Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Le 25/06/2024 à 13:31, Etienne Dublé a écrit : The bus addresses should be isolated to each pci controller if I am not mistaken, so changing the bus address was probably just done as a workaround because of some other unknown bug. OK. Hum, could be an issue in pci core of U-Boot that expect unique bus addresses across all pci controllers. Will run some quick tests on my R5C later. OK, thanks for looking at it. Hello Jonas, Could you have a look at this issue with the 2nd interface on the nanopi r5c, and your idea about a possible issue in U-Boot core about non-unique PCI bus addresses? Thanks, Etienne -- Etienne Dublé CNRS / LIG - Bâtiment IMAG 700 avenue Centrale - 38401 St Martin d'Hères Bureau 426 - Tel 0457421431
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hello Jonas, Le 25/06/2024 à 12:46, Jonas Karlman a écrit : Ahh, linux is still missing a patch to be able to use full address ranges as a root complex. Will re-run some tests on my R5C on both u-boot and linux to see if I can replicate your issue. OK. To use the second interface in u-boot, you first need to provide a ".bind()" member function in the rtl8169 driver, otherwise both interfaces have the same name (this is my [PATCH 1/3]). Then : Hit any key to stop autoboot: 0 => pci enum => net list eth0 : RTL8169#0 52:e8:a1:60:81:e7 active eth1 : RTL8169#1 52:e8:a1:60:81:e6 => setenv ethact "RTL8169#1" => dhcp The issue appears when sending the first packet. When activating DEBUG_RTL8169_TX in rtl8169.c it prints "tx timeout/error". The issue may be that U-Boot is not fully capable of having overlapping bus addresses for multiple pci controllers. To my knowledge these addresses should be internal to the pci controller and its devices. The addresses below tells us that system memory address 0x34000+, and 0x38000+ is mapped to bus address 0x4000+ of each pci controller. I see. [...] So I verified in the downstream repository of the board vendor (https://github.com/friendlyarm/uboot-rockchip/blob/nanopi5-v2017.09/arch/arm/dts/rk3568.dtsi#L1754C21-L1754C31 and the second address there was replaced with "*0x0* *0x8000*". Then, updating this was enough to make the second interface work in u-boot. The bus addresses should be isolated to each pci controller if I am not mistaken, so changing the bus address was probably just done as a workaround because of some other unknown bug. OK. Hum, could be an issue in pci core of U-Boot that expect unique bus addresses across all pci controllers. Will run some quick tests on my R5C later. OK, thanks for looking at it. Regards, Etienne -- Etienne Dublé CNRS / LIG - Bâtiment IMAG 700 avenue Centrale - 38401 St Martin d'Hères Bureau 426 - Tel 0457421431
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hello Etienne,
On 2024-06-25 12:12, Etienne Dublé wrote:
>
> Hello Jonas,
>
> Le 25/06/2024 à 10:29, Jonas Karlman a écrit :
>> I do not understand the need for such/this patch. The changed address is
>> the internal io address that is shared with the pci controller and
>> devices.
>>
>> Do you have any issue in linux or is it only in U-Boot?, I suspect this
>> change is only a workaround to an issue only found in U-Boot.
>
> On my board (Nanopi R5C with two RTL-8125B interfaces) both RTL-8125B
> interfaces work in Linux, and only the first one works in u-boot.
> But Linux is apparently using the second PCI region and u-boot is using
> the third (with the suspected address). These PCI regions are of the
> same type.
Ahh, linux is still missing a patch to be able to use full address ranges
as a root complex.
Will re-run some tests on my R5C on both u-boot and linux to see if I can
replicate your issue.
>
>> The rtl8169 driver or pci system of U-Boot may be of fault and should
>> probably be fixed instead of changing io addresses to work around issues
>> in software. E.g rtl8169 has a static ioaddr that is shared between all
>> drivers, something that may cause issues.
>
> I am not an expert, but I really believe the issue comes from this
> address in the device tree.
The issue may be that U-Boot is not fully capable of having overlapping
bus addresses for multiple pci controllers.
To my knowledge these addresses should be internal to the pci controller
and its devices.
The addresses below tells us that system memory address 0x34000+,
and 0x38000+ is mapped to bus address 0x4000+ of each pci
controller.
> We have these pcie entries in rk3568.dtsi:
>
>
> pcie3x1: pcie@fe27 {
> [...]
> ranges = <0x0100 0x0 0xf210 0x0 0xf210 0x0
> 0x0010>,
> <0x0200 0x0 0xf220 0x0 0xf220 0x0
> 0x01e0>,
> <0x0300 *0x0* *0x4000* 0x3 0x4000
> 0x0 0x4000>;
> [...]
> }
> [...]
> pcie3x2: pcie@fe28 {
> [...]
> ranges = <0x0100 0x0 0xf010 0x0 0xf010 0x0
> 0x0010>,
> <0x0200 0x0 0xf020 0x0 0xf020 0x0
> 0x01e0>,
> <0x0300 *0x0* *0x4000* 0x3 0x8000
> 0x0 0x4000>;
> [...]
> }
>
>
> Again, I am not an expert, but it seemed suspicious to me that those two
> entries were sharing the same PCI address.
> So I verified in the downstream repository of the board vendor
> (https://github.com/friendlyarm/uboot-rockchip/blob/nanopi5-v2017.09/arch/arm/dts/rk3568.dtsi#L1754C21-L1754C31
>
> and the second address there was replaced with "*0x0* *0x8000*".
> Then, updating this was enough to make the second interface work in u-boot.
The bus addresses should be isolated to each pci controller if I am not
mistaken, so changing the bus address was probably just done as a
workaround because of some other unknown bug.
>
> Like you, I initially suspected the code of rtl8169.c, which has many
> global & static variables, so I actually spent quite a lot of time on
> refactoring it, moving things to the private struct, but could never
> make it work until I found this suspecting PCI address.
Hum, could be an issue in pci core of U-Boot that expect unique bus
addresses across all pci controllers.
Will run some quick tests on my R5C later.
Regards,
Jonas
>
> Cheers,
> Etienne
>
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hello Jonas,
Le 25/06/2024 à 10:29, Jonas Karlman a écrit :
I do not understand the need for such/this patch. The changed address is
the internal io address that is shared with the pci controller and
devices.
Do you have any issue in linux or is it only in U-Boot?, I suspect this
change is only a workaround to an issue only found in U-Boot.
On my board (Nanopi R5C with two RTL-8125B interfaces) both RTL-8125B
interfaces work in Linux, and only the first one works in u-boot.
But Linux is apparently using the second PCI region and u-boot is using
the third (with the suspected address). These PCI regions are of the
same type.
The rtl8169 driver or pci system of U-Boot may be of fault and should
probably be fixed instead of changing io addresses to work around issues
in software. E.g rtl8169 has a static ioaddr that is shared between all
drivers, something that may cause issues.
I am not an expert, but I really believe the issue comes from this
address in the device tree.
We have these pcie entries in rk3568.dtsi:
pcie3x1: pcie@fe27 {
[...]
ranges = <0x0100 0x0 0xf210 0x0 0xf210 0x0
0x0010>,
<0x0200 0x0 0xf220 0x0 0xf220 0x0
0x01e0>,
<0x0300 *0x0* *0x4000* 0x3 0x4000
0x0 0x4000>;
[...]
}
[...]
pcie3x2: pcie@fe28 {
[...]
ranges = <0x0100 0x0 0xf010 0x0 0xf010 0x0
0x0010>,
<0x0200 0x0 0xf020 0x0 0xf020 0x0
0x01e0>,
<0x0300 *0x0* *0x4000* 0x3 0x8000
0x0 0x4000>;
[...]
}
Again, I am not an expert, but it seemed suspicious to me that those two
entries were sharing the same PCI address.
So I verified in the downstream repository of the board vendor
(https://github.com/friendlyarm/uboot-rockchip/blob/nanopi5-v2017.09/arch/arm/dts/rk3568.dtsi#L1754C21-L1754C31),
and the second address there was replaced with "*0x0* *0x8000*".
Then, updating this was enough to make the second interface work in u-boot.
Like you, I initially suspected the code of rtl8169.c, which has many
global & static variables, so I actually spent quite a lot of time on
refactoring it, moving things to the private struct, but could never
make it work until I found this suspecting PCI address.
Cheers,
Etienne
--
Etienne Dublé
CNRS / LIG - Bâtiment IMAG
700 avenue Centrale - 38401 St Martin d'Hères
Bureau 426 - Tel 0457421431
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
> Date: Tue, 25 Jun 2024 10:29:48 +0200 > From: Jonas Karlman Hi Jonas, > Hi Etienne, > > On 2024-06-25 08:47, Etienne Dublé wrote: > > Hi Quentin, > > > > Le 24/06/2024 à 15:15, Quentin Schulz a écrit : > >> Hi Etienne, > >> > >> On 6/24/24 2:40 PM, ETIENNE DUBLE wrote: > >>> One of the PCI ranges was wrong in this device tree. > >>> When testing with a FriendlyElec Nanopi R5C board, the > >>> 2nd ethernet interface (labelled "wan") was not working > >>> in u-boot because of that. > >>> > >>> With the correct value (found in FriendlyElec's downstream > >>> u-boot repository), this 2nd ethernet interface now works. > >>> > >>> Signed-off-by: Etienne Dublé > >>> --- > >>> > >>> dts/upstream/src/arm64/rockchip/rk3568.dtsi | 2 +- > >> > >> dts/upstream is only for patches coming from **merged** Linux kernel > >> (i.e. releases or -rc releases or master branch from Linus Torvalds). > >> > >> We do not accept U-Boot-only patches in dts/upstream. > >> > >> Please submit the patch to the Linux kernel first and it will > >> eventually make it to that directory either via a > >> dts/update-dts-subtree.sh pull or dts/update-dts-subtree.sh pick. > >> Depending on maintainer's opinion, fixing the range in > >> arch/arm/dts/rk3568-u-boot.dtsi could be an option, but we really need > >> the patch sent to upstream Linux kernel community first. > >> > >> c.f. https://www.kernel.org/doc/html/v6.5/process/submitting-patches.html > > > > I see, I will look at it. > > In version 2 of the series I will remove this second patch and just > > mention that we are waiting for this problem to be fixed upstream, in > > the cover letter. > > I do not understand the need for such/this patch. The changed address is > the internal io address that is shared with the pci controller and > devices. > > Do you have any issue in linux or is it only in U-Boot?, I suspect this > change is only a workaround to an issue only found in U-Boot. I do see similar issues on OpenBSD, where some configurations of the outbound address translation windows work fine and others don't. It does seem to depend on the PCIe device, but the only configuration that seems to work for everything is when the translation of the mmio windows is 1:1. So for OpenBSD we build U-Boot with a pacthed device tree. The downside of using a 1:1 translation for mmio is that this reduces the size of the 32-bit PCI mmio address space. Now I can't rule out that there are bugs in the OpenBSD driver for the RK3568 PCIe controller, but the driver seems to work fine on other PCIe controllers derived from the Synopsis DesignWare stuff. Etienne's diff isn't using 1:1 translation though. It just makes sure the lower 32 bits of the address are translated 1:1. I'll see if I can retest OpenBSD with such a setup. > The rtl8169 driver or pci system of U-Boot may be of fault and should > probably be fixed instead of changing io addresses to work around issues > in software. E.g rtl8169 has a static ioaddr that is shared between all > drivers, something that may cause issues. > > Regards, > Jonas > > > > > Cheers, > > Etienne > > > >
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hi Etienne, On 2024-06-25 08:47, Etienne Dublé wrote: > Hi Quentin, > > Le 24/06/2024 à 15:15, Quentin Schulz a écrit : >> Hi Etienne, >> >> On 6/24/24 2:40 PM, ETIENNE DUBLE wrote: >>> One of the PCI ranges was wrong in this device tree. >>> When testing with a FriendlyElec Nanopi R5C board, the >>> 2nd ethernet interface (labelled "wan") was not working >>> in u-boot because of that. >>> >>> With the correct value (found in FriendlyElec's downstream >>> u-boot repository), this 2nd ethernet interface now works. >>> >>> Signed-off-by: Etienne Dublé >>> --- >>> >>> dts/upstream/src/arm64/rockchip/rk3568.dtsi | 2 +- >> >> dts/upstream is only for patches coming from **merged** Linux kernel >> (i.e. releases or -rc releases or master branch from Linus Torvalds). >> >> We do not accept U-Boot-only patches in dts/upstream. >> >> Please submit the patch to the Linux kernel first and it will >> eventually make it to that directory either via a >> dts/update-dts-subtree.sh pull or dts/update-dts-subtree.sh pick. >> Depending on maintainer's opinion, fixing the range in >> arch/arm/dts/rk3568-u-boot.dtsi could be an option, but we really need >> the patch sent to upstream Linux kernel community first. >> >> c.f. https://www.kernel.org/doc/html/v6.5/process/submitting-patches.html > > I see, I will look at it. > In version 2 of the series I will remove this second patch and just > mention that we are waiting for this problem to be fixed upstream, in > the cover letter. I do not understand the need for such/this patch. The changed address is the internal io address that is shared with the pci controller and devices. Do you have any issue in linux or is it only in U-Boot?, I suspect this change is only a workaround to an issue only found in U-Boot. The rtl8169 driver or pci system of U-Boot may be of fault and should probably be fixed instead of changing io addresses to work around issues in software. E.g rtl8169 has a static ioaddr that is shared between all drivers, something that may cause issues. Regards, Jonas > > Cheers, > Etienne >
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hi Quentin, Le 24/06/2024 à 15:15, Quentin Schulz a écrit : Hi Etienne, On 6/24/24 2:40 PM, ETIENNE DUBLE wrote: One of the PCI ranges was wrong in this device tree. When testing with a FriendlyElec Nanopi R5C board, the 2nd ethernet interface (labelled "wan") was not working in u-boot because of that. With the correct value (found in FriendlyElec's downstream u-boot repository), this 2nd ethernet interface now works. Signed-off-by: Etienne Dublé --- dts/upstream/src/arm64/rockchip/rk3568.dtsi | 2 +- dts/upstream is only for patches coming from **merged** Linux kernel (i.e. releases or -rc releases or master branch from Linus Torvalds). We do not accept U-Boot-only patches in dts/upstream. Please submit the patch to the Linux kernel first and it will eventually make it to that directory either via a dts/update-dts-subtree.sh pull or dts/update-dts-subtree.sh pick. Depending on maintainer's opinion, fixing the range in arch/arm/dts/rk3568-u-boot.dtsi could be an option, but we really need the patch sent to upstream Linux kernel community first. c.f. https://www.kernel.org/doc/html/v6.5/process/submitting-patches.html I see, I will look at it. In version 2 of the series I will remove this second patch and just mention that we are waiting for this problem to be fixed upstream, in the cover letter. Cheers, Etienne -- Etienne Dublé CNRS / LIG - Bâtiment IMAG 700 avenue Centrale - 38401 St Martin d'Hères Bureau 426 - Tel 0457421431
Re: [PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
Hi Etienne, On 6/24/24 2:40 PM, ETIENNE DUBLE wrote: One of the PCI ranges was wrong in this device tree. When testing with a FriendlyElec Nanopi R5C board, the 2nd ethernet interface (labelled "wan") was not working in u-boot because of that. With the correct value (found in FriendlyElec's downstream u-boot repository), this 2nd ethernet interface now works. Signed-off-by: Etienne Dublé --- dts/upstream/src/arm64/rockchip/rk3568.dtsi | 2 +- dts/upstream is only for patches coming from **merged** Linux kernel (i.e. releases or -rc releases or master branch from Linus Torvalds). We do not accept U-Boot-only patches in dts/upstream. Please submit the patch to the Linux kernel first and it will eventually make it to that directory either via a dts/update-dts-subtree.sh pull or dts/update-dts-subtree.sh pick. Depending on maintainer's opinion, fixing the range in arch/arm/dts/rk3568-u-boot.dtsi could be an option, but we really need the patch sent to upstream Linux kernel community first. c.f. https://www.kernel.org/doc/html/v6.5/process/submitting-patches.html Cheers, Quentin
[PATCH 2/3] rockchip: fix wrong PCI range address in rk3568 dtsi
One of the PCI ranges was wrong in this device tree. When testing with a FriendlyElec Nanopi R5C board, the 2nd ethernet interface (labelled "wan") was not working in u-boot because of that. With the correct value (found in FriendlyElec's downstream u-boot repository), this 2nd ethernet interface now works. Signed-off-by: Etienne Dublé --- dts/upstream/src/arm64/rockchip/rk3568.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dts/upstream/src/arm64/rockchip/rk3568.dtsi b/dts/upstream/src/arm64/rockchip/rk3568.dtsi index f1be76a54c..06aac034ca 100644 --- a/dts/upstream/src/arm64/rockchip/rk3568.dtsi +++ b/dts/upstream/src/arm64/rockchip/rk3568.dtsi @@ -150,7 +150,7 @@ <0x0 0xf000 0x0 0x0010>; ranges = <0x0100 0x0 0xf010 0x0 0xf010 0x0 0x0010>, <0x0200 0x0 0xf020 0x0 0xf020 0x0 0x01e0>, -<0x0300 0x0 0x4000 0x3 0x8000 0x0 0x4000>; +<0x0300 0x0 0x8000 0x3 0x8000 0x0 0x4000>; reg-names = "dbi", "apb", "config"; resets = < SRST_PCIE30X2_POWERUP>; reset-names = "pipe"; -- 2.34.1
