Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
device-tree in order to support legacy I/O port based cards which have
hardcoded I/O ports in low address space.
Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
'ranges' DT property") this driver can work with I/O windows which have
a different address for CPU than for PCI bus (unless there is some
conflict with other A37xx mapping), without needing additional support
for this in the firmware.
Note that DDR on A37xx is mapped to bus address 0x0 and that mapping of
I/O space can be set to address 0x0 too because MEM space and I/O space
are separate and so they do not conflict.
Signed-off-by: Pali Rohár <[email protected]>
Reported-by: Arnd Bergmann <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 2 +-
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 6581092c2c90..7d1b9153a901 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -163,7 +163,7 @@
*/
#address-cells = <3>;
#size-cells = <2>;
- ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
+ ranges = <0x81000000 0 0x00000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
0x82000000 0 0xe9000000 0 0xe9000000 0 0x07000000>; /* Port 0 MEM */
/* enabled by U-Boot if PCIe module is present */
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 549c3f7c5b27..a099b7787429 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -515,7 +515,7 @@
* (totaling 127 MiB) for MEM.
*/
ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
- 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
+ 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc 0>,
<0 0 0 2 &pcie_intc 1>,
--
2.20.1
> Hello Pali,
>
>> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
>> device-tree in order to support legacy I/O port based cards which have
>> hardcoded I/O ports in low address space.
>>
>> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
>>
>> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
>> 'ranges' DT property") this driver can work with I/O windows which
>> have
>
> Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> resources from 'ranges' DT property")" tag ?
Waiting for your confirmation I tried to applied it but it failed.
Did you base this patch on v5.17-rc1 ?
Gregory
>
> Gregory
>
>> a different address for CPU than for PCI bus (unless there is some
>> conflict with other A37xx mapping), without needing additional support
>> for this in the firmware.
>>
>> Note that DDR on A37xx is mapped to bus address 0x0 and that mapping of
>> I/O space can be set to address 0x0 too because MEM space and I/O space
>> are separate and so they do not conflict.
>>
>> Signed-off-by: Pali Rohár <[email protected]>
>> Reported-by: Arnd Bergmann <[email protected]>
>> ---
>> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 2 +-
>> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> index 6581092c2c90..7d1b9153a901 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> @@ -163,7 +163,7 @@
>> */
>> #address-cells = <3>;
>> #size-cells = <2>;
>> - ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
>> + ranges = <0x81000000 0 0x00000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
>> 0x82000000 0 0xe9000000 0 0xe9000000 0 0x07000000>; /* Port 0 MEM */
>>
>> /* enabled by U-Boot if PCIe module is present */
>> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> index 549c3f7c5b27..a099b7787429 100644
>> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> @@ -515,7 +515,7 @@
>> * (totaling 127 MiB) for MEM.
>> */
>> ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
>> - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
>> + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
>> interrupt-map-mask = <0 0 0 7>;
>> interrupt-map = <0 0 0 1 &pcie_intc 0>,
>> <0 0 0 2 &pcie_intc 1>,
>> --
>> 2.20.1
>>
>
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
Hello Pali,
> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> device-tree in order to support legacy I/O port based cards which have
> hardcoded I/O ports in low address space.
>
> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
>
> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> 'ranges' DT property") this driver can work with I/O windows which
> have
Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
resources from 'ranges' DT property")" tag ?
Gregory
> a different address for CPU than for PCI bus (unless there is some
> conflict with other A37xx mapping), without needing additional support
> for this in the firmware.
>
> Note that DDR on A37xx is mapped to bus address 0x0 and that mapping of
> I/O space can be set to address 0x0 too because MEM space and I/O space
> are separate and so they do not conflict.
>
> Signed-off-by: Pali Rohár <[email protected]>
> Reported-by: Arnd Bergmann <[email protected]>
> ---
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 2 +-
> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 6581092c2c90..7d1b9153a901 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -163,7 +163,7 @@
> */
> #address-cells = <3>;
> #size-cells = <2>;
> - ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> + ranges = <0x81000000 0 0x00000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> 0x82000000 0 0xe9000000 0 0xe9000000 0 0x07000000>; /* Port 0 MEM */
>
> /* enabled by U-Boot if PCIe module is present */
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 549c3f7c5b27..a099b7787429 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -515,7 +515,7 @@
> * (totaling 127 MiB) for MEM.
> */
> ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> interrupt-map-mask = <0 0 0 7>;
> interrupt-map = <0 0 0 1 &pcie_intc 0>,
> <0 0 0 2 &pcie_intc 1>,
> --
> 2.20.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> > Hello Pali,
> >
> >> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> >> device-tree in order to support legacy I/O port based cards which have
> >> hardcoded I/O ports in low address space.
> >>
> >> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
> >>
> >> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> >> 'ranges' DT property") this driver can work with I/O windows which
> >> have
> >
> > Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> > resources from 'ranges' DT property")" tag ?
>
> Waiting for your confirmation I tried to applied it but it failed.
>
> Did you base this patch on v5.17-rc1 ?
>
> Gregory
Hello! This change is breaking booting of Turris Mox kernel with older
bootloader due to bugs in bootloader. So it is not possible to remap
PCI I/O space for turris-mox.dts file. I will send a new version of this
patch just for 37xx.dtsi file and add a comment into turris-mox.dts file
it is not possible here.
> >
> > Gregory
> >
> >> a different address for CPU than for PCI bus (unless there is some
> >> conflict with other A37xx mapping), without needing additional support
> >> for this in the firmware.
> >>
> >> Note that DDR on A37xx is mapped to bus address 0x0 and that mapping of
> >> I/O space can be set to address 0x0 too because MEM space and I/O space
> >> are separate and so they do not conflict.
> >>
> >> Signed-off-by: Pali Rohár <[email protected]>
> >> Reported-by: Arnd Bergmann <[email protected]>
> >> ---
> >> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 2 +-
> >> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> index 6581092c2c90..7d1b9153a901 100644
> >> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> @@ -163,7 +163,7 @@
> >> */
> >> #address-cells = <3>;
> >> #size-cells = <2>;
> >> - ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> >> + ranges = <0x81000000 0 0x00000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> >> 0x82000000 0 0xe9000000 0 0xe9000000 0 0x07000000>; /* Port 0 MEM */
> >>
> >> /* enabled by U-Boot if PCIe module is present */
> >> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> index 549c3f7c5b27..a099b7787429 100644
> >> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> @@ -515,7 +515,7 @@
> >> * (totaling 127 MiB) for MEM.
> >> */
> >> ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> >> - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> >> + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> >> interrupt-map-mask = <0 0 0 7>;
> >> interrupt-map = <0 0 0 1 &pcie_intc 0>,
> >> <0 0 0 2 &pcie_intc 1>,
> >> --
> >> 2.20.1
> >>
> >
> > --
> > Gregory Clement, Bootlin
> > Embedded Linux and Kernel engineering
> > http://bootlin.com
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
On Wed, 2 Mar 2022 14:06:01 +0100
Andrew Lunn <[email protected]> wrote:
> On Tue, Mar 01, 2022 at 10:25:39AM +0100, Pali Rohár wrote:
> > On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> > > > Hello Pali,
> > > >
> > > >> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> > > >> device-tree in order to support legacy I/O port based cards which have
> > > >> hardcoded I/O ports in low address space.
> > > >>
> > > >> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
> > > >>
> > > >> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> > > >> 'ranges' DT property") this driver can work with I/O windows which
> > > >> have
> > > >
> > > > Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> > > > resources from 'ranges' DT property")" tag ?
> > >
> > > Waiting for your confirmation I tried to applied it but it failed.
> > >
> > > Did you base this patch on v5.17-rc1 ?
> > >
> > > Gregory
> >
> > Hello! This change is breaking booting of Turris Mox kernel with older
> > bootloader due to bugs in bootloader.
>
> Do you know what actually goes wrong?
>
> I've not been involved in the discussion, but looking at the comments
> above, not changing the space can result in non-working cards. So it
> does sound like something which in general we want to do. Does the
> current code assume the bootloader has initialized some registers with
> specific values? Can that be moved into the driver so it also works
> with older bootloaders?
No. TF-A may remap CPU PCIe window, and so U-Boot fixes these addresses
in device-tree. But the fixup function was at first written in such a
way that it assumes that the ranges propreties contains specific
values. The proposed DT change, together with the fixup function in
older U-Boot, will break ranges property to non-functional state.
See corresponding U-Boot patches
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
The last patch is not merged yet.
Marek
On Wednesday 02 March 2022 14:06:01 Andrew Lunn wrote:
> On Tue, Mar 01, 2022 at 10:25:39AM +0100, Pali Rohár wrote:
> > On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> > > > Hello Pali,
> > > >
> > > >> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> > > >> device-tree in order to support legacy I/O port based cards which have
> > > >> hardcoded I/O ports in low address space.
> > > >>
> > > >> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
> > > >>
> > > >> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> > > >> 'ranges' DT property") this driver can work with I/O windows which
> > > >> have
> > > >
> > > > Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> > > > resources from 'ranges' DT property")" tag ?
> > >
> > > Waiting for your confirmation I tried to applied it but it failed.
> > >
> > > Did you base this patch on v5.17-rc1 ?
> > >
> > > Gregory
> >
> > Hello! This change is breaking booting of Turris Mox kernel with older
> > bootloader due to bugs in bootloader.
>
> Do you know what actually goes wrong?
Yes! There is already pending fix for U-Boot which will fix this bug:
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
But because older U-Boot version is already in production we cannot
change this.
> I've not been involved in the discussion, but looking at the comments
> above, not changing the space can result in non-working cards.
And changing it would result in non-bootable kernels or crashing
kernels... So possible non-working card is better choice.
Note that non-working cards are only those which do not support 32-bit
I/O ports, which is probably only some ancient PCI or ISA cards. I have
checked 3 random mPCIe SATA controllers which use I/O ports and they
support 32-bit I/O addressing, so I guess these cards should not be
affected at all.
> So it
> does sound like something which in general we want to do. Does the
> current code assume the bootloader has initialized some registers with
> specific values? Can that be moved into the driver so it also works
> with older bootloaders?
>
> Andrew
Yes, by converting DTS to board platform data, stop using DTS and
dynamically fill board platform data by kernel code... hehe :D nothing
which we want.
Probably it could be possible to write drivers which would ignore
address resources in DTS and fill kernel structured dynamically from HW
registers, in similar way how old platform data on arm32 worked in the
past. But this is too much work for which I do not see real usage. I'm
really not going to use ISA card connected to PCI-to-ISA bridge
connected itself to PCIe-to-PCI bridge and this connected to A3720 SoC.
If somebody is really want to use this setup, then it is easier to
upgrade bootloader (patch is already pending) and manually edit DTS file
to remap I/O space to bus address 0x0. This edit can be automated by
U-Boot script (or U-Boot driver).
It is really easier to do upgrade+fix bootloader and modify DTB on the
fly than hacking kernel to support older bootloaders which are already
in use.
On Wed, 2 Mar 2022 14:15:15 +0100
Marek Behún <[email protected]> wrote:
> On Wed, 2 Mar 2022 14:06:01 +0100
> Andrew Lunn <[email protected]> wrote:
>
> > On Tue, Mar 01, 2022 at 10:25:39AM +0100, Pali Rohár wrote:
> > > On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> > > > > Hello Pali,
> > > > >
> > > > >> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> > > > >> device-tree in order to support legacy I/O port based cards which have
> > > > >> hardcoded I/O ports in low address space.
> > > > >>
> > > > >> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
> > > > >>
> > > > >> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> > > > >> 'ranges' DT property") this driver can work with I/O windows which
> > > > >> have
> > > > >
> > > > > Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> > > > > resources from 'ranges' DT property")" tag ?
> > > >
> > > > Waiting for your confirmation I tried to applied it but it failed.
> > > >
> > > > Did you base this patch on v5.17-rc1 ?
> > > >
> > > > Gregory
> > >
> > > Hello! This change is breaking booting of Turris Mox kernel with older
> > > bootloader due to bugs in bootloader.
> >
> > Do you know what actually goes wrong?
> >
> > I've not been involved in the discussion, but looking at the comments
> > above, not changing the space can result in non-working cards. So it
> > does sound like something which in general we want to do. Does the
> > current code assume the bootloader has initialized some registers with
> > specific values? Can that be moved into the driver so it also works
> > with older bootloaders?
>
> No. TF-A may remap CPU PCIe window, and so U-Boot fixes these addresses
> in device-tree. But the fixup function was at first written in such a
> way that it assumes that the ranges propreties contains specific
> values. The proposed DT change, together with the fixup function in
> older U-Boot, will break ranges property to non-functional state.
>
> See corresponding U-Boot patches
>
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>
> The last patch is not merged yet.
To explain more:
- the first patch added the ranges property fixup. After that patch
(which was applied sometime not long after 8th April 2020) U-Boot
fixes the ranges property in a way that does not work with the
proposed DT change.
- the second patch extended the fixup, but it still won't work
correctly with the proposed DT change
- the third U-Boot patch will fix this issue, afterwards the DT change
won't break PCIe. This patch is not yet merged in U-Boot
It is questionable how many users have updated U-Boot to the version
with first fixup. AFAIK we at Turris did not make an automatic update
for U-Boot yet for Turris MOX, it was done manually only for some
boards that had some problems or users wanted certain features.
But we can't change the device-tree because it will break the
functinality for some users.
What we could do is add another patch to U-Boot that would change IO
window address if certain conditions are met (for example if the ranges
proprety was not changed by the user and thus contains a specific
value that can be checked for).
Marek
On Tue, Mar 01, 2022 at 10:25:39AM +0100, Pali Roh?r wrote:
> On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> > > Hello Pali,
> > >
> > >> Remap PCI I/O space to the bus address 0x0 in the Armada 37xx
> > >> device-tree in order to support legacy I/O port based cards which have
> > >> hardcoded I/O ports in low address space.
> > >>
> > >> Some legacy PCI I/O based cards do not support 32-bit I/O addressing.
> > >>
> > >> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> > >> 'ranges' DT property") this driver can work with I/O windows which
> > >> have
> > >
> > > Should we add a "Fixes: 64f160e19e92 ("PCI: aardvark: Configure PCIe
> > > resources from 'ranges' DT property")" tag ?
> >
> > Waiting for your confirmation I tried to applied it but it failed.
> >
> > Did you base this patch on v5.17-rc1 ?
> >
> > Gregory
>
> Hello! This change is breaking booting of Turris Mox kernel with older
> bootloader due to bugs in bootloader.
Do you know what actually goes wrong?
I've not been involved in the discussion, but looking at the comments
above, not changing the space can result in non-working cards. So it
does sound like something which in general we want to do. Does the
current code assume the bootloader has initialized some registers with
specific values? Can that be moved into the driver so it also works
with older bootloaders?
Andrew
On Monday 28 February 2022 17:42:03 Gregory CLEMENT wrote:
> Waiting for your confirmation I tried to applied it but it failed.
>
> Did you base this patch on v5.17-rc1 ?
This patch is based on top of the patch which increase size of IO space
from 64 kB to 1 MB:
https://lore.kernel.org/linux-arm-kernel/878rv95umu.fsf@BL-laptop/
You have wrote that this patch was applied to mvebu/fixes but I do not
see it applied in that branch. So I this is the reason why applying
failed.
Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
'ranges' DT property") kernel can set different PCIe address on CPU and
different on the bus for the one A37xx address mapping without any firmware
support in case the bus address does not conflict with other A37xx mapping.
So remap I/O space to the bus address 0x0 to enable support for old legacy
I/O port based cards which have hardcoded I/O ports in low address space.
Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
space can be set to address 0x0 too because MEM space and I/O space are
separate and so do not conflict.
Remapping IO space on Turris Mox to different address is not possible to
due bootloader bug.
Signed-off-by: Pali Rohár <[email protected]>
Reported-by: Arnd Bergmann <[email protected]>
Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
---
Changes in v2:
* Do not remap IO space on Turris Mox
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 6581092c2c90..2838e3f65ada 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -150,17 +150,22 @@
slot-power-limit = <10000>;
/*
* U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
* contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
- * 2 size cells and also expects that the second range starts at 16 MB offset. If these
+ * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
+ * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
+ * no remapping) and that this address is the lowest from all specified ranges. If these
* conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
* space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
* for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
* This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
* U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
* https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
* https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
* https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
+ * Bug related to requirement of same child and parent addresses for first range is fixed
+ * in U-Boot version 2022.04 by following commit:
+ * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
*/
#address-cells = <3>;
#size-cells = <2>;
ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 549c3f7c5b27..a099b7787429 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -514,9 +514,9 @@
* IO at the end and the remaining seven windows
* (totaling 127 MiB) for MEM.
*/
ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
- 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
+ 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc 0>,
<0 0 0 2 &pcie_intc 1>,
<0 0 0 3 &pcie_intc 2>,
--
2.20.1
Hello! Could you review this change?
On Friday 04 March 2022 17:30:27 Pali Rohár wrote:
> Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
>
> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> 'ranges' DT property") kernel can set different PCIe address on CPU and
> different on the bus for the one A37xx address mapping without any firmware
> support in case the bus address does not conflict with other A37xx mapping.
>
> So remap I/O space to the bus address 0x0 to enable support for old legacy
> I/O port based cards which have hardcoded I/O ports in low address space.
>
> Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
> space can be set to address 0x0 too because MEM space and I/O space are
> separate and so do not conflict.
>
> Remapping IO space on Turris Mox to different address is not possible to
> due bootloader bug.
>
> Signed-off-by: Pali Rohár <[email protected]>
> Reported-by: Arnd Bergmann <[email protected]>
> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
> Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
> Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
>
> ---
> Changes in v2:
> * Do not remap IO space on Turris Mox
> ---
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 6581092c2c90..2838e3f65ada 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -150,17 +150,22 @@
> slot-power-limit = <10000>;
> /*
> * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> - * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> + * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
> + * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
> + * no remapping) and that this address is the lowest from all specified ranges. If these
> * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> * for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
> * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> + * Bug related to requirement of same child and parent addresses for first range is fixed
> + * in U-Boot version 2022.04 by following commit:
> + * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
> */
> #address-cells = <3>;
> #size-cells = <2>;
> ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 549c3f7c5b27..a099b7787429 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -514,9 +514,9 @@
> * IO at the end and the remaining seven windows
> * (totaling 127 MiB) for MEM.
> */
> ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> interrupt-map-mask = <0 0 0 7>;
> interrupt-map = <0 0 0 1 &pcie_intc 0>,
> <0 0 0 2 &pcie_intc 1>,
> <0 0 0 3 &pcie_intc 2>,
> --
> 2.20.1
>
On Thursday 10 March 2022 11:22:38 Arnd Bergmann wrote:
> On Thu, Mar 10, 2022 at 11:09 AM Pali Rohár <[email protected]> wrote:
> > On Thursday 10 March 2022 11:05:00 Gregory CLEMENT wrote:
>
> > >
> > > This patch has been refused by Arnd so I removed it from the mvebu/fixes
> > > branch so you should not apply anything on top of it.
> >
> > Ok, so what is wrong with a change which increase size of IO space to 1 MB?
>
> It should not cause any harm, but there is really no point in this if no known
> devices use more than a few bytes, and Linux only maps the first 64KB of
> the I/O space for each host bridge. I don't actually see where we limit the
> size to 64KB, so maybe that changed recently.
>
> Arnd
Ok. Anyway, I was told that DTS should describe HW properties and not to
be bound with SW implementation or SW limitations, like case here if
Linux SW limits some sizes.
Hello Pali,
> Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
>
> Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> 'ranges' DT property") kernel can set different PCIe address on CPU and
> different on the bus for the one A37xx address mapping without any firmware
> support in case the bus address does not conflict with other A37xx mapping.
>
> So remap I/O space to the bus address 0x0 to enable support for old legacy
> I/O port based cards which have hardcoded I/O ports in low address space.
>
> Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
> space can be set to address 0x0 too because MEM space and I/O space are
> separate and so do not conflict.
>
> Remapping IO space on Turris Mox to different address is not possible to
> due bootloader bug.
>
> Signed-off-by: Pali Rohár <[email protected]>
> Reported-by: Arnd Bergmann <[email protected]>
> Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
> Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
>
Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
This patch has been refused by Arnd so I removed it from the mvebu/fixes
branch so you should not apply anything on top of it.
Actually I still try to first apply the old patch and then this one but
it still fail. And it is also failed when I applied this one on a
v5.17-rc1, so I wondered on which did create this patch.
Grégory
> ---
> Changes in v2:
> * Do not remap IO space on Turris Mox
> ---
> arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 6581092c2c90..2838e3f65ada 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -150,17 +150,22 @@
> slot-power-limit = <10000>;
> /*
> * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> - * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> + * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
> + * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
> + * no remapping) and that this address is the lowest from all specified ranges. If these
> * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> * for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
> * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> + * Bug related to requirement of same child and parent addresses for first range is fixed
> + * in U-Boot version 2022.04 by following commit:
> + * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
> */
> #address-cells = <3>;
> #size-cells = <2>;
> ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index 549c3f7c5b27..a099b7787429 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -514,9 +514,9 @@
> * IO at the end and the remaining seven windows
> * (totaling 127 MiB) for MEM.
> */
> ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> interrupt-map-mask = <0 0 0 7>;
> interrupt-map = <0 0 0 1 &pcie_intc 0>,
> <0 0 0 2 &pcie_intc 1>,
> <0 0 0 3 &pcie_intc 2>,
> --
> 2.20.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
On Thursday 10 March 2022 11:23:26 Gregory CLEMENT wrote:
> Pali Rohár <[email protected]> writes:
>
> > On Thursday 10 March 2022 11:05:00 Gregory CLEMENT wrote:
> >> Hello Pali,
> >>
> >> > Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
> >> >
> >> > Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> >> > 'ranges' DT property") kernel can set different PCIe address on CPU and
> >> > different on the bus for the one A37xx address mapping without any firmware
> >> > support in case the bus address does not conflict with other A37xx mapping.
> >> >
> >> > So remap I/O space to the bus address 0x0 to enable support for old legacy
> >> > I/O port based cards which have hardcoded I/O ports in low address space.
> >> >
> >> > Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
> >> > space can be set to address 0x0 too because MEM space and I/O space are
> >> > separate and so do not conflict.
> >> >
> >> > Remapping IO space on Turris Mox to different address is not possible to
> >> > due bootloader bug.
> >> >
> >> > Signed-off-by: Pali Rohár <[email protected]>
> >> > Reported-by: Arnd Bergmann <[email protected]>
> >> > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> >> > Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
> >> > Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
> >> >
> >> Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
> >>
> >> This patch has been refused by Arnd so I removed it from the mvebu/fixes
> >> branch so you should not apply anything on top of it.
> >
> > Ok, so what is wrong with a change which increase size of IO space to 1 MB?
>
> https://lore.kernel.org/linux-arm-kernel/CAK8P3a2D8Yv+KpM4NJyP9mosieqbhHh08=mdEy+OA84Vx6FVCQ@mail.gmail.com/
The whole discussion was about remapping IO to 0 and this is addressed
in this patch. Therefore I thought that it is resolved...
Now I see that Arnd wrote also about size increasing, so some
misunderstanding from my side.
> >
> >> Actually I still try to first apply the old patch and then this one but
> >> it still fail. And it is also failed when I applied this one on a
> >> v5.17-rc1, so I wondered on which did create this patch.
> >
> > Ok, at which branch / commit should I rebase it?
>
> Please create only one single patch on top of v5.17-rc1.
Done in v3. Hopefully now it is correct.
> Thanks!
>
> Gregory
>
> >
> >> Grégory
> >>
> >> > ---
> >> > Changes in v2:
> >> > * Do not remap IO space on Turris Mox
> >> > ---
> >> > arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
> >> > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> >> > 2 files changed, 7 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> > index 6581092c2c90..2838e3f65ada 100644
> >> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> >> > @@ -150,17 +150,22 @@
> >> > slot-power-limit = <10000>;
> >> > /*
> >> > * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> >> > * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> >> > - * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> >> > + * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
> >> > + * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
> >> > + * no remapping) and that this address is the lowest from all specified ranges. If these
> >> > * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> >> > * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> >> > * for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
> >> > * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> >> > * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> >> > * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> >> > * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> >> > * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> >> > + * Bug related to requirement of same child and parent addresses for first range is fixed
> >> > + * in U-Boot version 2022.04 by following commit:
> >> > + * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
> >> > */
> >> > #address-cells = <3>;
> >> > #size-cells = <2>;
> >> > ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> >> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> > index 549c3f7c5b27..a099b7787429 100644
> >> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> >> > @@ -514,9 +514,9 @@
> >> > * IO at the end and the remaining seven windows
> >> > * (totaling 127 MiB) for MEM.
> >> > */
> >> > ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> >> > - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> >> > + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> >> > interrupt-map-mask = <0 0 0 7>;
> >> > interrupt-map = <0 0 0 1 &pcie_intc 0>,
> >> > <0 0 0 2 &pcie_intc 1>,
> >> > <0 0 0 3 &pcie_intc 2>,
> >> > --
> >> > 2.20.1
> >> >
> >>
> >> --
> >> Gregory Clement, Bootlin
> >> Embedded Linux and Kernel engineering
> >> http://bootlin.com
>
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
On Thu, Mar 10, 2022 at 11:09 AM Pali Rohár <[email protected]> wrote:
> On Thursday 10 March 2022 11:05:00 Gregory CLEMENT wrote:
> >
> > This patch has been refused by Arnd so I removed it from the mvebu/fixes
> > branch so you should not apply anything on top of it.
>
> Ok, so what is wrong with a change which increase size of IO space to 1 MB?
It should not cause any harm, but there is really no point in this if no known
devices use more than a few bytes, and Linux only maps the first 64KB of
the I/O space for each host bridge. I don't actually see where we limit the
size to 64KB, so maybe that changed recently.
Arnd
On Thursday 10 March 2022 11:05:00 Gregory CLEMENT wrote:
> Hello Pali,
>
> > Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
> >
> > Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
> > 'ranges' DT property") kernel can set different PCIe address on CPU and
> > different on the bus for the one A37xx address mapping without any firmware
> > support in case the bus address does not conflict with other A37xx mapping.
> >
> > So remap I/O space to the bus address 0x0 to enable support for old legacy
> > I/O port based cards which have hardcoded I/O ports in low address space.
> >
> > Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
> > space can be set to address 0x0 too because MEM space and I/O space are
> > separate and so do not conflict.
> >
> > Remapping IO space on Turris Mox to different address is not possible to
> > due bootloader bug.
> >
> > Signed-off-by: Pali Rohár <[email protected]>
> > Reported-by: Arnd Bergmann <[email protected]>
> > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
> > Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
> > Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
> >
> Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
>
> This patch has been refused by Arnd so I removed it from the mvebu/fixes
> branch so you should not apply anything on top of it.
Ok, so what is wrong with a change which increase size of IO space to 1 MB?
> Actually I still try to first apply the old patch and then this one but
> it still fail. And it is also failed when I applied this one on a
> v5.17-rc1, so I wondered on which did create this patch.
Ok, at which branch / commit should I rebase it?
> Grégory
>
> > ---
> > Changes in v2:
> > * Do not remap IO space on Turris Mox
> > ---
> > arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
> > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
> > 2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > index 6581092c2c90..2838e3f65ada 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> > @@ -150,17 +150,22 @@
> > slot-power-limit = <10000>;
> > /*
> > * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
> > * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
> > - * 2 size cells and also expects that the second range starts at 16 MB offset. If these
> > + * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
> > + * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
> > + * no remapping) and that this address is the lowest from all specified ranges. If these
> > * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
> > * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
> > * for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
> > * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
> > * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
> > * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
> > * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
> > * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
> > + * Bug related to requirement of same child and parent addresses for first range is fixed
> > + * in U-Boot version 2022.04 by following commit:
> > + * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
> > */
> > #address-cells = <3>;
> > #size-cells = <2>;
> > ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > index 549c3f7c5b27..a099b7787429 100644
> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> > @@ -514,9 +514,9 @@
> > * IO at the end and the remaining seven windows
> > * (totaling 127 MiB) for MEM.
> > */
> > ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
> > - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
> > + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
> > interrupt-map-mask = <0 0 0 7>;
> > interrupt-map = <0 0 0 1 &pcie_intc 0>,
> > <0 0 0 2 &pcie_intc 1>,
> > <0 0 0 3 &pcie_intc 2>,
> > --
> > 2.20.1
> >
>
> --
> Gregory Clement, Bootlin
> Embedded Linux and Kernel engineering
> http://bootlin.com
Pali Rohár <[email protected]> writes:
> On Thursday 10 March 2022 11:05:00 Gregory CLEMENT wrote:
>> Hello Pali,
>>
>> > Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
>> >
>> > Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
>> > 'ranges' DT property") kernel can set different PCIe address on CPU and
>> > different on the bus for the one A37xx address mapping without any firmware
>> > support in case the bus address does not conflict with other A37xx mapping.
>> >
>> > So remap I/O space to the bus address 0x0 to enable support for old legacy
>> > I/O port based cards which have hardcoded I/O ports in low address space.
>> >
>> > Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
>> > space can be set to address 0x0 too because MEM space and I/O space are
>> > separate and so do not conflict.
>> >
>> > Remapping IO space on Turris Mox to different address is not possible to
>> > due bootloader bug.
>> >
>> > Signed-off-by: Pali Rohár <[email protected]>
>> > Reported-by: Arnd Bergmann <[email protected]>
>> > Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
>> > Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
>> > Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
>> >
>> Cc: [email protected] # ???????????? ("arm64: dts: marvell: armada-37xx: Increase PCIe IO size from 64 KiB to 1 MiB")
>>
>> This patch has been refused by Arnd so I removed it from the mvebu/fixes
>> branch so you should not apply anything on top of it.
>
> Ok, so what is wrong with a change which increase size of IO space to 1 MB?
https://lore.kernel.org/linux-arm-kernel/CAK8P3a2D8Yv+KpM4NJyP9mosieqbhHh08=mdEy+OA84Vx6FVCQ@mail.gmail.com/
>
>> Actually I still try to first apply the old patch and then this one but
>> it still fail. And it is also failed when I applied this one on a
>> v5.17-rc1, so I wondered on which did create this patch.
>
> Ok, at which branch / commit should I rebase it?
Please create only one single patch on top of v5.17-rc1.
Thanks!
Gregory
>
>> Grégory
>>
>> > ---
>> > Changes in v2:
>> > * Do not remap IO space on Turris Mox
>> > ---
>> > arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
>> > arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
>> > 2 files changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> > index 6581092c2c90..2838e3f65ada 100644
>> > --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> > +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
>> > @@ -150,17 +150,22 @@
>> > slot-power-limit = <10000>;
>> > /*
>> > * U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
>> > * contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
>> > - * 2 size cells and also expects that the second range starts at 16 MB offset. If these
>> > + * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
>> > + * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
>> > + * no remapping) and that this address is the lowest from all specified ranges. If these
>> > * conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
>> > * space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
>> > * for IO and the rest 112 MB (64+32+16) for MEM. Controller supports 32-bit IO mapping.
>> > * This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
>> > * U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
>> > * https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
>> > * https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
>> > * https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
>> > + * Bug related to requirement of same child and parent addresses for first range is fixed
>> > + * in U-Boot version 2022.04 by following commit:
>> > + * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
>> > */
>> > #address-cells = <3>;
>> > #size-cells = <2>;
>> > ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
>> > diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> > index 549c3f7c5b27..a099b7787429 100644
>> > --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> > +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
>> > @@ -514,9 +514,9 @@
>> > * IO at the end and the remaining seven windows
>> > * (totaling 127 MiB) for MEM.
>> > */
>> > ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
>> > - 0x81000000 0 0xeff00000 0 0xeff00000 0 0x00100000>; /* Port 0 IO*/
>> > + 0x81000000 0 0x00000000 0 0xeff00000 0 0x00100000>; /* Port 0 IO */
>> > interrupt-map-mask = <0 0 0 7>;
>> > interrupt-map = <0 0 0 1 &pcie_intc 0>,
>> > <0 0 0 2 &pcie_intc 1>,
>> > <0 0 0 3 &pcie_intc 2>,
>> > --
>> > 2.20.1
>> >
>>
>> --
>> Gregory Clement, Bootlin
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
Legacy and old PCI I/O based cards do not support 32-bit I/O addressing.
Since commit 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from
'ranges' DT property") kernel can set different PCIe address on CPU and
different on the bus for the one A37xx address mapping without any firmware
support in case the bus address does not conflict with other A37xx mapping.
So remap I/O space to the bus address 0x0 to enable support for old legacy
I/O port based cards which have hardcoded I/O ports in low address space.
Note that DDR on A37xx is mapped to bus address 0x0. And mapping of I/O
space can be set to address 0x0 too because MEM space and I/O space are
separate and so do not conflict.
Remapping IO space on Turris Mox to different address is not possible to
due bootloader bug.
Signed-off-by: Pali Rohár <[email protected]>
Reported-by: Arnd Bergmann <[email protected]>
Fixes: 76f6386b25cc ("arm64: dts: marvell: Add Aardvark PCIe support for Armada 3700")
Cc: [email protected] # 64f160e19e92 ("PCI: aardvark: Configure PCIe resources from 'ranges' DT property")
Cc: [email protected] # 514ef1e62d65 ("arm64: dts: marvell: armada-37xx: Extend PCIe MEM space")
---
Changes in v3:
* Rebase on v5.17-rc1
Changes in v2:
* Do not remap IO space on Turris Mox
---
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 7 ++++++-
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
index 04da07ae4420..4b377fe807e0 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
@@ -136,19 +136,24 @@
status = "okay";
reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
/*
* U-Boot port for Turris Mox has a bug which always expects that "ranges" DT property
* contains exactly 2 ranges with 3 (child) address cells, 2 (parent) address cells and
- * 2 size cells and also expects that the second range starts at 16 MB offset. If these
+ * 2 size cells and also expects that the second range starts at 16 MB offset. Also it
+ * expects that first range uses same address for PCI (child) and CPU (parent) cells (so
+ * no remapping) and that this address is the lowest from all specified ranges. If these
* conditions are not met then U-Boot crashes during loading kernel DTB file. PCIe address
* space is 128 MB long, so the best split between MEM and IO is to use fixed 16 MB window
* for IO and the rest 112 MB (64+32+16) for MEM, despite that maximal IO size is just 64 kB.
* This bug is not present in U-Boot ports for other Armada 3700 devices and is fixed in
* U-Boot version 2021.07. See relevant U-Boot commits (the last one contains fix):
* https://source.denx.de/u-boot/u-boot/-/commit/cb2ddb291ee6fcbddd6d8f4ff49089dfe580f5d7
* https://source.denx.de/u-boot/u-boot/-/commit/c64ac3b3185aeb3846297ad7391fc6df8ecd73bf
* https://source.denx.de/u-boot/u-boot/-/commit/4a82fca8e330157081fc132a591ebd99ba02ee33
+ * Bug related to requirement of same child and parent addresses for first range is fixed
+ * in U-Boot version 2022.04 by following commit:
+ * https://source.denx.de/u-boot/u-boot/-/commit/1fd54253bca7d43d046bba4853fe5fafd034bc17
*/
#address-cells = <3>;
#size-cells = <2>;
ranges = <0x81000000 0 0xe8000000 0 0xe8000000 0 0x01000000 /* Port 0 IO */
0x82000000 0 0xe9000000 0 0xe9000000 0 0x07000000>; /* Port 0 MEM */
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 673f4906eef9..fb78ef613b29 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -497,11 +497,11 @@
* with size a power of two. Use one 64 KiB window for
* IO at the end and the remaining seven windows
* (totaling 127 MiB) for MEM.
*/
ranges = <0x82000000 0 0xe8000000 0 0xe8000000 0 0x07f00000 /* Port 0 MEM */
- 0x81000000 0 0xefff0000 0 0xefff0000 0 0x00010000>; /* Port 0 IO */
+ 0x81000000 0 0x00000000 0 0xefff0000 0 0x00010000>; /* Port 0 IO */
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc 0>,
<0 0 0 2 &pcie_intc 1>,
<0 0 0 3 &pcie_intc 2>,
<0 0 0 4 &pcie_intc 3>;
--
2.20.1