[PATCH net-next 0/2] net: hibmcge: Add support for PHY LEDs on YT8521

[PATCH net-next 0/2] net: hibmcge: Add support for PHY LEDs on YT8521

[Jijie Shao]

net: hibmcge: Add support for PHY LEDs on YT8521

Jijie Shao (2):
  net: phy: motorcomm: Add support for PHY LEDs on YT8521
  net: hibmcge: Add support for PHY LEDs on YT8521

 drivers/net/ethernet/hisilicon/Kconfig        |   8 ++
 .../net/ethernet/hisilicon/hibmcge/Makefile   |   1 +
 .../net/ethernet/hisilicon/hibmcge/hbg_led.c  | 132 ++++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_led.h  |  17 +++
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |   7 +
 drivers/net/phy/motorcomm.c                   | 120 ++++++++++++++++
 6 files changed, 285 insertions(+)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_led.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_led.h

-- 
2.33.0

[PATCH net-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Jijie Shao]

Add minimal LED controller driver supporting
the most common uses with the 'netdev' trigger.

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

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 0e91f5d1a4fd..e1a1c3a1c9d0 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -213,6 +213,23 @@
 #define YT8521_RC1R_RGMII_2_100_NS		14
 #define YT8521_RC1R_RGMII_2_250_NS		15
 
+/* LED CONFIG */
+#define YT8521_MAX_LEDS				3
+#define YT8521_LED0_CFG_REG			0xA00C
+#define YT8521_LED1_CFG_REG			0xA00D
+#define YT8521_LED2_CFG_REG			0xA00E
+#define YT8521_LED_ACT_BLK_IND			BIT(13)
+#define YT8521_LED_FDX_ON_EN			BIT(12)
+#define YT8521_LED_HDX_ON_EN			BIT(11)
+#define YT8521_LED_TXACT_BLK_EN			BIT(10)
+#define YT8521_LED_RXACT_BLK_EN			BIT(9)
+/* 1000Mbps */
+#define YT8521_LED_GT_ON_EN			BIT(6)
+/* 100Mbps */
+#define YT8521_LED_HT_ON_EN			BIT(5)
+/* 10Mbps */
+#define YT8521_LED_BT_ON_EN			BIT(4)
+
 #define YTPHY_MISC_CONFIG_REG			0xA006
 #define YTPHY_MCR_FIBER_SPEED_MASK		BIT(0)
 #define YTPHY_MCR_FIBER_1000BX			(0x1 << 0)
@@ -1681,6 +1698,106 @@ static int yt8521_config_init(struct phy_device *phydev)
 	return phy_restore_page(phydev, old_page, ret);
 }
 
