[PATCH RFC net-next 3/6] net: hibmcge: create a software node for phy_led

Jijie Shao posted 6 patches 1 day, 15 hours ago
[PATCH RFC net-next 3/6] net: hibmcge: create a software node for phy_led
Posted by Jijie Shao 1 day, 15 hours ago
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
Re: [PATCH RFC net-next 3/6] net: hibmcge: create a software node for phy_led
Posted by Andrew Lunn 20 hours ago
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