Currently, phy_led supports of node, acpi node, and software node.
The hibmcge hardware(including leds) is on the BMC side, while
the driver runs on the host side.

An apic or of node needs to be created on the host side to
support phy_led; otherwise, phy_led will not be supported.

Therefore, in order to support phy_led, this patch will create
a software node when it detects that the phy device is not
bound to any fwnode.

Closes: https://lore.kernel.org/all/023a85e4-87e2-4bd3-9727-69a2bfdc4145@lunn.ch/
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  11 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 120 +++++++++++++++++-
 2 files changed, 124 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index 8e134da3e217..d20dd324e129 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -5,8 +5,10 @@
 #define __HBG_COMMON_H
 
 #include <linux/ethtool.h>
+#include <linux/fwnode.h>
 #include <linux/netdevice.h>
 #include <linux/pci.h>
+#include <linux/property.h>
 #include <net/page_pool/helpers.h>
 #include "hbg_reg.h"
 
@@ -130,6 +132,9 @@ struct hbg_vector {
 	u32 info_array_len;
 };
 
+#define HBG_LED_MAX_NUM			(sizeof(u32) / sizeof(u8))
+#define HBG_LED_SOFTWARE_NODE_MAX_NUM	(HBG_LED_MAX_NUM + 2)
+
 struct hbg_mac {
 	struct mii_bus *mdio_bus;
 	struct phy_device *phydev;
@@ -140,6 +145,12 @@ struct hbg_mac {
 	u32 autoneg;
 	u32 link_status;
 	u32 pause_autoneg;
+
+	struct software_node phy_node;
+	struct software_node leds_node;
+	struct software_node led_nodes[HBG_LED_MAX_NUM];
+	struct property_entry leds_props[HBG_LED_MAX_NUM][4];
+	const struct software_node *nodes[HBG_LED_SOFTWARE_NODE_MAX_NUM + 1];
 };
 
 struct hbg_mac_table_entry {
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
index b6f0a2780ea8..edd8ccefbb62 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -263,11 +263,119 @@ static int hbg_fixed_phy_init(struct hbg_priv *priv)
 	return hbg_phy_connect(priv);
 }
 
-int hbg_mdio_init(struct hbg_priv *priv)
+static void hbg_phy_device_free(void *data)
+{
+	phy_device_free((struct phy_device *)data);
+}
+
+static void hbg_phy_device_remove(void *data)
+{
+	phy_device_remove((struct phy_device *)data);
+}
+
+static void hbg_software_node_unregister_group(void *data)
+{
+	const struct software_node **node = data;
+
+	software_node_unregister_node_group(node);
+}
+
+static void hbg_device_remove_software_node(void *data)
+{
+	device_remove_software_node((struct device *)data);
+}
+
+static int hbg_register_phy_leds_software_node(struct hbg_priv *priv,
+					       struct phy_device *phydev)
 {
+	struct fwnode_handle *fwnode = dev_fwnode(&phydev->mdio.dev);
 	struct device *dev = &priv->pdev->dev;
 	struct hbg_mac *mac = &priv->mac;
+	u32 node_index = 0, i;
+	const char *label;
+	int ret;
+
+	if (fwnode) {
+		dev_dbg(dev, "phy fwnode is already exists\n");
+		return 0;
+	}
+
+	mac->phy_node.name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s-%u",
+					    "mii", dev_name(dev),
+					    mac->phy_addr);
+	mac->nodes[node_index++] = &mac->phy_node;
+
+	mac->leds_node.name = devm_kasprintf(dev, GFP_KERNEL, "leds");
+	mac->leds_node.parent = &mac->phy_node;
+	mac->nodes[node_index++] = &mac->leds_node;
+
+	for (i = 0; i < HBG_LED_MAX_NUM; i++) {
+		mac->leds_props[i][0] = PROPERTY_ENTRY_U32("reg", i);
+		label = devm_kasprintf(dev, GFP_KERNEL, "%u", i);
+		mac->leds_props[i][1] = PROPERTY_ENTRY_STRING("label", label);
+
+		mac->led_nodes[i].name = devm_kasprintf(dev, GFP_KERNEL,
+							"led@%u", i);
+		mac->led_nodes[i].properties = mac->leds_props[i];
+		mac->led_nodes[i].parent = &mac->leds_node;
+
+		mac->nodes[node_index++] = &mac->led_nodes[i];
+	}
+
+	ret = software_node_register_node_group(mac->nodes);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to register software node\n");
+
+	ret = devm_add_action_or_reset(dev, hbg_software_node_unregister_group,
+				       mac->nodes);
+	if (ret)
+		return ret;
+
+	ret = device_add_software_node(&phydev->mdio.dev, &mac->phy_node);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add software node\n");
+
+	return devm_add_action_or_reset(dev, hbg_device_remove_software_node,
+					&phydev->mdio.dev);
+}
+
+static int hbg_find_phy_device(struct hbg_priv *priv, struct mii_bus *mdio_bus)
+{
+	struct device *dev = &priv->pdev->dev;
 	struct phy_device *phydev;
+	int ret;
+
+	phydev = get_phy_device(mdio_bus, priv->mac.phy_addr, false);
+	if (IS_ERR(phydev))
+		return dev_err_probe(dev, -ENODEV,
+				     "failed to get phy device\n");
+
+	ret = devm_add_action_or_reset(dev, hbg_phy_device_free, phydev);
+	if (ret)
+		return ret;
+
+	ret = hbg_register_phy_leds_software_node(priv, phydev);
+	if (ret)
+		return ret;
+
+	ret = phy_device_register(phydev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to register phy device\n");
+
+	ret = devm_add_action_or_reset(dev, hbg_phy_device_remove, phydev);
+	if (ret)
+		return ret;
+
+	priv->mac.phydev = phydev;
+	return 0;
+}
+
+int hbg_mdio_init(struct hbg_priv *priv)
+{
+	struct device *dev = &priv->pdev->dev;
+	struct hbg_mac *mac = &priv->mac;
 	struct mii_bus *mdio_bus;
 	int ret;
 
@@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
 
 	mdio_bus->parent = dev;
 	mdio_bus->priv = priv;
-	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
+	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
 	mdio_bus->name = "hibmcge mii bus";
 	mac->mdio_bus = mdio_bus;
 
@@ -293,12 +401,10 @@ int hbg_mdio_init(struct hbg_priv *priv)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to register MDIO bus\n");
 
-	phydev = mdiobus_get_phy(mdio_bus, mac->phy_addr);
-	if (!phydev)
-		return dev_err_probe(dev, -ENODEV,
-				     "failed to get phy device\n");
+	ret = hbg_find_phy_device(priv, mdio_bus);
+	if (ret)
+		return ret;
 
-	mac->phydev = phydev;
 	hbg_mdio_init_hw(priv);
 	return hbg_phy_connect(priv);
 }
-- 
2.33.0

On Mon, Dec 15, 2025 at 08:57:02PM +0800, Jijie Shao wrote:
> Currently, phy_led supports of node, acpi node, and software node.
> The hibmcge hardware(including leds) is on the BMC side, while
> the driver runs on the host side.
> 
> An apic or of node needs to be created on the host side to
> support phy_led; otherwise, phy_led will not be supported.
> 
> Therefore, in order to support phy_led, this patch will create
> a software node when it detects that the phy device is not
> bound to any fwnode.
> 
> Closes: https://lore.kernel.org/all/023a85e4-87e2-4bd3-9727-69a2bfdc4145@lunn.ch/
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  .../ethernet/hisilicon/hibmcge/hbg_common.h   |  11 ++
>  .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 120 +++++++++++++++++-
>  2 files changed, 124 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
> index 8e134da3e217..d20dd324e129 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
> @@ -5,8 +5,10 @@
>  #define __HBG_COMMON_H
>  
>  #include <linux/ethtool.h>
> +#include <linux/fwnode.h>
>  #include <linux/netdevice.h>
>  #include <linux/pci.h>
> +#include <linux/property.h>
>  #include <net/page_pool/helpers.h>
>  #include "hbg_reg.h"
>  
> @@ -130,6 +132,9 @@ struct hbg_vector {
>  	u32 info_array_len;
>  };
>  
> +#define HBG_LED_MAX_NUM			(sizeof(u32) / sizeof(u8))
> +#define HBG_LED_SOFTWARE_NODE_MAX_NUM	(HBG_LED_MAX_NUM + 2)
> +
>  struct hbg_mac {
>  	struct mii_bus *mdio_bus;
>  	struct phy_device *phydev;
> @@ -140,6 +145,12 @@ struct hbg_mac {
>  	u32 autoneg;
>  	u32 link_status;
>  	u32 pause_autoneg;
> +
> +	struct software_node phy_node;
> +	struct software_node leds_node;
> +	struct software_node led_nodes[HBG_LED_MAX_NUM];
> +	struct property_entry leds_props[HBG_LED_MAX_NUM][4];
> +	const struct software_node *nodes[HBG_LED_SOFTWARE_NODE_MAX_NUM + 1];
>  };
>  
>  struct hbg_mac_table_entry {
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> index b6f0a2780ea8..edd8ccefbb62 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> @@ -263,11 +263,119 @@ static int hbg_fixed_phy_init(struct hbg_priv *priv)
>  	return hbg_phy_connect(priv);
>  }
>  
> -int hbg_mdio_init(struct hbg_priv *priv)
> +static void hbg_phy_device_free(void *data)
> +{
> +	phy_device_free((struct phy_device *)data);
> +}
> +
> +static void hbg_phy_device_remove(void *data)
> +{
> +	phy_device_remove((struct phy_device *)data);
> +}

Alarm bells are ringing! A MAC driver should not be doing anything
like this.

> +int hbg_mdio_init(struct hbg_priv *priv)
> +{
> +	struct device *dev = &priv->pdev->dev;
> +	struct hbg_mac *mac = &priv->mac;
>  	struct mii_bus *mdio_bus;
>  	int ret;
>  
> @@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
>  
>  	mdio_bus->parent = dev;
>  	mdio_bus->priv = priv;
> -	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
> +	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
>  	mdio_bus->name = "hibmcge mii bus";
>  	mac->mdio_bus = mdio_bus;

I think you are taking the wrong approach.

Please consider trying to use of_mdiobus_register(). Either load a DT
overlay, or see if you can construct a tree of properties as you have
been doing, and pass it to of_mdiobus_register(). You then don't need
to destroy and create PHY devices.

	Andrew

on 2025/12/16 15:17, Andrew Lunn wrote:
> On Mon, Dec 15, 2025 at 08:57:02PM +0800, Jijie Shao wrote:
>
>> +int hbg_mdio_init(struct hbg_priv *priv)
>> +{
>> +	struct device *dev = &priv->pdev->dev;
>> +	struct hbg_mac *mac = &priv->mac;
>>   	struct mii_bus *mdio_bus;
>>   	int ret;
>>   
>> @@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
>>   
>>   	mdio_bus->parent = dev;
>>   	mdio_bus->priv = priv;
>> -	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
>> +	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
>>   	mdio_bus->name = "hibmcge mii bus";
>>   	mac->mdio_bus = mdio_bus;
> I think you are taking the wrong approach.
>
> Please consider trying to use of_mdiobus_register(). Either load a DT
> overlay, or see if you can construct a tree of properties as you have
> been doing, and pass it to of_mdiobus_register(). You then don't need
> to destroy and create PHY devices.

This is not easy.
`of_mdiobus_register()` requires a device_node, but I currently don't have one.
It is precisely because there is no DT or ACPI node that I am using a software node as a substitute.

In fact, this patch does not destroy and recreate the phydev.
It only delays the timing of phy registration:
first, register the MDIO (without scanning and registering the PHY devices),
then obtain the PHY device, register the software node, and finally register the PHY device.

Thanks,
Jijie Shao

On Wed, Jan 07, 2026 at 06:09:28PM +0800, Jijie Shao wrote:
> 
> on 2025/12/16 15:17, Andrew Lunn wrote:
> > On Mon, Dec 15, 2025 at 08:57:02PM +0800, Jijie Shao wrote:
> > 
> > > +int hbg_mdio_init(struct hbg_priv *priv)
> > > +{
> > > +	struct device *dev = &priv->pdev->dev;
> > > +	struct hbg_mac *mac = &priv->mac;
> > >   	struct mii_bus *mdio_bus;
> > >   	int ret;
> > > @@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
> > >   	mdio_bus->parent = dev;
> > >   	mdio_bus->priv = priv;
> > > -	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
> > > +	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
> > >   	mdio_bus->name = "hibmcge mii bus";
> > >   	mac->mdio_bus = mdio_bus;
> > I think you are taking the wrong approach.
> > 
> > Please consider trying to use of_mdiobus_register(). Either load a DT
> > overlay, or see if you can construct a tree of properties as you have
> > been doing, and pass it to of_mdiobus_register(). You then don't need
> > to destroy and create PHY devices.
> 
> This is not easy.
> `of_mdiobus_register()` requires a device_node, but I currently don't have one.
> It is precisely because there is no DT or ACPI node that I am using a software node as a substitute.

Which is why i suggested DT overlay.

drivers/misc/lan966x_pci.dtso

	Andrew

on 2026/1/7 21:04, Andrew Lunn wrote:
> On Wed, Jan 07, 2026 at 06:09:28PM +0800, Jijie Shao wrote:
>> on 2025/12/16 15:17, Andrew Lunn wrote:
>>> On Mon, Dec 15, 2025 at 08:57:02PM +0800, Jijie Shao wrote:
>>>
>>>> +int hbg_mdio_init(struct hbg_priv *priv)
>>>> +{
>>>> +	struct device *dev = &priv->pdev->dev;
>>>> +	struct hbg_mac *mac = &priv->mac;
>>>>    	struct mii_bus *mdio_bus;
>>>>    	int ret;
>>>> @@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
>>>>    	mdio_bus->parent = dev;
>>>>    	mdio_bus->priv = priv;
>>>> -	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
>>>> +	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
>>>>    	mdio_bus->name = "hibmcge mii bus";
>>>>    	mac->mdio_bus = mdio_bus;
>>> I think you are taking the wrong approach.
>>>
>>> Please consider trying to use of_mdiobus_register(). Either load a DT
>>> overlay, or see if you can construct a tree of properties as you have
>>> been doing, and pass it to of_mdiobus_register(). You then don't need
>>> to destroy and create PHY devices.
>> This is not easy.
>> `of_mdiobus_register()` requires a device_node, but I currently don't have one.
>> It is precisely because there is no DT or ACPI node that I am using a software node as a substitute.
> Which is why i suggested DT overlay.
>
> drivers/misc/lan966x_pci.dtso
>
> 	Andrew

Thank you for the guidance.

I did some research, and it seems that our board uses ACPI,
so it might not be possible to use this method.

If we rely on ACPI, we would need to ask the BIOS team to add ACPI entries.
I need to communicate with the customer's BIOS team, which may not be easy.

Personally, I think that using software node is more efficient when OF cannot be used.

Note: Patches with questions will continue to be discussed.
I will sent the patches that currently have no issues to net-next for acceptance.

Thanks,
Jijie Shao

> I did some research, and it seems that our board uses ACPI,
> so it might not be possible to use this method.
> 
> If we rely on ACPI, we would need to ask the BIOS team to add ACPI entries.
> I need to communicate with the customer's BIOS team, which may not be easy.

If you are going the ACPI route, please take a look at:

Documentation/firmware-guide/acpi/dsd/phy.rst

The basic structure of how to list PHYs is defined here.

And you also want to look at:

Documentation/firmware-guide/acpi/dsd/leds.rst

Since you are describing LEDs on a PHY. You will probably need to
extend phy.rst to reference leds.rst.

      Andrew

on 2025/12/16 15:17, Andrew Lunn wrote:
> On Mon, Dec 15, 2025 at 08:57:02PM +0800, Jijie Shao wrote:
>> +int hbg_mdio_init(struct hbg_priv *priv)
>> +{
>> +	struct device *dev = &priv->pdev->dev;
>> +	struct hbg_mac *mac = &priv->mac;
>>   	struct mii_bus *mdio_bus;
>>   	int ret;
>>   
>> @@ -281,7 +389,7 @@ int hbg_mdio_init(struct hbg_priv *priv)
>>   
>>   	mdio_bus->parent = dev;
>>   	mdio_bus->priv = priv;
>> -	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
>> +	mdio_bus->phy_mask = 0xFFFFFFFF; /* not scan phy device */
>>   	mdio_bus->name = "hibmcge mii bus";
>>   	mac->mdio_bus = mdio_bus;
> I think you are taking the wrong approach.
>
> Please consider trying to use of_mdiobus_register(). Either load a DT
> overlay, or see if you can construct a tree of properties as you have
> been doing, and pass it to of_mdiobus_register(). You then don't need
> to destroy and create PHY devices.
>
> 	Andrew


Ok， I will try like this

Thanks
Jijie Shao