+static const unsigned long supported_trgs = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
+					     BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
+					     BIT(TRIGGER_NETDEV_LINK)        |
+					     BIT(TRIGGER_NETDEV_LINK_10)     |
+					     BIT(TRIGGER_NETDEV_LINK_100)    |
+					     BIT(TRIGGER_NETDEV_LINK_1000)   |
+					     BIT(TRIGGER_NETDEV_RX)          |
+					     BIT(TRIGGER_NETDEV_TX));
+
+static int yt8521_led_hw_is_supported(struct phy_device *phydev, u8 index,
+				      unsigned long rules)
+{
+	if (index >= YT8521_MAX_LEDS)
+		return -EINVAL;
+
+	/* All combinations of the supported triggers are allowed */
+	if (rules & ~supported_trgs)
+		return -EOPNOTSUPP;
+
+	return 0;
+}
+
+static int yt8521_led_hw_control_set(struct phy_device *phydev, u8 index,
+				     unsigned long rules)
+{
+	u16 val = 0;
+
+	if (index >= YT8521_MAX_LEDS)
+		return -EINVAL;
+
+	if (test_bit(TRIGGER_NETDEV_LINK, &rules)) {
+		val |= YT8521_LED_BT_ON_EN;
+		val |= YT8521_LED_HT_ON_EN;
+		val |= YT8521_LED_GT_ON_EN;
+	}
+
+	if (test_bit(TRIGGER_NETDEV_LINK_10, &rules))
+		val |= YT8521_LED_BT_ON_EN;
+
+	if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
+		val |= YT8521_LED_HT_ON_EN;
+
+	if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
+		val |= YT8521_LED_GT_ON_EN;
+
+	if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
+		val |= YT8521_LED_HDX_ON_EN;
+
+	if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
+		val |= YT8521_LED_FDX_ON_EN;
+
+	if (test_bit(TRIGGER_NETDEV_TX, &rules) ||
+	    test_bit(TRIGGER_NETDEV_RX, &rules))
+		val |= YT8521_LED_ACT_BLK_IND;
+
+	if (test_bit(TRIGGER_NETDEV_TX, &rules))
+		val |= YT8521_LED_TXACT_BLK_EN;
+
+	if (test_bit(TRIGGER_NETDEV_RX, &rules))
+		val |= YT8521_LED_RXACT_BLK_EN;
+
+	return ytphy_write_ext(phydev, YT8521_LED0_CFG_REG + index, val);
+}
+
+static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
+				     unsigned long *rules)
+{
+	int val;
+
+	if (index >= YT8521_MAX_LEDS)
+		return -EINVAL;
+
+	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
+	if (val < 0)
+		return val;
+
+	if (val & YT8521_LED_TXACT_BLK_EN)
+		set_bit(TRIGGER_NETDEV_TX, rules);
+
+	if (val & YT8521_LED_RXACT_BLK_EN)
+		set_bit(TRIGGER_NETDEV_RX, rules);
+
+	if (val & YT8521_LED_FDX_ON_EN)
+		set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
+
+	if (val & YT8521_LED_HDX_ON_EN)
+		set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
+
+	if (val & YT8521_LED_GT_ON_EN)
+		set_bit(TRIGGER_NETDEV_LINK_1000, rules);
+
+	if (val & YT8521_LED_HT_ON_EN)
+		set_bit(TRIGGER_NETDEV_LINK_100, rules);
+
+	if (val & YT8521_LED_BT_ON_EN)
+		set_bit(TRIGGER_NETDEV_LINK_10, rules);
+
+	return 0;
+}
+
 static int yt8531_config_init(struct phy_device *phydev)
 {
 	struct device_node *node = phydev->mdio.dev.of_node;
@@ -2920,6 +3037,9 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 		.soft_reset	= yt8521_soft_reset,
 		.suspend	= yt8521_suspend,
 		.resume		= yt8521_resume,
+		.led_hw_is_supported = yt8521_led_hw_is_supported,
+		.led_hw_control_set = yt8521_led_hw_control_set,
+		.led_hw_control_get = yt8521_led_hw_control_get,
 	},
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
-- 
2.33.0

[PATCH net-next 2/2] net: hibmcge: Add support for PHY LEDs on YT8521

[Jijie Shao]

hibmcge is a PCIE EP device, and its controller is
not on the board. And board uses ACPI not DTS
to create the device tree.

So, this makes it impossible to add a "reg" property(used in of_phy_led())
for hibmcge. Therefore, the PHY_LED framework cannot be used directly.

This patch creates a separate LED device for hibmcge
and directly calls the phy->drv->led_hw**() function to
operate the related LEDs.

Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
 drivers/net/ethernet/hisilicon/Kconfig        |   8 ++
 .../net/ethernet/hisilicon/hibmcge/Makefile   |   1 +
 .../net/ethernet/hisilicon/hibmcge/hbg_led.c  | 132 ++++++++++++++++++
 .../net/ethernet/hisilicon/hibmcge/hbg_led.h  |  17 +++
 .../net/ethernet/hisilicon/hibmcge/hbg_main.c |   7 +
 5 files changed, 165 insertions(+)
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_led.c
 create mode 100644 drivers/net/ethernet/hisilicon/hibmcge/hbg_led.h

diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
index 65302c41bfb1..143b25f329c7 100644
--- a/drivers/net/ethernet/hisilicon/Kconfig
+++ b/drivers/net/ethernet/hisilicon/Kconfig
@@ -157,4 +157,12 @@ config HIBMCGE
 
 	  If you are unsure, say N.
 
+config HIBMCGE_LEDS
+	def_bool LEDS_TRIGGER_NETDEV
+	depends on HIBMCGE && LEDS_CLASS
+	depends on LEDS_CLASS=y || HIBMCGE=m
+	help
+	  Optional support for controlling the NIC LED's with the netdev
+	  LED trigger.
+
 endif # NET_VENDOR_HISILICON
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/Makefile b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
index 1a9da564b306..a78057208064 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/Makefile
+++ b/drivers/net/ethernet/hisilicon/hibmcge/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_HIBMCGE) += hibmcge.o
 
 hibmcge-objs = hbg_main.o hbg_hw.o hbg_mdio.o hbg_irq.o hbg_txrx.o hbg_ethtool.o \
 		hbg_debugfs.o hbg_err.o hbg_diagnose.o
