From: Bjorn Helgaas <helgaas@kernel.org>
To: "Marek Behún" <kabel@kernel.org>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, pali@kernel.org
Subject: Re: [PATCH 09/13] PCI: aardvark: Implement re-issuing config requests on CRS response
Date: Sat, 2 Oct 2021 11:35:19 -0500 [thread overview]
Message-ID: <20211002163519.GA973164@bhelgaas> (raw)
In-Reply-To: <20211001195856.10081-10-kabel@kernel.org>
On Fri, Oct 01, 2021 at 09:58:52PM +0200, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
>
> Commit 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value") fixed
> handling of CRS response and when CRSSVE flag was not enabled it marked CRS
> response as failed transaction (due to simplicity).
>
> But pci-aardvark.c driver is already waiting up to the PIO_RETRY_CNT count
> for PIO config response and so we can with a small change implement
> re-issuing of config requests as described in PCIe base specification.
>
> This change implements re-issuing of config requests when response is CRS.
> Set upper bound of wait cycles to around PIO_RETRY_CNT, afterwards the
> transaction is marked as failed and an all-ones value is returned as
> before.
Does this fix a problem?
> We do this by returning appropriate error codes from function
> advk_pcie_check_pio_status(). On CRS we return -EAGAIN and caller then
> reissues transaction.
>
> Fixes: 43f5c77bcbd2 ("PCI: aardvark: Fix reporting CRS value")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <kabel@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
> drivers/pci/controller/pci-aardvark.c | 67 +++++++++++++++++----------
> 1 file changed, 43 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 51d2955d9cca..7b9870d0b81f 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -603,6 +603,7 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> u32 reg;
> unsigned int status;
> char *strcomp_status, *str_posted;
> + int ret;
>
> reg = advk_readl(pcie, PIO_STAT);
> status = (reg & PIO_COMPLETION_STATUS_MASK) >>
> @@ -627,6 +628,7 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> case PIO_COMPLETION_STATUS_OK:
> if (reg & PIO_ERR_STATUS) {
> strcomp_status = "COMP_ERR";
> + ret = -EFAULT;
> break;
> }
> /* Get the read result */
> @@ -634,9 +636,11 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> *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 && val) {
> @@ -654,6 +658,7 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> */
> *val = CFG_RD_CRS_VAL;
> strcomp_status = NULL;
> + ret = 0;
> break;
> }
> /* PCIe r4.0, sec 2.3.2, says:
> @@ -669,21 +674,24 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> * Request and taking appropriate action, e.g., complete the
> * Request to the host as a failed transaction.
> *
> - * To simplify implementation do not re-issue the Configuration
> - * Request and complete the Request as a failed transaction.
> + * So return -EAGAIN and caller (pci-aardvark.c driver) will
> + * re-issue request again up to the PIO_RETRY_CNT retries.
> */
> 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";
> @@ -693,7 +701,7 @@ static int advk_pcie_check_pio_status(struct advk_pcie *pcie, bool allow_crs, u3
> dev_dbg(dev, "%s PIO Response Status: %s, %#x @ %#x\n",
> str_posted, strcomp_status, reg, advk_readl(pcie, PIO_ADDR_LS));
>
> - return -EFAULT;
> + return ret;
> }
>
> static int advk_pcie_wait_pio(struct advk_pcie *pcie)
> @@ -701,13 +709,13 @@ static int advk_pcie_wait_pio(struct advk_pcie *pcie)
> struct device *dev = &pcie->pdev->dev;
> int i;
>
> - for (i = 0; i < PIO_RETRY_CNT; i++) {
> + for (i = 1; i <= PIO_RETRY_CNT; i++) {
> u32 start, isr;
>
> start = advk_readl(pcie, PIO_START);
> isr = advk_readl(pcie, PIO_ISR);
> if (!start && isr)
> - return 0;
> + return i;
> udelay(PIO_RETRY_DELAY);
> }
>
> @@ -898,6 +906,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
> int where, int size, u32 *val)
> {
> struct advk_pcie *pcie = bus->sysdata;
> + int retry_count;
> bool allow_crs;
> u32 reg;
> int ret;
> @@ -940,16 +949,22 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
> /* Program the data strobe */
> advk_writel(pcie, 0xf, PIO_WR_DATA_STRB);
>
> - /* Clear PIO DONE ISR and start the transfer */
> - advk_writel(pcie, 1, PIO_ISR);
> - advk_writel(pcie, 1, PIO_START);
> + retry_count = 0;
> + do {
> + /* Clear PIO DONE ISR and start the transfer */
> + advk_writel(pcie, 1, PIO_ISR);
> + advk_writel(pcie, 1, PIO_START);
>
> - ret = advk_pcie_wait_pio(pcie);
> - if (ret < 0)
> - goto try_crs;
> + ret = advk_pcie_wait_pio(pcie);
> + if (ret < 0)
> + goto try_crs;
> +
> + retry_count += ret;
> +
> + /* Check PIO status and get the read result */
> + ret = advk_pcie_check_pio_status(pcie, allow_crs, val);
> + } while (ret == -EAGAIN && retry_count < PIO_RETRY_CNT);
>
> - /* Check PIO status and get the read result */
> - ret = advk_pcie_check_pio_status(pcie, allow_crs, val);
> if (ret < 0)
> goto fail;
>
> @@ -981,6 +996,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> struct advk_pcie *pcie = bus->sysdata;
> u32 reg;
> u32 data_strobe = 0x0;
> + int retry_count;
> int offset;
> int ret;
>
> @@ -1022,19 +1038,22 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> /* Program the data strobe */
> advk_writel(pcie, data_strobe, PIO_WR_DATA_STRB);
>
> - /* Clear PIO DONE ISR and start the transfer */
> - advk_writel(pcie, 1, PIO_ISR);
> - advk_writel(pcie, 1, PIO_START);
> + retry_count = 0;
> + do {
> + /* Clear PIO DONE ISR and start the transfer */
> + advk_writel(pcie, 1, PIO_ISR);
> + advk_writel(pcie, 1, PIO_START);
>
> - ret = advk_pcie_wait_pio(pcie);
> - if (ret < 0)
> - return PCIBIOS_SET_FAILED;
> + ret = advk_pcie_wait_pio(pcie);
> + if (ret < 0)
> + return PCIBIOS_SET_FAILED;
>
> - ret = advk_pcie_check_pio_status(pcie, false, NULL);
> - if (ret < 0)
> - return PCIBIOS_SET_FAILED;
> + retry_count += ret;
>
> - return PCIBIOS_SUCCESSFUL;
> + ret = advk_pcie_check_pio_status(pcie, false, NULL);
> + } while (ret == -EAGAIN && retry_count < PIO_RETRY_CNT);
> +
> + return ret < 0 ? PCIBIOS_SET_FAILED : PCIBIOS_SUCCESSFUL;
> }
>
> static struct pci_ops advk_pcie_ops = {
> --
> 2.32.0
>
next prev parent reply other threads:[~2021-10-02 16:35 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 19:58 [PATCH 00/13] PCI: aardvark controller fixes Marek Behún
2021-10-01 19:58 ` [PATCH 01/13] PCI: Add PCI_EXP_DEVCTL_PAYLOAD_* macros Marek Behún
2021-10-02 16:05 ` Bjorn Helgaas
2021-10-04 8:43 ` Lorenzo Pieralisi
2021-10-04 9:32 ` Marek Behún
2021-10-04 9:35 ` Lorenzo Pieralisi
2021-10-01 19:58 ` [PATCH 02/13] PCI: aardvark: Fix PCIe Max Payload Size setting Marek Behún
2021-10-03 17:44 ` Marek Behún
2021-10-01 19:58 ` [PATCH 03/13] PCI: aardvark: Don't spam about PIO Response Status Marek Behún
2021-10-01 19:58 ` [PATCH 04/13] PCI: aardvark: Fix preserving PCI_EXP_RTCTL_CRSSVE flag on emulated bridge Marek Behún
2021-10-01 19:58 ` [PATCH 05/13] PCI: aardvark: Fix configuring Reference clock Marek Behún
2021-10-01 19:58 ` [PATCH 06/13] PCI: aardvark: Do not clear status bits of masked interrupts Marek Behún
2021-10-04 14:06 ` Lorenzo Pieralisi
2021-10-04 15:18 ` Marek Behún
2021-10-04 15:31 ` Marc Zyngier
2021-10-05 12:13 ` Marek Behún
2021-10-05 12:42 ` Marc Zyngier
2021-10-05 13:15 ` Pali Rohár
2021-10-05 15:42 ` Marc Zyngier
2021-10-05 18:48 ` Pali Rohár
2021-10-05 16:04 ` Lorenzo Pieralisi
2021-10-01 19:58 ` [PATCH 07/13] PCI: aardvark: Do not unmask unused interrupts Marek Behún
2021-10-01 19:58 ` [PATCH 08/13] PCI: aardvark: Deduplicate code in advk_pcie_rd_conf() Marek Behún
2021-10-01 19:58 ` [PATCH 09/13] PCI: aardvark: Implement re-issuing config requests on CRS response Marek Behún
2021-10-02 16:35 ` Bjorn Helgaas [this message]
2021-10-04 7:21 ` Marek Behún
2021-10-04 8:53 ` Lorenzo Pieralisi
2021-10-04 10:19 ` Marek Behún
2021-10-05 19:28 ` Bjorn Helgaas
2021-10-05 22:45 ` Marek Behún
2021-10-11 18:15 ` Usage of PCBIOS_* macros (Was: Re: [PATCH 09/13] PCI: aardvark: Implement re-issuing config requests on CRS response) Pali Rohár
2021-10-11 20:58 ` Bjorn Helgaas
2021-10-06 16:29 ` [PATCH 09/13] PCI: aardvark: Implement re-issuing config requests on CRS response Lorenzo Pieralisi
2021-10-06 20:13 ` Bjorn Helgaas
2021-10-07 11:51 ` Lorenzo Pieralisi
2021-10-07 12:36 ` Marek Behún
2021-10-07 19:25 ` Bjorn Helgaas
2021-10-01 19:58 ` [PATCH 10/13] PCI: aardvark: Simplify initialization of rootcap on virtual bridge Marek Behún
2021-10-04 9:44 ` Lorenzo Pieralisi
2021-10-04 9:56 ` Marek Behún
2021-10-04 10:10 ` Lorenzo Pieralisi
2021-10-01 19:58 ` [PATCH 11/13] PCI: aardvark: Fix link training Marek Behún
2021-10-01 19:58 ` [PATCH 12/13] PCI: aardvark: Fix checking for link up via LTSSM state Marek Behún
2021-10-04 13:35 ` Lorenzo Pieralisi
2021-10-01 19:58 ` [PATCH 13/13] PCI: aardvark: Fix reporting Data Link Layer Link Active Marek Behún
2021-10-04 9:53 ` [PATCH 00/13] PCI: aardvark controller fixes Lorenzo Pieralisi
2021-10-04 10:40 ` Marek Behún
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211002163519.GA973164@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=kabel@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=pali@kernel.org \
--cc=thomas.petazzoni@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).