[PATCH] arm: a37xx: pci: Fix processing PIO transfers
Stefan Roese
sr at denx.de
Mon Apr 26 11:22:50 CEST 2021
On 22.04.21 16:23, Pali Rohár wrote:
> Trying to clear PIO_START register when it is non-zero (which indicates
> that previous PIO transfer has not finished yet) causes an External
> Abort with SError 0xbf000002.
>
> This bug is currently worked around in TF-A by handling External Aborts
> in EL3 and ignoring this particular SError.
>
> This workaround was also discussed at:
> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=3c7dcdac5c50
> https://lore.kernel.org/linux-pci/20190316161243.29517-1-repk@triplefau.lt/
> https://lore.kernel.org/linux-pci/971be151d24312cc533989a64bd454b4@www.loen.fr/
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/1541
>
> Implement a proper fix to prevent this External Abort. As it is not
> possible to cancel a pending PIO transfer, simply do not start a new one
> if previous has not finished yet. In this case return an error to the
> caller.
>
> In most cases this SError happens when there is no PCIe card connected
> or when PCIe link is down. The reason is that in these cases a PIO
> transfer takes about 1.44 seconds. For this reason we also increase the
> wait timeout in pcie_advk_wait_pio() to 1.5 seconds.
>
> If PIO read transfer for PCI_VENDOR_ID register times out, or if it
> isn't possible to read it yet because previous transfer is not finished,
> return Completion Retry Status value instead of failing, to give the
> caller a chance to send a new read request.
>
> Signed-off-by: Pali Rohár <pali at kernel.org>
> Reviewed-by: Marek Behún <marek.behun at nic.cz>
Reviewed-by: Stefan Roese <sr at denx.de>
Thanks,
Stefan
> ---
> drivers/pci/pci-aardvark.c | 42 +++++++++++++++++++++++++-------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
> index 3b9309f52c..c43d4f309b 100644
> --- a/drivers/pci/pci-aardvark.c
> +++ b/drivers/pci/pci-aardvark.c
> @@ -132,8 +132,9 @@
> PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
>
> /* PCIe Retries & Timeout definitions */
> -#define MAX_RETRIES 10
> -#define PIO_WAIT_TIMEOUT 100
> +#define PIO_MAX_RETRIES 1500
> +#define PIO_WAIT_TIMEOUT 1000
> +#define LINK_MAX_RETRIES 10
> #define LINK_WAIT_TIMEOUT 100000
>
> #define CFG_RD_UR_VAL 0xFFFFFFFF
> @@ -192,7 +193,7 @@ static int pcie_advk_addr_valid(pci_dev_t bdf, int first_busno)
> *
> * @pcie: The PCI device to access
> *
> - * Wait up to 1 micro second for PIO access to be accomplished.
> + * 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.
> @@ -202,7 +203,7 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie)
> uint start, isr;
> uint count;
>
> - for (count = 0; count < MAX_RETRIES; count++) {
> + for (count = 0; count < PIO_MAX_RETRIES; count++) {
> start = advk_readl(pcie, PIO_START);
> isr = advk_readl(pcie, PIO_ISR);
> if (!start && isr)
> @@ -214,7 +215,7 @@ static int pcie_advk_wait_pio(struct pcie_advk *pcie)
> udelay(PIO_WAIT_TIMEOUT);
> }
>
> - dev_err(pcie->dev, "config read/write timed out\n");
> + dev_err(pcie->dev, "PIO read/write transfer time out\n");
> return 0;
> }
>
> @@ -323,9 +324,14 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
> return 0;
> }
>
> - /* Start PIO */
> - advk_writel(pcie, 0, PIO_START);
> - advk_writel(pcie, 1, PIO_ISR);
> + if (advk_readl(pcie, PIO_START)) {
> + dev_err(pcie->dev,
> + "Previous PIO read/write transfer is still running\n");
> + if (offset != PCI_VENDOR_ID)
> + return -EINVAL;
> + *valuep = CFG_RD_CRS_VAL;
> + return 0;
> + }
>
> /* Program the control register */
> reg = advk_readl(pcie, PIO_CTRL);
> @@ -342,10 +348,15 @@ static int pcie_advk_read_config(const struct udevice *bus, pci_dev_t bdf,
> advk_writel(pcie, 0, PIO_ADDR_MS);
>
> /* Start the transfer */
> + advk_writel(pcie, 1, PIO_ISR);
> advk_writel(pcie, 1, PIO_START);
>
> - if (!pcie_advk_wait_pio(pcie))
> - return -EINVAL;
> + if (!pcie_advk_wait_pio(pcie)) {
> + if (offset != PCI_VENDOR_ID)
> + return -EINVAL;
> + *valuep = CFG_RD_CRS_VAL;
> + return 0;
> + }
>
> /* Check PIO status and get the read result */
> ret = pcie_advk_check_pio_status(pcie, true, ®);
> @@ -420,9 +431,11 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
> return 0;
> }
>
> - /* Start PIO */
> - advk_writel(pcie, 0, PIO_START);
> - advk_writel(pcie, 1, PIO_ISR);
> + if (advk_readl(pcie, PIO_START)) {
> + dev_err(pcie->dev,
> + "Previous PIO read/write transfer is still running\n");
> + return -EINVAL;
> + }
>
> /* Program the control register */
> reg = advk_readl(pcie, PIO_CTRL);
> @@ -450,6 +463,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,
> dev_dbg(pcie->dev, "\tPIO req. - strb = 0x%02x\n", reg);
>
> /* Start the transfer */
> + advk_writel(pcie, 1, PIO_ISR);
> advk_writel(pcie, 1, PIO_START);
>
> if (!pcie_advk_wait_pio(pcie)) {
> @@ -494,7 +508,7 @@ static int pcie_advk_wait_for_link(struct pcie_advk *pcie)
> int retries;
>
> /* check if the link is up or not */
> - for (retries = 0; retries < MAX_RETRIES; retries++) {
> + for (retries = 0; retries < LINK_MAX_RETRIES; retries++) {
> if (pcie_advk_link_up(pcie)) {
> printf("PCIE-%d: Link up\n", pcie->first_busno);
> return 0;
>
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 at denx.de
More information about the U-Boot
mailing list