[PATCH] net: phy: avoid config_init failure on unattached PHY during resume

yicongsrfy@163.com posted 1 patch 3 weeks, 1 day ago
drivers/net/phy/phy_device.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 3 weeks, 1 day ago
From: Yi Cong <yicong@kylinos.cn>

When resuming a PHY device that is not attached to a MAC (i.e.
phydev->attached_dev is NULL), mdio_bus_phy_resume() may call into
phy_init_hw() -> phydev->drv->config_init(), which can return -EOPNOTSUPP
(-95) if the driver does not support initialization in this state.

This results in log messages like:
[ 1905.106209] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x180 [libphy] returns -95
[ 1905.106232] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
PM: failed to resume: error -95

In practice, only one PHY on the bus (e.g. XXXX:00:00) is typically
attached to a MAC interface; others (like XXXX:00:01) are probed but
not used, making such resume attempts unnecessary and misleading.

Add an early return in mdio_bus_phy_resume() when !phydev->attached_dev,
to prevent unneeded hardware initialization and avoids false error reports.

Fixes: 611d779af7ca ("net: phy: fix MDIO bus PM PHY resuming")
Signed-off-by: Yi Cong <yicong@kylinos.cn>
---
 drivers/net/phy/phy_device.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 7556aa3dd7ee..e8b4be967832 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -373,6 +373,9 @@ static __maybe_unused int mdio_bus_phy_resume(struct device *dev)
 	struct phy_device *phydev = to_phy_device(dev);
 	int ret;
 
+	if (!phydev->attached_dev)
+		return 0;
+
 	if (phydev->mac_managed_pm)
 		return 0;
 
-- 
2.25.1
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Russell King (Oracle) 3 weeks, 1 day ago
On Wed, Sep 10, 2025 at 10:58:26AM +0800, yicongsrfy@163.com wrote:
> From: Yi Cong <yicong@kylinos.cn>
> 
> When resuming a PHY device that is not attached to a MAC (i.e.
> phydev->attached_dev is NULL), mdio_bus_phy_resume() may call into
> phy_init_hw() -> phydev->drv->config_init(), which can return -EOPNOTSUPP
> (-95) if the driver does not support initialization in this state.
> 
> This results in log messages like:
> [ 1905.106209] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
> PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x180 [libphy] returns -95
> [ 1905.106232] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
> PM: failed to resume: error -95
> 
> In practice, only one PHY on the bus (e.g. XXXX:00:00) is typically
> attached to a MAC interface; others (like XXXX:00:01) are probed but
> not used, making such resume attempts unnecessary and misleading.
> 
> Add an early return in mdio_bus_phy_resume() when !phydev->attached_dev,
> to prevent unneeded hardware initialization and avoids false error reports.

PHYs are allowed to be attached without a net device. Your PHY
driver needs to cope with this condition.

-- 
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: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 3 weeks, 1 day ago
On Wed, 10 Sep 2025 09:15:59 +0100, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
>
> On Wed, Sep 10, 2025 at 10:58:26AM +0800, yicongsrfy@163.com wrote:
> > From: Yi Cong <yicong@kylinos.cn>
> >
> > When resuming a PHY device that is not attached to a MAC (i.e.
> > phydev->attached_dev is NULL), mdio_bus_phy_resume() may call into
> > phy_init_hw() -> phydev->drv->config_init(), which can return -EOPNOTSUPP
> > (-95) if the driver does not support initialization in this state.
> >
> > This results in log messages like:
> > [ 1905.106209] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
> > PM: dpm_run_callback(): mdio_bus_phy_resume+0x0/0x180 [libphy] returns -95
> > [ 1905.106232] YT8531S Gigabit Ethernet xxxxmac_mii_bus-XXXX:00:01:
> > PM: failed to resume: error -95
> >
> > In practice, only one PHY on the bus (e.g. XXXX:00:00) is typically
> > attached to a MAC interface; others (like XXXX:00:01) are probed but
> > not used, making such resume attempts unnecessary and misleading.
> >
> > Add an early return in mdio_bus_phy_resume() when !phydev->attached_dev,
> > to prevent unneeded hardware initialization and avoids false error reports.
>
> PHYs are allowed to be attached without a net device. Your PHY
> driver needs to cope with this condition.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Thank you for the reply!