+hibmcge-$(CONFIG_HIBMCGE_LEDS) += hbg_led.o
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.c
new file mode 100644
index 000000000000..013eae1c54f2
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2025 Hisilicon Limited.
+
+#include <linux/device.h>
+#include <linux/etherdevice.h>
+#include <linux/phy.h>
+#include "hbg_common.h"
+#include "hbg_led.h"
+
+#define PHY_ID_YT8521		0x0000011a
+
+#define to_hbg_led(lcdev) container_of(lcdev, struct hbg_led_classdev, led)
+#define to_hbg_phy_dev(lcdev) \
+	(((struct hbg_led_classdev *)to_hbg_led(lcdev))->priv->mac.phydev)
+
+static int hbg_led_hw_control_set(struct led_classdev *led_cdev,
+				  unsigned long rules)
+{
+	struct hbg_led_classdev *hbg_led = to_hbg_led(led_cdev);
+	struct phy_device *phydev = to_hbg_phy_dev(led_cdev);
+	int ret;
+
+	mutex_lock(&phydev->lock);
+	ret = phydev->drv->led_hw_control_set(phydev, hbg_led->index, rules);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+static int hbg_led_hw_control_get(struct led_classdev *led_cdev,
+				  unsigned long *rules)
+{
+	struct hbg_led_classdev *hbg_led = to_hbg_led(led_cdev);
+	struct phy_device *phydev = to_hbg_phy_dev(led_cdev);
+	int ret;
+
+	mutex_lock(&phydev->lock);
+	ret = phydev->drv->led_hw_control_get(phydev, hbg_led->index, rules);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+static int hbg_led_hw_is_supported(struct led_classdev *led_cdev,
+				   unsigned long rules)
+{
+	struct hbg_led_classdev *hbg_led = to_hbg_led(led_cdev);
+	struct phy_device *phydev = to_hbg_phy_dev(led_cdev);
+	int ret;
+
+	mutex_lock(&phydev->lock);
+	ret = phydev->drv->led_hw_is_supported(phydev, hbg_led->index, rules);
+	mutex_unlock(&phydev->lock);
+
+	return ret;
+}
+
+static struct device *
+	hbg_led_hw_control_get_device(struct led_classdev *led_cdev)
+{
+	struct hbg_led_classdev *hbg_led = to_hbg_led(led_cdev);
+
+	return &hbg_led->priv->netdev->dev;
+}
+
+static int hbg_setup_ldev(struct hbg_led_classdev *hbg_led)
+{
+	struct led_classdev *ldev = &hbg_led->led;
+	struct hbg_priv *priv = hbg_led->priv;
+	struct device *dev = &priv->pdev->dev;
+
+	ldev->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%s-%d",
+				    dev_driver_string(dev),
+				    pci_name(priv->pdev), hbg_led->index);
+	if (!ldev->name)
+		return -ENOMEM;
+
+	ldev->hw_control_trigger = "netdev";
+	ldev->hw_control_set = hbg_led_hw_control_set;
+	ldev->hw_control_get = hbg_led_hw_control_get;
+	ldev->hw_control_is_supported = hbg_led_hw_is_supported;
+	ldev->hw_control_get_device = hbg_led_hw_control_get_device;
+
+	return devm_led_classdev_register(dev, ldev);
+}
+
+static u32 hbg_get_phy_max_led_count(struct hbg_priv *priv)
+{
+	struct phy_device *phydev = priv->mac.phydev;
+
+	if (!phydev->drv->led_hw_is_supported ||
+	    !phydev->drv->led_hw_control_set ||
+	    !phydev->drv->led_hw_control_get)
+		return 0;
+
+	/* YT8521, support 3 leds */
+	if (phydev->drv->phy_id == PHY_ID_YT8521)
+		return 3;
+
+	return 0;
+}
+
+int hbg_leds_init(struct hbg_priv *priv)
+{
+	u32 led_count = hbg_get_phy_max_led_count(priv);
+	struct phy_device *phydev = priv->mac.phydev;
+	struct hbg_led_classdev *leds;
+	int ret;
+	int i;
+
+	if (!led_count)
+		return 0;
+
+	leds = devm_kcalloc(&priv->pdev->dev, led_count,
+			    sizeof(*leds), GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	for (i = 0; i < led_count; i++) {
+		/* for YT8521, we only have two lights, 0 and 2. */
+		if (phydev->drv->phy_id == PHY_ID_YT8521 && i == 1)
+			continue;
+
+		leds[i].priv = priv;
+		leds[i].index = i;
+		ret = hbg_setup_ldev(&leds[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.h
new file mode 100644
index 000000000000..463285077c91
--- /dev/null
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_led.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (c) 2025 Hisilicon Limited. */
+
+#ifndef __HBG_LED_H
+#define __HBG_LED_H
+
+#include "hbg_common.h"
+
+struct hbg_led_classdev {
+	struct hbg_priv *priv;
+	struct led_classdev led;
+	u32 index;
+};
+
+int hbg_leds_init(struct hbg_priv *priv);
+
+#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index 2e64dc1ab355..f2f8f651f3d2 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -12,6 +12,7 @@
 #include "hbg_ethtool.h"
 #include "hbg_hw.h"
 #include "hbg_irq.h"
+#include "hbg_led.h"
 #include "hbg_mdio.h"
 #include "hbg_txrx.h"
 #include "hbg_debugfs.h"
@@ -383,6 +384,12 @@ static int hbg_init(struct hbg_priv *priv)
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_HIBMCGE_LEDS)) {
+		ret = hbg_leds_init(priv);
+		if (ret)
+			return ret;
+	}
+
 	ret = hbg_mac_filter_init(priv);
 	if (ret)
 		return ret;
-- 
2.33.0

Re: [PATCH net-next 2/2] net: hibmcge: Add support for PHY LEDs on YT8521

[Russell King (Oracle)]

On Wed, Jul 16, 2025 at 06:00:41PM +0800, Jijie Shao wrote:
> hibmcge is a PCIE EP device, and its controller is
> not on the board. And board uses ACPI not DTS
> to create the device tree.
> 
> So, this makes it impossible to add a "reg" property(used in of_phy_led())
> for hibmcge. Therefore, the PHY_LED framework cannot be used directly.

It would be better to find a way to solve this problem, rather than
we end up with every ethernet driver having its own LED code. Each
driver having its own LED code simply won't scale for over-worked
kernel maintainers.

Sorry, but NAK.

-- 
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-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Andrew Lunn]

On Wed, Jul 16, 2025 at 06:00:40PM +0800, Jijie Shao wrote:
> Add minimal LED controller driver supporting
> the most common uses with the 'netdev' trigger.
> 
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> ---
>  drivers/net/phy/motorcomm.c | 120 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 0e91f5d1a4fd..e1a1c3a1c9d0 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -213,6 +213,23 @@
>  #define YT8521_RC1R_RGMII_2_100_NS		14
>  #define YT8521_RC1R_RGMII_2_250_NS		15
>  
> +/* LED CONFIG */
> +#define YT8521_MAX_LEDS				3
> +#define YT8521_LED0_CFG_REG			0xA00C
> +#define YT8521_LED1_CFG_REG			0xA00D
> +#define YT8521_LED2_CFG_REG			0xA00E
> +#define YT8521_LED_ACT_BLK_IND			BIT(13)
> +#define YT8521_LED_FDX_ON_EN			BIT(12)
> +#define YT8521_LED_HDX_ON_EN			BIT(11)
> +#define YT8521_LED_TXACT_BLK_EN			BIT(10)
> +#define YT8521_LED_RXACT_BLK_EN			BIT(9)
> +/* 1000Mbps */
> +#define YT8521_LED_GT_ON_EN			BIT(6)
> +/* 100Mbps */
> +#define YT8521_LED_HT_ON_EN			BIT(5)
> +/* 10Mbps */
> +#define YT8521_LED_BT_ON_EN			BIT(4)

Rather than comments, why not call these YT8521_LED_1000_ON_EN,
YT8521_LED_100_ON_EN, YT8521_LED_100_ON_EN ? That makes the rest of
the driver easier to read.

> +static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
> +				     unsigned long *rules)
> +{
> +	int val;
> +
> +	if (index >= YT8521_MAX_LEDS)
> +		return -EINVAL;
> +
> +	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & YT8521_LED_TXACT_BLK_EN)
> +		set_bit(TRIGGER_NETDEV_TX, rules);
> +
> +	if (val & YT8521_LED_RXACT_BLK_EN)
> +		set_bit(TRIGGER_NETDEV_RX, rules);

This looks to be missing YT8521_LED_ACT_BLK_IND.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next 2/2] net: hibmcge: Add support for PHY LEDs on YT8521

[Andrew Lunn]

On Wed, Jul 16, 2025 at 06:00:41PM +0800, Jijie Shao wrote:
> hibmcge is a PCIE EP device, and its controller is
> not on the board. And board uses ACPI not DTS
> to create the device tree.
> 
> So, this makes it impossible to add a "reg" property(used in of_phy_led())
> for hibmcge. Therefore, the PHY_LED framework cannot be used directly.
> 
> This patch creates a separate LED device for hibmcge
> and directly calls the phy->drv->led_hw**() function to
> operate the related LEDs.

Extending what Russell said, please take a look at:

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

and extend it to cover PHY LEDs.

	Andrew

Re: [PATCH net-next 2/2] net: hibmcge: Add support for PHY LEDs on YT8521

[Jijie Shao]

on 2025/7/17 0:42, Andrew Lunn wrote:
> On Wed, Jul 16, 2025 at 06:00:41PM +0800, Jijie Shao wrote:
>> hibmcge is a PCIE EP device, and its controller is
>> not on the board. And board uses ACPI not DTS
>> to create the device tree.
>>
>> So, this makes it impossible to add a "reg" property(used in of_phy_led())
>> for hibmcge. Therefore, the PHY_LED framework cannot be used directly.
>>
>> This patch creates a separate LED device for hibmcge
>> and directly calls the phy->drv->led_hw**() function to
>> operate the related LEDs.
> Extending what Russell said, please take a look at:
>
> Documentation/firmware-guide/acpi/dsd/phy.rst
>
> and extend it to cover PHY LEDs.
>
> 	Andrew

Sure. I'll take a look at this.

Thanks,
Jijie Shao

Re: [PATCH net-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Heiko Stübner]

Am Mittwoch, 16. Juli 2025, 12:00:40 Mitteleuropäische Sommerzeit schrieb Jijie Shao:
> Add minimal LED controller driver supporting
> the most common uses with the 'netdev' trigger.
> 
> Signed-off-by: Jijie Shao <shaojijie@huawei.com>

On a Qnap TS233 NAS using this phy, I get the expected device LEDs
to light up (with appropriate config via sysfs), so

Tested-by: Heiko Stuebner <heiko@sntech.de>

(haven't found a v2 yet yesterday, so hopefully still the right thread
to reply to ;-) )

