Re: [PATCH RFC leds + net-next v3 1/2] net: phy: add API for LEDs controlled by PHY HW
From: Andrew Lunn <andrew@lunn.ch>
Date: 2020-07-25 16:22:47
Also in:
linux-leds, lkml
On Fri, Jul 24, 2020 at 06:46:02PM +0200, Marek Behún wrote:
quoted hunk ↗ jump to hunk
Many PHYs support various HW control modes for LEDs connected directly to them. This code adds a new private LED trigger called phydev-hw-mode. When this trigger is enabled for a LED, the various HW control modes which the PHY supports for given LED can be get/set via hw_mode sysfs file. A PHY driver wishing to utilize this API needs to register the LEDs on its own and set the .trigger_type member of LED classdev to &phy_hw_led_trig_type. It also needs to implement the methods .led_iter_hw_mode, .led_set_hw_mode and .led_get_hw_mode in struct phydev. Signed-off-by: Marek Behún <redacted> --- drivers/net/phy/Kconfig | 9 +++ drivers/net/phy/Makefile | 1 + drivers/net/phy/phy_hw_led_mode.c | 96 +++++++++++++++++++++++++++++++ include/linux/phy.h | 15 +++++ 4 files changed, 121 insertions(+) create mode 100644 drivers/net/phy/phy_hw_led_mode.cdiff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index dd20c2c27c2f..ffea11f73acd 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig@@ -283,6 +283,15 @@ config LED_TRIGGER_PHY <Speed in megabits>Mbps OR <Speed in gigabits>Gbps OR link for any speed known to the PHY. +config LED_TRIGGER_PHY_HW + bool "Support HW LED control modes" + depends on LEDS_TRIGGERS + help + Many PHYs can control blinking of LEDs connected directly to them. + This adds a special LED trigger called phydev-hw-mode. When enabled, + the various control modes supported by the PHY on given LED can be + chosen via hw_mode sysfs file. + comment "MII PHY device drivers"diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index d84bab489a53..fd0253ab8097 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile@@ -20,6 +20,7 @@ endif obj-$(CONFIG_MDIO_DEVRES) += mdio_devres.o libphy-$(CONFIG_SWPHY) += swphy.o libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o +libphy-$(CONFIG_LED_TRIGGER_PHY_HW) += phy_hw_led_mode.o obj-$(CONFIG_PHYLINK) += phylink.o obj-$(CONFIG_PHYLIB) += libphy.odiff --git a/drivers/net/phy/phy_hw_led_mode.c b/drivers/net/phy/phy_hw_led_mode.c new file mode 100644 index 000000000000..b4c2f25266a5 --- /dev/null +++ b/drivers/net/phy/phy_hw_led_mode.c@@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * drivers/net/phy/phy_hw_led_mode.c + * + * PHY HW LED mode trigger + * + * Copyright (C) 2020 Marek Behun <marek.behun@nic.cz> + */ +#include <linux/leds.h> +#include <linux/phy.h> + +static void phy_hw_led_trig_deactivate(struct led_classdev *cdev) +{ + struct phy_device *phydev = to_phy_device(cdev->dev->parent); + int ret; + + ret = phydev->drv->led_set_hw_mode(phydev, cdev, NULL); + if (ret < 0) { + phydev_err(phydev, "failed deactivating HW mode on LED %s\n", cdev->name); + return; + }
The core holds the phydev mutex when calling into the driver. There are a few exceptions, but it would be good if all the LED calls into the driver also held the mutex.
+}
+
+static ssize_t hw_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *cdev = led_trigger_get_led(dev);
+ struct phy_device *phydev = to_phy_device(cdev->dev->parent);
+ const char *mode, *cur_mode;
+ void *iter = NULL;
+ int len = 0;Reverse christmas tree.
+static int __init phy_led_triggers_init(void)
+{
+ return led_trigger_register(&phy_hw_led_trig);
+}
+
+module_init(phy_led_triggers_init);
+
+static void __exit phy_led_triggers_exit(void)
+{
+ led_trigger_unregister(&phy_hw_led_trig);
+}
+
+module_exit(phy_led_triggers_exit);
It is a bit of a surprise to find the module init/exit calls here, and
not in phy.c. I think they should be moved.
Andrew