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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4a622284-66db-49c4-943f-5da1f1a6ba65@lunn.ch>
Date: Wed, 16 Jul 2025 18:39:00 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jijie Shao <shaojijie@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
	Frank.Sae@...or-comm.com, hkallweit1@...il.com,
	linux@...linux.org.uk, shenjian15@...wei.com,
	liuyonglong@...wei.com, chenhao418@...wei.com,
	jonathan.cameron@...wei.com, shameerali.kolothum.thodi@...wei.com,
	salil.mehta@...wei.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] net: phy: motorcomm: Add support for PHY
 LEDs on YT8521

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@...wei.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