devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
       [not found] <20250923113647.895686-2-randolph@andestech.com>
@ 2025-09-23 14:42 ` Bjorn Helgaas
  2025-09-24 12:58   ` Randolph Lin
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2025-09-23 14:42 UTC (permalink / raw)
  To: Randolph Lin
  Cc: linux-kernel, linux-pci, linux-riscv, devicetree, jingoohan1,
	mani, lpieralisi, kwilczynski, robh, bhelgaas, krzk+dt, conor+dt,
	alex, aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

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().

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.

> Signed-off-by: Randolph Lin <randolph@andestech.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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
  2025-09-23 14:42 ` [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue Bjorn Helgaas
@ 2025-09-24 12:58   ` Randolph Lin
  2025-09-26 21:10     ` Bjorn Helgaas
  0 siblings, 1 reply; 6+ messages in thread
From: Randolph Lin @ 2025-09-24 12:58 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, linux-pci, linux-riscv, devicetree, jingoohan1,
	mani, lpieralisi, kwilczynski, robh, bhelgaas, krzk+dt, conor+dt,
	alex, aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

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@andestech.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@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Sincerely,
Randolph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
  2025-09-24 12:58   ` Randolph Lin
@ 2025-09-26 21:10     ` Bjorn Helgaas
  2025-09-29 14:03       ` Rob Herring
  2025-09-29 14:25       ` Manivannan Sadhasivam
  0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2025-09-26 21:10 UTC (permalink / raw)
  To: Randolph Lin
  Cc: linux-kernel, linux-pci, linux-riscv, devicetree, jingoohan1,
	mani, lpieralisi, kwilczynski, robh, bhelgaas, krzk+dt, conor+dt,
	alex, aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

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@andestech.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@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> Sincerely,
> Randolph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
  2025-09-26 21:10     ` Bjorn Helgaas
@ 2025-09-29 14:03       ` Rob Herring
  2025-09-30 12:05         ` Randolph Lin
  2025-09-29 14:25       ` Manivannan Sadhasivam
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2025-09-29 14:03 UTC (permalink / raw)
  To: Bjorn Helgaas, Randolph Lin
  Cc: linux-kernel, linux-pci, linux-riscv, devicetree, jingoohan1,
	mani, lpieralisi, kwilczynski, bhelgaas, krzk+dt, conor+dt, alex,
	aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

On Fri, Sep 26, 2025 at 4:10 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> 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.

I believe it should probably be handled in the DT. The iATU is
programmed based on the bridge window resources which are in turn
based on DT ranges and dma-ranges. If there's a failure, then
ranges/dma-ranges is wrong. Or the driver could adjust the bridge
window resources before programming the iATU.

Please provide what the DT looks like (for ranges/dma-ranges) and
where problem entry is.

Rob

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
  2025-09-26 21:10     ` Bjorn Helgaas
  2025-09-29 14:03       ` Rob Herring
@ 2025-09-29 14:25       ` Manivannan Sadhasivam
  1 sibling, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-29 14:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Randolph Lin, linux-kernel, linux-pci, linux-riscv, devicetree,
	jingoohan1, lpieralisi, kwilczynski, robh, bhelgaas, krzk+dt,
	conor+dt, alex, aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

On Fri, Sep 26, 2025 at 04:10:23PM -0500, Bjorn Helgaas wrote:
> 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.
> > 

Why does the DT supplies broken 'ranges' in the first place? DT describes the
hardware and if the hardware only supports 32bit OB window, then DT has to
supply the range accordingly. It is not currently clear on what issue you are
trying to solve by skipping the range check for broken resources.

> > 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.
> > 

'num-viewport' property is deprecated. So should not be used in new DTs.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue
  2025-09-29 14:03       ` Rob Herring
@ 2025-09-30 12:05         ` Randolph Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Randolph Lin @ 2025-09-30 12:05 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Helgaas, linux-kernel, linux-pci, linux-riscv, devicetree,
	jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas, krzk+dt,
	conor+dt, alex, aou, palmer, paul.walmsley, ben717, inochiama,
	thippeswamy.havalige, namcao, shradha.t, randolph.sklin, tim609