Thanks
Heiko


> ---
>  drivers/net/phy/motorcomm.c | 120 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 0e91f5d1a4fd..e1a1c3a1c9d0 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -213,6 +213,23 @@
>  #define YT8521_RC1R_RGMII_2_100_NS		14
>  #define YT8521_RC1R_RGMII_2_250_NS		15
>  
> +/* LED CONFIG */
> +#define YT8521_MAX_LEDS				3
> +#define YT8521_LED0_CFG_REG			0xA00C
> +#define YT8521_LED1_CFG_REG			0xA00D
> +#define YT8521_LED2_CFG_REG			0xA00E
> +#define YT8521_LED_ACT_BLK_IND			BIT(13)
> +#define YT8521_LED_FDX_ON_EN			BIT(12)
> +#define YT8521_LED_HDX_ON_EN			BIT(11)
> +#define YT8521_LED_TXACT_BLK_EN			BIT(10)
> +#define YT8521_LED_RXACT_BLK_EN			BIT(9)
> +/* 1000Mbps */
> +#define YT8521_LED_GT_ON_EN			BIT(6)
> +/* 100Mbps */
> +#define YT8521_LED_HT_ON_EN			BIT(5)
> +/* 10Mbps */
> +#define YT8521_LED_BT_ON_EN			BIT(4)
> +
>  #define YTPHY_MISC_CONFIG_REG			0xA006
>  #define YTPHY_MCR_FIBER_SPEED_MASK		BIT(0)
>  #define YTPHY_MCR_FIBER_1000BX			(0x1 << 0)
> @@ -1681,6 +1698,106 @@ static int yt8521_config_init(struct phy_device *phydev)
>  	return phy_restore_page(phydev, old_page, ret);
>  }
>  
> +static const unsigned long supported_trgs = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
> +					     BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
> +					     BIT(TRIGGER_NETDEV_LINK)        |
> +					     BIT(TRIGGER_NETDEV_LINK_10)     |
> +					     BIT(TRIGGER_NETDEV_LINK_100)    |
> +					     BIT(TRIGGER_NETDEV_LINK_1000)   |
> +					     BIT(TRIGGER_NETDEV_RX)          |
> +					     BIT(TRIGGER_NETDEV_TX));
> +
> +static int yt8521_led_hw_is_supported(struct phy_device *phydev, u8 index,
> +				      unsigned long rules)
> +{
> +	if (index >= YT8521_MAX_LEDS)
> +		return -EINVAL;
> +
> +	/* All combinations of the supported triggers are allowed */
> +	if (rules & ~supported_trgs)
> +		return -EOPNOTSUPP;
> +
> +	return 0;
> +}
> +
> +static int yt8521_led_hw_control_set(struct phy_device *phydev, u8 index,
> +				     unsigned long rules)
> +{
> +	u16 val = 0;
> +
> +	if (index >= YT8521_MAX_LEDS)
> +		return -EINVAL;
> +
> +	if (test_bit(TRIGGER_NETDEV_LINK, &rules)) {
> +		val |= YT8521_LED_BT_ON_EN;
> +		val |= YT8521_LED_HT_ON_EN;
> +		val |= YT8521_LED_GT_ON_EN;
> +	}
> +
> +	if (test_bit(TRIGGER_NETDEV_LINK_10, &rules))
> +		val |= YT8521_LED_BT_ON_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
> +		val |= YT8521_LED_HT_ON_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
> +		val |= YT8521_LED_GT_ON_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
> +		val |= YT8521_LED_HDX_ON_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
> +		val |= YT8521_LED_FDX_ON_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_TX, &rules) ||
> +	    test_bit(TRIGGER_NETDEV_RX, &rules))
> +		val |= YT8521_LED_ACT_BLK_IND;
> +
> +	if (test_bit(TRIGGER_NETDEV_TX, &rules))
> +		val |= YT8521_LED_TXACT_BLK_EN;
> +
> +	if (test_bit(TRIGGER_NETDEV_RX, &rules))
> +		val |= YT8521_LED_RXACT_BLK_EN;
> +
> +	return ytphy_write_ext(phydev, YT8521_LED0_CFG_REG + index, val);
> +}
> +
> +static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
> +				     unsigned long *rules)
> +{
> +	int val;
> +
> +	if (index >= YT8521_MAX_LEDS)
> +		return -EINVAL;
> +
> +	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & YT8521_LED_TXACT_BLK_EN)
> +		set_bit(TRIGGER_NETDEV_TX, rules);
> +
> +	if (val & YT8521_LED_RXACT_BLK_EN)
> +		set_bit(TRIGGER_NETDEV_RX, rules);
> +
> +	if (val & YT8521_LED_FDX_ON_EN)
> +		set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
> +
> +	if (val & YT8521_LED_HDX_ON_EN)
> +		set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
> +
> +	if (val & YT8521_LED_GT_ON_EN)
> +		set_bit(TRIGGER_NETDEV_LINK_1000, rules);
> +
> +	if (val & YT8521_LED_HT_ON_EN)
> +		set_bit(TRIGGER_NETDEV_LINK_100, rules);
> +
> +	if (val & YT8521_LED_BT_ON_EN)
> +		set_bit(TRIGGER_NETDEV_LINK_10, rules);
> +
> +	return 0;
> +}
> +
>  static int yt8531_config_init(struct phy_device *phydev)
>  {
>  	struct device_node *node = phydev->mdio.dev.of_node;
> @@ -2920,6 +3037,9 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>  		.soft_reset	= yt8521_soft_reset,
>  		.suspend	= yt8521_suspend,
>  		.resume		= yt8521_resume,
> +		.led_hw_is_supported = yt8521_led_hw_is_supported,
> +		.led_hw_control_set = yt8521_led_hw_control_set,
> +		.led_hw_control_get = yt8521_led_hw_control_get,
>  	},
>  	{
>  		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
> 

Re: [PATCH net-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Jijie Shao]

on 2025/8/7 17:50, Heiko Stübner wrote:
> Am Mittwoch, 16. Juli 2025, 12:00:40 Mitteleuropäische Sommerzeit schrieb Jijie Shao:
>> Add minimal LED controller driver supporting
>> the most common uses with the 'netdev' trigger.
>>
>> Signed-off-by: Jijie Shao <shaojijie@huawei.com>
> On a Qnap TS233 NAS using this phy, I get the expected device LEDs
> to light up (with appropriate config via sysfs), so
>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
>
> (haven't found a v2 yet yesterday, so hopefully still the right thread
> to reply to ;-) )
>
> Thanks
> Heiko

