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

["Heiko Stübner" <heiko@sntech.de>]

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