Hi Rob,

On Mon, Sep 29, 2025 at 09:03:59AM -0500, Rob Herring wrote:
> [EXTERNAL MAIL]
> 
> On Fri, Sep 26, 2025 at 4:10 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > 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.
> 
> I believe it should probably be handled in the DT. The iATU is
> programmed based on the bridge window resources which are in turn
> based on DT ranges and dma-ranges. If there's a failure, then
> ranges/dma-ranges is wrong. Or the driver could adjust the bridge
> window resources before programming the iATU.
> 

Thank you very much. That’s a great hint for me.

My driver can handle most of the logic within the .init callback of the
dw_pcie_host_ops structure. This includes parsing the Device Tree data
and performing the required pre-initialization steps, such as counting
how many bridge window resources comply with the iATU limitations and
verifying the 32-bit address mappings within those bridge window
resources.

The following additional logic is still required to ensure
pci->num_ob_windows correctly reflects the driver’s pre-initialization
value, with the current approach remaining more generic and purposeful.

--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -907,8 +907,10 @@ void dw_pcie_iatu_detect(struct dw_pcie *pci)
                max = 0;
        }

-       pci->num_ob_windows = ob;
-       pci->num_ib_windows = ib;
+       if (!pci->num_ob_windows)
+               pci->num_ob_windows = ob;
+       if (!pci->num_ib_windows)
+               pci->num_ib_windows = ib;
        pci->region_align = 1 << fls(min);
        pci->region_limit = (max << 32) | (SZ_4G - 1);

> Please provide what the DT looks like (for ranges/dma-ranges) and
> where problem entry is.
> 

bus@80000000 {
	compatible = "simple-bus";
	#address-cells = <2>;
	#size-cells = <2>;
	dma-ranges = <0x44 0x00000000 0x04 0x00000000 0x04 0x00000000>;
	ranges = <0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
		 <0x00 0x04000000 0x00 0x04000000 0x00 0x00001000>,
		 <0x00 0x00000000 0x20 0x00000000 0x20 0x00000000>;

	pci@80000000 {
		compatible = "andestech,qilai-pcie";
		device_type = "pci";
		reg = <0x00 0x80000000 0x00 0x20000000>, /* DBI registers */
		      <0x00 0x04000000 0x00 0x00001000>, /* APB registers */
		      <0x00 0x00000000 0x00 0x00010000>; /* Configuration registers */
		reg-names = "dbi", "apb", "config";

		linux,pci-domain = <0>;
		bus-range = <0x0 0xff>;
		num-viewport = <4>;
		#address-cells = <3>;
		#size-cells = <2>;
		ranges = <0x02000000 0x00 0x10000000 0x00 0x10000000 0x00 0xf0000000>,
			 <0x43000000 0x01 0x00000000 0x01 0x00000000 0x1f 0x00000000>;

Just look at the last "ranges" property — the first line is the only one
we want to program into the iATU, as its size is below SZ_4G
(the iATU region limitation for this SoC).

The next line exceeds the iATU region limitation; therefore, we do not
want to program it into the iATU. It is natively wire-connected by
design and does not need to pass through the iATU.

> Rob

Sincerely,
Randolph

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-09-30 12:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250923113647.895686-2-randolph@andestech.com>
2025-09-23 14:42 ` [PATCH v3 1/5] PCI: dwc: Skip failed outbound iATU and continue Bjorn Helgaas
2025-09-24 12:58   ` Randolph Lin
2025-09-26 21:10     ` Bjorn Helgaas
2025-09-29 14:03       ` Rob Herring
2025-09-30 12:05         ` Randolph Lin
2025-09-29 14:25       ` Manivannan Sadhasivam

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).