My thought is, if a PHY device hasn't been attached to a net_device,
is it still necessary to go through the resume procedure?

My issue context is as follows:
After entering `mdio_bus_phy_resume`, the call flow proceeds to:
`phy_init_hw()`
    => `phydev->drv->config_init`
        => `yt8521_config_init`

Then, because `phydev->interface != PHY_INTERFACE_MODE_SGMII`, it attempts
to enter `ytphy_rgmii_clk_delay_config` to configure the RGMII tx/rx delay.
However, since this PHY device is not associated with any GMAC and is not
connected via an RGMII interface, the function returns `-EOPNOTSUPP`.

Of course, I could submit a fix patch to `motorcomm.c` to terminate
`config_init` early. But as I mentioned at the beginning, when a PHY
device hasn't been attached, do we really need to let it go through
the entire resume process?
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Russell King (Oracle) 3 weeks, 1 day ago
On Wed, Sep 10, 2025 at 05:17:03PM +0800, yicongsrfy@163.com wrote:
> Then, because `phydev->interface != PHY_INTERFACE_MODE_SGMII`, it attempts
> to enter `ytphy_rgmii_clk_delay_config` to configure the RGMII tx/rx delay.
> However, since this PHY device is not associated with any GMAC and is not
> connected via an RGMII interface, the function returns `-EOPNOTSUPP`.

