[PATCH] arm64: dts: rockchip: qnap-ts433: Simplify network PHY connection
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Mon Mar 4 14:44:48 PST 2024
Hello,
On Mon, Mar 04, 2024 at 05:43:08PM +0100, Diederik de Haas wrote:
> On Monday, 4 March 2024 16:59:38 CET Andrew Lunn wrote:
> > On Mon, Mar 04, 2024 at 04:32:03PM +0100, Diederik de Haas wrote:
> > > On Monday, 4 March 2024 14:09:15 CET Andrew Lunn wrote:
> > > > > > Andrew already pointed out when I posted the patch introducing the
> > > > > > gmac0 node that rgmii-id would be the preferred way to setup things.
> > > > > > Back then this didn't happen because this change broke reception of
> > > > > > network packets. However this only happend because I didn't have the
> > > > > > right phy driver loaded.
> > > > >
> > > > > It could be that the PHY is strapped to not use its internal RX delay.
> > > > > And the PHY has some weird default TX delay, so having the driver
> > > > > put some sensible values in is probably better.
> > > >
> > > > It could also be the bootloader putting odd values into the PHY.
> > > >
> > > > Anyway, it will work better with the correct PHY, and enable WoL
> > > > support.
> > >
> > > Not sure if this is the right place or way, but here we go...
> > >
> > > A few days ago on #debian-kernel at OFTC:
> > > [28.02 16:35] <ukleinek> u-boot should be out of the game
> > > [28.02 16:36] <diederik> I'm not so sure anymore. On Quartz64 Model A and
> > > B
> > > (rk3566) I had massive packet loss and tracked it down to a change in
> > > u-boot [28.02 16:37] <ukleinek> diederik: sounds like the Linux network
> > > driver on that machine could do something better
> > > [28.02 16:38] <diederik> yeah, probably
> > >
> > > I reported this about a month ago to Jonas Karlman as I bisected the
> > > problem>
> > > to a change in u-boot:
> > > > diederik at bagend:~/dev/u-boot/u-boot$ git bisect bad
> > > > 25f56459aebced8e4bb7d01061dcb1b765b197e2 is the first bad commit
> > > > commit 25f56459aebced8e4bb7d01061dcb1b765b197e2
> > > > Author: Jonas Karlman <jonas at kwiboo.se>
> > > > Date: Sun Oct 1 19:17:21 2023 +0000
> > > >
> > > > configs: rockchip: Enable ethernet driver on RK356x boards
> > > >
> > > > Enable DWC_ETH_QOS_ROCKCHIP and related PHY driver on RK356x boards
> > > > that
> > > > have an enabled gmac node.
> > >
> > > I just checked and both Quartz64 Model A and B have `phy-mode = "rgmii";`
> > > and set `tx_delay` and `rx_delay` to some (other) values.
> > > Without knowing nor understanding the details, this seem very much
> > > related?
> >
> > If you don't have a specific Linux PHY driver, you are at the mercies
> > of how the bootloader, or strapping set up the PHY. So it is always
> > best to use the correct PHY driver.
>
> This part is a bit over my head (that's ok, no need to explain it).
>
> > The Linux PHY driver should assume
> > nothing and setup the hardware from scratch, removing anything odd the
> > bootloader did. However, the fall back generic PHY driver has no chip
> > specific knowledge, so it cannot undo whatever the bootloader did.
> >
> > So, in an ideal world, we don't care about what the bootloader did,
> > just use the correct MAC and PHY driver and it should work. And if it
> > does not work, it is a Linux bug, which needs to be found and fixed.
>
> I agree.
>
> > > Not sure if this is the right place or way, but here we go...
>
> That was because it's actually a bug report (wrt Quartz64 A and B), but
> especially your remark made all the pieces I found earlier fall into place.
> Therefor I 'abused' this thread/patch to report it.
>
> I'm happy to test patches, but I lack the knowledge to come up with one
> myself.
I guess that would be:
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
index 59843a7a199c..f4d1deba3110 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-a.dts
@@ -269,7 +269,7 @@ &gmac1 {
assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru SCLK_GMAC1>, <&gmac1_clkin>;
clock_in_out = "input";
phy-supply = <&vcc_3v3>;
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
pinctrl-names = "default";
pinctrl-0 = <&gmac1m0_miim
&gmac1m0_tx_bus2
@@ -281,8 +281,6 @@ &gmac1m0_clkinout
snps,reset-active-low;
/* Reset time is 20ms, 100ms for rtl8211f */
snps,reset-delays-us = <0 20000 100000>;
- tx_delay = <0x30>;
- rx_delay = <0x10>;
phy-handle = <&rgmii_phy1>;
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
index 2d92713be2a0..ec1351a171d4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3566-quartz64-b.dts
@@ -176,7 +176,7 @@ &gmac1 {
assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru SCLK_GMAC1>;
assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru SCLK_GMAC1>, <&gmac1_clkin>;
clock_in_out = "input";
- phy-mode = "rgmii";
+ phy-mode = "rgmii-id";
phy-supply = <&vcc_3v3>;
pinctrl-names = "default";
pinctrl-0 = <&gmac1m1_miim
@@ -189,8 +189,6 @@ &gmac1m1_clkinout
snps,reset-active-low;
/* Reset time is 20ms, 100ms for rtl8211f, also works well here */
snps,reset-delays-us = <0 20000 100000>;
- tx_delay = <0x4f>;
- rx_delay = <0x24>;
phy-handle = <&rgmii_phy1>;
status = "okay";
};
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240304/c7822eb7/attachment.sig>
More information about the linux-arm-kernel
mailing list