[PATCH RFC net-next 0/6] Support PHY LED for hibmcge driver

[PATCH RFC net-next 0/6] Support PHY LED for hibmcge driver

[Jijie Shao]

Support PHY LED for hibmcge driver

Jijie Shao (6):
  net: phy: change of_phy_leds() to fwnode_phy_leds()
  net: phy: add support to set default rules
  net: hibmcge: create a software node for phy_led
  net: hibmcge: support get phy_leds_reg from spec register
  net: hibmcge: support get phy device from apci
  net: phy: motorcomm: fix duplex setting error for phy leds

 .../ethernet/hisilicon/hibmcge/hbg_common.h   |  11 ++
 .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 153 +++++++++++++++++-
 .../net/ethernet/hisilicon/hibmcge/hbg_reg.h  |   5 +
 drivers/net/phy/motorcomm.c                   |   4 +-
 drivers/net/phy/phy_device.c                  |  62 ++++---
 5 files changed, 206 insertions(+), 29 deletions(-)

-- 
2.33.0

[PATCH RFC net-next 1/6] net: phy: change of_phy_leds() to fwnode_phy_leds()

[Jijie Shao]

Change of_phy_leds() to fwnode_phy_leds(), to support
of node, acpi node, and software node together.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/phy/phy_device.c | 37 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 81984d4ebb7c..c5ce057f88ff 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3208,8 +3208,8 @@ static void phy_leds_unregister(struct phy_device *phydev)
 	}
 }
 
