| Message ID | 20221116200150.4657-2-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [linux-next-v2,1/5] arm64: dts: rockchip: Fix gmac phy mode to rgmii on Rock 3A SBC. | expand |
On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote: > > On rk356x ethernet phy support reduced media independent interface (RMII) > and reduced gigabit media independent interface (RGMII). > So set the phy mode to rgmii to support clock delay, also > add TX and RX delay for phy-mode. Controller based clock delay, the various rgmii-id modes (rgmii-txid and rgmii-rxid are also valid) apply the delays in the phy. They are usually at a fixed amount, but some phys support variable delays. You want your commit message to accurately describe the problem, such as "In rgmii-id mode, the phy on the rock-3a is unreliable due to incorrect delays. Switch to rgmii mode in order to handle the delays in the controller." > > Fix following warning > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 I've been meaning to make this a dev_debug message, because in the various rgmii-id modes it is feasible for these to be non-existent in the device-tree. In rgmii-id mode these are disabled, no matter what they are set to in the dt. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > V2: Fix commit message and added the RX and TX clock delay. > --- > arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > index ea74ba32fbbd..e1c75532dcee 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > @@ -253,13 +253,16 @@ &gmac1 { > assigned-clock-rates = <0>, <125000000>; > clock_in_out = "output"; > phy-handle = <&rgmii_phy1>; > - phy-mode = "rgmii-id"; > + phy-mode = "rgmii"; > pinctrl-names = "default"; > pinctrl-0 = <&gmac1m1_miim > &gmac1m1_tx_bus2 > &gmac1m1_rx_bus2 > &gmac1m1_rgmii_clk > &gmac1m1_rgmii_bus>; > + > + tx_delay = <0x4f>; > + rx_delay = <0x26>; These are pretty far off from the default, have you verified the upper and lower bounds for the rock-3a? These should be roughly in the middle of that range. > status = "okay"; > }; > > -- > 2.38.1 >
Hi Peter On Thu, 17 Nov 2022 at 02:06, Peter Geis <pgwipeout@gmail.com> wrote: > > On Wed, Nov 16, 2022 at 3:02 PM Anand Moon <linux.amoon@gmail.com> wrote: > > > > On rk356x ethernet phy support reduced media independent interface (RMII) > > and reduced gigabit media independent interface (RGMII). > > So set the phy mode to rgmii to support clock delay, also > > add TX and RX delay for phy-mode. > > Controller based clock delay, the various rgmii-id modes (rgmii-txid > and rgmii-rxid are also valid) apply the delays in the phy. They are > usually at a fixed amount, but some phys support variable delays. > > You want your commit message to accurately describe the problem, such > as "In rgmii-id mode, the phy on the rock-3a is unreliable due to > incorrect delays. Switch to rgmii mode in order to handle the delays > in the controller." > > > > > Fix following warning > > > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 > > I've been meaning to make this a dev_debug message, because in the > various rgmii-id modes it is feasible for these to be non-existent in > the device-tree. In rgmii-id mode these are disabled, no matter what > they are set to in the dt. As per the public datasheet share by Radxa below [0] https://dl.radxa.com/rock3/docs/hw/datasheet/RTL8211F-CG-Datasheet.pdf This Ethernet controller supports RGMII mode. > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > V2: Fix commit message and added the RX and TX clock delay. > > --- > > arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > > index ea74ba32fbbd..e1c75532dcee 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > > @@ -253,13 +253,16 @@ &gmac1 { > > assigned-clock-rates = <0>, <125000000>; > > clock_in_out = "output"; > > phy-handle = <&rgmii_phy1>; > > - phy-mode = "rgmii-id"; > > + phy-mode = "rgmii"; > > pinctrl-names = "default"; > > pinctrl-0 = <&gmac1m1_miim > > &gmac1m1_tx_bus2 > > &gmac1m1_rx_bus2 > > &gmac1m1_rgmii_clk > > &gmac1m1_rgmii_bus>; > > + > > + tx_delay = <0x4f>; > > + rx_delay = <0x26>; > > These are pretty far off from the default, have you verified the upper > and lower bounds for the rock-3a? These should be roughly in the > middle of that range. > No I have not tested this way but as per the datasheet, it supports TXDLY Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG) RXDLY Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG) I feel I should ignore the above warning and add it below. rx-internal-delay-ps = <2000>; tx-internal-delay-ps = <2000>; Thanks -Anand > > status = "okay"; > > }; > > > > -- > > 2.38.1 > >
Hi Anand, On 11/16/22 21:01, Anand Moon wrote: > On rk356x ethernet phy support reduced media independent interface (RMII) > and reduced gigabit media independent interface (RGMII). > So set the phy mode to rgmii to support clock delay, also > add TX and RX delay for phy-mode. Based on this commit message I still don't understand what you are actually trying to fix here. If you encounter network problems/stability issues, please let me know what test triggers the faulty behavior. Please describe the problem you are facing in detail here or in the cover letter. > Fix following warning > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 If the only purpose of this patch is to get rid of this warnings, it may make sense to set them to dev_dbg as Peter pointed out. Best regards, Michael > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > V2: Fix commit message and added the RX and TX clock delay. > --- > arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > index ea74ba32fbbd..e1c75532dcee 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts > @@ -253,13 +253,16 @@ &gmac1 { > assigned-clock-rates = <0>, <125000000>; > clock_in_out = "output"; > phy-handle = <&rgmii_phy1>; > - phy-mode = "rgmii-id"; > + phy-mode = "rgmii"; > pinctrl-names = "default"; > pinctrl-0 = <&gmac1m1_miim > &gmac1m1_tx_bus2 > &gmac1m1_rx_bus2 > &gmac1m1_rgmii_clk > &gmac1m1_rgmii_bus>; > + > + tx_delay = <0x4f>; > + rx_delay = <0x26>; > status = "okay"; > }; >
Hi Michael, On Fri, 18 Nov 2022 at 12:33, Michael Riesch <michael.riesch@wolfvision.net> wrote: > > Hi Anand, > > On 11/16/22 21:01, Anand Moon wrote: > > On rk356x ethernet phy support reduced media independent interface (RMII) > > and reduced gigabit media independent interface (RGMII). > > So set the phy mode to rgmii to support clock delay, also > > add TX and RX delay for phy-mode. > > Based on this commit message I still don't understand what you are > actually trying to fix here. If you encounter network problems/stability > issues, please let me know what test triggers the faulty behavior. > Please describe the problem you are facing in detail here or in the > cover letter. > Ok, Ethernet does not work on my Radxa 3A see boot logs. [0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2 Please find this updated commit message. As per the schematic and datasheet PHY mode is RGMII Use 2ns clock delay to RXC for RXD and TXC for TXD latching. > > Fix the following warning > > > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 > > If the only purpose of this patch is to get rid of this warnings, it may No, the intent is to fix the PHY mode to RGMII and fix the delay. [ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID > make sense to set them to dev_dbg as Peter pointed out. > Ok, will update this in the next version. > Best regards, > Michael > Thanks -Anand
On Fri, Nov 18, 2022 at 4:35 AM Anand Moon <linux.amoon@gmail.com> wrote: > > Hi Michael, > > On Fri, 18 Nov 2022 at 12:33, Michael Riesch > <michael.riesch@wolfvision.net> wrote: > > > > Hi Anand, > > > > On 11/16/22 21:01, Anand Moon wrote: > > > On rk356x ethernet phy support reduced media independent interface (RMII) > > > and reduced gigabit media independent interface (RGMII). > > > So set the phy mode to rgmii to support clock delay, also > > > add TX and RX delay for phy-mode. > > > > Based on this commit message I still don't understand what you are > > actually trying to fix here. If you encounter network problems/stability > > issues, please let me know what test triggers the faulty behavior. > > Please describe the problem you are facing in detail here or in the > > cover letter. > > > > Ok, Ethernet does not work on my Radxa 3A see boot logs. > > [0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2 > > Please find this updated commit message. > > As per the schematic and datasheet PHY mode is RGMII > Use 2ns clock delay to RXC for RXD and TXC for TXD latching. rgmii-id mode does exactly this in the phy (your realtek chip). By setting the mode to rgmii, you're telling the phy that delays are set elsewhere, either in hardware or in the controller. You're then handling them in the controller. While the delays aren't documented in the TRM, I've long suspected that the defaults of 0x30 and 0x10 equate to the standard 2ns delay. So you're setting the delays much higher than the default means you need to add *more* than the standard 2ns delay for your device to work. This is why I've been asking if you have tested these. You need to set each value and find the lowest and highest possible values that work, then take the median value between those two. > > > > Fix the following warning > > > > > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > > > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > > > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > > > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 > > > > If the only purpose of this patch is to get rid of this warnings, it may > > No, the intent is to fix the PHY mode to RGMII and fix the delay. > [ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID > > > make sense to set them to dev_dbg as Peter pointed out. > > > Ok, will update this in the next version. > > > Best regards, > > Michael > > > Thanks > -Anand
Hi Peter / Michael On Fri, 18 Nov 2022 at 23:43, Peter Geis <pgwipeout@gmail.com> wrote: > > On Fri, Nov 18, 2022 at 4:35 AM Anand Moon <linux.amoon@gmail.com> wrote: > > > > Hi Michael, > > > > On Fri, 18 Nov 2022 at 12:33, Michael Riesch > > <michael.riesch@wolfvision.net> wrote: > > > > > > Hi Anand, > > > > > > On 11/16/22 21:01, Anand Moon wrote: > > > > On rk356x ethernet phy support reduced media independent interface (RMII) > > > > and reduced gigabit media independent interface (RGMII). > > > > So set the phy mode to rgmii to support clock delay, also > > > > add TX and RX delay for phy-mode. > > > > > > Based on this commit message I still don't understand what you are > > > actually trying to fix here. If you encounter network problems/stability > > > issues, please let me know what test triggers the faulty behavior. > > > Please describe the problem you are facing in detail here or in the > > > cover letter. > > > > > > > Ok, Ethernet does not work on my Radxa 3A see boot logs. > > > > [0] https://gist.github.com/moonlinux/bb56c787031226fbb9f69121564e76a2 > > > > Please find this updated commit message. > > > > As per the schematic and datasheet PHY mode is RGMII > > Use 2ns clock delay to RXC for RXD and TXC for TXD latching. > > rgmii-id mode does exactly this in the phy (your realtek chip). By > setting the mode to rgmii, you're telling the phy that delays are set > elsewhere, either in hardware or in the controller. You're then > handling them in the controller. While the delays aren't documented in > the TRM, I've long suspected that the defaults of 0x30 and 0x10 equate > to the standard 2ns delay. So you're setting the delays much higher > than the default means you need to add *more* than the standard 2ns > delay for your device to work. > TX and RX clock delay might have been updated for ID: 0x30, Synopsys ID: 0x51 DWMAC4/5 As per the schematic [0] https://dl.radxa.com/rock3/docs/hw/3a/rock3a_v1.3_sch.pdf Pull-up for additional 2ns delay to RXC for data latching Pull-up for additional 2ns delay to TXC for data latching As per the datasheet [1] https://dl.radxa.com/rock3/docs/hw/datasheet/RTL8211F-CG-Datasheet.pdf TXDLY RGMII Transmit Clock Timing Control. 1: Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG) 0: No delay (via 4.7k-ohm to GND) RXDLY RGMII Receive Clock Timing Control. 1: Add 2ns delay to RXC for RXD latching (via 4.7k-ohm to DVDD_RG) 0: No delay (via 4.7k-ohm to GND) tx_delay and rx_delay might be suitable for old DWMAC hence it does not apply to this new Ethernet controller. I have tested with remove these from the dts and adding the following as this is required for rgmii mode. rx-internal-delay-ps = <2000>; tx-internal-delay-ps = <2000>; I don't mind setting the defaults of 0x30 and 0x10 to equate to the standard 2ns delay. but it does not have any effect on the current ethernet controller. > This is why I've been asking if you have tested these. You need to set > each value and find the lowest and highest possible values that work, > then take the median value between those two. > Yes, I have tested with and without those values. Thanks -Anand > > > > > > Fix the following warning > > > > > > > > [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. > > > > [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 > > > > [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. > > > > [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 > > > > > > If the only purpose of this patch is to get rid of this warnings, it may > > > > No, the intent is to fix the PHY mode to RGMII and fix the delay. > > [ 7.066357] rk_gmac-dwmac fe010000.ethernet: init for RGMII_ID > > > > > make sense to set them to dev_dbg as Peter pointed out. > > > > > Ok, will update this in the next version. > > > > > Best regards, > > > Michael > > > > > Thanks > > -Anand
On Wed, 16 Nov 2022 20:01:43 +0000, Anand Moon wrote: > On rk356x ethernet phy support reduced media independent interface (RMII) > and reduced gigabit media independent interface (RGMII). > So set the phy mode to rgmii to support clock delay, also > add TX and RX delay for phy-mode. > > Fix following warning > > [...] Applied, thanks! [2/5] arm64: dts: rockchip: Add support of external clock to ethernet node on Rock 3A SBC commit: ef9f4b4a50206bedd931f45dd9fd57fd4c1714a6 [3/5] arm64: dts: rockchip: Add support of regulator for ethernet node on Rock 3A SBC commit: 79aa02ddc682558edb9bd56522ad841759c99201 Best regards,
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts index ea74ba32fbbd..e1c75532dcee 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts @@ -253,13 +253,16 @@ &gmac1 { assigned-clock-rates = <0>, <125000000>; clock_in_out = "output"; phy-handle = <&rgmii_phy1>; - phy-mode = "rgmii-id"; + phy-mode = "rgmii"; pinctrl-names = "default"; pinctrl-0 = <&gmac1m1_miim &gmac1m1_tx_bus2 &gmac1m1_rx_bus2 &gmac1m1_rgmii_clk &gmac1m1_rgmii_bus>; + + tx_delay = <0x4f>; + rx_delay = <0x26>; status = "okay"; };
On rk356x ethernet phy support reduced media independent interface (RMII) and reduced gigabit media independent interface (RGMII). So set the phy mode to rgmii to support clock delay, also add TX and RX delay for phy-mode. Fix following warning [ 7.365215] rk_gmac-dwmac fe010000.ethernet: Can not read property: tx_delay. [ 7.365219] rk_gmac-dwmac fe010000.ethernet: set tx_delay to 0x30 [ 7.365224] rk_gmac-dwmac fe010000.ethernet: Can not read property: rx_delay. [ 7.365228] rk_gmac-dwmac fe010000.ethernet: set rx_delay to 0x10 Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- V2: Fix commit message and added the RX and TX clock delay. --- arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)