drivers/net/phy/phy_device.c | 3 +++ 1 file changed, 3 insertions(+)
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
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!
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?
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!
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!
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!
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!
> 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
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?
> 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
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.
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!
© 2016 - 2025 Red Hat, Inc.