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

[Jijie Shao <shaojijie@huawei.com>]

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),
>>
>
>
>
>