-static int of_phy_led(struct phy_device *phydev,
-		      struct device_node *led)
+static int fwnode_phy_led(struct phy_device *phydev,
+			  struct fwnode_handle *led)
 {
 	struct device *dev = &phydev->mdio.dev;
 	struct led_init_data init_data = {};
@@ -3226,17 +3226,17 @@ static int of_phy_led(struct phy_device *phydev,
 	cdev = &phyled->led_cdev;
 	phyled->phydev = phydev;
 
-	err = of_property_read_u32(led, "reg", &index);
+	err = fwnode_property_read_u32(led, "reg", &index);
 	if (err)
 		return err;
 	if (index > U8_MAX)
 		return -EINVAL;
 
-	if (of_property_read_bool(led, "active-high"))
+	if (fwnode_property_read_bool(led, "active-high"))
 		set_bit(PHY_LED_ACTIVE_HIGH, &modes);
-	if (of_property_read_bool(led, "active-low"))
+	if (fwnode_property_read_bool(led, "active-low"))
 		set_bit(PHY_LED_ACTIVE_LOW, &modes);
-	if (of_property_read_bool(led, "inactive-high-impedance"))
+	if (fwnode_property_read_bool(led, "inactive-high-impedance"))
 		set_bit(PHY_LED_INACTIVE_HIGH_IMPEDANCE, &modes);
 
 	if (WARN_ON(modes & BIT(PHY_LED_ACTIVE_LOW) &&
@@ -3273,7 +3273,7 @@ static int of_phy_led(struct phy_device *phydev,
 #endif
 	cdev->max_brightness = 1;
 	init_data.devicename = dev_name(&phydev->mdio.dev);
-	init_data.fwnode = of_fwnode_handle(led);
+	init_data.fwnode = led;
 	init_data.devname_mandatory = true;
 
 	err = led_classdev_register_ext(dev, cdev, &init_data);
@@ -3285,19 +3285,16 @@ static int of_phy_led(struct phy_device *phydev,
 	return 0;
 }
 
-static int of_phy_leds(struct phy_device *phydev)
+static int fwnode_phy_leds(struct phy_device *phydev)
 {
-	struct device_node *node = phydev->mdio.dev.of_node;
-	struct device_node *leds;
+	struct fwnode_handle *fwnode = dev_fwnode(&phydev->mdio.dev);
+	struct fwnode_handle *leds, *led;
 	int err;
 
-	if (!IS_ENABLED(CONFIG_OF_MDIO))
-		return 0;
-
-	if (!node)
+	if (!fwnode)
 		return 0;
 
-	leds = of_get_child_by_name(node, "leds");
+	leds = fwnode_get_named_child_node(fwnode, "leds");
 	if (!leds)
 		return 0;
 
@@ -3311,17 +3308,17 @@ static int of_phy_leds(struct phy_device *phydev)
 		goto exit;
 	}
 
-	for_each_available_child_of_node_scoped(leds, led) {
-		err = of_phy_led(phydev, led);
+	fwnode_for_each_available_child_node(leds, led) {
+		err = fwnode_phy_led(phydev, led);
 		if (err) {
-			of_node_put(leds);
+			fwnode_handle_put(leds);
 			phy_leds_unregister(phydev);
 			return err;
 		}
 	}
 
 exit:
-	of_node_put(leds);
+	fwnode_handle_put(leds);
 	return 0;
 }
 
@@ -3516,7 +3513,7 @@ static int phy_probe(struct device *dev)
 	 * LEDs for them.
 	 */
 	if (IS_ENABLED(CONFIG_PHYLIB_LEDS) && !phy_driver_is_genphy(phydev))
-		err = of_phy_leds(phydev);
+		err = fwnode_phy_leds(phydev);
 
 out:
 	/* Re-assert the reset signal on error */
-- 
2.33.0

[PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Jijie Shao]

The node of led need add new property: rules,
and rules can be set as:
BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX)

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/phy/phy_device.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index c5ce057f88ff..65bd0bf11e78 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3208,6 +3208,26 @@ static void phy_leds_unregister(struct phy_device *phydev)
 	}
 }
 
+static int fwnode_phy_led_set_rules(struct phy_device *phydev,
+				    struct fwnode_handle *led, u32 index)
+{
+	u32 rules;
+	int err;
+
+	if (!fwnode_property_present(led, "rules"))
+		return 0;
+
+	err = fwnode_property_read_u32(led, "rules", &rules);
+	if (err)
+		return err;
+
+	err = phydev->drv->led_hw_is_supported(phydev, index, rules);
+	if (err)
+		return err;
+
+	return phydev->drv->led_hw_control_set(phydev, index, rules);
+}
+
 static int fwnode_phy_led(struct phy_device *phydev,
 			  struct fwnode_handle *led)
 {
@@ -3253,6 +3273,11 @@ static int fwnode_phy_led(struct phy_device *phydev,
 			return err;
 	}
 
+	err = fwnode_phy_led_set_rules(phydev, led, index);
+	if (err)
+		phydev_warn(phydev, "failed to set rules for led%u, err = %d\n",
+			    index, err);
+
 	phyled->index = index;
 	if (phydev->drv->led_brightness_set)
 		cdev->brightness_set_blocking = phy_led_set_brightness;
-- 
2.33.0

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

[Jijie Shao]

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

[PATCH RFC net-next 4/6] net: hibmcge: support get phy_leds_reg from spec register

[Jijie Shao]

support get phy_leds_reg from spec register,
and init the software node according to the value.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 12 ++++++++++--
 drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h  |  5 +++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
index edd8ccefbb62..fb6ece3935e7 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -289,9 +289,11 @@ 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);
+	u32 phy_reg_num = hbg_reg_read(priv, HBG_REG_PHY_LEDS_REG);
 	struct device *dev = &priv->pdev->dev;
 	struct hbg_mac *mac = &priv->mac;
-	u32 node_index = 0, i;
+	u32 node_index = 0, i, reg;
+	unsigned long rules;
 	const char *label;
 	int ret;
 
@@ -310,12 +312,18 @@ static int hbg_register_phy_leds_software_node(struct hbg_priv *priv,
 	mac->nodes[node_index++] = &mac->leds_node;
 
 	for (i = 0; i < HBG_LED_MAX_NUM; i++) {
+		reg = (phy_reg_num >> (8 * i)) & 0xFF;
+
 		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);
 
+		rules = hbg_reg_read(priv,
+				     HBG_REG_PHY_LED0_RULES_ADDR + i * 4);
+		mac->leds_props[i][2] = PROPERTY_ENTRY_U32("rules", rules);
+
 		mac->led_nodes[i].name = devm_kasprintf(dev, GFP_KERNEL,
-							"led@%u", i);
+							"led@%u", reg);
 		mac->led_nodes[i].properties = mac->leds_props[i];
 		mac->led_nodes[i].parent = &mac->leds_node;
 
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index 30b3903c8f2d..2ad7b60874c7 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -12,12 +12,17 @@
 #define HBG_REG_MAC_ADDR_ADDR			0x0010
 #define HBG_REG_MAC_ADDR_HIGH_ADDR		0x0014
 #define HBG_REG_UC_MAC_NUM_ADDR			0x0018
+#define HBG_REG_PHY_LEDS_REG			0x0020
 #define HBG_REG_MDIO_FREQ_ADDR			0x0024
 #define HBG_REG_MAX_MTU_ADDR			0x0028
 #define HBG_REG_MIN_MTU_ADDR			0x002C
 #define HBG_REG_TX_FIFO_NUM_ADDR		0x0030
 #define HBG_REG_RX_FIFO_NUM_ADDR		0x0034
 #define HBG_REG_VLAN_LAYERS_ADDR		0x0038
+#define HBG_REG_PHY_LED0_RULES_ADDR		0x003C
+#define HBG_REG_PHY_LED1_RULES_ADDR		0x0040
+#define HBG_REG_PHY_LED2_RULES_ADDR		0x0044
+#define HBG_REG_PHY_LED3_RULES_ADDR		0x0048
 #define HBG_REG_PUSH_REQ_ADDR			0x00F0
 #define HBG_REG_MSG_HEADER_ADDR			0x00F4
 #define HBG_REG_MSG_HEADER_OPCODE_M		GENMASK(7, 0)
-- 
2.33.0

[PATCH RFC net-next 5/6] net: hibmcge: support get phy device from apci

[Jijie Shao]

support get phy device from apci.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
index fb6ece3935e7..36997634486b 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 // Copyright (c) 2024 Hisilicon Limited.
 
+#include <linux/acpi.h>
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 #include <linux/rtnetlink.h>
@@ -348,12 +349,36 @@ static int hbg_register_phy_leds_software_node(struct hbg_priv *priv,
 					&phydev->mdio.dev);
 }
 
+static int hbg_find_phy_device_from_acpi(struct hbg_priv *priv)
+{
+	struct device *dev = &priv->pdev->dev;
+	struct fwnode_handle *phy_fwnode;
+	struct phy_device *phydev;
+
+	phy_fwnode = fwnode_get_phy_node(dev_fwnode(dev));
+	if (IS_ERR(phy_fwnode))
+		return dev_err_probe(dev, PTR_ERR(phydev),
+				     "failed to get phy fwnode\n");
+
+	phydev = fwnode_phy_find_device(phy_fwnode);
+	/* We're done with the phy_node handle */
+	fwnode_handle_put(phy_fwnode);
+	if (!phydev)
+		return dev_err_probe(dev, -ENODEV,
+				     "failed to get phy fwnode device\n");
+	priv->mac.phydev = phydev;
+	return 0;
+}
+
 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;
 
+	if (has_acpi_companion(dev))
+		return hbg_find_phy_device_from_acpi(priv);
+
 	phydev = get_phy_device(mdio_bus, priv->mac.phy_addr, false);
 	if (IS_ERR(phydev))
 		return dev_err_probe(dev, -ENODEV,
-- 
2.33.0

[PATCH RFC net-next 6/6] net: phy: motorcomm: fix duplex setting error for phy leds

[Jijie Shao]

fix duplex setting error for phy leds

Fixes: 355b82c54c12 ("net: phy: motorcomm: Add support for PHY LEDs on YT8521")
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/phy/motorcomm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 89b5b19a9bd2..42d46b5758fc 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -1741,10 +1741,10 @@ static int yt8521_led_hw_control_set(struct phy_device *phydev, u8 index,
 		val |= YT8521_LED_1000_ON_EN;
 
 	if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
-		val |= YT8521_LED_HDX_ON_EN;
+		val |= YT8521_LED_FDX_ON_EN;
 
 	if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
-		val |= YT8521_LED_FDX_ON_EN;
+		val |= YT8521_LED_HDX_ON_EN;
 
 	if (test_bit(TRIGGER_NETDEV_TX, &rules) ||
 	    test_bit(TRIGGER_NETDEV_RX, &rules))
-- 
2.33.0

Re: [PATCH RFC net-next 0/6] Support PHY LED for hibmcge driver

[Simon Horman]

On Mon, Dec 15, 2025 at 08:56:59PM +0800, Jijie Shao wrote:
> Support PHY LED for hibmcge driver
> 
> Jijie Shao (6):
>   net: phy: change of_phy_leds() to fwnode_phy_leds()
>   net: phy: add support to set default rules
>   net: hibmcge: create a software node for phy_led
>   net: hibmcge: support get phy_leds_reg from spec register
>   net: hibmcge: support get phy device from apci
>   net: phy: motorcomm: fix duplex setting error for phy leds

...

## Form letter - net-next-closed

net-next is currently closed for new drivers, features, code refactoring and
optimizations. We are currently accepting bug fixes only.

net-next was closed when the merge window for v6.19 began. And due to a
combination of the travel commitments of the maintainers, and the holiday
season, net-next will not re-open until after 2nd January.

Please repost when net-next reopens.

RFC patches sent for review only are welcome at any time.

Thanks for your understanding.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle

-- 
pw-bot: defer

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Andrew Lunn]

On Mon, Dec 15, 2025 at 08:57:01PM +0800, Jijie Shao wrote:
> The node of led need add new property: rules,
> and rules can be set as:
> BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX)

Please could you expand this description. It is not clear to my why it
is needed. OF systems have not needed it so far. What is special about
your hardware?

Also, it looks like you are expanding the DT binding, so you need to
update the binding .yaml file.

	Andrew

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

[Andrew Lunn]

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

Re: [PATCH RFC net-next 4/6] net: hibmcge: support get phy_leds_reg from spec register

[Andrew Lunn]

On Mon, Dec 15, 2025 at 08:57:03PM +0800, Jijie Shao wrote:
> support get phy_leds_reg from spec register,

What is the spec register?

	Andrew

Re: [PATCH RFC net-next 6/6] net: phy: motorcomm: fix duplex setting error for phy leds

[Andrew Lunn]

On Mon, Dec 15, 2025 at 08:57:05PM +0800, Jijie Shao wrote:
> fix duplex setting error for phy leds
> 
> Fixes: 355b82c54c12 ("net: phy: motorcomm: Add support for PHY LEDs on YT8521")
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

Please don't mix new development and fixes in one patchset. Please
base this patch on net, and send it on its own.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Fixes are accepted any time, it does not matter about the merge
window.

	Andrew

Re: [PATCH RFC net-next 1/6] net: phy: change of_phy_leds() to fwnode_phy_leds()

[Jonathan Cameron]

On Mon, 15 Dec 2025 20:57:00 +0800
Jijie Shao <shaojijie@huawei.com> wrote:

> Change of_phy_leds() to fwnode_phy_leds(), to support
> of node, acpi node, and software node together.
> 
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

One minor suggestion inline. It is a 'while you are here'
and whilst there are uses of the _scoped loops
in drivers/net I'm not sure how much appetite there is
for using them wider.

Jonathan


> ---
>  drivers/net/phy/phy_device.c | 37 +++++++++++++++++-------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 81984d4ebb7c..c5ce057f88ff 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c


> -static int of_phy_leds(struct phy_device *phydev)
> +static int fwnode_phy_leds(struct phy_device *phydev)
>  {
> -	struct device_node *node = phydev->mdio.dev.of_node;
> -	struct device_node *leds;
> +	struct fwnode_handle *fwnode = dev_fwnode(&phydev->mdio.dev);
> +	struct fwnode_handle *leds, *led;
>  	int err;
>  
> -	if (!IS_ENABLED(CONFIG_OF_MDIO))
> -		return 0;
> -
> -	if (!node)
> +	if (!fwnode)
>  		return 0;
>  
> -	leds = of_get_child_by_name(node, "leds");
> +	leds = fwnode_get_named_child_node(fwnode, "leds");
>  	if (!leds)
>  		return 0;
>  
> @@ -3311,17 +3308,17 @@ static int of_phy_leds(struct phy_device *phydev)
>  		goto exit;
>  	}
>  
> -	for_each_available_child_of_node_scoped(leds, led) {
> -		err = of_phy_led(phydev, led);
> +	fwnode_for_each_available_child_node(leds, led) {

Maybe use the _scoped version to simplify this a little given
you are changing it.

> +		err = fwnode_phy_led(phydev, led);
>  		if (err) {
> -			of_node_put(leds);
> +			fwnode_handle_put(leds);
>  			phy_leds_unregister(phydev);
>  			return err;
>  		}
>  	}
>  
>  exit:
> -	of_node_put(leds);
> +	fwnode_handle_put(leds);
>  	return 0;
>  }
>  
> @@ -3516,7 +3513,7 @@ static int phy_probe(struct device *dev)
>  	 * LEDs for them.
>  	 */
>  	if (IS_ENABLED(CONFIG_PHYLIB_LEDS) && !phy_driver_is_genphy(phydev))
> -		err = of_phy_leds(phydev);
> +		err = fwnode_phy_leds(phydev);
>  
>  out:
>  	/* Re-assert the reset signal on error */

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Jijie Shao]