Thank you for testing; I will resend this patch shortly.

>
>
>> ---
>>   drivers/net/phy/motorcomm.c | 120 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 120 insertions(+)
>>
>> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
>> index 0e91f5d1a4fd..e1a1c3a1c9d0 100644
>> --- a/drivers/net/phy/motorcomm.c
>> +++ b/drivers/net/phy/motorcomm.c
>> @@ -213,6 +213,23 @@
>>   #define YT8521_RC1R_RGMII_2_100_NS		14
>>   #define YT8521_RC1R_RGMII_2_250_NS		15
>>   
>> +/* LED CONFIG */
>> +#define YT8521_MAX_LEDS				3
>> +#define YT8521_LED0_CFG_REG			0xA00C
>> +#define YT8521_LED1_CFG_REG			0xA00D
>> +#define YT8521_LED2_CFG_REG			0xA00E
>> +#define YT8521_LED_ACT_BLK_IND			BIT(13)
>> +#define YT8521_LED_FDX_ON_EN			BIT(12)
>> +#define YT8521_LED_HDX_ON_EN			BIT(11)
>> +#define YT8521_LED_TXACT_BLK_EN			BIT(10)
>> +#define YT8521_LED_RXACT_BLK_EN			BIT(9)
>> +/* 1000Mbps */
>> +#define YT8521_LED_GT_ON_EN			BIT(6)
>> +/* 100Mbps */
>> +#define YT8521_LED_HT_ON_EN			BIT(5)
>> +/* 10Mbps */
>> +#define YT8521_LED_BT_ON_EN			BIT(4)
>> +
>>   #define YTPHY_MISC_CONFIG_REG			0xA006
>>   #define YTPHY_MCR_FIBER_SPEED_MASK		BIT(0)
>>   #define YTPHY_MCR_FIBER_1000BX			(0x1 << 0)
>> @@ -1681,6 +1698,106 @@ static int yt8521_config_init(struct phy_device *phydev)
>>   	return phy_restore_page(phydev, old_page, ret);
>>   }
>>   
>> +static const unsigned long supported_trgs = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
>> +					     BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
>> +					     BIT(TRIGGER_NETDEV_LINK)        |
>> +					     BIT(TRIGGER_NETDEV_LINK_10)     |
>> +					     BIT(TRIGGER_NETDEV_LINK_100)    |
>> +					     BIT(TRIGGER_NETDEV_LINK_1000)   |
>> +					     BIT(TRIGGER_NETDEV_RX)          |
>> +					     BIT(TRIGGER_NETDEV_TX));
>> +
>> +static int yt8521_led_hw_is_supported(struct phy_device *phydev, u8 index,
>> +				      unsigned long rules)
>> +{
>> +	if (index >= YT8521_MAX_LEDS)
>> +		return -EINVAL;
>> +
>> +	/* All combinations of the supported triggers are allowed */
>> +	if (rules & ~supported_trgs)
>> +		return -EOPNOTSUPP;
>> +
>> +	return 0;
>> +}
>> +
>> +static int yt8521_led_hw_control_set(struct phy_device *phydev, u8 index,
>> +				     unsigned long rules)
>> +{
>> +	u16 val = 0;
>> +
>> +	if (index >= YT8521_MAX_LEDS)
>> +		return -EINVAL;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_LINK, &rules)) {
>> +		val |= YT8521_LED_BT_ON_EN;
>> +		val |= YT8521_LED_HT_ON_EN;
>> +		val |= YT8521_LED_GT_ON_EN;
>> +	}
>> +
>> +	if (test_bit(TRIGGER_NETDEV_LINK_10, &rules))
>> +		val |= YT8521_LED_BT_ON_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_LINK_100, &rules))
>> +		val |= YT8521_LED_HT_ON_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules))
>> +		val |= YT8521_LED_GT_ON_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_FULL_DUPLEX, &rules))
>> +		val |= YT8521_LED_HDX_ON_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_HALF_DUPLEX, &rules))
>> +		val |= YT8521_LED_FDX_ON_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_TX, &rules) ||
>> +	    test_bit(TRIGGER_NETDEV_RX, &rules))
>> +		val |= YT8521_LED_ACT_BLK_IND;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_TX, &rules))
>> +		val |= YT8521_LED_TXACT_BLK_EN;
>> +
>> +	if (test_bit(TRIGGER_NETDEV_RX, &rules))
>> +		val |= YT8521_LED_RXACT_BLK_EN;
>> +
>> +	return ytphy_write_ext(phydev, YT8521_LED0_CFG_REG + index, val);
>> +}
>> +
>> +static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
>> +				     unsigned long *rules)
>> +{
>> +	int val;
>> +
>> +	if (index >= YT8521_MAX_LEDS)
>> +		return -EINVAL;
>> +
>> +	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
>> +	if (val < 0)
>> +		return val;
>> +
>> +	if (val & YT8521_LED_TXACT_BLK_EN)
>> +		set_bit(TRIGGER_NETDEV_TX, rules);
>> +
>> +	if (val & YT8521_LED_RXACT_BLK_EN)
>> +		set_bit(TRIGGER_NETDEV_RX, rules);
>> +
>> +	if (val & YT8521_LED_FDX_ON_EN)
>> +		set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
>> +
>> +	if (val & YT8521_LED_HDX_ON_EN)
>> +		set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
>> +
>> +	if (val & YT8521_LED_GT_ON_EN)
>> +		set_bit(TRIGGER_NETDEV_LINK_1000, rules);
>> +
>> +	if (val & YT8521_LED_HT_ON_EN)
>> +		set_bit(TRIGGER_NETDEV_LINK_100, rules);
>> +
>> +	if (val & YT8521_LED_BT_ON_EN)
>> +		set_bit(TRIGGER_NETDEV_LINK_10, rules);
>> +
>> +	return 0;
>> +}
>> +
>>   static int yt8531_config_init(struct phy_device *phydev)
>>   {
>>   	struct device_node *node = phydev->mdio.dev.of_node;
>> @@ -2920,6 +3037,9 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>>   		.soft_reset	= yt8521_soft_reset,
>>   		.suspend	= yt8521_suspend,
>>   		.resume		= yt8521_resume,
>> +		.led_hw_is_supported = yt8521_led_hw_is_supported,
>> +		.led_hw_control_set = yt8521_led_hw_control_set,
>> +		.led_hw_control_get = yt8521_led_hw_control_get,
>>   	},
>>   	{
>>   		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
>>
>
>
>
>

Re: [PATCH net-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Russell King (Oracle)]

On Thu, Aug 07, 2025 at 11:50:06AM +0200, Heiko Stübner wrote:
> > +static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
> > +				     unsigned long *rules)
> > +{
> > +	int val;
> > +
> > +	if (index >= YT8521_MAX_LEDS)
> > +		return -EINVAL;
> > +
> > +	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
> > +	if (val < 0)
> > +		return val;
> > +
> > +	if (val & YT8521_LED_TXACT_BLK_EN)
> > +		set_bit(TRIGGER_NETDEV_TX, rules);
> > +
> > +	if (val & YT8521_LED_RXACT_BLK_EN)
> > +		set_bit(TRIGGER_NETDEV_RX, rules);
> > +
> > +	if (val & YT8521_LED_FDX_ON_EN)
> > +		set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
> > +
> > +	if (val & YT8521_LED_HDX_ON_EN)
> > +		set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
> > +
> > +	if (val & YT8521_LED_GT_ON_EN)
> > +		set_bit(TRIGGER_NETDEV_LINK_1000, rules);
> > +
> > +	if (val & YT8521_LED_HT_ON_EN)
> > +		set_bit(TRIGGER_NETDEV_LINK_100, rules);
> > +
> > +	if (val & YT8521_LED_BT_ON_EN)
> > +		set_bit(TRIGGER_NETDEV_LINK_10, rules);

Sorry, I don't have the original to hand.

Please use __set_bit() where the more expensive atomic operation that
set_bit() gives is not necessary.

-- 
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-next 1/2] net: phy: motorcomm: Add support for PHY LEDs on YT8521

