* [PATCH 1/2] arm: a37xx: pci: Disable returning CRS response
@ 2021-08-27 12:14 Pali Rohár
2021-08-27 12:14 ` [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on " Pali Rohár
2021-08-31 5:57 ` [PATCH 1/2] arm: a37xx: pci: Disable returning " Stefan Roese
0 siblings, 2 replies; 4+ messages in thread
From: Pali Rohár @ 2021-08-27 12:14 UTC (permalink / raw)
To: Stefan Roese, Konstantin Porotchkin; +Cc: Marek Behún, u-boot
There was mistake in commit 4cd61c43fd51 ("arm: a37xx: pci: Fix handling
PIO config error responses"). U-Boot does not support handling of CRS
return value for PCI_VENDOR_ID config read request and also does not set
CRSSVE bit.
Therefore disable returning CRS response for now.
Signed-off-by: Pali Rohár <pali@kernel.org>
Fixes: 4cd61c43fd51 ("arm: a37xx: pci: Fix handling PIO config error responses")
---
drivers/pci/pci-aardvark.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 815b26162f15..d3ef8f203d97 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -358,7 +358,18 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
return 0;
}
- allow_crs = (offset == PCI_VENDOR_ID) && (size == 4);
+ /*
+ * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to
+ * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and
+ * only when CRSSVE bit in Root Port PCIe device is enabled. In all
+ * other error PCIe Root Complex must return all-ones.
+ * Aardvark HW does not have Root Port PCIe device and U-Boot does not
+ * implement emulation of this device.
+ * U-Boot currently does not support handling of CRS return value for
+ * PCI_VENDOR_ID config read request and also does not set CRSSVE bit.
+ * Therefore disable returning CRS response for now.
+ */
+ allow_crs = false;
if (advk_readl(pcie, PIO_START)) {
dev_err(pcie->dev,
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on CRS response 2021-08-27 12:14 [PATCH 1/2] arm: a37xx: pci: Disable returning CRS response Pali Rohár @ 2021-08-27 12:14 ` Pali Rohár 2021-08-31 5:58 ` Stefan Roese 2021-08-31 5:57 ` [PATCH 1/2] arm: a37xx: pci: Disable returning " Stefan Roese 1 sibling, 1 reply; 4+ messages in thread From: Pali Rohár @ 2021-08-27 12:14 UTC (permalink / raw) To: Stefan Roese, Konstantin Porotchkin; +Cc: Marek Behún, u-boot According to PCIe base specification, if CRS Software Visibility is not enabled, the Root Complex must re-issue the Configuration Request as a new Request. Normally this part of Root Complex is implemented in hardware but aardvark is somehow special and does not implement it in hardware and expect that handling of config requests are fully implemented in software. This re-issuing functionality is required also because U-Boot does not support CRS Software Visibility feature and therefore expects that Root Complex re-issues requests as is specified in PCIe base specification. Retry / re-issue config request up to the PIO_MAX_RETRIES, to prevent infinite loop. After retry count exceed PIO_MAX_RETRIES, returns failure. Signed-off-by: Pali Rohár <pali@kernel.org> --- drivers/pci/pci-aardvark.c | 58 ++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c index d3ef8f203d97..74797e984cb8 100644 --- a/drivers/pci/pci-aardvark.c +++ b/drivers/pci/pci-aardvark.c @@ -234,19 +234,19 @@ static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno) * * Wait up to 1.5 seconds for PIO access to be accomplished. * - * Return 1 (true) if PIO access is accomplished. - * Return 0 (false) if PIO access is timed out. + * Return positive - retry count if PIO access is accomplished. + * Return negative - error if PIO access is timed out. */ static int pcie_advk_wait_pio(struct pcie_advk *pcie) { uint start, isr; uint count; - for (count = 0; count < PIO_MAX_RETRIES; count++) { + for (count = 1; count <= PIO_MAX_RETRIES; count++) { start = advk_readl(pcie, PIO_START); isr = advk_readl(pcie, PIO_ISR); if (!start && isr) - return 1; + return count; /* * Do not check the PIO state too frequently, * 100us delay is appropriate. @@ -255,7 +255,7 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie) } dev_err(pcie->dev, "PIO read/write transfer time out\n"); - return 0; + return -ETIMEDOUT; } /** @@ -265,11 +265,13 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie) * @allow_crs: Only for read requests, if CRS response is allowed * @read_val: Pointer to the read result * + * Return: 0 on success */ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, bool allow_crs, uint *read_val) { + int ret; uint reg; unsigned int status; char *strcomp_status, *str_posted; @@ -282,6 +284,7 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, case PIO_COMPLETION_STATUS_OK: if (reg & PIO_ERR_STATUS) { strcomp_status = "COMP_ERR"; + ret = -EFAULT; break; } /* Get the read result */ @@ -289,29 +292,35 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, *read_val = advk_readl(pcie, PIO_RD_DATA); /* No error */ strcomp_status = NULL; + ret = 0; break; case PIO_COMPLETION_STATUS_UR: strcomp_status = "UR"; + ret = -EOPNOTSUPP; break; case PIO_COMPLETION_STATUS_CRS: if (allow_crs && read_val) { /* For reading, CRS is not an error status. */ *read_val = CFG_RD_CRS_VAL; strcomp_status = NULL; + ret = 0; } else { strcomp_status = "CRS"; + ret = -EAGAIN; } break; case PIO_COMPLETION_STATUS_CA: strcomp_status = "CA"; + ret = -ECANCELED; break; default: strcomp_status = "Unknown"; + ret = -EINVAL; break; } if (!strcomp_status) - return 0; + return ret; if (reg & PIO_NON_POSTED_REQ) str_posted = "Non-posted"; @@ -322,7 +331,7 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, str_posted, strcomp_status, reg, advk_readl(pcie, PIO_ADDR_LS)); - return -EFAULT; + return ret; } /** @@ -345,6 +354,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct pcie_advk *pcie = dev_get_priv(bus); + int retry_count; bool allow_crs; uint reg; int ret; @@ -379,7 +389,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, return 0; } *valuep = pci_get_ff(size); - return -EINVAL; + return -EAGAIN; } /* Program the control register */ @@ -396,21 +406,29 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, advk_writel(pcie, reg, PIO_ADDR_LS); advk_writel(pcie, 0, PIO_ADDR_MS); + retry_count = 0; + +retry: /* Start the transfer */ advk_writel(pcie, 1, PIO_ISR); advk_writel(pcie, 1, PIO_START); - if (!pcie_advk_wait_pio(pcie)) { + ret = pcie_advk_wait_pio(pcie); + if (ret < 0) { if (allow_crs) { *valuep = CFG_RD_CRS_VAL; return 0; } *valuep = pci_get_ff(size); - return -EINVAL; + return ret; } + retry_count += ret; + /* Check PIO status and get the read result */ ret = pcie_advk_check_pio_status(pcie, allow_crs, ®); + if (ret == -EAGAIN && retry_count < PIO_MAX_RETRIES) + goto retry; if (ret) { *valuep = pci_get_ff(size); return ret; @@ -472,7 +490,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, enum pci_size_t size) { struct pcie_advk *pcie = dev_get_priv(bus); + int retry_count; uint reg; + int ret; dev_dbg(pcie->dev, "PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ", PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); @@ -487,7 +507,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, if (advk_readl(pcie, PIO_START)) { dev_err(pcie->dev, "Previous PIO read/write transfer is still running\n"); - return -EINVAL; + return -EAGAIN; } /* Program the control register */ @@ -515,16 +535,24 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, advk_writel(pcie, reg, PIO_WR_DATA_STRB); dev_dbg(pcie->dev, "\tPIO req. - strb = 0x%02x\n", reg); + retry_count = 0; + +retry: /* Start the transfer */ advk_writel(pcie, 1, PIO_ISR); advk_writel(pcie, 1, PIO_START); - if (!pcie_advk_wait_pio(pcie)) { - return -EINVAL; - } + ret = pcie_advk_wait_pio(pcie); + if (ret < 0) + return ret; + + retry_count += ret; /* Check PIO status */ - return pcie_advk_check_pio_status(pcie, false, NULL); + ret = pcie_advk_check_pio_status(pcie, false, NULL); + if (ret == -EAGAIN && retry_count < PIO_MAX_RETRIES) + goto retry; + return ret; } /** -- 2.20.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on CRS response 2021-08-27 12:14 ` [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on " Pali Rohár @ 2021-08-31 5:58 ` Stefan Roese 0 siblings, 0 replies; 4+ messages in thread From: Stefan Roese @ 2021-08-31 5:58 UTC (permalink / raw) To: Pali Rohár, Konstantin Porotchkin; +Cc: Marek Behún, u-boot On 27.08.21 14:14, Pali Rohár wrote: > According to PCIe base specification, if CRS Software Visibility is not > enabled, the Root Complex must re-issue the Configuration Request as a new > Request. > > Normally this part of Root Complex is implemented in hardware but aardvark > is somehow special and does not implement it in hardware and expect that > handling of config requests are fully implemented in software. > > This re-issuing functionality is required also because U-Boot does not > support CRS Software Visibility feature and therefore expects that Root > Complex re-issues requests as is specified in PCIe base specification. > > Retry / re-issue config request up to the PIO_MAX_RETRIES, to prevent > infinite loop. After retry count exceed PIO_MAX_RETRIES, returns failure. > > Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 58 ++++++++++++++++++++++++++++---------- > 1 file changed, 43 insertions(+), 15 deletions(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index d3ef8f203d97..74797e984cb8 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -234,19 +234,19 @@ static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno) > * > * Wait up to 1.5 seconds for PIO access to be accomplished. > * > - * Return 1 (true) if PIO access is accomplished. > - * Return 0 (false) if PIO access is timed out. > + * Return positive - retry count if PIO access is accomplished. > + * Return negative - error if PIO access is timed out. > */ > static int pcie_advk_wait_pio(struct pcie_advk *pcie) > { > uint start, isr; > uint count; > > - for (count = 0; count < PIO_MAX_RETRIES; count++) { > + for (count = 1; count <= PIO_MAX_RETRIES; count++) { > start = advk_readl(pcie, PIO_START); > isr = advk_readl(pcie, PIO_ISR); > if (!start && isr) > - return 1; > + return count; > /* > * Do not check the PIO state too frequently, > * 100us delay is appropriate. > @@ -255,7 +255,7 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie) > } > > dev_err(pcie->dev, "PIO read/write transfer time out\n"); > - return 0; > + return -ETIMEDOUT; > } > > /** > @@ -265,11 +265,13 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie) > * @allow_crs: Only for read requests, if CRS response is allowed > * @read_val: Pointer to the read result > * > + * Return: 0 on success > */ > static int pcie_advk_check_pio_status(struct pcie_advk *pcie, > bool allow_crs, > uint *read_val) > { > + int ret; > uint reg; > unsigned int status; > char *strcomp_status, *str_posted; > @@ -282,6 +284,7 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, > case PIO_COMPLETION_STATUS_OK: > if (reg & PIO_ERR_STATUS) { > strcomp_status = "COMP_ERR"; > + ret = -EFAULT; > break; > } > /* Get the read result */ > @@ -289,29 +292,35 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, > *read_val = advk_readl(pcie, PIO_RD_DATA); > /* No error */ > strcomp_status = NULL; > + ret = 0; > break; > case PIO_COMPLETION_STATUS_UR: > strcomp_status = "UR"; > + ret = -EOPNOTSUPP; > break; > case PIO_COMPLETION_STATUS_CRS: > if (allow_crs && read_val) { > /* For reading, CRS is not an error status. */ > *read_val = CFG_RD_CRS_VAL; > strcomp_status = NULL; > + ret = 0; > } else { > strcomp_status = "CRS"; > + ret = -EAGAIN; > } > break; > case PIO_COMPLETION_STATUS_CA: > strcomp_status = "CA"; > + ret = -ECANCELED; > break; > default: > strcomp_status = "Unknown"; > + ret = -EINVAL; > break; > } > > if (!strcomp_status) > - return 0; > + return ret; > > if (reg & PIO_NON_POSTED_REQ) > str_posted = "Non-posted"; > @@ -322,7 +331,7 @@ static int pcie_advk_check_pio_status(struct pcie_advk *pcie, > str_posted, strcomp_status, reg, > advk_readl(pcie, PIO_ADDR_LS)); > > - return -EFAULT; > + return ret; > } > > /** > @@ -345,6 +354,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > enum pci_size_t size) > { > struct pcie_advk *pcie = dev_get_priv(bus); > + int retry_count; > bool allow_crs; > uint reg; > int ret; > @@ -379,7 +389,7 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > return 0; > } > *valuep = pci_get_ff(size); > - return -EINVAL; > + return -EAGAIN; > } > > /* Program the control register */ > @@ -396,21 +406,29 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > advk_writel(pcie, reg, PIO_ADDR_LS); > advk_writel(pcie, 0, PIO_ADDR_MS); > > + retry_count = 0; > + > +retry: > /* Start the transfer */ > advk_writel(pcie, 1, PIO_ISR); > advk_writel(pcie, 1, PIO_START); > > - if (!pcie_advk_wait_pio(pcie)) { > + ret = pcie_advk_wait_pio(pcie); > + if (ret < 0) { > if (allow_crs) { > *valuep = CFG_RD_CRS_VAL; > return 0; > } > *valuep = pci_get_ff(size); > - return -EINVAL; > + return ret; > } > > + retry_count += ret; > + > /* Check PIO status and get the read result */ > ret = pcie_advk_check_pio_status(pcie, allow_crs, ®); > + if (ret == -EAGAIN && retry_count < PIO_MAX_RETRIES) > + goto retry; > if (ret) { > *valuep = pci_get_ff(size); > return ret; > @@ -472,7 +490,9 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > enum pci_size_t size) > { > struct pcie_advk *pcie = dev_get_priv(bus); > + int retry_count; > uint reg; > + int ret; > > dev_dbg(pcie->dev, "PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ", > PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf)); > @@ -487,7 +507,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > if (advk_readl(pcie, PIO_START)) { > dev_err(pcie->dev, > "Previous PIO read/write transfer is still running\n"); > - return -EINVAL; > + return -EAGAIN; > } > > /* Program the control register */ > @@ -515,16 +535,24 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf, > advk_writel(pcie, reg, PIO_WR_DATA_STRB); > dev_dbg(pcie->dev, "\tPIO req. - strb = 0x%02x\n", reg); > > + retry_count = 0; > + > +retry: > /* Start the transfer */ > advk_writel(pcie, 1, PIO_ISR); > advk_writel(pcie, 1, PIO_START); > > - if (!pcie_advk_wait_pio(pcie)) { > - return -EINVAL; > - } > + ret = pcie_advk_wait_pio(pcie); > + if (ret < 0) > + return ret; > + > + retry_count += ret; > > /* Check PIO status */ > - return pcie_advk_check_pio_status(pcie, false, NULL); > + ret = pcie_advk_check_pio_status(pcie, false, NULL); > + if (ret == -EAGAIN && retry_count < PIO_MAX_RETRIES) > + goto retry; > + return ret; > } > > /** > Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] arm: a37xx: pci: Disable returning CRS response 2021-08-27 12:14 [PATCH 1/2] arm: a37xx: pci: Disable returning CRS response Pali Rohár 2021-08-27 12:14 ` [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on " Pali Rohár @ 2021-08-31 5:57 ` Stefan Roese 1 sibling, 0 replies; 4+ messages in thread From: Stefan Roese @ 2021-08-31 5:57 UTC (permalink / raw) To: Pali Rohár, Konstantin Porotchkin; +Cc: Marek Behún, u-boot On 27.08.21 14:14, Pali Rohár wrote: > There was mistake in commit 4cd61c43fd51 ("arm: a37xx: pci: Fix handling > PIO config error responses"). U-Boot does not support handling of CRS > return value for PCI_VENDOR_ID config read request and also does not set > CRSSVE bit. > > Therefore disable returning CRS response for now. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Fixes: 4cd61c43fd51 ("arm: a37xx: pci: Fix handling PIO config error responses") Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/pci/pci-aardvark.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c > index 815b26162f15..d3ef8f203d97 100644 > --- a/drivers/pci/pci-aardvark.c > +++ b/drivers/pci/pci-aardvark.c > @@ -358,7 +358,18 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf, > return 0; > } > > - allow_crs = (offset == PCI_VENDOR_ID) && (size == 4); > + /* > + * Returning fabricated CRS value (0xFFFF0001) by PCIe Root Complex to > + * OS is allowed only for 4-byte PCI_VENDOR_ID config read request and > + * only when CRSSVE bit in Root Port PCIe device is enabled. In all > + * other error PCIe Root Complex must return all-ones. > + * Aardvark HW does not have Root Port PCIe device and U-Boot does not > + * implement emulation of this device. > + * U-Boot currently does not support handling of CRS return value for > + * PCI_VENDOR_ID config read request and also does not set CRSSVE bit. > + * Therefore disable returning CRS response for now. > + */ > + allow_crs = false; > > if (advk_readl(pcie, PIO_START)) { > dev_err(pcie->dev, > Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-31 5:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-27 12:14 [PATCH 1/2] arm: a37xx: pci: Disable returning CRS response Pali Rohár 2021-08-27 12:14 ` [PATCH 2/2] arm: a37xx: pci: Implement re-issuing config requests on " Pali Rohár 2021-08-31 5:58 ` Stefan Roese 2021-08-31 5:57 ` [PATCH 1/2] arm: a37xx: pci: Disable returning " Stefan Roese
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.