diff mbox series

[RFC,v6,09/11] leds: trigger: netdev: add additional hardware only triggers

Message ID 20220503151633.18760-10-ansuelsmth@gmail.com
State Superseded
Headers show
Series Adds support for PHY LEDs with offload triggers | expand

Commit Message

Christian Marangi May 3, 2022, 3:16 p.m. UTC 
Add additional hardware only triggers commonly supported by switch LEDs.

Additional modes:
link_10: LED on with link up AND speed 10mbps
link_100: LED on with link up AND speed 100mbps
link_1000: LED on with link up AND speed 1000mbps
half_duplex: LED on with link up AND half_duplex mode
full_duplex: LED on with link up AND full duplex mode

Additional blink interval modes:
blink_2hz: LED blink on any even at 2Hz (250ms)
blink_4hz: LED blink on any even at 4Hz (125ms)
blink_8hz: LED blink on any even at 8Hz (62ms)

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/leds/trigger/ledtrig-netdev.c | 58 +++++++++++++++++++++++++++
 include/linux/leds.h                  | 10 +++++
 2 files changed, 68 insertions(+)

Comments

Andrew Lunn May 5, 2022, 1:29 a.m. UTC | #1 
On Tue, May 03, 2022 at 05:16:31PM +0200, Ansuel Smith wrote:
> Add additional hardware only triggers commonly supported by switch LEDs.
> 
> Additional modes:
> link_10: LED on with link up AND speed 10mbps
> link_100: LED on with link up AND speed 100mbps
> link_1000: LED on with link up AND speed 1000mbps
> half_duplex: LED on with link up AND half_duplex mode
> full_duplex: LED on with link up AND full duplex mode
> 
> Additional blink interval modes:
> blink_2hz: LED blink on any even at 2Hz (250ms)
> blink_4hz: LED blink on any even at 4Hz (125ms)
> blink_8hz: LED blink on any even at 8Hz (62ms)

I would suggest separating blink intervals into a patch of their own,
because they are orthogonal to the other modes. Most PHYs are not
going to support them, or they have to be the same across all LEDs, or
don't for example make sense with duplex etc. We need to first
concentrate on the basics, get that correct. Then we can add nice to
have features like this.

     Andrew
Christian Marangi May 5, 2022, 1:30 p.m. UTC | #2 
On Thu, May 05, 2022 at 03:29:26AM +0200, Andrew Lunn wrote:
> On Tue, May 03, 2022 at 05:16:31PM +0200, Ansuel Smith wrote:
> > Add additional hardware only triggers commonly supported by switch LEDs.
> > 
> > Additional modes:
> > link_10: LED on with link up AND speed 10mbps
> > link_100: LED on with link up AND speed 100mbps
> > link_1000: LED on with link up AND speed 1000mbps
> > half_duplex: LED on with link up AND half_duplex mode
> > full_duplex: LED on with link up AND full duplex mode
> > 
> > Additional blink interval modes:
> > blink_2hz: LED blink on any even at 2Hz (250ms)
> > blink_4hz: LED blink on any even at 4Hz (125ms)
> > blink_8hz: LED blink on any even at 8Hz (62ms)
> 
> I would suggest separating blink intervals into a patch of their own,
> because they are orthogonal to the other modes. Most PHYs are not
> going to support them, or they have to be the same across all LEDs, or
> don't for example make sense with duplex etc. We need to first
> concentrate on the basics, get that correct. Then we can add nice to
> have features like this.
> 
>      Andrew

Okok will just drop this. I agree that adds additional complexity to the
code and would make this even harder to merge.
diff mbox series

Patch

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index d88b0c6a910e..bced05fafb1c 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -29,8 +29,20 @@ 
  *
  * device_name - network device name to monitor
  * interval - duration of LED blink, in milliseconds
+ *            (in hardware mode 2hz (62ms), 4hz (125ms) or 8hz (250ms)
+ *             are supported)
  * link -  LED's normal state reflects whether the link is up
  *         (has carrier) or not
+ * link_10 - LED's normal state reflects whether the link is
+ *           up and at 10mbps speed (hardware only)
+ * link_100 - LED's normal state reflects whether the link is
+ *            up and at 100mbps speed (hardware only)
+ * link_1000 - LED's normal state reflects whether the link is
+ *             up and at 1000mbps speed (hardware only)
+ * half_duplex - LED's normal state reflects whether the link is
+ *               up and hafl duplex (hardware only)
+ * full_duplex - LED's normal state reflects whether the link is
+ *               up and full duplex (hardware only)
  * tx -  LED blinks on transmitted data
  * rx -  LED blinks on receive data
  * available_mode - Display available mode and how they can be handled
