Message-ID: <20250926211023.GA2128495@bhelgaas>
Date: Fri, 26 Sep 2025 16:10:23 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Randolph Lin <randolph@...estech.com>
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

On Wed, Sep 24, 2025 at 08:58:11PM +0800, Randolph Lin wrote:
> On Tue, Sep 23, 2025 at 09:42:23AM -0500, Bjorn Helgaas wrote:
> > 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.

I don't know the DesignWare iATU design very well, so I don't know if
this issue is something unique to Qilai or if it's something that
could be handled via devicetree.

Will look for input/acks from DesignWare maintainers (Jingoo, Mani).

> 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