| Message ID | 20240112124505.2054212-1-sjoerd@collabora.com |
|---|---|
| State | Accepted |
| Commit | 3c3f2d13d3c8e748ebdebfc6d8a72024240227e9 |
| Headers | show |
| Series | arm64: dts: ti: k3-am625-beagleplay: Use the builtin mdio bus | expand |
On Fri, 2024-01-12 at 08:25 -0600, Nishanth Menon wrote: > On 15:20-20240112, Sjoerd Simons wrote: > > On Fri, 2024-01-12 at 07:50 -0600, Nishanth Menon wrote: > > > On 13:44-20240112, Sjoerd Simons wrote: > > > > The beagleplay dts was using a bit-bang gpio mdio bus as a > > > > work- > > > > around > > > > for errata i2329. However since commit d04807b80691 ("net: > > > > ethernet: ti: > > > > davinci_mdio: Add workaround for errata i2329") the mdio driver > > > > itself > > > > already takes care of this errata for effected silicon, which > > > > landed > > > > well before the beagleplay dts. So i suspect the reason for the > > > > workaround in upstream was simply due to copying the vendor > > > > dts. > > > > > > > > Switch the dts to the ti,cpsw-mdio instead so it described the > > > > actual > > > > hardware and is consistent with other AM625 based boards > > > > > > > > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com> > > > > > > > > --- > > > > > > We have had issues with the ethernet integration previously (also > > > why > > > ethernet in u-boot is not yet functional on beagleplay[1]). > > > > > > https://openbeagle.org/beagleplay/beagleplay/-/issues/101 > > > > > > we should probably do a 1000 boot nfs test or something to ensure > > > this > > > doesn't introduce regressions (I recollect mdio wasn't stable on > > > beagleplay) and switching to bitbang driver stopped all > > > complains. > > > > I can do a longer test with that over the weekend sure; For > > reference > > I'm seeing issues in u-boot as well on initial probe with these > > changes, but i've not seen the same on the linux side. > > > > Do you remember with what kernel versions users saw the mdio > > instabilities? I wonder if that was a version with the commit > > mentioned > > that includes the errata fix for the mdio driver. > > > > we were on TI 5.10 kernel (the image that went into production) and > it > did have errata fix (I am not sure if it is similar or same as what > finally went into upstream now).. I have started an automated > multiple > boot test on my end as well.. > > The signature as I recollect was spurious link Down and Up logs > (though the cable is not physically disconnected).. been a bit of > time.. so my memory on exact signature might be vague. For reference over the weekend our test infra did over 1600 boots with an NFS root fs (see [0] for an example job) without a single boot failure. So with the upstream kernel this seems pretty solid. 0: https://staging.lava.collabora.dev/scheduler/job/572063
On 12/01/2024 14:44, Sjoerd Simons wrote: > The beagleplay dts was using a bit-bang gpio mdio bus as a work-around > for errata i2329. However since commit d04807b80691 ("net: ethernet: ti: > davinci_mdio: Add workaround for errata i2329") the mdio driver itself > already takes care of this errata for effected silicon, which landed > well before the beagleplay dts. So i suspect the reason for the > workaround in upstream was simply due to copying the vendor dts. > > Switch the dts to the ti,cpsw-mdio instead so it described the actual > hardware and is consistent with other AM625 based boards > > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com> Reviewed-by: Roger Quadros <rogerq@kernel.org>
diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts index eadbdd9ffe37..49fb21ba62b0 100644 --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts @@ -29,7 +29,6 @@ aliases { i2c3 = &main_i2c3; i2c4 = &wkup_i2c0; i2c5 = &mcu_i2c0; - mdio-gpio0 = &mdio0; mmc0 = &sdhci0; mmc1 = &sdhci1; mmc2 = &sdhci2; @@ -231,27 +230,6 @@ simple-audio-card,codec { }; }; - /* Workaround for errata i2329 - just use mdio bitbang */ - mdio0: mdio { - compatible = "virtual,mdio-gpio"; - pinctrl-names = "default"; - pinctrl-0 = <&mdio0_pins_default>; - gpios = <&main_gpio0 86 GPIO_ACTIVE_HIGH>, /* MDC */ - <&main_gpio0 85 GPIO_ACTIVE_HIGH>; /* MDIO */ - #address-cells = <1>; - #size-cells = <0>; - - cpsw3g_phy0: ethernet-phy@0 { - reg = <0>; - }; - - cpsw3g_phy1: ethernet-phy@1 { - reg = <1>; - reset-gpios = <&main_gpio1 5 GPIO_ACTIVE_LOW>; - reset-assert-us = <25>; - reset-deassert-us = <60000>; /* T2 */ - }; - }; }; &main_pmx0 { @@ -312,8 +290,8 @@ AM62X_IOPAD(0x00b4, PIN_INPUT_PULLUP, 1) /* (K24) GPMC0_CSn3.I2C2_SDA */ mdio0_pins_default: mdio0-default-pins { pinctrl-single,pins = < - AM62X_IOPAD(0x0160, PIN_OUTPUT, 7) /* (AD24) MDIO0_MDC.GPIO0_86 */ - AM62X_IOPAD(0x015c, PIN_INPUT, 7) /* (AB22) MDIO0_MDIO.GPIO0_85 */ + AM62X_IOPAD(0x0160, PIN_OUTPUT, 0) /* (AD24) MDIO0_MDC */ + AM62X_IOPAD(0x015c, PIN_INPUT, 0) /* (AB22) MDIO0_MDIO */ >; }; @@ -611,8 +589,20 @@ &cpsw_port2 { }; &cpsw3g_mdio { - /* Workaround for errata i2329 - Use mdio bitbang */ - status = "disabled"; + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&mdio0_pins_default>; + + cpsw3g_phy0: ethernet-phy@0 { + reg = <0>; + }; + + cpsw3g_phy1: ethernet-phy@1 { + reg = <1>; + reset-gpios = <&main_gpio1 5 GPIO_ACTIVE_LOW>; + reset-assert-us = <25>; + reset-deassert-us = <60000>; /* T2 */ + }; }; &main_gpio0 {
The beagleplay dts was using a bit-bang gpio mdio bus as a work-around for errata i2329. However since commit d04807b80691 ("net: ethernet: ti: davinci_mdio: Add workaround for errata i2329") the mdio driver itself already takes care of this errata for effected silicon, which landed well before the beagleplay dts. So i suspect the reason for the workaround in upstream was simply due to copying the vendor dts. Switch the dts to the ti,cpsw-mdio instead so it described the actual hardware and is consistent with other AM625 based boards Signed-off-by: Sjoerd Simons <sjoerd@collabora.com> --- .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-)