@@ -64,8 +76,19 @@  struct netdev_led_attr_detail {
 
 static struct netdev_led_attr_detail attr_details[] = {
 	{ .name = "link", .bit = TRIGGER_NETDEV_LINK},
+	{ .name = "link_10", .hardware_only = true, .bit = TRIGGER_NETDEV_LINK_10},
+	{ .name = "link_100", .hardware_only = true, .bit = TRIGGER_NETDEV_LINK_100},
+	{ .name = "link_1000", .hardware_only = true, .bit = TRIGGER_NETDEV_LINK_1000},
+	{ .name = "half_duplex", .hardware_only = true, .bit = TRIGGER_NETDEV_HALF_DUPLEX},
+	{ .name = "full_duplex", .hardware_only = true, .bit = TRIGGER_NETDEV_FULL_DUPLEX},
 	{ .name = "tx", .bit = TRIGGER_NETDEV_TX},
 	{ .name = "rx", .bit = TRIGGER_NETDEV_RX},
+	{ .name = "hw blink 2hz (interval set to 62)", .hardware_only = true,
+	  .bit = TRIGGER_NETDEV_BLINK_2HZ},
+	{ .name = "hw blink 4hz (interval set to 125)", .hardware_only = true,
+	  .bit = TRIGGER_NETDEV_BLINK_4HZ},
+	{ .name = "hw blink 8hz (interval set to 250)", .hardware_only = true,
+	  .bit = TRIGGER_NETDEV_BLINK_8HZ},
 };
 
 static bool validate_baseline_state(struct led_netdev_data *trigger_data)
@@ -259,6 +282,11 @@  static ssize_t netdev_led_attr_show(struct device *dev, char *buf,
 
 	switch (attr) {
 	case TRIGGER_NETDEV_LINK:
+	case TRIGGER_NETDEV_LINK_10:
+	case TRIGGER_NETDEV_LINK_100:
+	case TRIGGER_NETDEV_LINK_1000:
+	case TRIGGER_NETDEV_HALF_DUPLEX:
+	case TRIGGER_NETDEV_FULL_DUPLEX:
 	case TRIGGER_NETDEV_TX:
 	case TRIGGER_NETDEV_RX:
 		bit = attr;
@@ -284,6 +312,11 @@  static ssize_t netdev_led_attr_store(struct device *dev, const char *buf,
 
 	switch (attr) {
 	case TRIGGER_NETDEV_LINK:
+	case TRIGGER_NETDEV_LINK_10:
+	case TRIGGER_NETDEV_LINK_100:
+	case TRIGGER_NETDEV_LINK_1000:
+	case TRIGGER_NETDEV_HALF_DUPLEX:
+	case TRIGGER_NETDEV_FULL_DUPLEX:
 	case TRIGGER_NETDEV_TX:
 	case TRIGGER_NETDEV_RX:
 		bit = attr;
@@ -324,6 +357,11 @@  static ssize_t netdev_led_attr_store(struct device *dev, const char *buf,
 	static DEVICE_ATTR_RW(trigger_name)
 
 DEFINE_NETDEV_TRIGGER(link, TRIGGER_NETDEV_LINK);
+DEFINE_NETDEV_TRIGGER(link_10, TRIGGER_NETDEV_LINK_10);
+DEFINE_NETDEV_TRIGGER(link_100, TRIGGER_NETDEV_LINK_100);
+DEFINE_NETDEV_TRIGGER(link_1000, TRIGGER_NETDEV_LINK_1000);
+DEFINE_NETDEV_TRIGGER(half_duplex, TRIGGER_NETDEV_HALF_DUPLEX);
+DEFINE_NETDEV_TRIGGER(full_duplex, TRIGGER_NETDEV_FULL_DUPLEX);
 DEFINE_NETDEV_TRIGGER(tx, TRIGGER_NETDEV_TX);
 DEFINE_NETDEV_TRIGGER(rx, TRIGGER_NETDEV_RX);
 
@@ -356,6 +394,21 @@  static ssize_t interval_store(struct device *dev,
 
 	cancel_delayed_work_sync(&trigger_data->work);
 
+	if (trigger_data->blink_mode == HARDWARE_CONTROLLED) {
+		/* Interval are handled as triggers. Reset them. */
+		trigger_data->mode &= ~(BIT(TRIGGER_NETDEV_BLINK_8HZ) |
+					BIT(TRIGGER_NETDEV_BLINK_4HZ) |
+					BIT(TRIGGER_NETDEV_BLINK_2HZ));
+
+		/* Support a common value of 2Hz, 4Hz and 8Hz. */
+		if (value > 5 && value <= 62) /* 8Hz */
+			trigger_data->mode |= BIT(TRIGGER_NETDEV_BLINK_8HZ);
+		else if (value > 63 && value <= 125) /* 4Hz */
+			trigger_data->mode |= BIT(TRIGGER_NETDEV_BLINK_4HZ);
+		else /* 2Hz */
+			trigger_data->mode |= BIT(TRIGGER_NETDEV_BLINK_2HZ);
+	}
+
 	atomic_set(&trigger_data->interval, msecs_to_jiffies(value));
 
 	if (!validate_baseline_state(trigger_data)) {
@@ -404,6 +457,11 @@  static DEVICE_ATTR_RO(available_mode);
 static struct attribute *netdev_trig_attrs[] = {
 	&dev_attr_device_name.attr,
 	&dev_attr_link.attr,
+	&dev_attr_link_10.attr,
+	&dev_attr_link_100.attr,
+	&dev_attr_link_1000.attr,
+	&dev_attr_half_duplex.attr,
+	&dev_attr_full_duplex.attr,
 	&dev_attr_rx.attr,
 	&dev_attr_tx.attr,
 	&dev_attr_interval.attr,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 13862f8b1e07..5fcc6d233757 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -551,8 +551,18 @@  static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 /* Trigger specific enum */
 enum led_trigger_netdev_modes {
 	TRIGGER_NETDEV_LINK = 1,
+	TRIGGER_NETDEV_LINK_10,
+	TRIGGER_NETDEV_LINK_100,
+	TRIGGER_NETDEV_LINK_1000,
+	TRIGGER_NETDEV_HALF_DUPLEX,
+	TRIGGER_NETDEV_FULL_DUPLEX,
 	TRIGGER_NETDEV_TX,
 	TRIGGER_NETDEV_RX,
+
+	/* Hardware Interval options */
+	TRIGGER_NETDEV_BLINK_2HZ,
+	TRIGGER_NETDEV_BLINK_4HZ,
+	TRIGGER_NETDEV_BLINK_8HZ,
 };
 
 /* Trigger specific functions */