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/Docu...

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