[PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
@ 2021-04-07 12:34 Kornel Duleba
  2021-04-16 11:36 ` Kornel Dulęba
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Kornel Duleba @ 2021-04-07 12:34 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree, linux-kernel
  Cc: shawnguo, leoyang.li, robh+dt, mw, tn, upstream, Kornel Duleba

Currently all PCIE windows point to bus address 0x0, which does not match
the values obtained from hardware during EA.
Replace those values with CPU addresses, since in reality we
have a 1:1 mapping between the two.

Signed-off-by: Kornel Duleba <mindal@semihalf.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 262fbad8f0ec..85c62a6fabb6 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -994,19 +994,19 @@ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
 			msi-map = <0 &its 0x17 0xe>;
 			iommu-map = <0 &smmu 0x17 0xe>;
 				  /* PF0-6 BAR0 - non-prefetchable memory */
-			ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000  0x0 0x160000
+			ranges = <0x82000000 0x1 0xf8000000  0x1 0xf8000000  0x0 0x160000
 				  /* PF0-6 BAR2 - prefetchable memory */
-				  0xc2000000 0x0 0x00000000  0x1 0xf8160000  0x0 0x070000
+				  0xc2000000 0x1 0xf8160000  0x1 0xf8160000  0x0 0x070000
 				  /* PF0: VF0-1 BAR0 - non-prefetchable memory */
-				  0x82000000 0x0 0x00000000  0x1 0xf81d0000  0x0 0x020000
+				  0x82000000 0x1 0xf81d0000  0x1 0xf81d0000  0x0 0x020000
 				  /* PF0: VF0-1 BAR2 - prefetchable memory */
-				  0xc2000000 0x0 0x00000000  0x1 0xf81f0000  0x0 0x020000
+				  0xc2000000 0x1 0xf81f0000  0x1 0xf81f0000  0x0 0x020000
 				  /* PF1: VF0-1 BAR0 - non-prefetchable memory */
-				  0x82000000 0x0 0x00000000  0x1 0xf8210000  0x0 0x020000
+				  0x82000000 0x1 0xf8210000  0x1 0xf8210000  0x0 0x020000
 				  /* PF1: VF0-1 BAR2 - prefetchable memory */
-				  0xc2000000 0x0 0x00000000  0x1 0xf8230000  0x0 0x020000
+				  0xc2000000 0x1 0xf8230000  0x1 0xf8230000  0x0 0x020000
 				  /* BAR4 (PF5) - non-prefetchable memory */
-				  0x82000000 0x0 0x00000000  0x1 0xfc000000  0x0 0x400000>;
+				  0x82000000 0x1 0xfc000000  0x1 0xfc000000  0x0 0x400000>;
 
 			enetc_port0: ethernet@0,0 {
 				compatible = "fsl,enetc";
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-04-07 12:34 [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges Kornel Duleba
@ 2021-04-16 11:36 ` Kornel Dulęba
  2021-05-11  3:06 ` Shawn Guo
  2021-05-22 12:27 ` Shawn Guo
  2 siblings, 0 replies; 11+ messages in thread
From: Kornel Dulęba @ 2021-04-16 11:36 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree, linux-kernel
  Cc: shawnguo, leoyang.li, robh+dt, Marcin Wojtas, Tomasz Nowicki, upstream

Hi,

On Wed, Apr 7, 2021 at 2:35 PM Kornel Duleba <mindal@semihalf.com> wrote:
>
> Currently all PCIE windows point to bus address 0x0, which does not match
> the values obtained from hardware during EA.
> Replace those values with CPU addresses, since in reality we
> have a 1:1 mapping between the two.
>
> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 262fbad8f0ec..85c62a6fabb6 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -994,19 +994,19 @@ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
>                         msi-map = <0 &its 0x17 0xe>;
>                         iommu-map = <0 &smmu 0x17 0xe>;
>                                   /* PF0-6 BAR0 - non-prefetchable memory */
> -                       ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000  0x0 0x160000
> +                       ranges = <0x82000000 0x1 0xf8000000  0x1 0xf8000000  0x0 0x160000
>                                   /* PF0-6 BAR2 - prefetchable memory */
> -                                 0xc2000000 0x0 0x00000000  0x1 0xf8160000  0x0 0x070000
> +                                 0xc2000000 0x1 0xf8160000  0x1 0xf8160000  0x0 0x070000
>                                   /* PF0: VF0-1 BAR0 - non-prefetchable memory */
> -                                 0x82000000 0x0 0x00000000  0x1 0xf81d0000  0x0 0x020000
> +                                 0x82000000 0x1 0xf81d0000  0x1 0xf81d0000  0x0 0x020000
>                                   /* PF0: VF0-1 BAR2 - prefetchable memory */
> -                                 0xc2000000 0x0 0x00000000  0x1 0xf81f0000  0x0 0x020000
> +                                 0xc2000000 0x1 0xf81f0000  0x1 0xf81f0000  0x0 0x020000
>                                   /* PF1: VF0-1 BAR0 - non-prefetchable memory */
> -                                 0x82000000 0x0 0x00000000  0x1 0xf8210000  0x0 0x020000
> +                                 0x82000000 0x1 0xf8210000  0x1 0xf8210000  0x0 0x020000
>                                   /* PF1: VF0-1 BAR2 - prefetchable memory */
> -                                 0xc2000000 0x0 0x00000000  0x1 0xf8230000  0x0 0x020000
> +                                 0xc2000000 0x1 0xf8230000  0x1 0xf8230000  0x0 0x020000
>                                   /* BAR4 (PF5) - non-prefetchable memory */
> -                                 0x82000000 0x0 0x00000000  0x1 0xfc000000  0x0 0x400000>;
> +                                 0x82000000 0x1 0xfc000000  0x1 0xfc000000  0x0 0x400000>;
>
>                         enetc_port0: ethernet@0,0 {
>                                 compatible = "fsl,enetc";
> --
> 2.31.1
>

Have you had a chance to to review the patch? Any questions or remarks?

Regards,
Kornel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-04-07 12:34 [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges Kornel Duleba
  2021-04-16 11:36 ` Kornel Dulęba
@ 2021-05-11  3:06 ` Shawn Guo
  2021-05-11  9:48   ` Claudiu Manoil
  2021-05-22 12:27 ` Shawn Guo
  2 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2021-05-11  3:06 UTC (permalink / raw)
  To: Kornel Duleba, Claudiu Manoil
  Cc: linux-arm-kernel, devicetree, linux-kernel, leoyang.li, robh+dt,
	mw, tn, upstream

+ Claudiu

On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> Currently all PCIE windows point to bus address 0x0, which does not match
> the values obtained from hardware during EA.
> Replace those values with CPU addresses, since in reality we
> have a 1:1 mapping between the two.
> 
> Signed-off-by: Kornel Duleba <mindal@semihalf.com>

Claudiu,

Do you have any comment on this?

Shawn

> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 262fbad8f0ec..85c62a6fabb6 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -994,19 +994,19 @@ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
>  			msi-map = <0 &its 0x17 0xe>;
>  			iommu-map = <0 &smmu 0x17 0xe>;
>  				  /* PF0-6 BAR0 - non-prefetchable memory */
> -			ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000  0x0 0x160000
> +			ranges = <0x82000000 0x1 0xf8000000  0x1 0xf8000000  0x0 0x160000
>  				  /* PF0-6 BAR2 - prefetchable memory */
> -				  0xc2000000 0x0 0x00000000  0x1 0xf8160000  0x0 0x070000
> +				  0xc2000000 0x1 0xf8160000  0x1 0xf8160000  0x0 0x070000
>  				  /* PF0: VF0-1 BAR0 - non-prefetchable memory */
> -				  0x82000000 0x0 0x00000000  0x1 0xf81d0000  0x0 0x020000
> +				  0x82000000 0x1 0xf81d0000  0x1 0xf81d0000  0x0 0x020000
>  				  /* PF0: VF0-1 BAR2 - prefetchable memory */
> -				  0xc2000000 0x0 0x00000000  0x1 0xf81f0000  0x0 0x020000
> +				  0xc2000000 0x1 0xf81f0000  0x1 0xf81f0000  0x0 0x020000
>  				  /* PF1: VF0-1 BAR0 - non-prefetchable memory */
> -				  0x82000000 0x0 0x00000000  0x1 0xf8210000  0x0 0x020000
> +				  0x82000000 0x1 0xf8210000  0x1 0xf8210000  0x0 0x020000
>  				  /* PF1: VF0-1 BAR2 - prefetchable memory */
> -				  0xc2000000 0x0 0x00000000  0x1 0xf8230000  0x0 0x020000
> +				  0xc2000000 0x1 0xf8230000  0x1 0xf8230000  0x0 0x020000
>  				  /* BAR4 (PF5) - non-prefetchable memory */
> -				  0x82000000 0x0 0x00000000  0x1 0xfc000000  0x0 0x400000>;
> +				  0x82000000 0x1 0xfc000000  0x1 0xfc000000  0x0 0x400000>;
>  
>  			enetc_port0: ethernet@0,0 {
>  				compatible = "fsl,enetc";
> -- 
> 2.31.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-11  3:06 ` Shawn Guo
@ 2021-05-11  9:48   ` Claudiu Manoil
  2021-05-13  2:12     ` Shawn Guo
  0 siblings, 1 reply; 11+ messages in thread
From: Claudiu Manoil @ 2021-05-11  9:48 UTC (permalink / raw)
  To: Shawn Guo, Kornel Duleba
  Cc: linux-arm-kernel, devicetree, linux-kernel, Leo Li, robh+dt, mw,
	tn, upstream, Vladimir Oltean, Alexandru Marginean

>-----Original Message-----
>From: Shawn Guo <shawnguo@kernel.org>
>Sent: Tuesday, May 11, 2021 6:07 AM
[...]
>Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
>ranges
>
>+ Claudiu
>
>On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
>> Currently all PCIE windows point to bus address 0x0, which does not match
>> the values obtained from hardware during EA.
>> Replace those values with CPU addresses, since in reality we
>> have a 1:1 mapping between the two.
>>
>> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
>
>Claudiu,
>
>Do you have any comment on this?
>

Well, probing is still working with this change, I've just tested it.

PCI listing at boot time changes from:

pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000

to:

pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000

and looks reasonable.
Adding Vladimir and Alex just in case.

Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>

>Shawn
>
>> ---
>>  arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
>> index 262fbad8f0ec..85c62a6fabb6 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
>> @@ -994,19 +994,19 @@ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
>>  			msi-map = <0 &its 0x17 0xe>;
>>  			iommu-map = <0 &smmu 0x17 0xe>;
>>  				  /* PF0-6 BAR0 - non-prefetchable memory */
>> -			ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000 0x0 0x160000
>> +			ranges = <0x82000000 0x1 0xf8000000  0x1 0xf8000000 0x0 0x160000
>>  				  /* PF0-6 BAR2 - prefetchable memory */
>> -				  0xc2000000 0x0 0x00000000  0x1 0xf8160000 0x0 0x070000
>> +				  0xc2000000 0x1 0xf8160000  0x1 0xf8160000 0x0 0x070000
>>  				  /* PF0: VF0-1 BAR0 - non-prefetchable memory */
>> -				  0x82000000 0x0 0x00000000  0x1 0xf81d0000 0x0 0x020000
>> +				  0x82000000 0x1 0xf81d0000  0x1 0xf81d0000 0x0 0x020000
>>  				  /* PF0: VF0-1 BAR2 - prefetchable memory */
>> -				  0xc2000000 0x0 0x00000000  0x1 0xf81f0000 0x0 0x020000
>> +				  0xc2000000 0x1 0xf81f0000  0x1 0xf81f0000 0x0 0x020000
>>  				  /* PF1: VF0-1 BAR0 - non-prefetchable memory */
>> -				  0x82000000 0x0 0x00000000  0x1 0xf8210000 0x0 0x020000
>> +				  0x82000000 0x1 0xf8210000  0x1 0xf8210000 0x0 0x020000
>>  				  /* PF1: VF0-1 BAR2 - prefetchable memory */
>> -				  0xc2000000 0x0 0x00000000  0x1 0xf8230000 0x0 0x020000
>> +				  0xc2000000 0x1 0xf8230000  0x1 0xf8230000 0x0 0x020000
>>  				  /* BAR4 (PF5) - non-prefetchable memory */
>> -				  0x82000000 0x0 0x00000000  0x1 0xfc000000 0x0 0x400000>;
>> +				  0x82000000 0x1 0xfc000000  0x1 0xfc000000 0x0 0x400000>;
>>
>>  			enetc_port0: ethernet@0,0 {
>>  				compatible = "fsl,enetc";
>> --
>> 2.31.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-11  9:48   ` Claudiu Manoil
@ 2021-05-13  2:12     ` Shawn Guo
  2021-05-13 14:19       ` Vladimir Oltean
  0 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2021-05-13  2:12 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: Kornel Duleba, linux-arm-kernel, devicetree, linux-kernel,
	Leo Li, robh+dt, mw, tn, upstream, Vladimir Oltean,
	Alexandru Marginean

On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> >-----Original Message-----
> >From: Shawn Guo <shawnguo@kernel.org>
> >Sent: Tuesday, May 11, 2021 6:07 AM
> [...]
> >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> >ranges
> >
> >+ Claudiu
> >
> >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> >> Currently all PCIE windows point to bus address 0x0, which does not match
> >> the values obtained from hardware during EA.
> >> Replace those values with CPU addresses, since in reality we
> >> have a 1:1 mapping between the two.
> >>
> >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> >
> >Claudiu,
> >
> >Do you have any comment on this?
> >
> 
> Well, probing is still working with this change, I've just tested it.
> 
> PCI listing at boot time changes from:
> 
> pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> 
> to:
> 
> pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> 
> and looks reasonable.
> Adding Vladimir and Alex just in case.
> 
> Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>

Thanks, Claudiu.

Kornel,

Do we need a Fixes tag for this patch?

Shawn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-13  2:12     ` Shawn Guo
@ 2021-05-13 14:19       ` Vladimir Oltean
  2021-05-13 17:54         ` Marcin Wojtas
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-05-13 14:19 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Claudiu Manoil, Kornel Duleba, linux-arm-kernel, devicetree,
	linux-kernel, Leo Li, robh+dt, mw, tn, upstream,
	Alexandru Marginean, Bjorn Helgaas

On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:
> On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> > >-----Original Message-----
> > >From: Shawn Guo <shawnguo@kernel.org>
> > >Sent: Tuesday, May 11, 2021 6:07 AM
> > [...]
> > >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> > >ranges
> > >
> > >+ Claudiu
> > >
> > >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> > >> Currently all PCIE windows point to bus address 0x0, which does not match
> > >> the values obtained from hardware during EA.
> > >> Replace those values with CPU addresses, since in reality we
> > >> have a 1:1 mapping between the two.
> > >>
> > >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> > >
> > >Claudiu,
> > >
> > >Do you have any comment on this?
> > >
> >
> > Well, probing is still working with this change, I've just tested it.
> >
> > PCI listing at boot time changes from:
> >
> > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> >
> > to:
> >
> > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> >
> > and looks reasonable.
> > Adding Vladimir and Alex just in case.
> >
> > Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>
> Thanks, Claudiu.
>
> Kornel,
>
> Do we need a Fixes tag for this patch?
>
> Shawn

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

I am not sure whether "incorrect data that is unused" deserves a Fixes:
tag or not, probably not.

Bjorn Helgaas did point out before that "The fact that all these windows
map to PCI bus address 0 looks broken", so there's that:

https://patchwork.kernel.org/project/linux-pci/cover/20201129230743.3006978-1-kw@linux.com/

And while it does look "broken", with the Enhanced Allocation capability
and the pci-host-ecam-generic driver, there is no address translation
taking place, so no inbound/outbound windows are configured, so the
range.pci_addr calculated in devm_of_pci_get_host_bridge_resources() is
not used for anything except for printing.

FWIW here's a more complete image of what changes with Kornel's patch
("-" is before, "+" is after) - again all is limited to the dmesg output.

 pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
 pci-host-generic 1f0000000.pcie: Parsing ranges property...
-pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8000000 end 0x01f815ffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8000000
-pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8000000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8160000 end 0x01f81cffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8160000
-pci-host-generic 1f0000000.pcie:      MEM 0x01f81d0000..0x01f81effff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8160000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f81d0000..0x01f81effff -> 0x01f81d0000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f81d0000 end 0x01f81effff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f81d0000
-pci-host-generic 1f0000000.pcie:      MEM 0x01f81f0000..0x01f820ffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f81d0000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f81f0000..0x01f820ffff -> 0x01f81f0000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f81f0000 end 0x01f820ffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f81f0000
-pci-host-generic 1f0000000.pcie:      MEM 0x01f8210000..0x01f822ffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f81f0000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f8210000..0x01f822ffff -> 0x01f8210000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8210000 end 0x01f822ffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8210000
-pci-host-generic 1f0000000.pcie:      MEM 0x01f8230000..0x01f824ffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8210000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01f8230000..0x01f824ffff -> 0x01f8230000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8230000 end 0x01f824ffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8230000
-pci-host-generic 1f0000000.pcie:      MEM 0x01fc000000..0x01fc3fffff -> 0x0000000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8230000 => offset 0x0000000000
+pci-host-generic 1f0000000.pcie:      MEM 0x01fc000000..0x01fc3fffff -> 0x01fc000000
 pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01fc000000 end 0x01fc3fffff
-pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01fc000000
+pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01fc000000 => offset 0x0000000000
 pci-host-generic 1f0000000.pcie: ECAM at [mem 0x1f0000000-0x1f00fffff] for [bus 00]
 pci-host-generic 1f0000000.pcie: PCI host bridge to bus 0000:00
 pci_bus 0000:00: root bus resource [bus 00]
-pci_bus 0000:00: root bus resource [mem 0x1f8000000-0x1f815ffff] (bus address [0x00000000-0x0015ffff])
-pci_bus 0000:00: root bus resource [mem 0x1f8160000-0x1f81cffff pref] (bus address [0x00000000-0x0006ffff])
-pci_bus 0000:00: root bus resource [mem 0x1f81d0000-0x1f81effff] (bus address [0x00000000-0x0001ffff])
-pci_bus 0000:00: root bus resource [mem 0x1f81f0000-0x1f820ffff pref] (bus address [0x00000000-0x0001ffff])
-pci_bus 0000:00: root bus resource [mem 0x1f8210000-0x1f822ffff] (bus address [0x00000000-0x0001ffff])
-pci_bus 0000:00: root bus resource [mem 0x1f8230000-0x1f824ffff pref] (bus address [0x00000000-0x0001ffff])
-pci_bus 0000:00: root bus resource [mem 0x1fc000000-0x1fc3fffff] (bus address [0x00000000-0x003fffff])
+pci_bus 0000:00: root bus resource [mem 0x1f8000000-0x1f815ffff]
+pci_bus 0000:00: root bus resource [mem 0x1f8160000-0x1f81cffff pref]
+pci_bus 0000:00: root bus resource [mem 0x1f81d0000-0x1f81effff]
+pci_bus 0000:00: root bus resource [mem 0x1f81f0000-0x1f820ffff pref]
+pci_bus 0000:00: root bus resource [mem 0x1f8210000-0x1f822ffff]
+pci_bus 0000:00: root bus resource [mem 0x1f8230000-0x1f824ffff pref]
+pci_bus 0000:00: root bus resource [mem 0x1fc000000-0x1fc3fffff]
 pci 0000:00:00.0: [1957:e100] type 00 class 0x020001
 pci 0000:00:00.0: BAR 0: [mem 0x1f8000000-0x1f803ffff 64bit] (from Enhanced Allocation, properties 0x0)
 pci 0000:00:00.0: BAR 2: [mem 0x1f8160000-0x1f816ffff 64bit pref] (from Enhanced Allocation, properties 0x1)

My understanding might be wrong, but it should be possible for the PCIe
host bridge driver to initialize some of its resources by enumerating
the functions which have the EA capability, and not require the device
tree writer to add a "ranges" entry for them at all. Then this discussion
would be moot - that resource would have no way to be incorrect.

$ lspci -vvv
0000:00:00.0 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01)
	Subsystem: Freescale Semiconductor Inc Device e100
(...)
	Capabilities: [9c] Enhanced Allocation (EA): NumEntries=4
		Entry 0: Enable+ Writable- EntrySize=3
			 BAR Equivalent Indicator: BAR 0
			 PrimaryProperties: memory space, non-prefetchable
			 SecondaryProperties: entry unavailable for use, PrimaryProperties should be used
			 Base: 1f8000000
			 MaxOffset: 0003ffff
		Entry 1: Enable+ Writable- EntrySize=3
			 BAR Equivalent Indicator: BAR 2
			 PrimaryProperties: memory space, prefetchable
			 SecondaryProperties: memory space, non-prefetchable
			 Base: 1f8160000
			 MaxOffset: 0000ffff
		Entry 2: Enable+ Writable- EntrySize=3
			 BAR Equivalent Indicator: VF-BAR 0
			 PrimaryProperties: VF memory space, non-prefetchable
			 SecondaryProperties: entry unavailable for use, PrimaryProperties should be used
			 Base: 1f81d0000
			 MaxOffset: 0000ffff
		Entry 3: Enable+ Writable- EntrySize=3
			 BAR Equivalent Indicator: VF-BAR 2
			 PrimaryProperties: VF memory space, prefetchable
			 SecondaryProperties: VF memory space, prefetchable
			 Base: 1f81f0000
			 MaxOffset: 0000ffff

This information, which is already present in the hardware, needs to be
duplicated here (now I do see that the 'ranges' property declares them
larger than they really are, too):

				  /* PF0-6 BAR0 - non-prefetchable memory */
			ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000  0x0 0x160000
				  /* PF0-6 BAR2 - prefetchable memory */
				  0xc2000000 0x0 0x00000000  0x1 0xf8160000  0x0 0x070000
				  /* PF0: VF0-1 BAR0 - non-prefetchable memory */
				  0x82000000 0x0 0x00000000  0x1 0xf81d0000  0x0 0x020000
				  /* PF0: VF0-1 BAR2 - prefetchable memory */
				  0xc2000000 0x0 0x00000000  0x1 0xf81f0000  0x0 0x020000
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-13 14:19       ` Vladimir Oltean
@ 2021-05-13 17:54         ` Marcin Wojtas
  2021-05-13 18:31           ` Vladimir Oltean
  0 siblings, 1 reply; 11+ messages in thread
From: Marcin Wojtas @ 2021-05-13 17:54 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Shawn Guo, Claudiu Manoil, Kornel Duleba, linux-arm-kernel,
	devicetree, linux-kernel, Leo Li, robh+dt, tn, upstream,
	Alexandru Marginean, Bjorn Helgaas

Hi Vladimir,

czw., 13 maj 2021 o 16:19 Vladimir Oltean <vladimir.oltean@nxp.com> napisał(a):
>
> On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:
> > On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> > > >-----Original Message-----
> > > >From: Shawn Guo <shawnguo@kernel.org>
> > > >Sent: Tuesday, May 11, 2021 6:07 AM
> > > [...]
> > > >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> > > >ranges
> > > >
> > > >+ Claudiu
> > > >
> > > >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> > > >> Currently all PCIE windows point to bus address 0x0, which does not match
> > > >> the values obtained from hardware during EA.
> > > >> Replace those values with CPU addresses, since in reality we
> > > >> have a 1:1 mapping between the two.
> > > >>
> > > >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> > > >
> > > >Claudiu,
> > > >
> > > >Do you have any comment on this?
> > > >
> > >
> > > Well, probing is still working with this change, I've just tested it.
> > >
> > > PCI listing at boot time changes from:
> > >
> > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> > >
> > > to:
> > >
> > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> > >
> > > and looks reasonable.
> > > Adding Vladimir and Alex just in case.
> > >
> > > Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> >
> > Thanks, Claudiu.
> >
> > Kornel,
> >
> > Do we need a Fixes tag for this patch?
> >
> > Shawn
>
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> I am not sure whether "incorrect data that is unused" deserves a Fixes:
> tag or not, probably not.
>
> Bjorn Helgaas did point out before that "The fact that all these windows
> map to PCI bus address 0 looks broken", so there's that:
>
> https://patchwork.kernel.org/project/linux-pci/cover/20201129230743.3006978-1-kw@linux.com/
>
> And while it does look "broken", with the Enhanced Allocation capability
> and the pci-host-ecam-generic driver, there is no address translation
> taking place, so no inbound/outbound windows are configured, so the
> range.pci_addr calculated in devm_of_pci_get_host_bridge_resources() is
> not used for anything except for printing.

...in Linux. Please note Linux device trees can be used as-is by other
projects. Regardless my opinion on how that's unfortunate, FreeBSD
does additional ranges check before performing EA and fails. Since the
current DT description is imo broken and the change is transparent for
Linux, it would be great to get this change merged into tree in case
there are are no objections.

Thanks,
Marcin

>
> FWIW here's a more complete image of what changes with Kornel's patch
> ("-" is before, "+" is after) - again all is limited to the dmesg output.
>
>  pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
>  pci-host-generic 1f0000000.pcie: Parsing ranges property...
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8000000 end 0x01f815ffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8000000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8000000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8160000 end 0x01f81cffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8160000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f81d0000..0x01f81effff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8160000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f81d0000..0x01f81effff -> 0x01f81d0000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f81d0000 end 0x01f81effff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f81d0000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f81f0000..0x01f820ffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f81d0000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f81f0000..0x01f820ffff -> 0x01f81f0000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f81f0000 end 0x01f820ffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f81f0000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f8210000..0x01f822ffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f81f0000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f8210000..0x01f822ffff -> 0x01f8210000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8210000 end 0x01f822ffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8210000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01f8230000..0x01f824ffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8210000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01f8230000..0x01f824ffff -> 0x01f8230000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01f8230000 end 0x01f824ffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01f8230000
> -pci-host-generic 1f0000000.pcie:      MEM 0x01fc000000..0x01fc3fffff -> 0x0000000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01f8230000 => offset 0x0000000000
> +pci-host-generic 1f0000000.pcie:      MEM 0x01fc000000..0x01fc3fffff -> 0x01fc000000
>  pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: tmp_res start 0x01fc000000 end 0x01fc3fffff
> -pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x0000000000 => offset 0x01fc000000
> +pci-host-generic 1f0000000.pcie: devm_of_pci_get_host_bridge_resources: pci_addr 0x01fc000000 => offset 0x0000000000
>  pci-host-generic 1f0000000.pcie: ECAM at [mem 0x1f0000000-0x1f00fffff] for [bus 00]
>  pci-host-generic 1f0000000.pcie: PCI host bridge to bus 0000:00
>  pci_bus 0000:00: root bus resource [bus 00]
> -pci_bus 0000:00: root bus resource [mem 0x1f8000000-0x1f815ffff] (bus address [0x00000000-0x0015ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1f8160000-0x1f81cffff pref] (bus address [0x00000000-0x0006ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1f81d0000-0x1f81effff] (bus address [0x00000000-0x0001ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1f81f0000-0x1f820ffff pref] (bus address [0x00000000-0x0001ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1f8210000-0x1f822ffff] (bus address [0x00000000-0x0001ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1f8230000-0x1f824ffff pref] (bus address [0x00000000-0x0001ffff])
> -pci_bus 0000:00: root bus resource [mem 0x1fc000000-0x1fc3fffff] (bus address [0x00000000-0x003fffff])
> +pci_bus 0000:00: root bus resource [mem 0x1f8000000-0x1f815ffff]
> +pci_bus 0000:00: root bus resource [mem 0x1f8160000-0x1f81cffff pref]
> +pci_bus 0000:00: root bus resource [mem 0x1f81d0000-0x1f81effff]
> +pci_bus 0000:00: root bus resource [mem 0x1f81f0000-0x1f820ffff pref]
> +pci_bus 0000:00: root bus resource [mem 0x1f8210000-0x1f822ffff]
> +pci_bus 0000:00: root bus resource [mem 0x1f8230000-0x1f824ffff pref]
> +pci_bus 0000:00: root bus resource [mem 0x1fc000000-0x1fc3fffff]
>  pci 0000:00:00.0: [1957:e100] type 00 class 0x020001
>  pci 0000:00:00.0: BAR 0: [mem 0x1f8000000-0x1f803ffff 64bit] (from Enhanced Allocation, properties 0x0)
>  pci 0000:00:00.0: BAR 2: [mem 0x1f8160000-0x1f816ffff 64bit pref] (from Enhanced Allocation, properties 0x1)
>
> My understanding might be wrong, but it should be possible for the PCIe
> host bridge driver to initialize some of its resources by enumerating
> the functions which have the EA capability, and not require the device
> tree writer to add a "ranges" entry for them at all. Then this discussion
> would be moot - that resource would have no way to be incorrect.
>
> $ lspci -vvv
> 0000:00:00.0 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01)
>         Subsystem: Freescale Semiconductor Inc Device e100
> (...)
>         Capabilities: [9c] Enhanced Allocation (EA): NumEntries=4
>                 Entry 0: Enable+ Writable- EntrySize=3
>                          BAR Equivalent Indicator: BAR 0
>                          PrimaryProperties: memory space, non-prefetchable
>                          SecondaryProperties: entry unavailable for use, PrimaryProperties should be used
>                          Base: 1f8000000
>                          MaxOffset: 0003ffff
>                 Entry 1: Enable+ Writable- EntrySize=3
>                          BAR Equivalent Indicator: BAR 2
>                          PrimaryProperties: memory space, prefetchable
>                          SecondaryProperties: memory space, non-prefetchable
>                          Base: 1f8160000
>                          MaxOffset: 0000ffff
>                 Entry 2: Enable+ Writable- EntrySize=3
>                          BAR Equivalent Indicator: VF-BAR 0
>                          PrimaryProperties: VF memory space, non-prefetchable
>                          SecondaryProperties: entry unavailable for use, PrimaryProperties should be used
>                          Base: 1f81d0000
>                          MaxOffset: 0000ffff
>                 Entry 3: Enable+ Writable- EntrySize=3
>                          BAR Equivalent Indicator: VF-BAR 2
>                          PrimaryProperties: VF memory space, prefetchable
>                          SecondaryProperties: VF memory space, prefetchable
>                          Base: 1f81f0000
>                          MaxOffset: 0000ffff
>
> This information, which is already present in the hardware, needs to be
> duplicated here (now I do see that the 'ranges' property declares them
> larger than they really are, too):
>
>                                   /* PF0-6 BAR0 - non-prefetchable memory */
>                         ranges = <0x82000000 0x0 0x00000000  0x1 0xf8000000  0x0 0x160000
>                                   /* PF0-6 BAR2 - prefetchable memory */
>                                   0xc2000000 0x0 0x00000000  0x1 0xf8160000  0x0 0x070000
>                                   /* PF0: VF0-1 BAR0 - non-prefetchable memory */
>                                   0x82000000 0x0 0x00000000  0x1 0xf81d0000  0x0 0x020000
>                                   /* PF0: VF0-1 BAR2 - prefetchable memory */
>                                   0xc2000000 0x0 0x00000000  0x1 0xf81f0000  0x0 0x020000

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-13 17:54         ` Marcin Wojtas
@ 2021-05-13 18:31           ` Vladimir Oltean
  2021-05-14  8:25             ` Kornel Dulęba
  0 siblings, 1 reply; 11+ messages in thread
From: Vladimir Oltean @ 2021-05-13 18:31 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: Shawn Guo, Claudiu Manoil, Kornel Duleba, linux-arm-kernel,
	devicetree, linux-kernel, Leo Li, robh+dt, tn, upstream,
	Alexandru Marginean, Bjorn Helgaas

Hi Marcin,

On Thu, May 13, 2021 at 07:54:15PM +0200, Marcin Wojtas wrote:
> Hi Vladimir,
> 
> czw., 13 maj 2021 o 16:19 Vladimir Oltean <vladimir.oltean@nxp.com> napisał(a):
> >
> > On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:
> > > On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> > > > >-----Original Message-----
> > > > >From: Shawn Guo <shawnguo@kernel.org>
> > > > >Sent: Tuesday, May 11, 2021 6:07 AM
> > > > [...]
> > > > >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> > > > >ranges
> > > > >
> > > > >+ Claudiu
> > > > >
> > > > >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> > > > >> Currently all PCIE windows point to bus address 0x0, which does not match
> > > > >> the values obtained from hardware during EA.
> > > > >> Replace those values with CPU addresses, since in reality we
> > > > >> have a 1:1 mapping between the two.
> > > > >>
> > > > >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> > > > >
> > > > >Claudiu,
> > > > >
> > > > >Do you have any comment on this?
> > > > >
> > > >
> > > > Well, probing is still working with this change, I've just tested it.
> > > >
> > > > PCI listing at boot time changes from:
> > > >
> > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> > > >
> > > > to:
> > > >
> > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> > > >
> > > > and looks reasonable.
> > > > Adding Vladimir and Alex just in case.
> > > >
> > > > Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> > >
> > > Thanks, Claudiu.
> > >
> > > Kornel,
> > >
> > > Do we need a Fixes tag for this patch?
> > >
> > > Shawn
> >
> > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > I am not sure whether "incorrect data that is unused" deserves a Fixes:
> > tag or not, probably not.
> >
> > Bjorn Helgaas did point out before that "The fact that all these windows
> > map to PCI bus address 0 looks broken", so there's that:
> >
> > https://patchwork.kernel.org/project/linux-pci/cover/20201129230743.3006978-1-kw@linux.com/
> >
> > And while it does look "broken", with the Enhanced Allocation capability
> > and the pci-host-ecam-generic driver, there is no address translation
> > taking place, so no inbound/outbound windows are configured, so the
> > range.pci_addr calculated in devm_of_pci_get_host_bridge_resources() is
> > not used for anything except for printing.
> 
> ...in Linux. Please note Linux device trees can be used as-is by other
> projects. Regardless my opinion on how that's unfortunate, FreeBSD
> does additional ranges check before performing EA and fails. Since the
> current DT description is imo broken and the change is transparent for
> Linux, it would be great to get this change merged into tree in case
> there are are no objections.

Just for my curiosity, can you please link me to the extra FreeBSD checks?

Anyway, I'm not sure what is more "broken", to have a "ranges" property
when no address translation takes place, or for that "ranges" property
to be set to a confusing "child address space" value. That's not to say
I have an objection against Shawn merging the patch.

My main point was slightly different though, the "ranges" property is
currently mandatory, although in this case it provides no information
which cannot be retrieved directly from the config space. Properties
that have no other use except to be pedantic are, well, useless.
Maybe we can do something about that too.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-13 18:31           ` Vladimir Oltean
@ 2021-05-14  8:25             ` Kornel Dulęba
  2021-05-19 14:28               ` Kornel Dulęba
  0 siblings, 1 reply; 11+ messages in thread
From: Kornel Dulęba @ 2021-05-14  8:25 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Marcin Wojtas, Shawn Guo, Claudiu Manoil, linux-arm-kernel,
	devicetree, linux-kernel, Leo Li, robh+dt, tn, upstream,
	Alexandru Marginean, Bjorn Helgaas

Hi Vladimir,

On Thu, May 13, 2021 at 8:31 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> Hi Marcin,
>
> On Thu, May 13, 2021 at 07:54:15PM +0200, Marcin Wojtas wrote:
> > Hi Vladimir,
> >
> > czw., 13 maj 2021 o 16:19 Vladimir Oltean <vladimir.oltean@nxp.com> napisał(a):
> > >
> > > On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:
> > > > On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> > > > > >-----Original Message-----
> > > > > >From: Shawn Guo <shawnguo@kernel.org>
> > > > > >Sent: Tuesday, May 11, 2021 6:07 AM
> > > > > [...]
> > > > > >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> > > > > >ranges
> > > > > >
> > > > > >+ Claudiu
> > > > > >
> > > > > >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> > > > > >> Currently all PCIE windows point to bus address 0x0, which does not match
> > > > > >> the values obtained from hardware during EA.
> > > > > >> Replace those values with CPU addresses, since in reality we
> > > > > >> have a 1:1 mapping between the two.
> > > > > >>
> > > > > >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> > > > > >
> > > > > >Claudiu,
> > > > > >
> > > > > >Do you have any comment on this?
> > > > > >
> > > > >
> > > > > Well, probing is still working with this change, I've just tested it.
> > > > >
> > > > > PCI listing at boot time changes from:
> > > > >
> > > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> > > > >
> > > > > to:
> > > > >
> > > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> > > > >
> > > > > and looks reasonable.
> > > > > Adding Vladimir and Alex just in case.
> > > > >
> > > > > Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> > > >
> > > > Thanks, Claudiu.
> > > >
> > > > Kornel,
> > > >
> > > > Do we need a Fixes tag for this patch?
> > > >
> > > > Shawn
> > >
> > > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > I am not sure whether "incorrect data that is unused" deserves a Fixes:
> > > tag or not, probably not.
> > >
> > > Bjorn Helgaas did point out before that "The fact that all these windows
> > > map to PCI bus address 0 looks broken", so there's that:
> > >
> > > https://patchwork.kernel.org/project/linux-pci/cover/20201129230743.3006978-1-kw@linux.com/
> > >
> > > And while it does look "broken", with the Enhanced Allocation capability
> > > and the pci-host-ecam-generic driver, there is no address translation
> > > taking place, so no inbound/outbound windows are configured, so the
> > > range.pci_addr calculated in devm_of_pci_get_host_bridge_resources() is
> > > not used for anything except for printing.
> >
> > ...in Linux. Please note Linux device trees can be used as-is by other
> > projects. Regardless my opinion on how that's unfortunate, FreeBSD
> > does additional ranges check before performing EA and fails. Since the
> > current DT description is imo broken and the change is transparent for
> > Linux, it would be great to get this change merged into tree in case
> > there are are no objections.
>
> Just for my curiosity, can you please link me to the extra FreeBSD checks?

FreeBSD parses values from "ranges" and uses "rman" API to store them.
Now "rman", or Resource manager is used in the FreeBSD kernel to
manage memory regions.
In particular it checks if any two regions inserted into the same
"manager" overlap.
If they do it is treated as a fatal error, which in our case causes
the PCI driver to fail to probe.
code: https://github.com/freebsd/freebsd-src/blob/main/sys/dev/pci/pci_host_generic.c#L148.

>
> Anyway, I'm not sure what is more "broken", to have a "ranges" property
> when no address translation takes place, or for that "ranges" property
> to be set to a confusing "child address space" value. That's not to say
> I have an objection against Shawn merging the patch.
>
> My main point was slightly different though, the "ranges" property is
> currently mandatory, although in this case it provides no information
> which cannot be retrieved directly from the config space. Properties
> that have no other use except to be pedantic are, well, useless.
> Maybe we can do something about that too.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-05-14  8:25             ` Kornel Dulęba
@ 2021-05-19 14:28               ` Kornel Dulęba
  0 siblings, 0 replies; 11+ messages in thread
From: Kornel Dulęba @ 2021-05-19 14:28 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Vladimir Oltean, Marcin Wojtas, Claudiu Manoil, linux-arm-kernel,
	devicetree, linux-kernel, Leo Li, robh+dt, tn, upstream,
	Alexandru Marginean, Bjorn Helgaas

Hi Shawn,

On Fri, May 14, 2021 at 10:25 AM Kornel Dulęba <mindal@semihalf.com> wrote:
>
> Hi Vladimir,
>
> On Thu, May 13, 2021 at 8:31 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> >
> > Hi Marcin,
> >
> > On Thu, May 13, 2021 at 07:54:15PM +0200, Marcin Wojtas wrote:
> > > Hi Vladimir,
> > >
> > > czw., 13 maj 2021 o 16:19 Vladimir Oltean <vladimir.oltean@nxp.com> napisał(a):
> > > >
> > > > On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:
> > > > > On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> > > > > > >-----Original Message-----
> > > > > > >From: Shawn Guo <shawnguo@kernel.org>
> > > > > > >Sent: Tuesday, May 11, 2021 6:07 AM
> > > > > > [...]
> > > > > > >Subject: Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window
> > > > > > >ranges
> > > > > > >
> > > > > > >+ Claudiu
> > > > > > >
> > > > > > >On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> > > > > > >> Currently all PCIE windows point to bus address 0x0, which does not match
> > > > > > >> the values obtained from hardware during EA.
> > > > > > >> Replace those values with CPU addresses, since in reality we
> > > > > > >> have a 1:1 mapping between the two.
> > > > > > >>
> > > > > > >> Signed-off-by: Kornel Duleba <mindal@semihalf.com>
> > > > > > >
> > > > > > >Claudiu,
> > > > > > >
> > > > > > >Do you have any comment on this?
> > > > > > >
> > > > > >
> > > > > > Well, probing is still working with this change, I've just tested it.
> > > > > >
> > > > > > PCI listing at boot time changes from:
> > > > > >
> > > > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x0000000000
> > > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x0000000000
> > > > > >
> > > > > > to:
> > > > > >
> > > > > > pci-host-generic 1f0000000.pcie: host bridge /soc/pcie@1f0000000 ranges:
> > > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8000000..0x01f815ffff -> 0x01f8000000
> > > > > > pci-host-generic 1f0000000.pcie:      MEM 0x01f8160000..0x01f81cffff -> 0x01f8160000
> > > > > >
> > > > > > and looks reasonable.
> > > > > > Adding Vladimir and Alex just in case.
> > > > > >
> > > > > > Acked-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> > > > >
> > > > > Thanks, Claudiu.
> > > > >
> > > > > Kornel,
> > > > >
> > > > > Do we need a Fixes tag for this patch?
> > > > >
> > > > > Shawn
> > > >
> > > > Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > >
> > > > I am not sure whether "incorrect data that is unused" deserves a Fixes:
> > > > tag or not, probably not.
> > > >
> > > > Bjorn Helgaas did point out before that "The fact that all these windows
> > > > map to PCI bus address 0 looks broken", so there's that:
> > > >
> > > > https://patchwork.kernel.org/project/linux-pci/cover/20201129230743.3006978-1-kw@linux.com/
> > > >
> > > > And while it does look "broken", with the Enhanced Allocation capability
> > > > and the pci-host-ecam-generic driver, there is no address translation
> > > > taking place, so no inbound/outbound windows are configured, so the
> > > > range.pci_addr calculated in devm_of_pci_get_host_bridge_resources() is
> > > > not used for anything except for printing.
> > >
> > > ...in Linux. Please note Linux device trees can be used as-is by other
> > > projects. Regardless my opinion on how that's unfortunate, FreeBSD
> > > does additional ranges check before performing EA and fails. Since the
> > > current DT description is imo broken and the change is transparent for
> > > Linux, it would be great to get this change merged into tree in case
> > > there are are no objections.
> >
> > Just for my curiosity, can you please link me to the extra FreeBSD checks?
>
> FreeBSD parses values from "ranges" and uses "rman" API to store them.
> Now "rman", or Resource manager is used in the FreeBSD kernel to
> manage memory regions.
> In particular it checks if any two regions inserted into the same
> "manager" overlap.
> If they do it is treated as a fatal error, which in our case causes
> the PCI driver to fail to probe.
> code: https://github.com/freebsd/freebsd-src/blob/main/sys/dev/pci/pci_host_generic.c#L148.
>
> >
> > Anyway, I'm not sure what is more "broken", to have a "ranges" property
> > when no address translation takes place, or for that "ranges" property
> > to be set to a confusing "child address space" value. That's not to say
> > I have an objection against Shawn merging the patch.
> >
> > My main point was slightly different though, the "ranges" property is
> > currently mandatory, although in this case it provides no information
> > which cannot be retrieved directly from the config space. Properties
> > that have no other use except to be pedantic are, well, useless.
> > Maybe we can do something about that too.

Do you have any more remarks regarding this patch?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges
  2021-04-07 12:34 [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges Kornel Duleba
  2021-04-16 11:36 ` Kornel Dulęba
  2021-05-11  3:06 ` Shawn Guo
@ 2021-05-22 12:27 ` Shawn Guo
  2 siblings, 0 replies; 11+ messages in thread
From: Shawn Guo @ 2021-05-22 12:27 UTC (permalink / raw)
  To: Kornel Duleba
  Cc: linux-arm-kernel, devicetree, linux-kernel, leoyang.li, robh+dt,
	mw, tn, upstream

On Wed, Apr 07, 2021 at 02:34:38PM +0200, Kornel Duleba wrote:
> Currently all PCIE windows point to bus address 0x0, which does not match
> the values obtained from hardware during EA.
> Replace those values with CPU addresses, since in reality we
> have a 1:1 mapping between the two.
> 
> Signed-off-by: Kornel Duleba <mindal@semihalf.com>

Applied, thanks.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2021-05-22 12:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 12:34 [PATCH] arm64: dts: fsl-ls1028a: Correct ECAM PCIE window ranges Kornel Duleba
2021-04-16 11:36 ` Kornel Dulęba
2021-05-11  3:06 ` Shawn Guo
2021-05-11  9:48   ` Claudiu Manoil
2021-05-13  2:12     ` Shawn Guo
2021-05-13 14:19       ` Vladimir Oltean
2021-05-13 17:54         ` Marcin Wojtas
2021-05-13 18:31           ` Vladimir Oltean
2021-05-14  8:25             ` Kornel Dulęba
2021-05-19 14:28               ` Kornel Dulęba
2021-05-22 12:27 ` Shawn Guo

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