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

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

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

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!

on 2026/1/2 19:26, Russell King (Oracle) wrote:
> 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.


Actually, in my code, `default_rules` is an optional configuration;
you can choose not to set default rules.

Thanks,
Jijie Shao

On Wed, Jan 07, 2026 at 05:43:08PM +0800, Jijie Shao wrote:
> 
> on 2026/1/2 19:26, Russell King (Oracle) wrote:
> > 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.
> 
> 
> Actually, in my code, `default_rules` is an optional configuration;
> you can choose not to set default rules.

How is that achieved?

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

on 2026/1/7 17:50, Russell King (Oracle) wrote:
> On Wed, Jan 07, 2026 at 05:43:08PM +0800, Jijie Shao wrote:
>> on 2026/1/2 19:26, Russell King (Oracle) wrote:
>>> 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.
>>
>> Actually, in my code, `default_rules` is an optional configuration;
>> you can choose not to set default rules.
> How is that achieved?

I use `fwnode_property_present()` to determine whether the rules exist,
and if they do not exist, I skip the configuration.

+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;

Thanks,
Jijie Shao

> 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

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

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.

As i said, DT describes hardware, the fact there is an LED, what bus
it is on, colour etc, is describing hardware. How you blink it is
policy, so i expect the DT Maintainers will push back on putting
policy into DT.

So i think your best way forwards is as Russell suggests, some form of
firmware sets the LED before Linux takes control of it. Or udev.

	Andrew

on 2026/1/7 21:09, Andrew Lunn wrote:
> 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.
> As i said, DT describes hardware, the fact there is an LED, what bus
> it is on, colour etc, is describing hardware. How you blink it is
> policy, so i expect the DT Maintainers will push back on putting
> policy into DT.
>
> So i think your best way forwards is as Russell suggests, some form of
> firmware sets the LED before Linux takes control of it. Or udev.
>
> 	Andrew

Yes, at present, this seems to be the only solution.
Recently, I have also been developing a UEFI driver for PXE,
which might be a solution.

However, there is no way to handle the issue with another patch;
I cannot directly modify the ACPI table (a risky operation).

Thanks,
Jijie Shao

> However, there is no way to handle the issue with another patch;
> I cannot directly modify the ACPI table (a risky operation).

It should not be risky. ACPI tables have as many bugs as any other
software. You have to assume they are buggy and will get updated
during their lifetime, so you have processes in place to allow them to
be safely upgraded.

   Andrew

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

on 2025/12/18 18:12, Andrew Lunn wrote:
> 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.

Yes, a feasible approach is for the MAC driver to immediately
call `phydriver->led_hw_control_set()` after `phy_probe()` to configure this rule.
At this point, the netdev trigger for the LED has not yet been activated.

When the netdev trigger is activated, it will retrieve the initial state
from `phydriver->yt8521_led_hw_control_get()`, which is the rule we want to configure.

Thanks,
Jijie Shao