on 2025/12/16 15:09, Andrew Lunn wrote:
> On Mon, Dec 15, 2025 at 08:57:01PM +0800, Jijie Shao wrote:
>> The node of led need add new property: rules,
>> and rules can be set as:
>> BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX)
> Please could you expand this description. It is not clear to my why it
> is needed. OF systems have not needed it so far. What is special about
> your hardware?

I hope to configure the default rules.
Currently, the LED does not configure rules during initialization; it uses the default rules in the PHY registers.
I would like to change the default rules during initialization.


Some of our boards have only one LED, and we want to indicate both link and active(TX and RX) status simultaneously.

Thanks,
Jijie Shao

>
> Also, it looks like you are expanding the DT binding, so you need to
> update the binding .yaml file.
>
> 	Andrew

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

[Jijie Shao]

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

Re: [PATCH RFC net-next 4/6] net: hibmcge: support get phy_leds_reg from spec register

[Jijie Shao]

on 2025/12/16 15:19, Andrew Lunn wrote:
> On Mon, Dec 15, 2025 at 08:57:03PM +0800, Jijie Shao wrote:
>> support get phy_leds_reg from spec register,
> What is the spec register?

These are some MAC and PHY-related properties, such as MTU specifications, MDIO frequency, etc.,
similar to the properties in the device tree.
However, these properties come from the BAR space registers.

