| Message ID | 1637048356-73662-1-git-send-email-pthombar@cadence.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] PCI: cadence: Clear FLR in device capabilities register | expand |
Ping ! Regards, Parshuram Thombare >-----Original Message----- >From: Parshuram Raju Thombare <pthombar@cadence.com> >Sent: Tuesday, November 16, 2021 1:09 PM >To: bhelgaas@google.com; kishon@ti.com; Tom Joseph ><tjoseph@cadence.com>; lorenzo.pieralisi@arm.com; robh@kernel.org; >kw@linux.com >Cc: linux-omap@vger.kernel.org; linux-pci@vger.kernel.org; linux-arm- >kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Milind Parab ><mparab@cadence.com>; Parshuram Raju Thombare ><pthombar@cadence.com> >Subject: [PATCH v2] PCI: cadence: Clear FLR in device capabilities register > >From: Parshuram Thombare <pthombar@cadence.com> > >Clear FLR (Function Level Reset) from device capabilities >registers for all physical functions. > >During FLR, the Margining Lane Status and Margining Lane Control >registers should not be reset, as per PCIe specification. >However, the controller incorrectly resets these registers upon FLR. >This causes PCISIG compliance FLR test to fail. Hence preventing >all functions from advertising FLR support if flag quirk_disable_flr >is set. > >Signed-off-by: Parshuram Thombare <pthombar@cadence.com> >--- >Changes since v1: >Changes suggested by Bjorn in the description. > >--- > drivers/pci/controller/cadence/pci-j721e.c | 3 +++ > drivers/pci/controller/cadence/pcie-cadence-ep.c | 18 +++++++++++++++++- > drivers/pci/controller/cadence/pcie-cadence.h | 3 +++ > 3 files changed, 23 insertions(+), 1 deletion(-) > >diff --git a/drivers/pci/controller/cadence/pci-j721e.c >b/drivers/pci/controller/cadence/pci-j721e.c >index ffb176d..635e36c 100644 >--- a/drivers/pci/controller/cadence/pci-j721e.c >+++ b/drivers/pci/controller/cadence/pci-j721e.c >@@ -70,6 +70,7 @@ struct j721e_pcie_data { > enum j721e_pcie_mode mode; > unsigned int quirk_retrain_flag:1; > unsigned int quirk_detect_quiet_flag:1; >+ unsigned int quirk_disable_flr:1; > u32 linkdown_irq_regfield; > unsigned int byte_access_allowed:1; > }; >@@ -308,6 +309,7 @@ static int cdns_ti_pcie_config_write(struct pci_bus *bus, >unsigned int devfn, > static const struct j721e_pcie_data j7200_pcie_ep_data = { > .mode = PCI_MODE_EP, > .quirk_detect_quiet_flag = true, >+ .quirk_disable_flr = true, > }; > > static const struct j721e_pcie_data am64_pcie_rc_data = { >@@ -510,6 +512,7 @@ static int j721e_pcie_probe(struct platform_device >*pdev) > goto err_get_sync; > } > ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; >+ ep->quirk_disable_flr = data->quirk_disable_flr; > > cdns_pcie = &ep->pcie; > cdns_pcie->dev = dev; >diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c >b/drivers/pci/controller/cadence/pcie-cadence-ep.c >index 88e05b9..4b1c4bc 100644 >--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c >+++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c >@@ -565,7 +565,8 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > struct cdns_pcie_ep *ep = epc_get_drvdata(epc); > struct cdns_pcie *pcie = &ep->pcie; > struct device *dev = pcie->dev; >- int ret; >+ int max_epfs = sizeof(epc->function_num_map) * 8; >+ int ret, value, epf; > > /* > * BIT(0) is hardwired to 1, hence function 0 is always enabled >@@ -573,6 +574,21 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) > */ > cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, epc- >>function_num_map); > >+ if (ep->quirk_disable_flr) { >+ for (epf = 0; epf < max_epfs; epf++) { >+ if (!(epc->function_num_map & BIT(epf))) >+ continue; >+ >+ value = cdns_pcie_ep_fn_readl(pcie, epf, >+ > CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + >+ PCI_EXP_DEVCAP); >+ value &= ~PCI_EXP_DEVCAP_FLR; >+ cdns_pcie_ep_fn_writel(pcie, epf, >+ > CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + >+ PCI_EXP_DEVCAP, value); >+ } >+ } >+ > ret = cdns_pcie_start_link(pcie); > if (ret) { > dev_err(dev, "Failed to start link\n"); >diff --git a/drivers/pci/controller/cadence/pcie-cadence.h >b/drivers/pci/controller/cadence/pcie-cadence.h >index 262421e..e978e7c 100644 >--- a/drivers/pci/controller/cadence/pcie-cadence.h >+++ b/drivers/pci/controller/cadence/pcie-cadence.h >@@ -123,6 +123,7 @@ > > #define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90 > #define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET 0xb0 >+#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET 0xc0 > #define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET 0x200 > > /* >@@ -357,6 +358,7 @@ struct cdns_pcie_epf { > * minimize time between read and write > * @epf: Structure to hold info about endpoint function > * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk >+ * @quirk_disable_flr: Disable FLR (Function Level Reset) quirk flag > */ > struct cdns_pcie_ep { > struct cdns_pcie pcie; >@@ -372,6 +374,7 @@ struct cdns_pcie_ep { > spinlock_t lock; > struct cdns_pcie_epf *epf; > unsigned int quirk_detect_quiet_flag:1; >+ unsigned int quirk_disable_flr:1; > }; > > >-- >1.9.1
On Mon, 15 Nov 2021 23:39:16 -0800, Parshuram Raju Thombare wrote: > From: Parshuram Thombare <pthombar@cadence.com> > > Clear FLR (Function Level Reset) from device capabilities > registers for all physical functions. > > During FLR, the Margining Lane Status and Margining Lane Control > registers should not be reset, as per PCIe specification. > However, the controller incorrectly resets these registers upon FLR. > This causes PCISIG compliance FLR test to fail. Hence preventing > all functions from advertising FLR support if flag quirk_disable_flr > is set. > > [...] Applied to pci/cadence, thanks! [1/1] PCI: cadence: Clear FLR in device capabilities register https://git.kernel.org/lpieralisi/pci/c/d3dbd4d862 Thanks, Lorenzo
On Wed, May 11, 2022 at 05:02:35PM +0100, Lorenzo Pieralisi wrote: > On Mon, 15 Nov 2021 23:39:16 -0800, Parshuram Raju Thombare wrote: > > From: Parshuram Thombare <pthombar@cadence.com> > > > > Clear FLR (Function Level Reset) from device capabilities > > registers for all physical functions. > > > > During FLR, the Margining Lane Status and Margining Lane Control > > registers should not be reset, as per PCIe specification. > > However, the controller incorrectly resets these registers upon FLR. > > This causes PCISIG compliance FLR test to fail. Hence preventing > > all functions from advertising FLR support if flag quirk_disable_flr > > is set. > > > > [...] > > Applied to pci/cadence, thanks! > > [1/1] PCI: cadence: Clear FLR in device capabilities register > https://git.kernel.org/lpieralisi/pci/c/d3dbd4d862 Obviously you've already seen the kbuild report: https://lore.kernel.org/r/202205120700.X76G7aC2-lkp@intel.com but it looks like most of this patch got lost somehow :) Happy to fix it up for you if you want! Bjorn
On Thu, May 12, 2022 at 02:06:26PM -0500, Bjorn Helgaas wrote: > On Wed, May 11, 2022 at 05:02:35PM +0100, Lorenzo Pieralisi wrote: > > On Mon, 15 Nov 2021 23:39:16 -0800, Parshuram Raju Thombare wrote: > > > From: Parshuram Thombare <pthombar@cadence.com> > > > > > > Clear FLR (Function Level Reset) from device capabilities > > > registers for all physical functions. > > > > > > During FLR, the Margining Lane Status and Margining Lane Control > > > registers should not be reset, as per PCIe specification. > > > However, the controller incorrectly resets these registers upon FLR. > > > This causes PCISIG compliance FLR test to fail. Hence preventing > > > all functions from advertising FLR support if flag quirk_disable_flr > > > is set. > > > > > > [...] > > > > Applied to pci/cadence, thanks! > > > > [1/1] PCI: cadence: Clear FLR in device capabilities register > > https://git.kernel.org/lpieralisi/pci/c/d3dbd4d862 > > Obviously you've already seen the kbuild report: > https://lore.kernel.org/r/202205120700.X76G7aC2-lkp@intel.com > > but it looks like most of this patch got lost somehow :) Happy to fix > it up for you if you want! I have messed up the merge, now rebuilt my pci/cadence branch, it _should_ be fixed, apologies. Lorenzo
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c index ffb176d..635e36c 100644 --- a/drivers/pci/controller/cadence/pci-j721e.c +++ b/drivers/pci/controller/cadence/pci-j721e.c @@ -70,6 +70,7 @@ struct j721e_pcie_data { enum j721e_pcie_mode mode; unsigned int quirk_retrain_flag:1; unsigned int quirk_detect_quiet_flag:1; + unsigned int quirk_disable_flr:1; u32 linkdown_irq_regfield; unsigned int byte_access_allowed:1; }; @@ -308,6 +309,7 @@ static int cdns_ti_pcie_config_write(struct pci_bus *bus, unsigned int devfn, static const struct j721e_pcie_data j7200_pcie_ep_data = { .mode = PCI_MODE_EP, .quirk_detect_quiet_flag = true, + .quirk_disable_flr = true, }; static const struct j721e_pcie_data am64_pcie_rc_data = { @@ -510,6 +512,7 @@ static int j721e_pcie_probe(struct platform_device *pdev) goto err_get_sync; } ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag; + ep->quirk_disable_flr = data->quirk_disable_flr; cdns_pcie = &ep->pcie; cdns_pcie->dev = dev; diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c index 88e05b9..4b1c4bc 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c @@ -565,7 +565,8 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) struct cdns_pcie_ep *ep = epc_get_drvdata(epc); struct cdns_pcie *pcie = &ep->pcie; struct device *dev = pcie->dev; - int ret; + int max_epfs = sizeof(epc->function_num_map) * 8; + int ret, value, epf; /* * BIT(0) is hardwired to 1, hence function 0 is always enabled @@ -573,6 +574,21 @@ static int cdns_pcie_ep_start(struct pci_epc *epc) */ cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, epc->function_num_map); + if (ep->quirk_disable_flr) { + for (epf = 0; epf < max_epfs; epf++) { + if (!(epc->function_num_map & BIT(epf))) + continue; + + value = cdns_pcie_ep_fn_readl(pcie, epf, + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + + PCI_EXP_DEVCAP); + value &= ~PCI_EXP_DEVCAP_FLR; + cdns_pcie_ep_fn_writel(pcie, epf, + CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET + + PCI_EXP_DEVCAP, value); + } + } + ret = cdns_pcie_start_link(pcie); if (ret) { dev_err(dev, "Failed to start link\n"); diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h index 262421e..e978e7c 100644 --- a/drivers/pci/controller/cadence/pcie-cadence.h +++ b/drivers/pci/controller/cadence/pcie-cadence.h @@ -123,6 +123,7 @@ #define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90 #define CDNS_PCIE_EP_FUNC_MSIX_CAP_OFFSET 0xb0 +#define CDNS_PCIE_EP_FUNC_DEV_CAP_OFFSET 0xc0 #define CDNS_PCIE_EP_FUNC_SRIOV_CAP_OFFSET 0x200 /* @@ -357,6 +358,7 @@ struct cdns_pcie_epf { * minimize time between read and write * @epf: Structure to hold info about endpoint function * @quirk_detect_quiet_flag: LTSSM Detect Quiet min delay set as quirk + * @quirk_disable_flr: Disable FLR (Function Level Reset) quirk flag */ struct cdns_pcie_ep { struct cdns_pcie pcie; @@ -372,6 +374,7 @@ struct cdns_pcie_ep { spinlock_t lock; struct cdns_pcie_epf *epf; unsigned int quirk_detect_quiet_flag:1; + unsigned int quirk_disable_flr:1; };