diff mbox series

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

Message ID 20230209094405.12462-1-oliver.graute@kococonnector.com
State New
Headers
Series [RFC] linux: net: phy: realtek: changing LED behaviour for RTL8211F |

Commit Message

Oliver Graute Feb. 9, 2023, 9:44 a.m. UTC 
  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(+)

Comments

Simon Horman Feb. 9, 2023, 10:57 a.m. UTC | #1 
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
>
Oliver Graute Feb. 9, 2023, 12:39 p.m. UTC | #2 
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
Michael Walle Feb. 9, 2023, 1:30 p.m. UTC | #3 
> 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/
Oliver Graute Feb. 9, 2023, 2:20 p.m. UTC | #4 
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
diff mbox series

Patch

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);
 }