Thanks,
Jijie Shao

Re: [PATCH RFC net-next 6/6] net: phy: motorcomm: fix duplex setting error for phy leds

[Jijie Shao]

on 2025/12/16 15:21, Andrew Lunn wrote:
> On Mon, Dec 15, 2025 at 08:57:05PM +0800, Jijie Shao wrote:
>> fix duplex setting error for phy leds
>>
>> Fixes: 355b82c54c12 ("net: phy: motorcomm: Add support for PHY LEDs on YT8521")
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> Please don't mix new development and fixes in one patchset. Please
> base this patch on net, and send it on its own.
>
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>
> Fixes are accepted any time, it does not matter about the merge
> window.
>
> 	Andrew

Yes, this is RFC, just for code review.
I sent it to net when requesting a merge.

Thanks,
Jijie Shao

Re: [PATCH RFC net-next 1/6] net: phy: change of_phy_leds() to fwnode_phy_leds()

[Jijie Shao]

on 2025/12/17 18:37, Jonathan Cameron wrote:
> On Mon, 15 Dec 2025 20:57:00 +0800
> Jijie Shao <shaojijie@huawei.com> wrote:
>
>> Change of_phy_leds() to fwnode_phy_leds(), to support
>> of node, acpi node, and software node together.
>>
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> One minor suggestion inline. It is a 'while you are here'
> and whilst there are uses of the _scoped loops
> in drivers/net I'm not sure how much appetite there is
> for using them wider.
>
> Jonathan
>
>
>> ---
>>   drivers/net/phy/phy_device.c | 37 +++++++++++++++++-------------------
>>   1 file changed, 17 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 8
>> -
>> -	if (!node)
>> +	if (!fwnode)
>>   		return 0;
>>   
>> -	leds = of_get_child_by_name(node, "leds");
>> +	leds = fwnode_get_named_child_node(fwnode, "leds");
>>   	if (!leds)
>>   		return 0;
>>   
>> @@ -3311,17 +3308,17 @@ static int of_phy_leds(struct phy_device *phydev)
>>   		goto exit;
>>   	}
>>   
>> -	for_each_available_child_of_node_scoped(leds, led) {
>> -		err = of_phy_led(phydev, led);
>> +	fwnode_for_each_available_child_node(leds, led) {
> Maybe use the _scoped version to simplify this a little given
> you are changing it.


Yes, it is indeed necessary to use fwnode_for_each_available_child_node_scoped()

Thanks,
Jijie Shao

Re: [PATCH RFC net-next 6/6] net: phy: motorcomm: fix duplex setting error for phy leds

[Andrew Lunn]

On Wed, Dec 17, 2025 at 09:05:20PM +0800, Jijie Shao wrote:
> 
> on 2025/12/16 15:21, Andrew Lunn wrote:
> > On Mon, Dec 15, 2025 at 08:57:05PM +0800, Jijie Shao wrote:
> > > fix duplex setting error for phy leds
> > > 
> > > Fixes: 355b82c54c12 ("net: phy: motorcomm: Add support for PHY LEDs on YT8521")
> > > Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> > Please don't mix new development and fixes in one patchset. Please
> > base this patch on net, and send it on its own.
> > 
> > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> > 
> > Fixes are accepted any time, it does not matter about the merge
> > window.
> > 
> > 	Andrew
> 
> Yes, this is RFC, just for code review.
> I sent it to net when requesting a merge.

This is however a real fix. The motorcomm driver is broken. Why wait
to fix it?

	Andrew

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Andrew Lunn]

> Some of our boards have only one LED, and we want to indicate both
> link and active(TX and RX) status simultaneously.

Configuration is generally policy, which is normally done in
userspace. I would suggest a udev rule.

	Andrew

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Jijie Shao]

