Message-ID: <aNPq42O1Ml3ppF2M@swlinux02>
Date: Wed, 24 Sep 2025 20:58:11 +0800
From: Randolph Lin <randolph@...estech.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
        <linux-riscv@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <jingoohan1@...il.com>, <mani@...nel.org>, <lpieralisi@...nel.org>,
        <kwilczynski@...nel.org>, <robh@...nel.org>, <bhelgaas@...gle.com>,
        <krzk+dt@...nel.org>, <conor+dt@...nel.org>, <alex@...ti.fr>,
        <aou@...s.berkeley.edu>, <palmer@...belt.com>,
        <paul.walmsley@...ive.com>, <ben717@...estech.com>,
        <inochiama@...il.com>, <thippeswamy.havalige@....com>,
        <namcao@...utronix.de>, <shradha.t@...sung.com>,
        <randolph.sklin@...il.com>, <tim609@...estech.com>
Subject: Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue

Hi Bjorn,

Sorry, I forgot to reply to the email before sending the patch.
I missed the email.

On Tue, Sep 23, 2025 at 09:42:23AM -0500, Bjorn Helgaas wrote:
> [EXTERNAL MAIL]
> 
> On Tue, Sep 23, 2025 at 07:36:43PM +0800, Randolph Lin wrote:
> > Previously, outbound iATU programming included range checks based
> > on hardware limitations. If a configuration did not meet these
> > constraints, the loop would stop immediately.
> >
> > This patch updates the behavior to enhance flexibility. Instead of
> > stopping at the first issue, it now logs a warning with details of
> > the affected window and proceeds to program the remaining iATU
> > entries.
> >
> > This enables partial configuration to complete in cases where some
> > iATU windows may not meet requirements, improving overall
> > compatibility.
> 
> It's not really clear why this is needed.  I assume it's related to
> dropping qilai_pcie_outbound_atu_addr_valid().
> 

Yes, I want to drop the previous atu_addr_valid function.

> I guess dw_pcie_prog_outbound_atu() must return an error for one of
> the QiLai ranges?  Which one, and what exactly is the problem?  I
> still suspect something wrong in the devicetree description.
> 

The main issue is not the returned error; just need to avoid terminating
the process when the configuration exceeds the hardware’s expected limits.

There are two methods to fix the issue on the Qilai SoC:

1. Simply skip the entries that do not match the designware hardware iATU limitations.
An error will be returned, but it can be ignored. On the Qilai SoC, the iATU
limitation is the 4GB boundary. Qilai SoC only need to configure iATU support
to translate addresses below the "32-bits" address range. Although 64-bits
addresses do not match the designware hardware iATU limitations, there is no
need to configure 64-bits addresses, since the connection is hard-wired.

2. Set the devicetree only 2 viewport for iATU and force using devicetree value.
This is a workaround in the devicetree, but the fix logic is not easy to document.
Instead, we should enforce the use of the viewport defined in the devicetree and
modify the designware generic code accordingly — using the viewport values
from the devicetree instead of reading them from the designware registers.
Since only two viewports are available for iATU, we should reserve one for
the configuration registers and the other for 32-bit address access.
Therefore, reverse logic still needs to be added to the designware generic code.

Method 2 adds excessive complexity to the designware generic code. Instead,
directly configuring the iATU and reporting an error when the configuration
exceeds the hardware iATU limitations is a simpler and more effective
approach to applying the fix.

Conclusion:
1. The iATU needs to be configured for 32-bits address space.
   In compliance with hardware limitations.
2. The iATU needs to be configured for config space.
   In compliance with hardware limitations.
3. The iATU needs to be configured for 64-bit address space.
   This does not comply with hardware limitations and will print an error.
   As long as it does not return an error value that terminates subsequent
   operations, it is acceptable.
   Simply skipping this entry when configuring the iATU is acceptable.

> > Signed-off-by: Randolph Lin <randolph@...estech.com>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index 952f8594b501..91ee6b903934 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -756,7 +756,7 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >               if (resource_type(entry->res) != IORESOURCE_MEM)
> >                       continue;
> >
> > -             if (pci->num_ob_windows <= ++i)
> > +             if (pci->num_ob_windows <= i)
> >                       break;
> >
> >               atu.index = i;
> > @@ -773,9 +773,10 @@ static int dw_pcie_iatu_setup(struct dw_pcie_rp *pp)
> >
> >               ret = dw_pcie_prog_outbound_atu(pci, &atu);
> >               if (ret) {
> > -                     dev_err(pci->dev, "Failed to set MEM range %pr\n",
> > -                             entry->res);
> > -                     return ret;
> > +                     dev_warn(pci->dev, "Failed to set MEM range %pr\n",
> > +                              entry->res);
> > +             } else {
> > +                     i++;
> >               }
> >       }
> >
> > --
> > 2.34.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Sincerely,
Randolph

Powered by blists - more mailing lists