| Message ID | 20231130203919.757-1-artur@conclusive.pl |
|---|---|
| State | Changes Requested |
| Delegated to: | Sean Anderson |
| Headers | show |
| Series | net: phy: realtek: Add support for LED configuration | expand |
Hi Artur/Jakub, On 11/30/23 15:39, Artur Rojek wrote: > From: Jakub Klama <jakub@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@conclusive.pl> > Signed-off-by: Artur Rojek <artur@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); >
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);