[PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting

Chen-Yu Tsai posted 10 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting
Posted by Chen-Yu Tsai 1 month, 3 weeks ago
From: Chen-Yu Tsai <wens@csie.org>

The external Ethernet PHY has a reset pin that is connected to the SoC.
It is missing from the original submission.

Add it to complete the description.

Fixes: acca163f3f51 ("arm64: dts: allwinner: a527: add EMAC0 to Radxa A5E board")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
index 70d439bc845c..d4cee2222104 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
@@ -94,6 +94,9 @@ &mdio0 {
 	ext_rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
 		reg = <1>;
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
+		reset-assert-us = <10000>;
+		reset-deassert-us = <150000>;
 	};
 };
 
-- 
2.39.5
Re: [PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting
Posted by Russell King (Oracle) 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 10:55:36PM +0800, Chen-Yu Tsai wrote:
> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> index 70d439bc845c..d4cee2222104 100644
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> @@ -94,6 +94,9 @@ &mdio0 {
>  	ext_rgmii_phy: ethernet-phy@1 {
>  		compatible = "ethernet-phy-ieee802.3-c22";
>  		reg = <1>;
> +		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
> +		reset-assert-us = <10000>;
> +		reset-deassert-us = <150000>;

Please verify that kexec works with this, as if the calling kernel
places the PHY in reset and then kexec's, and the reset remains
asserted, the PHY will not be detected.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting
Posted by Chen-Yu Tsai 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 11:12 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, Aug 13, 2025 at 10:55:36PM +0800, Chen-Yu Tsai wrote:
> > diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > index 70d439bc845c..d4cee2222104 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > @@ -94,6 +94,9 @@ &mdio0 {
> >       ext_rgmii_phy: ethernet-phy@1 {
> >               compatible = "ethernet-phy-ieee802.3-c22";
> >               reg = <1>;
> > +             reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
> > +             reset-assert-us = <10000>;
> > +             reset-deassert-us = <150000>;
>
> Please verify that kexec works with this, as if the calling kernel
> places the PHY in reset and then kexec's, and the reset remains
> asserted, the PHY will not be detected.

I found this to be a bit confusing to be honest.

If I put the reset description in the PHY (where I think it belongs),
then it wouldn't work if the reset isn't by default deasserted (through
some pull-up). This would be similar to the kexec scenario.

Whereas if I put the reset under the MDIO bus, then the core would
deassert the reset before scanning the bus.

It's confusing to me because the code already goes through the MDIO bus
device tree node and *knows* that there are PHYs under it, and that the
PHYs might have a reset. And it can even handle them _after_ the initial
bus scan.

Describing the PHY reset as a bus reset IMHO isn't correct.


ChenYu
Re: [PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting
Posted by Russell King (Oracle) 1 month, 3 weeks ago
On Wed, Aug 13, 2025 at 11:51:18PM +0800, Chen-Yu Tsai wrote:
> On Wed, Aug 13, 2025 at 11:12 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Aug 13, 2025 at 10:55:36PM +0800, Chen-Yu Tsai wrote:
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > index 70d439bc845c..d4cee2222104 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > @@ -94,6 +94,9 @@ &mdio0 {
> > >       ext_rgmii_phy: ethernet-phy@1 {
> > >               compatible = "ethernet-phy-ieee802.3-c22";
> > >               reg = <1>;
> > > +             reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
> > > +             reset-assert-us = <10000>;
> > > +             reset-deassert-us = <150000>;
> >
> > Please verify that kexec works with this, as if the calling kernel
> > places the PHY in reset and then kexec's, and the reset remains
> > asserted, the PHY will not be detected.
> 
> I found this to be a bit confusing to be honest.
> 
> If I put the reset description in the PHY (where I think it belongs),
> then it wouldn't work if the reset isn't by default deasserted (through
> some pull-up). This would be similar to the kexec scenario.

The reason for this is quite simple. While it's logical to put it in
there, the problem is that the PHY doesn't respond on the MDIO bus
while it's reset pin is asserted.

Consequently, when we probe the MDIO bus to detect PHYs and discover
the PHY IDs, we get no response, and thus we believe there isn't a
device at the address. That means we don't create a device, and thus
there's no mdio device for the address.

There is a work-around, which is to encode the PHY ID in the DT
compatible (check the ethernet-phy binding). However, note that we
will then not read the actual PHY ID (maybe we should?) which means
if the driver wants to know e.g. the revision, or during production
the PHY changes, it will require DT to change.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next v2 06/10] arm64: dts: allwinner: a527: cubie-a5e: Add ethernet PHY reset setting
Posted by Chen-Yu Tsai 1 month, 1 week ago
On Wed, Aug 13, 2025 at 6:39 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, Aug 13, 2025 at 11:51:18PM +0800, Chen-Yu Tsai wrote:
> > On Wed, Aug 13, 2025 at 11:12 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Wed, Aug 13, 2025 at 10:55:36PM +0800, Chen-Yu Tsai wrote:
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > > index 70d439bc845c..d4cee2222104 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun55i-a527-cubie-a5e.dts
> > > > @@ -94,6 +94,9 @@ &mdio0 {
> > > >       ext_rgmii_phy: ethernet-phy@1 {
> > > >               compatible = "ethernet-phy-ieee802.3-c22";
> > > >               reg = <1>;
> > > > +             reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
> > > > +             reset-assert-us = <10000>;
> > > > +             reset-deassert-us = <150000>;
> > >
> > > Please verify that kexec works with this, as if the calling kernel
> > > places the PHY in reset and then kexec's, and the reset remains
> > > asserted, the PHY will not be detected.
> >
> > I found this to be a bit confusing to be honest.
> >
> > If I put the reset description in the PHY (where I think it belongs),
> > then it wouldn't work if the reset isn't by default deasserted (through
> > some pull-up). This would be similar to the kexec scenario.
>
> The reason for this is quite simple. While it's logical to put it in
> there, the problem is that the PHY doesn't respond on the MDIO bus
> while it's reset pin is asserted.
>
> Consequently, when we probe the MDIO bus to detect PHYs and discover
> the PHY IDs, we get no response, and thus we believe there isn't a
> device at the address. That means we don't create a device, and thus
> there's no mdio device for the address.

It feels like a limitation of the implementation though. With the split
of mdio_device and phy_device, maybe it's possible to add some API that
registers mdio_device first based on information from the DT, have its
reset deasserted, read back the PHY ID, then create the PHY device?

This limitation also applies to handling regulator supplies for the PHY,
which we currently resort to sticking under the MAC, which is even worse?

> There is a work-around, which is to encode the PHY ID in the DT
> compatible (check the ethernet-phy binding). However, note that we
> will then not read the actual PHY ID (maybe we should?) which means
> if the driver wants to know e.g. the revision, or during production
> the PHY changes, it will require DT to change.

Judging from previous board iterations, I think this is quite likely
to happen. If the additional SoC internal delay values stay the same,
I would prefer we not run into this.


Thanks
ChenYu