linux: net: phy: realtek: changing LED behaviour for RTL8211F

[PATCH RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F

Posted by Oliver Graute 2 years, 7 months ago

This enable the LEDs for network activity and 100/1000Link for the RTL8211F

Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
---
 drivers/net/phy/realtek.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 3d99fd6664d7..5c796883cad3 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -416,6 +416,11 @@ static int rtl8211f_config_init(struct phy_device *phydev)
 		}
 	}
 
+        phy_write(phydev, RTL821x_PAGE_SELECT, 0xd04);
+        phy_write(phydev, 0x10, 0x15B);
+
+        phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
+
 	return genphy_soft_reset(phydev);
 }
 
-- 
2.17.1

Re: [PATCH RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F

Posted by Simon Horman 2 years, 7 months ago

On Thu, Feb 09, 2023 at 10:44:05AM +0100, Oliver Graute wrote:
> This enable the LEDs for network activity and 100/1000Link for the RTL8211F
> 
> Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
> ---
>  drivers/net/phy/realtek.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 3d99fd6664d7..5c796883cad3 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -416,6 +416,11 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  		}
>  	}
>  
> +        phy_write(phydev, RTL821x_PAGE_SELECT, 0xd04);
> +        phy_write(phydev, 0x10, 0x15B);
> +
> +        phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
> +

nit: it looks like the indentation in the new lines above should
     be using a single tab rather than 8 spaces.

>  	return genphy_soft_reset(phydev);
>  }
>  
> -- 
> 2.17.1
>

Re: [PATCH RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F

Posted by Oliver Graute 2 years, 7 months ago

On 09/02/23, Simon Horman wrote:
> On Thu, Feb 09, 2023 at 10:44:05AM +0100, Oliver Graute wrote:
> > This enable the LEDs for network activity and 100/1000Link for the RTL8211F
> > 
> > Signed-off-by: Oliver Graute <oliver.graute@kococonnector.com>
> > ---
> >  drivers/net/phy/realtek.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index 3d99fd6664d7..5c796883cad3 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -416,6 +416,11 @@ static int rtl8211f_config_init(struct phy_device *phydev)
> >  		}
> >  	}
> >  
> > +        phy_write(phydev, RTL821x_PAGE_SELECT, 0xd04);
> > +        phy_write(phydev, 0x10, 0x15B);
> > +
> > +        phy_write(phydev, RTL821x_PAGE_SELECT, 0x0);
> > +
> 
> nit: it looks like the indentation in the new lines above should
>      be using a single tab rather than 8 spaces.

thx, I will fix the indentation. 

is this the right place to turn on the realtek phy LEDs for RTL8211F?

Best regards,

Oliver

Re: [PATCH RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F

Posted by Michael Walle 2 years, 7 months ago

> is this the right place to turn on the realtek phy LEDs for RTL8211F?

Probably not. There are a few issues. This will only work one particular
board. Therefore, you'd need some kind of runtime configuration to also
support other boards. But lately any LED related patches for PHYs were
NAK'd because they need to integrate with the LED subsystem. See [1].

-michael

[1] https://lore.kernel.org/r/YyxOTKJ8OTxXgWcA@lunn.ch/

Re: [PATCH RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F

Posted by Oliver Graute 2 years, 7 months ago

On 09/02/23, Michael Walle wrote:
> > is this the right place to turn on the realtek phy LEDs for RTL8211F?
> 
> Probably not. There are a few issues. This will only work one particular
> board. Therefore, you'd need some kind of runtime configuration to also
> support other boards. But lately any LED related patches for PHYs were
> NAK'd because they need to integrate with the LED subsystem. See [1].
> 
> -michael
> 
> [1] https://lore.kernel.org/r/YyxOTKJ8OTxXgWcA@lunn.ch/

ok thx for this information.

Best Regards,

Oliver