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

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 [off-list ref] napisał(a):

quoted

On Thu, May 13, 2021 at 10:12:15AM +0800, Shawn Guo wrote:

quoted

On Tue, May 11, 2021 at 09:48:22AM +0000, Claudiu Manoil wrote:

quoted

quoted

-----Original Message-----
From: Shawn Guo <shawnguo@kernel.org>
Sent: Tuesday, May 11, 2021 6:07 AM

[...]

quoted

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:

quoted

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

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