It seems the problem is this code:

        /* set rgmii delay mode */
        if (phydev->interface != PHY_INTERFACE_MODE_SGMII) {
                ret = ytphy_rgmii_clk_delay_config(phydev);

which assumes that phydev->interface will be either SGMII or one of
the RGMII modes. This is not the case with a PHY that has been
freshly probed unless phydev->interface is set in the probe function.

I see the probe function decodes the PHYs operating mode and
configures stuff based on that. Maybe, as it only supports RGMII
and SGMII, it should also initialise phydev->interface to the initial
operating condition of the PHY if other code in the driver relies
upon this being set to either SGMII or one of the RGMII types.

-- 
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: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 3 weeks ago
On Wed, 10 Sep 2025 10:54:05 +0100, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
>
> On Wed, Sep 10, 2025 at 05:17:03PM +0800, yicongsrfy@163.com wrote:
> > Then, because `phydev->interface != PHY_INTERFACE_MODE_SGMII`, it attempts
> > to enter `ytphy_rgmii_clk_delay_config` to configure the RGMII tx/rx delay.
> > However, since this PHY device is not associated with any GMAC and is not
> > connected via an RGMII interface, the function returns `-EOPNOTSUPP`.
>
> It seems the problem is this code:
>
>         /* set rgmii delay mode */
>         if (phydev->interface != PHY_INTERFACE_MODE_SGMII) {
>                 ret = ytphy_rgmii_clk_delay_config(phydev);
>
> which assumes that phydev->interface will be either SGMII or one of
> the RGMII modes. This is not the case with a PHY that has been
> freshly probed unless phydev->interface is set in the probe function.
>
> I see the probe function decodes the PHYs operating mode and
> configures stuff based on that. Maybe, as it only supports RGMII
> and SGMII, it should also initialise phydev->interface to the initial
> operating condition of the PHY if other code in the driver relies
> upon this being set to either SGMII or one of the RGMII types.
>

Thank you for your reply!

What you mentioned above is correct.
However, there is a particular scenario as follows:

Some PHY chips support two addresses, using address 0 as a broadcast address
and address 1 as the hardware address. Both addresses respond to GMAC's MDIO
read/write operations. As a result, during 'mdio_scan', both PHY addresses are
detected, leading to the creation of two PHY device instances (for example,
as in my previous email: xxxxmac_mii_bus-XXXX:00:00 and xxxxmac_mii_bus-XXXX:00:01).

The GMAC will only attach to one of these PHY addresses. Some GMAC drivers
select the first available PHY address (via calling 'phy_find_first'), leaving
the other address idle and unattached. However, during the system resume
process, it attempts to resume this unused PHY instance.

When resuming this PHY instance, because 'phydev->interface' hasn't been
set to a valid interface mode supported by the chip (such as RGMII or SGMII),
we encounter the EOPNOTSUPP error.

I've tried modifying motorcomm.c as a workaround, but logically speaking,
it's not an ideal solution:
1. If I return 0 from the driver's resume function, mdio_bus_phy_resume will proceed.
2. If I return an error, the system will still report an error.

The key issue is: does the current kernel provide any field or mechanism
to indicate that two PHY addresses (instances) actually refer to the same
physical PHY device?

I haven't found any such mechanism in the kernel. Do you have any
suggestions on how to properly handle this?

Looking forward to your advice. Thank you!
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Russell King (Oracle) 2 weeks, 1 day ago
On Thu, Sep 11, 2025 at 07:25:25PM +0800, yicongsrfy@163.com wrote:
> However, there is a particular scenario as follows:
> 
> Some PHY chips support two addresses, using address 0 as a broadcast address
> and address 1 as the hardware address. Both addresses respond to GMAC's MDIO
> read/write operations. As a result, during 'mdio_scan', both PHY addresses are
> detected, leading to the creation of two PHY device instances (for example,
> as in my previous email: xxxxmac_mii_bus-XXXX:00:00 and xxxxmac_mii_bus-XXXX:00:01).

IEEE 802.3 makes no provision for a broadcast address. Here is what it
says about the PHY address:

22.2.4.5.5 PHYAD (PHY Address)

The PHY Address is five bits, allowing 32 unique PHY addresses. The
first PHY address bit transmitted and received is the MSB of the
address. A PHY that is connected to the station management entity
via the mechanical interface defined in 22.6 shall always respond to
transactions addressed to PHY Address zero <00000>. A station
management entity that is attached to multiple PHYs must have prior
knowledge of the appropriate PHY Address for each PHY.

So, phylib code can't make any special exception for address zero.
We definitely have network adapters where their PHYs are at address
zero, so blocking that address will break them.

As for Andrew's suggestion, returning an error in the probe function
doesn't prevent the phy_device being created, and it will mean that
if a MAC driver attempts to attach to that instance, phylib will use
the generic PHY driver instead. So, this doesn't solve the problem.

I don't see that there is anything that a PHY driver can do to solve
this as the code currently stands, especially if the MAC driver is
coded to "use the lowest PHY address present on the MDIO bus" (which
in this case will be your vendor's idea of a broadcast address.)

I really don't think we should start adding hacks for this kind of
stuff into phylib's core - we can't know that PHY address 0 is a
duplicate of another address.

The only thing I can come up with is:

1. There must be a way to configure the PHY to disable its non-
   standard "broadcast MDIO address" to make it compliant with 802.3.
   I suggest that board firmware needs to set that to make the PHY
   compliant.

2. As a hard reset of the PHY will likely clear that setting, this is
   another nail in the coffin of PHY hard reset handling in the kernel
   which has been the cause of many issues. (Another nail in that
   coffin is that some MACs require the RX clock from the PHY to be
   running in order to properly reset.)

-- 
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: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 1 week, 6 days ago
On Wed, 17 Sep 2025 11:06:37 +0100, "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
>
> I don't see that there is anything that a PHY driver can do to solve
> this as the code currently stands, especially if the MAC driver is
> coded to "use the lowest PHY address present on the MDIO bus" (which
> in this case will be your vendor's idea of a broadcast address.)
>
> I really don't think we should start adding hacks for this kind of
> stuff into phylib's core - we can't know that PHY address 0 is a
> duplicate of another address.
>
> The only thing I can come up with is:
>
> 1. There must be a way to configure the PHY to disable its non-
>    standard "broadcast MDIO address" to make it compliant with 802.3.
>    I suggest that board firmware needs to set that to make the PHY
>    compliant.
>
> 2. As a hard reset of the PHY will likely clear that setting, this is
>    another nail in the coffin of PHY hard reset handling in the kernel
>    which has been the cause of many issues. (Another nail in that
>    coffin is that some MACs require the RX clock from the PHY to be
>    running in order to properly reset.)

Thank you for your reply!

Since this issue cannot be fundamentally resolved within phylib,
we need to seek a solution within the PHY driver itself.

Let's return to the original problem: how to solve the suspend/resume
issue caused by a single physical PHY device generating multiple
`phy_device` instances?

A key requirement for a device's suspend/resume functionality is
power saving, which ultimately needs to take effect on the physical
hardware. Therefore, we only need to ensure that one `phy_device`
instance operates on the actual physical device.

Based on this idea, I've made the following modifications:
1.  Add a check within the PHY driver to identify the broadcast address.
2.  In `config_init`, `suspend`, and `resume` functions, if the broadcast
address is encountered, return immediately without further processing.

This approach assumes that the MAC driver correctly uses the PHY's
hardware address, rather than finding address 0 via `phy_find_first`,
which might be the default broadcast address for some PHY chips.

Here is a partial patch:
```
+/**
+ * yt8521_check_broadcast_phyaddr()
+ * - check is it a phydev with broadcast addr.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * returns true or false.
+ */
+static bool yt8521_check_broadcast_phyaddr(struct phy_device *phydev)
+{
+	if (/* addr 0 is broaddcast OR other broadcast addr */)
+		return true;
+
+	return false;
+}
+

static int yt8521_probe(struct phy_device *phydev) {
    ...
+	/* Logically speaking, it should return directly here,
+	 * although it don't have the expected effect.
+	 */
+	if (yt8521_check_broadcast_phyaddr(phydev))
+		return -ENODEV;
    ...
}

static int yt8521_suspend(struct phy_device *phydev) {
    ...
+	if (yt8521_check_broadcast_phyaddr(phydev))
+		return 0;
    ...
}

static int yt8521_resume(struct phy_device *phydev) {
    ...
+	if (yt8521_check_broadcast_phyaddr(phydev))
+		return 0;
    ...
}

static int yt8521_config_init(struct phy_device *phydev) {
    ...
+	if (yt8521_check_broadcast_phyaddr(phydev))
+		return 0;
    ...
}
```

Testing has confirmed that this fix is effective.

Could you please review if this fix approach is appropriate? If it's acceptable, I will send a complete patch v2.
Thank you very much!
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Andrew Lunn 1 week, 6 days ago
> Since this issue cannot be fundamentally resolved within phylib,
> we need to seek a solution within the PHY driver itself.

How about this...

Allow a node in DT which looks like this:

mdio {
	phy@0 {
		# Broadcast address, ignore
		compatible = "ethernet-phy-idffff.ffff";
		reg = <0>;
	}

	phy@16 {
		# The real address of the PHY
		reg = <16>;
	}
}

The idea being, you can use a compatible to correct the ID of a PHY.
The ID of mostly F is considered to mean there is no PHY there, its
just the pull-up resistor on the data line. So the PHY is returning
the wrong ID...

of_mdiobus_child_is_phy() then needs to change from a bool to an int,
and return -ENODEV for "ethernet-phy-idffff.ffff", and the caller
needs to correctly handle that and not create the device.

I would also suggest the PHY driver disables the broadcast address
when it gets probed on its real address.

	Andrew
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 1 week, 3 days ago
On Fri, 19 Sep 2025 14:45:12 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Since this issue cannot be fundamentally resolved within phylib,
> > we need to seek a solution within the PHY driver itself.
>
> How about this...
>
> Allow a node in DT which looks like this:
>
> mdio {
> 	phy@0 {
> 		# Broadcast address, ignore
> 		compatible = "ethernet-phy-idffff.ffff";
> 		reg = <0>;
> 	}
>
> 	phy@16 {
> 		# The real address of the PHY
> 		reg = <16>;
> 	}
> }
>
> The idea being, you can use a compatible to correct the ID of a PHY.
> The ID of mostly F is considered to mean there is no PHY there, its
> just the pull-up resistor on the data line. So the PHY is returning
> the wrong ID...
>
> of_mdiobus_child_is_phy() then needs to change from a bool to an int,
> and return -ENODEV for "ethernet-phy-idffff.ffff", and the caller
> needs to correctly handle that and not create the device.
>
> I would also suggest the PHY driver disables the broadcast address
> when it gets probed on its real address.

Thank you for your reply!

Disabling the broadcast address from ACPI or DTS is indeed a good
approach. However, should we also consider upgrade support for
existing devices? After all, modifying ACPI or DTS for already-deployed
devices is not a simple task. In cases like this, do we need to focus
solely on solutions implemented within the driver?
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Andrew Lunn 3 weeks ago
> Some PHY chips support two addresses, using address 0 as a broadcast address
> and address 1 as the hardware address. Both addresses respond to GMAC's MDIO
> read/write operations. As a result, during 'mdio_scan', both PHY addresses are
> detected, leading to the creation of two PHY device instances (for example,
> as in my previous email: xxxxmac_mii_bus-XXXX:00:00 and xxxxmac_mii_bus-XXXX:00:01).

I would say the PHY driver is broken, or at least, not correctly
handling the situation. When the scan finds the PHY at address 0, the
PHY driver is probed, and it should look at the strapping and decided,
if the strapping has put the PHY at address 0, or its the broadcast
address being used. If it is the broadcast address, turn off broadcast
address, and return -ENODEV. phylib should then not create a PHY at
address 0. The scan will continue and find the PHY at its correct
address. Your problem then goes away, and phylib has a correct
representation of the hardware.

The harder bit is backwards compatibility. Are there DT files which
make use of the broadcast address, and disabling it will break them?

We have seen patches disabling broadcast, but i don't remember for
which PHY.

	Andrew
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by yicongsrfy@163.com 3 weeks, 1 day ago
On Wed, 10 Sep 2025 17:17:03 +0800, yicongsrfy@163.com <> wrote:
> My thought is, if a PHY device hasn't been attached to a net_device,
> is it still necessary to go through the resume procedure?
>
> My issue context is as follows:
> After entering `mdio_bus_phy_resume`, the call flow proceeds to:
> `phy_init_hw()`
>     => `phydev->drv->config_init`
>         => `yt8521_config_init`
>
> Then, because `phydev->interface != PHY_INTERFACE_MODE_SGMII`, it attempts
> to enter `ytphy_rgmii_clk_delay_config` to configure the RGMII tx/rx delay.
> However, since this PHY device is not associated with any GMAC and is not
> connected via an RGMII interface, the function returns `-EOPNOTSUPP`.
>
> Of course, I could submit a fix patch to `motorcomm.c` to terminate
> `config_init` early. But as I mentioned at the beginning, when a PHY
> device hasn't been attached, do we really need to let it go through
> the entire resume process?

One more point: in the suspend flow as shown below:
`mdio_bus_phy_suspend`
    => `mdio_bus_phy_may_suspend`
there is also a check whether `phydev->attached_dev` is NULL.

Therefore, my idea is to make this modification to maintain consistency in the logic.
Re: [PATCH] net: phy: avoid config_init failure on unattached PHY during resume
Posted by Russell King (Oracle) 3 weeks, 1 day ago
On Wed, Sep 10, 2025 at 05:31:00PM +0800, yicongsrfy@163.com wrote:
> One more point: in the suspend flow as shown below:
> `mdio_bus_phy_suspend`
>     => `mdio_bus_phy_may_suspend`
> there is also a check whether `phydev->attached_dev` is NULL.

Yes, it checks, but only to bypass checking for things that are
dependent on whether a netdev exists. It doesn't _stop_ a netdev-less
PHY being suspended. What you're proposing _stops_ a netdev-less PHY
being resumed. You are proposing to break stuff.

> Therefore, my idea is to make this modification to maintain consistency in the logic.

No, it's making inconsistency. mdio_bus_phy_resume() will only resume a
PHY that has previously been suspended by mdio_bus_phy_suspend(). See
the phydev->suspended_by_mdio_bus flag.

Fix the motorcomm driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!