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 <[email protected]>
---
.../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 42 +++++++------------
1 file changed, 16 insertions(+), 26 deletions(-)
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 {
--
2.43.0
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 <[email protected]>
>
> ---
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.
[1] https://lore.kernel.org/u-boot/[email protected]/
>
> .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 42 +++++++------------
> 1 file changed, 16 insertions(+), 26 deletions(-)
>
> 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 {
> --
> 2.43.0
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
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 <[email protected]>
> >
> > ---
>
> 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.
>
> [1]
> https://lore.kernel.org/u-boot/[email protected]/
> >
> > .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 42 +++++++--------
> > ----
> > 1 file changed, 16 insertions(+), 26 deletions(-)
> >
> > 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 {
> > --
> > 2.43.0
> >
>
--
Sjoerd Simons
Collabora Ltd.
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 <[email protected]>
> > >
> > > ---
> >
> > 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.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
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 <[email protected]>
> > > >
> > > > ---
> > >
> > > 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
--
Sjoerd Simons
Collabora Ltd.
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 <[email protected]>
Reviewed-by: Roger Quadros <[email protected]>
Hi Sjoerd Simons,
On Fri, 12 Jan 2024 13:44:55 +0100, 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.
>
> [...]
I have applied the following to branch ti-k3-dts-next on [1].
Thank you!
[1/1] arm64: dts: ti: k3-am625-beagleplay: Use the builtin mdio bus
commit: 3c3f2d13d3c8e748ebdebfc6d8a72024240227e9
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent up the chain during
the next merge window (or sooner if it is a relevant bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux.git
--
Vignesh