on 2025/12/17 21:53, Andrew Lunn wrote:
>> Some of our boards have only one LED, and we want to indicate both
>> link and active(TX and RX) status simultaneously.
> Configuration is generally policy, which is normally done in
> userspace. I would suggest a udev rule.
>
> 	Andrew

Yes, the PHY LED framework supports configuration from user space,
allowing users to configure their preferred policies according to their own requirements.
I believe this is the original intention of the LED framework.

However, we cannot require users to actively configure policies,
nor can we restrict the types of OS versions they use.
Therefore, I personally think that the driver should still provide a reasonable default policy
to ensure that the LED behavior meets the needs of most scenarios.


Thanks!
Jijie Shao

Re: [PATCH RFC net-next 6/6] net: phy: motorcomm: fix duplex setting error for phy leds

[Jijie Shao]

on 2025/12/17 21:49, Andrew Lunn wrote:
> On Wed, Dec 17, 2025 at 09:05:20PM +0800, Jijie Shao wrote:
>> on 2025/12/16 15:21, Andrew Lunn wrote:
>>> On Mon, Dec 15, 2025 at 08:57:05PM +0800, Jijie Shao wrote:
>>>> fix duplex setting error for phy leds
>>>>
>>>> Fixes: 355b82c54c12 ("net: phy: motorcomm: Add support for PHY LEDs on YT8521")
>>>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
>>> Please don't mix new development and fixes in one patchset. Please
>>> base this patch on net, and send it on its own.
>>>
>>> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
>>>
>>> Fixes are accepted any time, it does not matter about the merge
>>> window.
>>>
>>> 	Andrew
>> Yes, this is RFC, just for code review.
>> I sent it to net when requesting a merge.
> This is however a real fix. The motorcomm driver is broken. Why wait
> to fix it?
>
> 	Andrew


