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 <[email protected]>
---
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
Hi,
On Wed, Apr 7, 2021 at 2:35 PM Kornel Duleba <[email protected]> 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 <[email protected]>
> ---
> 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
+ 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 <[email protected]>
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
>
>-----Original Message-----
>From: Shawn Guo <[email protected]>
>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 <[email protected]>
>
>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 <[email protected]>
>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
>>
On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:
> >-----Original Message-----
> >From: Shawn Guo <[email protected]>
> >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 <[email protected]>
> >
> >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 <[email protected]>
Thanks, Claudiu.
Kornel,
Do we need a Fixes tag for this patch?
Shawn
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 <[email protected]>
> > >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 <[email protected]>
> > >
> > >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 <[email protected]>
>
> Thanks, Claudiu.
>
> Kornel,
>
> Do we need a Fixes tag for this patch?
>
> Shawn
Reviewed-by: Vladimir Oltean <[email protected]>
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/[email protected]/
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
Hi Vladimir,
czw., 13 maj 2021 o 16:19 Vladimir Oltean <[email protected]> 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 <[email protected]>
> > > >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 <[email protected]>
> > > >
> > > >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 <[email protected]>
> >
> > Thanks, Claudiu.
> >
> > Kornel,
> >
> > Do we need a Fixes tag for this patch?
> >
> > Shawn
>
> Reviewed-by: Vladimir Oltean <[email protected]>
>
> 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/[email protected]/
>
> 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
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 <[email protected]> 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 <[email protected]>
> > > > >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 <[email protected]>
> > > > >
> > > > >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 <[email protected]>
> > >
> > > Thanks, Claudiu.
> > >
> > > Kornel,
> > >
> > > Do we need a Fixes tag for this patch?
> > >
> > > Shawn
> >
> > Reviewed-by: Vladimir Oltean <[email protected]>
> >
> > 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/[email protected]/
> >
> > 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.
Hi Vladimir,
On Thu, May 13, 2021 at 8:31 PM Vladimir Oltean <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > >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 <[email protected]>
> > > > > >
> > > > > >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 <[email protected]>
> > > >
> > > > Thanks, Claudiu.
> > > >
> > > > Kornel,
> > > >
> > > > Do we need a Fixes tag for this patch?
> > > >
> > > > Shawn
> > >
> > > Reviewed-by: Vladimir Oltean <[email protected]>
> > >
> > > 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/[email protected]/
> > >
> > > 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.
Hi Shawn,
On Fri, May 14, 2021 at 10:25 AM Kornel Dulęba <[email protected]> wrote:
>
> Hi Vladimir,
>
> On Thu, May 13, 2021 at 8:31 PM Vladimir Oltean <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > > > > >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 <[email protected]>
> > > > > > >
> > > > > > >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 <[email protected]>
> > > > >
> > > > > Thanks, Claudiu.
> > > > >
> > > > > Kornel,
> > > > >
> > > > > Do we need a Fixes tag for this patch?
> > > > >
> > > > > Shawn
> > > >
> > > > Reviewed-by: Vladimir Oltean <[email protected]>
> > > >
> > > > 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/[email protected]/
> > > >
> > > > 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?
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 <[email protected]>
Applied, thanks.