| Message ID | 20220919143340.4527-3-vidyas@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Disable PTM for endpoint mode | expand |
On Mon, Sep 19, 2022, Vidya Sagar <vidyas@nvidia.com> wrote: > > Dual mode DesignWare PCIe IP has PTM capability enabled (if supported) even > in the EP mode. The PCIe compliance for the EP mode expects PTM > capabilities (ROOT_CAPABLE, RES_CAPABLE, CLK_GRAN) be disabled. > Hence disable PTM for the EP mode. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 7e9529ae3824..dc3057b18f36 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -646,7 +646,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > struct dw_pcie_ep_func *ep_func; > struct device *dev = pci->dev; > struct pci_epc *epc = ep->epc; > - unsigned int offset; > + unsigned int offset, ptm_cap_base; Is there any reason to mix these 2 variables into 1 line? If not, please add a new line for ptm_cap_base. unsigned int offset; unsigned int ptm_cap_base; Also, it looks ok. When you send a new patch, just add my Acked-by. Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > unsigned int nbars; > u8 hdr_type; > u8 func_no; > @@ -698,6 +698,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > } > > offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > dw_pcie_dbi_ro_wr_en(pci); > > @@ -710,6 +711,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > } > > + /* > + * PTM responder capability can be disabled only after disabling > + * PTM root capability. > + */ > + if (ptm_cap_base) { > + dw_pcie_dbi_ro_wr_en(pci); > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~PCI_PTM_CAP_ROOT; > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + dw_pcie_dbi_ro_wr_dis(pci); > + } > + > dw_pcie_setup(pci); > dw_pcie_dbi_ro_wr_dis(pci); > > -- > 2.17.1 >
On Mon, Sep 19, 2022 at 08:03:40PM +0530, Vidya Sagar wrote: > Dual mode DesignWare PCIe IP has PTM capability enabled (if supported) even > in the EP mode. The PCIe compliance for the EP mode expects PTM > capabilities (ROOT_CAPABLE, RES_CAPABLE, CLK_GRAN) be disabled. > Hence disable PTM for the EP mode. "PCIe compliance" - what is this referring to ? Was this reported to Synopsys ? Thanks, Lorenzo > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 7e9529ae3824..dc3057b18f36 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -646,7 +646,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > struct dw_pcie_ep_func *ep_func; > struct device *dev = pci->dev; > struct pci_epc *epc = ep->epc; > - unsigned int offset; > + unsigned int offset, ptm_cap_base; > unsigned int nbars; > u8 hdr_type; > u8 func_no; > @@ -698,6 +698,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > } > > offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > dw_pcie_dbi_ro_wr_en(pci); > > @@ -710,6 +711,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > } > > + /* > + * PTM responder capability can be disabled only after disabling > + * PTM root capability. > + */ > + if (ptm_cap_base) { > + dw_pcie_dbi_ro_wr_en(pci); > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~PCI_PTM_CAP_ROOT; > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + dw_pcie_dbi_ro_wr_dis(pci); > + } > + > dw_pcie_setup(pci); > dw_pcie_dbi_ro_wr_dis(pci); > > -- > 2.17.1 >
On Thu, Oct 27, 2022 Lorenzo Pieralisi <lpieralisi@kernel.org> wrote: > > On Mon, Sep 19, 2022 at 08:03:40PM +0530, Vidya Sagar wrote: > > Dual mode DesignWare PCIe IP has PTM capability enabled (if supported) even > > in the EP mode. The PCIe compliance for the EP mode expects PTM > > capabilities (ROOT_CAPABLE, RES_CAPABLE, CLK_GRAN) be disabled. > > Hence disable PTM for the EP mode. > > "PCIe compliance" - what is this referring to ? Hi Lorenzo, You can refer to the following link: https://pcisig.com/developers/compliance-program PCIe compliance means 'PCIe Compliance test "Compliance tests allow for product testing against PCI-SIG test modules". Usually, USB specs and PCIe specs define compliance tests where product manufacturers need to pass specific test procedures. For example, in USB cases, USB.org says that "To qualify for the right to display the certified USB logo in conjunction with a product, the product must pass USB-IF compliance testing for product quality." To Vidiya, If my understanding is not correct, please let us know what you intended. > > Was this reported to Synopsys ? To Vidiya, Would you confirm that? Thank you. Best regards, Jingoo Han > > Thanks, > Lorenzo > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 19 ++++++++++++++++++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 7e9529ae3824..dc3057b18f36 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -646,7 +646,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > > struct dw_pcie_ep_func *ep_func; > > struct device *dev = pci->dev; > > struct pci_epc *epc = ep->epc; > > - unsigned int offset; > > + unsigned int offset, ptm_cap_base; > > unsigned int nbars; > > u8 hdr_type; > > u8 func_no; > > @@ -698,6 +698,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > > } > > > > offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > > > dw_pcie_dbi_ro_wr_en(pci); > > > > @@ -710,6 +711,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > > dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > > } > > > > + /* > > + * PTM responder capability can be disabled only after disabling > > + * PTM root capability. > > + */ > > + if (ptm_cap_base) { > > + dw_pcie_dbi_ro_wr_en(pci); > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > + reg &= ~PCI_PTM_CAP_ROOT; > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > + > > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > > + dw_pcie_dbi_ro_wr_dis(pci); > > + } > > + > > dw_pcie_setup(pci); > > dw_pcie_dbi_ro_wr_dis(pci); > > > > -- > > 2.17.1 > >
On Mon, Sep 19, 2022 at 08:03:40PM +0530, Vidya Sagar wrote: > Dual mode DesignWare PCIe IP has PTM capability enabled (if supported) even > in the EP mode. The PCIe compliance for the EP mode expects PTM > capabilities (ROOT_CAPABLE, RES_CAPABLE, CLK_GRAN) be disabled. > Hence disable PTM for the EP mode. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 7e9529ae3824..dc3057b18f36 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -646,7 +646,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > struct dw_pcie_ep_func *ep_func; > struct device *dev = pci->dev; > struct pci_epc *epc = ep->epc; > - unsigned int offset; > + unsigned int offset, ptm_cap_base; > unsigned int nbars; > u8 hdr_type; > u8 func_no; > @@ -698,6 +698,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > } > > offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); > + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); > > dw_pcie_dbi_ro_wr_en(pci); > > @@ -710,6 +711,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) > dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); > } > > + /* > + * PTM responder capability can be disabled only after disabling > + * PTM root capability. > + */ > + if (ptm_cap_base) { > + dw_pcie_dbi_ro_wr_en(pci); > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~PCI_PTM_CAP_ROOT; > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + > + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); > + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); > + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); > + dw_pcie_dbi_ro_wr_dis(pci); Per spec, PTM Responder Capable, PTM Root Capable, and Local Clock Granularity may only be set for Root Ports, RCRBs, and Switches (PCIe r6.0, sec 7.9.15.2). And this is just a matter of making an Endpoint comply with the spec, i.e., configures the Endpoint so it doesn't advertise that it can be a PTM Responder, right? But the Endpoint probably still *can* be a PTM Requester? > + } > + > dw_pcie_setup(pci); > dw_pcie_dbi_ro_wr_dis(pci); > > -- > 2.17.1 >
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 7e9529ae3824..dc3057b18f36 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -646,7 +646,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) struct dw_pcie_ep_func *ep_func; struct device *dev = pci->dev; struct pci_epc *epc = ep->epc; - unsigned int offset; + unsigned int offset, ptm_cap_base; unsigned int nbars; u8 hdr_type; u8 func_no; @@ -698,6 +698,7 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) } offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR); + ptm_cap_base = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_PTM); dw_pcie_dbi_ro_wr_en(pci); @@ -710,6 +711,22 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep) dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0); } + /* + * PTM responder capability can be disabled only after disabling + * PTM root capability. + */ + if (ptm_cap_base) { + dw_pcie_dbi_ro_wr_en(pci); + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); + reg &= ~PCI_PTM_CAP_ROOT; + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); + + reg = dw_pcie_readl_dbi(pci, ptm_cap_base + PCI_PTM_CAP); + reg &= ~(PCI_PTM_CAP_RES | PCI_PTM_GRANULARITY_MASK); + dw_pcie_writel_dbi(pci, ptm_cap_base + PCI_PTM_CAP, reg); + dw_pcie_dbi_ro_wr_dis(pci); + } + dw_pcie_setup(pci); dw_pcie_dbi_ro_wr_dis(pci);
Dual mode DesignWare PCIe IP has PTM capability enabled (if supported) even in the EP mode. The PCIe compliance for the EP mode expects PTM capabilities (ROOT_CAPABLE, RES_CAPABLE, CLK_GRAN) be disabled. Hence disable PTM for the EP mode. Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- .../pci/controller/dwc/pcie-designware-ep.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)