Alright, this patch shouldn't require an RFC
and has no direct relation to this patch set.

I'll send it directly to net.

Thanks!
Shao Jijie

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Andrew Lunn]

On Thu, Dec 18, 2025 at 09:35:44AM +0800, Jijie Shao wrote:
> 
> on 2025/12/17 21:53, Andrew Lunn wrote:
> > > Some of our boards have only one LED, and we want to indicate both
> > > link and active(TX and RX) status simultaneously.
> > Configuration is generally policy, which is normally done in
> > userspace. I would suggest a udev rule.
> > 
> > 	Andrew
> 
> Yes, the PHY LED framework supports configuration from user space,
> allowing users to configure their preferred policies according to their own requirements.
> I believe this is the original intention of the LED framework.
> 
> However, we cannot require users to actively configure policies,
> nor can we restrict the types of OS versions they use.
> Therefore, I personally think that the driver should still provide a reasonable default policy
> to ensure that the LED behavior meets the needs of most scenarios.

The DT Maintainers are likely to push back if you add this to the DT
binding. You are not really describing the hardware.

In your case, it is the MAC driver which has access to the
configuration you want to use. So please think about how you can add
an API a MAC driver could use. It is not so easy, since the PHY led is
just a Linux LED. It can be used for anything, any trigger can be
assigned to it, like the heartbeat, CPU load, disc activity, etc. You
also need to look at keeping the state synchronised. The netdev
trigger will read the state of the LED when it is bound to the
LED. After that, it assumes it is controlling the LED. Any changes
which go direct to the LED will not be seen by the LED trigger.

	Andrew

Re: [PATCH RFC net-next 2/6] net: phy: add support to set default rules

[Russell King (Oracle)]

On Wed, Dec 17, 2025 at 08:54:59PM +0800, Jijie Shao wrote:
> 
> on 2025/12/16 15:09, Andrew Lunn wrote:
> > On Mon, Dec 15, 2025 at 08:57:01PM +0800, Jijie Shao wrote:
> > > The node of led need add new property: rules,
> > > and rules can be set as:
> > > BIT(TRIGGER_NETDEV_LINK) | BIT(TRIGGER_NETDEV_RX)
> > Please could you expand this description. It is not clear to my why it
> > is needed. OF systems have not needed it so far. What is special about
> > your hardware?
> 
> I hope to configure the default rules.
> Currently, the LED does not configure rules during initialization; it uses the default rules in the PHY registers.
> I would like to change the default rules during initialization.

One of the issues here is that there are boards out there where the boot
loader has configured the PHY LED configuration - and doesn't supply it
via DT (because PHY LED configuration in the kernel is a new thing.)

Adding default rules for LEDs will break these platforms.

Please find a way to provide the LED rules via firmware rather than
introducing some kind of rule defaulting.

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