[PATCH] net: phy: realtek: Add support for LED configuration

[Sean Anderson]

Hi Artur/Jakub,

On 11/30/23 15:39, Artur Rojek wrote:
> From: Jakub Klama <jakub at conclusive.pl>
> 
> Introduce an ability to configure LED and Fiber LEDs found in RTL8211F
> PHYs. This is achieved through two optional Device Tree properties:
> * rtl,lcr  for LED control
> * rtl,flcr for Fiber LED control

The Linux netdev maintainers do not like this sort of LED binding (see
e.g. [1] for reasoning). They would prefer that PHY drivers use an
LED-subsystem-style binding for LEDS [2]. So I can't see this sort of
binding being accepted in Linux.

Unfortunately, U-Boot's LED support is a much more primitive than
Linux's. In particular, there is no comprehensive trigger system like in
Linux. So this would be a lot more work than your initial patch.

In Linux, LEDs are configured by userspace, but in U-Boot there really
isn't a userspace to do this. So I think maybe we need a u-boot,trigger
(like linux,default-trigger) to determine the blink mode. This would
need to be more expressive than Linux's.

Ideally there would be a way for LED triggers to hook into the net
send/receive functions. But I think it is probably fine for any initial
attempt to just parse a Linux-style binding and configure hardware
control.

Alternatively, you can just write these registers in board code, which
is not ideal but which is what everyone does anyway...

--Sean

[1] https://lpc.events/event/17/contributions/1604/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-phy.yaml#n206

> Signed-off-by: Jakub Klama <jakub at conclusive.pl>
> Signed-off-by: Artur Rojek <artur at conclusive.pl>
> ---
>  drivers/net/phy/realtek.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 396cac76d6..d078f41bee 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -59,6 +59,7 @@
>  #define MIIM_RTL8211F_TX_DELAY		0x100
>  #define MIIM_RTL8211F_RX_DELAY		0x8
>  #define MIIM_RTL8211F_LCR		0x10
> +#define MIIM_RTL8211F_FLCR		0x12
>  
>  #define RTL8201F_RMSR			0x10
>  
> @@ -220,6 +221,8 @@ default_delay:
>  
>  static int rtl8211f_config(struct phy_device *phydev)
>  {
> +	ofnode node = phy_get_ofnode(phydev);
> +	u32 lcr, flcr;
>  	u16 reg;
>  
>  	if (phydev->flags & PHY_RTL8211F_FORCE_EEE_RXC_ON) {
> @@ -254,14 +257,17 @@ static int rtl8211f_config(struct phy_device *phydev)
>  		reg &= ~MIIM_RTL8211F_RX_DELAY;
>  	phy_write(phydev, MDIO_DEVAD_NONE, 0x15, reg);
>  
> -	/* restore to default page 0 */
> -	phy_write(phydev, MDIO_DEVAD_NONE,
> -		  MIIM_RTL8211F_PAGE_SELECT, 0x0);
> -
> -	/* Set green LED for Link, yellow LED for Active */
>  	phy_write(phydev, MDIO_DEVAD_NONE,
>  		  MIIM_RTL8211F_PAGE_SELECT, 0xd04);
> -	phy_write(phydev, MDIO_DEVAD_NONE, 0x10, 0x617f);
> +
> +	if (ofnode_valid(node) && !ofnode_read_u32(node, "rtl,lcr", &lcr))
> +		phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_LCR, lcr);
> +	else /* Set green LED for Link, yellow LED for Active */
> +		phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_LCR, 0x617f);
> +
> +	if (ofnode_valid(node) && !ofnode_read_u32(node, "rtl,flcr", &flcr))
> +		phy_write(phydev, MDIO_DEVAD_NONE, MIIM_RTL8211F_FLCR, flcr);
> +
>  	phy_write(phydev, MDIO_DEVAD_NONE,
>  		  MIIM_RTL8211F_PAGE_SELECT, 0x0);
>