[Jijie Shao]

on 2025/8/11 23:01, Russell King (Oracle) wrote:
> On Thu, Aug 07, 2025 at 11:50:06AM +0200, Heiko Stübner wrote:
>>> +static int yt8521_led_hw_control_get(struct phy_device *phydev, u8 index,
>>> +				     unsigned long *rules)
>>> +{
>>> +	int val;
>>> +
>>> +	if (index >= YT8521_MAX_LEDS)
>>> +		return -EINVAL;
>>> +
>>> +	val = ytphy_read_ext(phydev, YT8521_LED0_CFG_REG + index);
>>> +	if (val < 0)
>>> +		return val;
>>> +
>>> +	if (val & YT8521_LED_TXACT_BLK_EN)
>>> +		set_bit(TRIGGER_NETDEV_TX, rules);
>>> +
>>> +	if (val & YT8521_LED_RXACT_BLK_EN)
>>> +		set_bit(TRIGGER_NETDEV_RX, rules);
>>> +
>>> +	if (val & YT8521_LED_FDX_ON_EN)
>>> +		set_bit(TRIGGER_NETDEV_FULL_DUPLEX, rules);
>>> +
>>> +	if (val & YT8521_LED_HDX_ON_EN)
>>> +		set_bit(TRIGGER_NETDEV_HALF_DUPLEX, rules);
>>> +
>>> +	if (val & YT8521_LED_GT_ON_EN)
>>> +		set_bit(TRIGGER_NETDEV_LINK_1000, rules);
>>> +
>>> +	if (val & YT8521_LED_HT_ON_EN)
>>> +		set_bit(TRIGGER_NETDEV_LINK_100, rules);
>>> +
>>> +	if (val & YT8521_LED_BT_ON_EN)
>>> +		set_bit(TRIGGER_NETDEV_LINK_10, rules);
> Sorry, I don't have the original to hand.
>
> Please use __set_bit() where the more expensive atomic operation that
> set_bit() gives is not necessary.

Okay, got it.