[RFC PATCH 0/6] Adds support for PHY LEDs with offload triggers

[RFC PATCH 0/6] Adds support for PHY LEDs with offload triggers

[Ansuel Smith]

This is another attempt in adding support for PHY LEDs. Most of the
times Switch/PHY have connected multiple LEDs that are controlled by HW
based on some rules/event. Currently we lack any support for a generic
way to control the HW part and normally we either never implement the
feature or only add control for brightness or hw blink.

This is based on Marek idea of providing some API to cled but use a
different implementation that in theory should be more generilized.

The current idea is:
- LED driver implement 2 API (trigger and configure). They are used to
  put the LED in offload mode and to configure the various trigger.
- We have offload triggers that are used to expose to userspace the
  supported offload triggers and set the offload mode on trigger
  activation.
- The PHY will declare the supported offload triggers using the
  linux,supported-offload-triggers binding in the dts.
- Only supported triggers from linux,supported-offload-triggers will be
  exposed by the offload trigger to userspace via sysfs. Other won't be
  configurable as they are not supported by the LED driver.
- The LED driver will set how the Offload mode is activated/disabled and
  will set a configure function to set each supported offload triggers.
  This function provide 3 different mode enable/disable/read that
  will enable or disable the offload trigger or read the current status.
- On offload trigger activation, only the offload mode is triggered but
  the offload triggers are not configured. This means that the LED will
  run in offload mode but will have the default rules/event set by the
  device by default. To change this the user will have to operate via
  userspace (or we can consider adding another binding in the dts to
  declare also a default trigger configuration)

Each LED driver will have to declare explicit support for the offload
trigger (or return not supported error code) as we pass a u32 flag that
the LED driver will elaborate and understand what is referring to (based
on the current active trigger).

I posted a user for this new implementation that will benefit from this
and will add a big feature to it. Currently qca8k can have up to 3 LEDs
connected to each PHY port and we have some device that have only one of
them connected and the default configuration won't work for that.

Ansuel Smith (5):
  leds: permit to declare supported offload triggers
  leds: add function to configure offload leds
  leds: trigger: add offload-phy-activity trigger
  net: dsa: qca8k: add LEDs support
  dt-bindings: net: dsa: qca8k: add LEDs definition example

Marek Behún (1):
  leds: trigger: add API for HW offloading of triggers

 .../devicetree/bindings/net/dsa/qca8k.yaml    |  30 ++
 Documentation/leds/leds-class.rst             |  38 ++
 drivers/leds/led-class.c                      |  15 +-
 drivers/leds/led-triggers.c                   |   1 +
 drivers/leds/trigger/Kconfig                  |  35 ++
 drivers/leds/trigger/Makefile                 |   1 +
 .../trigger/ledtrig-offload-phy-activity.c    | 151 ++++++++
 drivers/net/dsa/Kconfig                       |   9 +
 drivers/net/dsa/Makefile                      |   1 +
 drivers/net/dsa/qca8k-leds.c                  | 361 ++++++++++++++++++
 drivers/net/dsa/qca8k.c                       |   4 +-
 drivers/net/dsa/qca8k.h                       |  64 ++++
 include/linux/leds.h                          |  76 ++++
 13 files changed, 783 insertions(+), 3 deletions(-)
 create mode 100644 drivers/leds/trigger/ledtrig-offload-phy-activity.c
 create mode 100644 drivers/net/dsa/qca8k-leds.c

-- 
2.32.0

[RFC PATCH 1/6] leds: trigger: add API for HW offloading of triggers

[Ansuel Smith]

From: Marek Behún <kabel@kernel.org>

Add method trigger_offload() and member variable `offloaded` to struct
led_classdev. Add helper functions led_trigger_offload() and
led_trigger_offload_stop().

The trigger_offload() method, when implemented by the LED driver, should
be called (via led_trigger_offload() function) from trigger code wanting
to be offloaded at the moment when configuration of the trigger changes.

If the trigger is successfully offloaded, this method returns 0 and the
trigger does not have to blink the LED in software.

If the trigger with given configuration cannot be offloaded, the method
should return -EOPNOTSUPP, in which case the trigger must blink the LED
in SW.

The second argument to trigger_offload() being false means that the
offloading is being disabled. In this case the function must return 0,
errors are not permitted.

An additional config CONFIG_LEDS_OFFLOAD_TRIGGERS is added to add support
for these special trigger offload driven.

Signed-off-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/leds/leds-class.rst | 22 +++++++++++++++++++++
 drivers/leds/led-triggers.c       |  1 +
 drivers/leds/trigger/Kconfig      | 10 ++++++++++
 include/linux/leds.h              | 33 +++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index cd155ead8703..035a738afc4a 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -169,6 +169,28 @@ Setting the brightness to zero with brightness_set() callback function
 should completely turn off the LED and cancel the previously programmed
 hardware blinking function, if any.
 
+Hardware offloading of LED triggers
+===================================
+
+Some LEDs can offload SW triggers to hardware (for example a LED connected to
+an ethernet PHY or an ethernet switch can be configured to blink on activity on
+the network, which in software is done by the netdev trigger).
+
+To do such offloading, LED driver must support the this and a deficated offload
+trigger must be used. The LED must implement the trigger_offload() method and
+the trigger code must try to call this method (via led_trigger_offload()
+function) when configuration of the trigger (trigger_data) changes.
+
+The implementation of the trigger_offload() method by the LED driver must return
+0 if the offload is successful and -EOPNOTSUPP if the requested trigger
+configuration is not supported and the trigger should be executed in software.
+If trigger_offload() returns negative value, the triggering will be done in
+software, so any active offloading must also be disabled.
+
+If the second argument (enable) to the trigger_offload() method is false, any
+active HW offloading must be deactivated. In this case errors are not permitted
+in the trigger_offload() method.
+
 
 Known Issues
 ============
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 072491d3e17b..281c52914f42 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -179,6 +179,7 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 
 		cancel_work_sync(&led_cdev->set_brightness_work);
 		led_stop_software_blink(led_cdev);
+		led_trigger_offload_stop(led_cdev);
 		if (led_cdev->trigger->deactivate)
 			led_cdev->trigger->deactivate(led_cdev);
 		device_remove_groups(led_cdev->dev, led_cdev->trigger->groups);
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index dc6816d36d06..c073e64e0a37 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -9,6 +9,16 @@ menuconfig LEDS_TRIGGERS
 
 if LEDS_TRIGGERS
 
+config LEDS_OFFLOAD_TRIGGERS
+	bool "LED Offload Trigger support"
+	help
+	  This option enabled offload triggers support used by leds that
+	  can be driven in HW by declaring some specific triggers.
+	  A offload trigger will expose a sysfs dir to configure the
+	  different blinking trigger and the available hw trigger.
+
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_TIMER
 	tristate "LED Timer Trigger"
 	help
diff --git a/include/linux/leds.h b/include/linux/leds.h
index ba4861ec73d3..949ab461287f 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -154,6 +154,13 @@ struct led_classdev {
 
 	/* LEDs that have private triggers have this set */
 	struct led_hw_trigger_type	*trigger_type;
+
+#ifdef CONFIG_LEDS_OFFLOAD_TRIGGERS
+	int			offloaded;
+	/* some LEDs cne be driven by HW */
+	int			(*trigger_offload)(struct led_classdev *led_cdev,
+						   bool enable);
+#endif
 #endif
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
@@ -409,6 +416,32 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 	return led_cdev->trigger_data;
 }
 
+#ifdef CONFIG_LEDS_OFFLOAD_TRIGGERS
+static inline int led_trigger_offload(struct led_classdev *led_cdev)
+{
+	int ret;
+
+	if (!led_cdev->trigger_offload)
+		return -EOPNOTSUPP;
+
+	ret = led_cdev->trigger_offload(led_cdev, true);
+	led_cdev->offloaded = !ret;
+
+	return ret;
+}
+
+static inline void led_trigger_offload_stop(struct led_classdev *led_cdev)
+{
+	if (!led_cdev->trigger_offload)
+		return;
+
+	if (led_cdev->offloaded) {
+		led_cdev->trigger_offload(led_cdev, false);
+		led_cdev->offloaded = false;
+	}
+}
+#endif
+
 /**
  * led_trigger_rename_static - rename a trigger
  * @name: the new trigger name
-- 
2.32.0

[RFC PATCH 2/6] leds: permit to declare supported offload triggers

[Ansuel Smith]

With LEDs that can be offload driven, permit to declare supported triggers
in the dts and add them to the cled struct to be used by the related
offload trigger. This is particurally useful for phy that have support
for HW blinking on tx/rx traffic or based on the speed link.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/leds/leds-class.rst |  4 ++++
 drivers/leds/led-class.c          | 15 ++++++++++++++-
 include/linux/leds.h              |  5 ++++-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index 035a738afc4a..ab50b58d6a21 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -191,6 +191,10 @@ If the second argument (enable) to the trigger_offload() method is false, any
 active HW offloading must be deactivated. In this case errors are not permitted
 in the trigger_offload() method.
 
+LEDs can declare the supported offload trigger using linux,supported-offload-triggers
+binding in the dts. This is just an array of string that will be used by any
+offload trigger to check the supported triggers and configure the LED offload mode
+and bheaviour.
 
 Known Issues
 ============
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f4bb02f6e042..56f75e70b81e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -339,7 +339,7 @@ int led_classdev_register_ext(struct device *parent,
 	char composed_name[LED_MAX_NAME_SIZE];
 	char final_name[LED_MAX_NAME_SIZE];
 	const char *proposed_name = composed_name;
-	int ret;
+	int count, ret;
 
 	if (init_data) {
 		if (init_data->devname_mandatory && !init_data->devicename) {
@@ -358,6 +358,19 @@ int led_classdev_register_ext(struct device *parent,
 			if (fwnode_property_present(init_data->fwnode,
 						    "retain-state-shutdown"))
 				led_cdev->flags |= LED_RETAIN_AT_SHUTDOWN;
+
+			if (fwnode_property_present(init_data->fwnode,
+						    "linux,supported-offload-triggers")) {
+				count = fwnode_property_string_array_count(
+				    init_data->fwnode, "linux,supported-offload-triggers");
+
+				led_cdev->supported_offload_triggers =
+				    kcalloc(count, sizeof(char *), GFP_KERNEL);
+				fwnode_property_read_string_array(
+				    init_data->fwnode, "linux,supported-offload-triggers",
+				    led_cdev->supported_offload_triggers, count);
+				led_cdev->supported_offload_triggers_count = count;
+			}
 		}
 	} else {
 		proposed_name = led_cdev->name;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 949ab461287f..ff1f903f8079 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -157,7 +157,10 @@ struct led_classdev {
 
 #ifdef CONFIG_LEDS_OFFLOAD_TRIGGERS
 	int			offloaded;
-	/* some LEDs cne be driven by HW */
+	/* LEDs can have multiple offload triggers */
+	int			supported_offload_triggers_count;
+	const char		**supported_offload_triggers;
+	/* some LEDs may be able to offload some SW triggers to HW */
 	int			(*trigger_offload)(struct led_classdev *led_cdev,
 						   bool enable);
 #endif
-- 
2.32.0

[RFC PATCH 3/6] leds: add function to configure offload leds

[Ansuel Smith]

Add configure_offload helper to configure how the LED should work in
offload mode. The function require to support the particular trigger and
will use the passed offload_flags to elaborate the data and apply the
correct configuration. This function will then be used by the offload
trigger to request and update offload configuration.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/leds/leds-class.rst | 12 ++++++++++++
 include/linux/leds.h              | 16 ++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index ab50b58d6a21..af84cce09068 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -191,6 +191,18 @@ If the second argument (enable) to the trigger_offload() method is false, any
 active HW offloading must be deactivated. In this case errors are not permitted
 in the trigger_offload() method.
 
+The offload trigger will use the function configure_offload() provided by the driver
+that will configure the offloaded mode for the LED.
+This function pass as the first argument (offload_flags) a u32 flag, it's in the LED
+driver interest how to elaborate this flags and to declare support for a particular
+offload trigger.
+The second argument (cmd) of the configure_offload() method can be used to do various
+operation for the specific trigger. We currently support ENABLE, DISABLE and READ to
+enable, disable and read the state of the offload trigger for the LED driver.
+If the driver return -ENOTSUPP on configure_offload, the trigger activation will
+fail as the driver doesn't support that specific offload trigger or don't know
+how to handle the provided flags.
+
 LEDs can declare the supported offload trigger using linux,supported-offload-triggers
 binding in the dts. This is just an array of string that will be used by any
 offload trigger to check the supported triggers and configure the LED offload mode
diff --git a/include/linux/leds.h b/include/linux/leds.h
index ff1f903f8079..eb06d812bc24 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -67,6 +67,14 @@ struct led_hw_trigger_type {
 	int dummy;
 };
 
+#ifdef CONFIG_LEDS_OFFLOAD_TRIGGERS
+enum offload_trigger_cmd {
+	TRIGGER_ENABLE,
+	TRIGGER_DISABLE,
+	TRIGGER_READ
+};
+#endif
+
 struct led_classdev {
 	const char		*name;
 	unsigned int brightness;
@@ -163,6 +171,14 @@ struct led_classdev {
 	/* some LEDs may be able to offload some SW triggers to HW */
 	int			(*trigger_offload)(struct led_classdev *led_cdev,
 						   bool enable);
+	/* Function to configure how the LEDs should work in offload mode.
+	 * The function require to support the trigger and will use the
+	 * passed flags to elaborate the trigger requested and apply the
+	 * correct configuration.
+	 */
+	int			(*configure_offload)(struct led_classdev *led_cdev,
+						     u32 offload_flags,
+						     enum offload_trigger_cmd cmd);
 #endif
 #endif
 
-- 
2.32.0

[RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Ansuel Smith]

Add Offload Trigger for PHY Activity. This special trigger is used to
configure and expose the different HW trigger that are provided by the
PHY. Each offload trigger can be configured by sysfs and on trigger
activation the offload mode is enabled.

This currently implement these hw triggers:
  - blink_tx: Blink LED on tx packet receive
  - blink_rx: Blink LED on rx packet receive
  - blink_collision: Blink LED on collision detection
  - link_10m: Keep LED on with 10m link speed
  - link_100m: Keep LED on with 100m link speed
  - link_1000m: Keep LED on with 1000m link speed
  - half_duplex: Keep LED on with half duplex link
  - full_duplex: Keep LED on with full duplex link
  - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
  - power_on_reset: Keep LED on with switch reset
  - blink_2hz: Set blink speed at 2hz for every blink event
  - blink_4hz: Set blink speed at 4hz for every blink event
  - blink_8hz: Set blink speed at 8hz for every blink event
  - blink_auto: Set blink speed at 2hz for 10m link speed,
      4hz for 100m and 8hz for 1000m

The trigger will read the supported offload trigger in the led cdev and
will expose the offload triggers in sysfs and then activate the offload
mode for the led in offload mode has it configured by default. A flag is
passed to configure_offload with the related rule from this trigger to
active or disable.
It's in the led driver interest the detection and knowing how to
elaborate the passed flags.

The different hw triggers are exposed in the led sysfs dir under the
offload-phy-activity subdir.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/leds/trigger/Kconfig                  |  25 +++
 drivers/leds/trigger/Makefile                 |   1 +
 .../trigger/ledtrig-offload-phy-activity.c    | 151 ++++++++++++++++++
 include/linux/leds.h                          |  24 +++
 4 files changed, 201 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-offload-phy-activity.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index c073e64e0a37..377a8bceb0b7 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -164,4 +164,29 @@ config LEDS_TRIGGER_TTY
 
 	  When build as a module this driver will be called ledtrig-tty.
 
+config LEDS_OFFLOAD_TRIGGER_PHY_ACTIVITY
+	tristate "LED Offload Trigger for PHY Activity"
+	depends on LEDS_OFFLOAD_TRIGGERS
+	help
+	  This allows LEDs to be configured to run by HW and offloaded based
+	  on some rules. The LED will blink or be on based on the PHY Activity
+	  for example on packet receive of based on the link speed.
+	  The current rules are:
+	  - blink_tx: Blink LED on tx packet receive
+	  - blink_rx: Blink LED on rx packet receive
+	  - blink_collision: Blink LED on collision detection
+	  - link_10m: Keep LED on with 10m link speed
+	  - link_100m: Keep LED on with 100m link speed
+	  - link_1000m: Keep LED on with 1000m link speed
+	  - half_duplex: Keep LED on with half duplex link
+	  - full_duplex: Keep LED on with full duplex link
+	  - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
+	  - power_on_reset: Keep LED on with switch reset
+	  - blink_2hz: Set blink speed at 2hz for every blink event
+	  - blink_4hz: Set blink speed at 4hz for every blink event
+	  - blink_8hz: Set blink speed at 8hz for every blink event
+	  - blink_auto: Set blink speed at 2hz for 10m link speed,
+	                4hz for 100m and 8hz for 1000m
+
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 25c4db97cdd4..392ca8568588 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_LEDS_TRIGGER_NETDEV)	+= ledtrig-netdev.o
 obj-$(CONFIG_LEDS_TRIGGER_PATTERN)	+= ledtrig-pattern.o
 obj-$(CONFIG_LEDS_TRIGGER_AUDIO)	+= ledtrig-audio.o
 obj-$(CONFIG_LEDS_TRIGGER_TTY)		+= ledtrig-tty.o
+obj-$(CONFIG_LEDS_OFFLOAD_TRIGGER_PHY_ACTIVITY) += ledtrig-offload-phy-activity.o
diff --git a/drivers/leds/trigger/ledtrig-offload-phy-activity.c b/drivers/leds/trigger/ledtrig-offload-phy-activity.c
new file mode 100644
index 000000000000..ca0500b1d37a
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-offload-phy-activity.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
+
+#define PHY_ACTIVITY_MAX_TRIGGERS 14
+
+#define DEFINE_OFFLOAD_TRIGGER(trigger_name, trigger) \
+	static ssize_t trigger_name##_show(struct device *dev, \
+				struct device_attribute *attr, char *buf) \
+	{ \
+		struct led_classdev *led_cdev = led_trigger_get_led(dev); \
+		int val; \
+		val = led_cdev->configure_offload(led_cdev, trigger, TRIGGER_READ); \
+		return sprintf(buf, "%d\n", val ? 1 : 0); \
+	} \
+	static ssize_t trigger_name##_store(struct device *dev, \
+					struct device_attribute *attr, \
+					const char *buf, size_t size) \
+	{ \
+		struct led_classdev *led_cdev = led_trigger_get_led(dev); \
+		unsigned long state; \
+		int cmd, ret; \
+		ret = kstrtoul(buf, 0, &state); \
+		if (ret) \
+			return ret; \
+		cmd = !!state ? TRIGGER_ENABLE : TRIGGER_DISABLE; \
+		/* Update the configuration with every change */ \
+		led_cdev->configure_offload(led_cdev, trigger, cmd); \
+		return size; \
+	} \
+	DEVICE_ATTR_RW(trigger_name)
+
+/* Expose sysfs for every blink to be configurable from userspace */
+DEFINE_OFFLOAD_TRIGGER(blink_tx, BLINK_TX);
+DEFINE_OFFLOAD_TRIGGER(blink_rx, BLINK_RX);
+DEFINE_OFFLOAD_TRIGGER(blink_collision, BLINK_COLLISION);
+DEFINE_OFFLOAD_TRIGGER(keep_link_10m, KEEP_LINK_10M);
+DEFINE_OFFLOAD_TRIGGER(keep_link_100m, KEEP_LINK_100M);
+DEFINE_OFFLOAD_TRIGGER(keep_link_1000m, KEEP_LINK_1000M);
+DEFINE_OFFLOAD_TRIGGER(keep_half_duplex, KEEP_HALF_DUPLEX);
+DEFINE_OFFLOAD_TRIGGER(keep_full_duplex, KEEP_FULL_DUPLEX);
+DEFINE_OFFLOAD_TRIGGER(option_linkup_over, OPTION_LINKUP_OVER);
+DEFINE_OFFLOAD_TRIGGER(option_power_on_reset, OPTION_POWER_ON_RESET);
+DEFINE_OFFLOAD_TRIGGER(option_blink_2hz, OPTION_BLINK_2HZ);
+DEFINE_OFFLOAD_TRIGGER(option_blink_4hz, OPTION_BLINK_4HZ);
+DEFINE_OFFLOAD_TRIGGER(option_blink_8hz, OPTION_BLINK_8HZ);
+DEFINE_OFFLOAD_TRIGGER(option_blink_auto, OPTION_BLINK_AUTO);
+
+/* The attrs will be placed dynamically based on the supported triggers */
+static struct attribute *phy_activity_attrs[PHY_ACTIVITY_MAX_TRIGGERS + 1];
+
+static int offload_phy_activity_activate(struct led_classdev *led_cdev)
+{
+	const char *offload_trigger;
+	int i, trigger, ret;
+
+	/* This triggered is supported only by LEDs that can be offload driven */
+	if (!led_cdev->trigger_offload || !led_cdev->trigger_offload)
+		return -EOPNOTSUPP;
+
+	/* Scan the supported offload triggers and expose them in sysfs if supported */
+	for (trigger = 0, i = 0; trigger < led_cdev->supported_offload_triggers_count; trigger++) {
+		offload_trigger = led_cdev->supported_offload_triggers[trigger];
+
+		if (i >= PHY_ACTIVITY_MAX_TRIGGERS)
+			break;
+
+		if (!strcmp(offload_trigger, "tx-blink"))
+			phy_activity_attrs[i++] = &dev_attr_blink_tx.attr;
+
+		if (!strcmp(offload_trigger, "rx-blink"))
+			phy_activity_attrs[i++] = &dev_attr_blink_rx.attr;
+
+		if (!strcmp(offload_trigger, "collision-blink"))
+			phy_activity_attrs[i++] = &dev_attr_blink_collision.attr;
+
+		if (!strcmp(offload_trigger, "link-10M"))
+			phy_activity_attrs[i++] = &dev_attr_keep_link_10m.attr;
+
+		if (!strcmp(offload_trigger, "link-100M"))
+			phy_activity_attrs[i++] = &dev_attr_keep_link_100m.attr;
+
+		if (!strcmp(offload_trigger, "link-1000M"))
+			phy_activity_attrs[i++] = &dev_attr_keep_link_1000m.attr;
+
+		if (!strcmp(offload_trigger, "half-duplex"))
+			phy_activity_attrs[i++] = &dev_attr_keep_half_duplex.attr;
+
+		if (!strcmp(offload_trigger, "full-duplex"))
+			phy_activity_attrs[i++] = &dev_attr_keep_full_duplex.attr;
+
+		if (!strcmp(offload_trigger, "linkup-over"))
+			phy_activity_attrs[i++] = &dev_attr_option_linkup_over.attr;
+
+		if (!strcmp(offload_trigger, "power-on-reset"))
+			phy_activity_attrs[i++] = &dev_attr_option_power_on_reset.attr;
+
+		if (!strcmp(offload_trigger, "blink-2hz"))
+			phy_activity_attrs[i++] = &dev_attr_option_blink_2hz.attr;
+
+		if (!strcmp(offload_trigger, "blink-4hz"))
+			phy_activity_attrs[i++] = &dev_attr_option_blink_4hz.attr;
+
+		if (!strcmp(offload_trigger, "blink-8hz"))
+			phy_activity_attrs[i++] = &dev_attr_option_blink_8hz.attr;
+
+		if (!strcmp(offload_trigger, "blink-auto"))
+			phy_activity_attrs[i++] = &dev_attr_option_blink_auto.attr;
+	}
+
+	/* Enable offload mode. No custom configuration is applied,
+	 * the LED driver will use whatever default configuration is
+	 * currently configured.
+	 */
+	ret = led_cdev->trigger_offload(led_cdev, true);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void offload_phy_activity_deactivate(struct led_classdev *led_cdev)
+{
+	led_cdev->trigger_offload(led_cdev, false);
+}
+
+static const struct attribute_group phy_activity_group = {
+	.name = "offload-phy-activity",
+	.attrs = phy_activity_attrs,
+};
+
+static const struct attribute_group *phy_activity_groups[] = {
+	&phy_activity_group,
+	NULL,
+};
+
+static struct led_trigger offload_phy_activity_trigger = {
+	.name       = "offload-phy-activity",
+	.activate   = offload_phy_activity_activate,
+	.deactivate = offload_phy_activity_deactivate,
+	.groups     = phy_activity_groups,
+};
+
+static int __init offload_phy_activity_init(void)
+{
+	return led_trigger_register(&offload_phy_activity_trigger);
+}
+device_initcall(offload_phy_activity_init);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index eb06d812bc24..1e8d1efcb44f 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -537,6 +537,30 @@ static inline void ledtrig_flash_ctrl(bool on) {}
 static inline void ledtrig_torch_ctrl(bool on) {}
 #endif
 
+#ifdef CONFIG_LEDS_OFFLOAD_TRIGGER_PHY_ACTIVITY
+/* 3 main category for offload trigger:
+ * - blink: the led will blink on trigger
+ * - keep: the led will be kept on with condition
+ * - option: a configuration value internal to the led on how offload works
+ */
+enum offload_phy_activity {
+	BLINK_TX = BIT(0), /* Blink on TX traffic */
+	BLINK_RX = BIT(1), /* Blink on RX traffic */
+	BLINK_COLLISION = BIT(2), /* Blink on Collision packet */
+	KEEP_LINK_10M = BIT(3), /* LED on with 10M link */
+	KEEP_LINK_100M = BIT(4), /* LED on with 100M link */
+	KEEP_LINK_1000M = BIT(5), /* LED on with 1000M link */
+	KEEP_HALF_DUPLEX = BIT(6), /* LED on with Half-Duplex link */
+	KEEP_FULL_DUPLEX = BIT(7), /* LED on with Full-Duplex link */
+	OPTION_LINKUP_OVER = BIT(8), /* LED will be on with KEEP offload_triggers and blink on BLINK traffic */
+	OPTION_POWER_ON_RESET = BIT(9), /* LED will be on Switch reset */
+	OPTION_BLINK_2HZ = BIT(10), /* LED will blink with any offload_trigger at 2Hz */
+	OPTION_BLINK_4HZ = BIT(11), /* LED will blink with any offload_trigger at 4Hz */
+	OPTION_BLINK_8HZ = BIT(12), /* LED will blink with any offload_trigger at 8Hz */
+	OPTION_BLINK_AUTO = BIT(13), /* LED will blink differently based on the Link Speed */
+};
+#endif
+
 /*
  * Generic LED platform data for describing LED names and default triggers.
  */
-- 
2.32.0

[RFC PATCH 5/6] net: dsa: qca8k: add LEDs support

[Ansuel Smith]

Add LEDs support for qca8k Switch Family. This will provide the LEDs
offload API to permit the PHY LED to be driven offloaded.
Each port have at least 3 LEDs and they can HW blink, set on/off or
set to be driven by some rules applied by the offload trigger.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/Kconfig      |   9 +
 drivers/net/dsa/Makefile     |   1 +
 drivers/net/dsa/qca8k-leds.c | 361 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/qca8k.c      |   4 +-
 drivers/net/dsa/qca8k.h      |  64 +++++++
 5 files changed, 437 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/dsa/qca8k-leds.c

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 7b1457a6e327..89e36f3a8195 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -67,6 +67,15 @@ config NET_DSA_QCA8K
 	  This enables support for the Qualcomm Atheros QCA8K Ethernet
 	  switch chips.
 
+config NET_DSA_QCA8K_LEDS_SUPPORT
+	tristate "Qualcomm Atheros QCA8K Ethernet switch family LEDs support"
+	select NET_DSA_QCA8K
+	select LEDS_OFFLOAD_TRIGGERS
+	help
+	  Thsi enabled support for LEDs present on the Qualcomm Atheros
+	  QCA8K Ethernet switch chips. This require the LEDs offload
+	  triggers support as it can run in offload mode.
+
 config NET_DSA_REALTEK_SMI
 	tristate "Realtek SMI Ethernet switch family support"
 	select NET_DSA_TAG_RTL4_A
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index 8da1569a34e6..fb1e7d0cf126 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_NET_DSA_LANTIQ_GSWIP) += lantiq_gswip.o
 obj-$(CONFIG_NET_DSA_MT7530)	+= mt7530.o
 obj-$(CONFIG_NET_DSA_MV88E6060) += mv88e6060.o
 obj-$(CONFIG_NET_DSA_QCA8K)	+= qca8k.o
+obj-$(CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT) += qca8k-leds.o
 obj-$(CONFIG_NET_DSA_REALTEK_SMI) += realtek-smi.o
 realtek-smi-objs		:= realtek-smi-core.o rtl8366.o rtl8366rb.o rtl8365mb.o
 obj-$(CONFIG_NET_DSA_SMSC_LAN9303) += lan9303-core.o
diff --git a/drivers/net/dsa/qca8k-leds.c b/drivers/net/dsa/qca8k-leds.c
new file mode 100644
index 000000000000..0f86a30c7440
--- /dev/null
+++ b/drivers/net/dsa/qca8k-leds.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <net/dsa.h>
+
+#include "qca8k.h"
+
+static int
+qca8k_get_enable_led_reg(int port_num, int led_num, struct qca8k_led_pattern_en *reg_info)
+{
+	int shift;
+
+	switch (port_num) {
+	case 0:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = 14;
+		break;
+	case 1:
+	case 2:
+	case 3:
+		reg_info->reg = QCA8K_LED_CTRL_REG(3);
+		shift = 2 * led_num + (6 * (port_num - 1));
+
+		reg_info->shift = 8 + shift;
+
+		break;
+	case 4:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = 30;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+qca8k_get_control_led_reg(int port_num, int led_num, struct qca8k_led_pattern_en *reg_info)
+{
+	reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+
+	/* 6 total control rule:
+	 * 3 control rules for phy0-3 that applies to all their leds
+	 * 3 control rules for phy4
+	 */
+	if (port_num == 4)
+		reg_info->shift = 16;
+	else
+		reg_info->shift = 0;
+
+	return 0;
+}
+
+static int
+qca8k_cled_configure_offload(struct led_classdev *ldev, u32 rules, enum offload_trigger_cmd cmd)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+	struct led_trigger *trigger = ldev->trigger;
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 offload_trigger = 0, mask, val;
+	int ret;
+
+	/* Check trigger compatibility */
+	if (strcmp(trigger->name, "offload-phy-activity"))
+		return -EOPNOTSUPP;
+
+	switch (rules) {
+	case BLINK_TX:
+		offload_trigger = QCA8K_LED_TX_BLINK_MASK;
+		break;
+	case BLINK_RX:
+		offload_trigger = QCA8K_LED_RX_BLINK_MASK;
+		break;
+	case BLINK_COLLISION:
+		offload_trigger = QCA8K_LED_COL_BLINK_MASK;
+		break;
+	case KEEP_LINK_10M:
+		offload_trigger = QCA8K_LED_LINK_10M_EN_MASK;
+		break;
+	case KEEP_LINK_100M:
+		offload_trigger = QCA8K_LED_LINK_100M_EN_MASK;
+		break;
+	case KEEP_LINK_1000M:
+		offload_trigger = QCA8K_LED_LINK_1000M_EN_MASK;
+		break;
+	case KEEP_HALF_DUPLEX:
+		offload_trigger = QCA8K_LED_HALF_DUPLEX_MASK;
+		break;
+	case KEEP_FULL_DUPLEX:
+		offload_trigger = QCA8K_LED_FULL_DUPLEX_MASK;
+		break;
+	case OPTION_LINKUP_OVER:
+		offload_trigger = QCA8K_LED_LINKUP_OVER_MASK;
+		break;
+	case OPTION_BLINK_2HZ:
+			offload_trigger = QCA8K_LED_BLINK_2HZ;
+		break;
+	case OPTION_BLINK_4HZ:
+			offload_trigger = QCA8K_LED_BLINK_4HZ;
+		break;
+	case OPTION_BLINK_8HZ:
+			offload_trigger = QCA8K_LED_BLINK_8HZ;
+		break;
+	case OPTION_BLINK_AUTO:
+			offload_trigger = QCA8K_LED_BLINK_AUTO;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	if (rules == OPTION_BLINK_2HZ || rules == OPTION_BLINK_4HZ ||
+	    rules == OPTION_BLINK_8HZ || rules == OPTION_BLINK_AUTO) {
+		mask = QCA8K_LED_BLINK_FREQ_MASK;
+	} else {
+		mask = offload_trigger;
+	}
+
+	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+
+	switch (cmd) {
+	case TRIGGER_ENABLE:
+		ret = qca8k_rmw(priv, reg_info.reg,
+				mask << reg_info.shift,
+				offload_trigger << reg_info.shift);
+		break;
+	case TRIGGER_DISABLE:
+		ret = qca8k_rmw(priv, reg_info.reg,
+				mask << reg_info.shift,
+				0);
+		break;
+	case TRIGGER_READ:
+		ret = qca8k_read(priv, reg_info.reg, &val);
+		if (ret)
+			return ret;
+
+		val >>= reg_info.shift;
+		val &= offload_trigger;
+
+		/* Special handling for LED_BLINK_2HZ */
+		if (!val && offload_trigger == QCA8K_LED_BLINK_2HZ)
+			val = 1;
+
+		return val;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
+static void
+qca8k_led_brightness_set(struct qca8k_led *led,
+			 enum led_brightness b)
+{
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 val = QCA8K_LED_ALWAYS_OFF;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (b)
+		val = QCA8K_LED_ALWAYS_ON;
+
+	qca8k_rmw(priv, reg_info.reg,
+		  GENMASK(1, 0) << reg_info.shift,
+		  val << reg_info.shift);
+}
+
+static void
+qca8k_cled_brightness_set(struct led_classdev *ldev,
+			  enum led_brightness b)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+
+	return qca8k_led_brightness_set(led, b);
+}
+
+static enum led_brightness
+qca8k_led_brightness_get(struct qca8k_led *led)
+{
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 val;
+	int ret;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	ret = qca8k_read(priv, reg_info.reg, &val);
+	if (ret)
+		return 0;
+
+	val >>= reg_info.shift;
+	val &= GENMASK(1, 0);
+
+	return val > 0 ? 1 : 0;
+}
+
+static enum led_brightness
+qca8k_cled_brightness_get(struct led_classdev *ldev)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+
+	return qca8k_led_brightness_get(led);
+}
+
+static int
+qca8k_cled_blink_set(struct led_classdev *ldev,
+		     unsigned long *delay_on,
+		     unsigned long *delay_off)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 125;
+		*delay_off = 125;
+	}
+
+	if (*delay_on != 125 || *delay_off != 125) {
+		/* The hardware only supports blinking at 4Hz. Fall back
+		 * to software implementation in other cases.
+		 */
+		return -EINVAL;
+	}
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	qca8k_rmw(priv, reg_info.reg,
+		  GENMASK(1, 0) << reg_info.shift,
+		  QCA8K_LED_ALWAYS_BLINK_4HZ << reg_info.shift);
+
+	return 0;
+}
+
+static int
+qca8k_cled_trigger_offload(struct led_classdev *ldev, bool enable)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 val = QCA8K_LED_ALWAYS_OFF;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (enable)
+		val = QCA8K_LED_RULE_CONTROLLED;
+
+	return qca8k_rmw(priv, reg_info.reg,
+			 GENMASK(1, 0) << reg_info.shift,
+			 val << reg_info.shift);
+}
+
+static int
+qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
+{
+	struct fwnode_handle *led = NULL, *leds = NULL;
+	struct led_init_data init_data = { };
+	struct qca8k_led *port_led;
+	int led_num, port_index;
+	const char *state;
+	int ret;
+
+	leds = fwnode_get_named_child_node(port, "leds");
+	if (!leds) {
+		dev_info(priv->dev, "No Leds node specified in device tree for port %d!\n",
+			 port_num);
+		return 0;
+	}
+
+	fwnode_for_each_child_node(leds, led) {
+		/* Reg rapresent the led number of the port.
+		 * Each port can have at least 3 leds attached
+		 * Commonly:
+		 * 1. is gigabit led
+		 * 2. is mbit led
+		 * 3. additional status led
+		 */
+		if (fwnode_property_read_u32(led, "reg", &led_num))
+			continue;
+
+		if (led_num >= QCA8K_LED_PORT_COUNT) {
+			dev_warn(priv->dev, "Invalid LED reg defined %d", port_num);
+			continue;
+		}
+
+		port_index = 3 * port_num + led_num;
+
+		port_led = &priv->ports_led[port_index];
+		port_led->port_num = port_num;
+		port_led->led_num = led_num;
+		port_led->priv = priv;
+
+		ret = fwnode_property_read_string(led, "default-state", &state);
+		if (!ret) {
+			if (!strcmp(state, "on")) {
+				port_led->cdev.brightness = 1;
+				qca8k_led_brightness_set(port_led, 1);
+			} else if (!strcmp(state, "off")) {
+				port_led->cdev.brightness = 0;
+				qca8k_led_brightness_set(port_led, 0);
+			} else if (!strcmp(state, "keep")) {
+				port_led->cdev.brightness =
+					qca8k_led_brightness_get(port_led);
+			}
+		}
+
+		/* 3 brightness settings can be applied from Documentation:
+		 * 0 always off
+		 * 1 blink at 4Hz
+		 * 2 always on
+		 * 3 rule controlled
+		 * Suppots only 2 mode: (pcb limitation, with always on and blink
+		 * only the last led is set to this mode)
+		 * 0 always off (sets all leds off)
+		 * 3 rule controlled
+		 */
+		port_led->cdev.max_brightness = 1;
+		port_led->cdev.brightness_set = qca8k_cled_brightness_set;
+		port_led->cdev.brightness_get = qca8k_cled_brightness_get;
+		port_led->cdev.blink_set = qca8k_cled_blink_set;
+		port_led->cdev.configure_offload = qca8k_cled_configure_offload;
+		port_led->cdev.trigger_offload = qca8k_cled_trigger_offload;
+		init_data.default_label = ":port";
+		init_data.devicename = "qca8k";
+		init_data.fwnode = led;
+
+		ret = devm_led_classdev_register_ext(priv->dev, &port_led->cdev, &init_data);
+		if (ret)
+			dev_warn(priv->dev, "Failed to int led");
+	}
+
+	return 0;
+}
+
+int
+qca8k_setup_led_ctrl(struct qca8k_priv *priv)
+{
+	struct fwnode_handle *mdio, *port;
+	int port_num;
+	int ret;
+
+	mdio = device_get_named_child_node(priv->dev, "mdio");
+	if (!mdio) {
+		dev_info(priv->dev, "No MDIO node specified in device tree!\n");
+		return 0;
+	}
+
+	fwnode_for_each_child_node(mdio, port) {
+		if (fwnode_property_read_u32(port, "reg", &port_num))
+			continue;
+
+		/* Each port can have at least 3 different leds attached */
+		ret = qca8k_parse_port_leds(priv, port, port_num);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index a429c9750add..b5c81e2d9f8a 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -148,7 +148,7 @@ qca8k_set_page(struct mii_bus *bus, u16 page)
 	return 0;
 }
 
-static int
+int
 qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val)
 {
 	struct mii_bus *bus = priv->bus;
@@ -192,7 +192,7 @@ qca8k_write(struct qca8k_priv *priv, u32 reg, u32 val)
 	return ret;
 }
 
-static int
+int
 qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val)
 {
 	struct mii_bus *bus = priv->bus;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 128b8cf85e08..b11385ff9bce 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/regmap.h>
 #include <linux/gpio.h>
+#include <linux/leds.h>
 
 #define QCA8K_NUM_PORTS					7
 #define QCA8K_NUM_CPU_PORTS				2
@@ -74,6 +75,42 @@
 #define   QCA8K_MDIO_MASTER_DATA_MASK			GENMASK(15, 0)
 #define   QCA8K_MDIO_MASTER_MAX_PORTS			5
 #define   QCA8K_MDIO_MASTER_MAX_REG			32
+
+/* LED control register */
+#define QCA8K_LED_COUNT					15
+#define QCA8K_LED_PORT_COUNT				3
+#define QCA8K_LED_RULE_COUNT				6
+#define QCA8K_LED_RULE_MAX				11
+#define QCA8K_LED_CTRL_REG(_i)				(0x050 + (_i) * 4)
+#define QCA8K_LED_CTRL0_REG				0x50
+#define QCA8K_LED_CTRL1_REG				0x54
+#define QCA8K_LED_CTRL2_REG				0x58
+#define QCA8K_LED_CTRL3_REG				0x5C
+#define   QCA8K_LED_CTRL_SHIFT(_i)			(((_i) % 2) * 16)
+#define   QCA8K_LED_CTRL_MASK				GENMASK(15, 0)
+#define QCA8K_LED_BLINK_FREQ_MASK			GENMASK(1, 0)
+#define QCA8K_LED_BLINK_FREQ_SHITF			0
+#define   QCA8K_LED_BLINK_2HZ				0
+#define   QCA8K_LED_BLINK_4HZ				1
+#define   QCA8K_LED_BLINK_8HZ				2
+#define   QCA8K_LED_BLINK_AUTO				3
+#define QCA8K_LED_LINKUP_OVER_MASK			BIT(2)
+#define QCA8K_LED_TX_BLINK_MASK				BIT(4)
+#define QCA8K_LED_RX_BLINK_MASK				BIT(5)
+#define QCA8K_LED_COL_BLINK_MASK			BIT(7)
+#define QCA8K_LED_LINK_10M_EN_MASK			BIT(8)
+#define QCA8K_LED_LINK_100M_EN_MASK			BIT(9)
+#define QCA8K_LED_LINK_1000M_EN_MASK			BIT(10)
+#define QCA8K_LED_POWER_ON_LIGHT_MASK			BIT(11)
+#define QCA8K_LED_HALF_DUPLEX_MASK			BIT(12)
+#define QCA8K_LED_FULL_DUPLEX_MASK			BIT(13)
+#define QCA8K_LED_PATTERN_EN_MASK			GENMASK(15, 14)
+#define QCA8K_LED_PATTERN_EN_SHIFT			14
+#define   QCA8K_LED_ALWAYS_OFF				0
+#define   QCA8K_LED_ALWAYS_BLINK_4HZ			1
+#define   QCA8K_LED_ALWAYS_ON				2
+#define   QCA8K_LED_RULE_CONTROLLED			3
+
 #define QCA8K_GOL_MAC_ADDR0				0x60
 #define QCA8K_GOL_MAC_ADDR1				0x64
 #define QCA8K_MAX_FRAME_SIZE				0x78
@@ -279,6 +316,19 @@ struct qca8k_ports_config {
 	u8 rgmii_tx_delay[QCA8K_NUM_CPU_PORTS]; /* 0: CPU port0, 1: CPU port6 */
 };
 
+struct qca8k_led_pattern_en {
+	u32 reg;
+	u8 shift;
+};
+
+struct qca8k_led {
+	u8 port_num;
+	u8 led_num;
+	u16 old_rule;
+	struct qca8k_priv *priv;
+	struct led_classdev cdev;
+};
+
 struct qca8k_priv {
 	u8 switch_id;
 	u8 switch_revision;
@@ -293,6 +343,7 @@ struct qca8k_priv {
 	struct dsa_switch_ops ops;
 	struct gpio_desc *reset_gpio;
 	unsigned int port_mtu[QCA8K_NUM_PORTS];
+	struct qca8k_led ports_led[QCA8K_LED_COUNT];
 };
 
 struct qca8k_mib_desc {
@@ -308,4 +359,17 @@ struct qca8k_fdb {
 	u8 mac[6];
 };
 
+/* Common function used by the LEDs module */
+int qca8k_read(struct qca8k_priv *priv, u32 reg, u32 *val);
+int qca8k_rmw(struct qca8k_priv *priv, u32 reg, u32 mask, u32 write_val);
+
+#ifdef CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT
+int qca8k_setup_led_ctrl(struct qca8k_priv *priv);
+#else
+static inline int qca8k_setup_led_ctrl(struct qca8k_priv *priv)
+{
+	return 0;
+}
+#endif
+
 #endif /* __QCA8K_H */
-- 
2.32.0

[RFC PATCH 6/6] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Ansuel Smith]

Add LEDs definition example for qca8k using the offload trigger as the
default trigger and add all the supported offload triggers by the
switch.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/net/dsa/qca8k.yaml    | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 48de0ace265d..d8394f625d03 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -64,6 +64,8 @@ properties:
                  internal mdio access is used.
                  With the legacy mapping the reg corresponding to the internal
                  mdio is the switch reg with an offset of -1.
+                 Each phy have at least 3 LEDs connected and can be declared
+                 using the standard LEDs structure.
 
     properties:
       '#address-cells':
@@ -340,6 +342,34 @@ examples:
 
                 internal_phy_port1: ethernet-phy@0 {
                     reg = <0>;
+
+                    leds {
+                        led@0 {
+                            reg = <0>;
+                            color = <LED_COLOR_ID_WHITE>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            linux,default-trigger = "offload-phy-activity";
+                            linux,supported-offload-triggers = "rx-blink", "tx-blink", "ollision-blink",
+                                                        "link-10M", "link-100M", "link-1000M",
+                                                        "half-duplex", "full-duplex", "linkup-over",
+                                                        "power-on-reset", "blink-2hz", "blink-4hz",
+                                                        "blink-8hz", "blink-auto";
+                        };
+
+                        led@1 {
+                            reg = <1>;
+                            color = <LED_COLOR_ID_AMBER>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            linux,default-trigger = "offload-phy-activity";
+                            linux,supported-offload-triggers = "rx-blink", "tx-blink", "ollision-blink",
+                                                        "link-10M", "link-100M", "link-1000M",
+                                                        "half-duplex", "full-duplex", "linkup-over",
+                                                        "power-on-reset", "blink-2hz", "blink-4hz",
+                                                        "blink-8hz", "blink-auto";
+                        };
+                    };
                 };
 
                 internal_phy_port2: ethernet-phy@1 {
-- 
2.32.0

Re: [RFC PATCH 2/6] leds: permit to declare supported offload triggers

[Marek Behún]

On Sun,  7 Nov 2021 18:57:14 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:

> With LEDs that can be offload driven, permit to declare supported triggers
> in the dts and add them to the cled struct to be used by the related
> offload trigger. This is particurally useful for phy that have support
> for HW blinking on tx/rx traffic or based on the speed link.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

NAK. The device-tree shouldn't define this, only the LED's function as
designated by the manufacturer of the device.

Marek

Re: [RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Marek Behún]

On Sun,  7 Nov 2021 18:57:16 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:

> Add Offload Trigger for PHY Activity. This special trigger is used to
> configure and expose the different HW trigger that are provided by the
> PHY. Each offload trigger can be configured by sysfs and on trigger
> activation the offload mode is enabled.
> 
> This currently implement these hw triggers:
>   - blink_tx: Blink LED on tx packet receive
>   - blink_rx: Blink LED on rx packet receive
>   - blink_collision: Blink LED on collision detection
>   - link_10m: Keep LED on with 10m link speed
>   - link_100m: Keep LED on with 100m link speed
>   - link_1000m: Keep LED on with 1000m link speed
>   - half_duplex: Keep LED on with half duplex link
>   - full_duplex: Keep LED on with full duplex link
>   - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
>   - power_on_reset: Keep LED on with switch reset
>   - blink_2hz: Set blink speed at 2hz for every blink event
>   - blink_4hz: Set blink speed at 4hz for every blink event
>   - blink_8hz: Set blink speed at 8hz for every blink event
>   - blink_auto: Set blink speed at 2hz for 10m link speed,
>       4hz for 100m and 8hz for 1000m
> 
> The trigger will read the supported offload trigger in the led cdev and
> will expose the offload triggers in sysfs and then activate the offload
> mode for the led in offload mode has it configured by default. A flag is
> passed to configure_offload with the related rule from this trigger to
> active or disable.
> It's in the led driver interest the detection and knowing how to
> elaborate the passed flags.
> 
> The different hw triggers are exposed in the led sysfs dir under the
> offload-phy-activity subdir.

NAK. The current plan is to use netdev trigger, and if it can
transparently offload the settings to HW, it will.

Yes, netdev trigger currently does not support all these settings.
But it supports indicating link and blinking on activity.

So the plan is to start with offloading the blinking on activity, i.e.
I the user does
  $ cd /sys/class/leds/<LED>
  $ echo netdev >trigger
  $ echo 1 >rx
  $ echo eth0 >device_name

this would, instead of doing blinking in software, do it in HW instead.

After this is implemented, we can start working on extending netdev
trigger to support more complicated features.

Marek

Re: [RFC PATCH 2/6] leds: permit to declare supported offload triggers

[Ansuel Smith]

On Sun, Nov 07, 2021 at 11:06:24PM +0100, Marek Behún wrote:
> On Sun,  7 Nov 2021 18:57:14 +0100
> Ansuel Smith <ansuelsmth@gmail.com> wrote:
> 
> > With LEDs that can be offload driven, permit to declare supported triggers
> > in the dts and add them to the cled struct to be used by the related
> > offload trigger. This is particurally useful for phy that have support
> > for HW blinking on tx/rx traffic or based on the speed link.
> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> 
> NAK. The device-tree shouldn't define this, only the LED's function as
> designated by the manufacturer of the device.
> 
> Marek

Sure I will add a way to ask the led driver if the trigger is supported
and report it.

-- 
	Ansuel

Re: [RFC PATCH 5/6] net: dsa: qca8k: add LEDs support

[Randy Dunlap]

On 11/7/21 9:57 AM, Ansuel Smith wrote:
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 7b1457a6e327..89e36f3a8195 100644
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -67,6 +67,15 @@ config NET_DSA_QCA8K
>   	  This enables support for the Qualcomm Atheros QCA8K Ethernet
>   	  switch chips.
>   
> +config NET_DSA_QCA8K_LEDS_SUPPORT
> +	tristate "Qualcomm Atheros QCA8K Ethernet switch family LEDs support"
> +	select NET_DSA_QCA8K
> +	select LEDS_OFFLOAD_TRIGGERS
> +	help
> +	  Thsi enabled support for LEDs present on the Qualcomm Atheros

	  This enables

> +	  QCA8K Ethernet switch chips. This require the LEDs offload

	                                    requires

> +	  triggers support as it can run in offload mode.


-- 
~Randy

Re: [RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Randy Dunlap]

On 11/7/21 9:57 AM, Ansuel Smith wrote:
> +config LEDS_OFFLOAD_TRIGGER_PHY_ACTIVITY
> +	tristate "LED Offload Trigger for PHY Activity"
> +	depends on LEDS_OFFLOAD_TRIGGERS
> +	help
> +	  This allows LEDs to be configured to run by HW and offloaded based
> +	  on some rules. The LED will blink or be on based on the PHY Activity
> +	  for example on packet receive of based on the link speed.

Cannot parse:                           of based on

> +	  The current rules are:


-- 
~Randy

Re: [RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Ansuel Smith]

On Sun, Nov 07, 2021 at 11:10:09PM +0100, Marek Behún wrote:
> On Sun,  7 Nov 2021 18:57:16 +0100
> Ansuel Smith <ansuelsmth@gmail.com> wrote:
> 
> > Add Offload Trigger for PHY Activity. This special trigger is used to
> > configure and expose the different HW trigger that are provided by the
> > PHY. Each offload trigger can be configured by sysfs and on trigger
> > activation the offload mode is enabled.
> > 
> > This currently implement these hw triggers:
> >   - blink_tx: Blink LED on tx packet receive
> >   - blink_rx: Blink LED on rx packet receive
> >   - blink_collision: Blink LED on collision detection
> >   - link_10m: Keep LED on with 10m link speed
> >   - link_100m: Keep LED on with 100m link speed
> >   - link_1000m: Keep LED on with 1000m link speed
> >   - half_duplex: Keep LED on with half duplex link
> >   - full_duplex: Keep LED on with full duplex link
> >   - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
> >   - power_on_reset: Keep LED on with switch reset
> >   - blink_2hz: Set blink speed at 2hz for every blink event
> >   - blink_4hz: Set blink speed at 4hz for every blink event
> >   - blink_8hz: Set blink speed at 8hz for every blink event
> >   - blink_auto: Set blink speed at 2hz for 10m link speed,
> >       4hz for 100m and 8hz for 1000m
> > 
> > The trigger will read the supported offload trigger in the led cdev and
> > will expose the offload triggers in sysfs and then activate the offload
> > mode for the led in offload mode has it configured by default. A flag is
> > passed to configure_offload with the related rule from this trigger to
> > active or disable.
> > It's in the led driver interest the detection and knowing how to
> > elaborate the passed flags.
> > 
> > The different hw triggers are exposed in the led sysfs dir under the
> > offload-phy-activity subdir.
> 
> NAK. The current plan is to use netdev trigger, and if it can
> transparently offload the settings to HW, it will.
> 
> Yes, netdev trigger currently does not support all these settings.
> But it supports indicating link and blinking on activity.
> 
> So the plan is to start with offloading the blinking on activity, i.e.
> I the user does
>   $ cd /sys/class/leds/<LED>
>   $ echo netdev >trigger
>   $ echo 1 >rx
>   $ echo eth0 >device_name
> 
> this would, instead of doing blinking in software, do it in HW instead.
> 
> After this is implemented, we can start working on extending netdev
> trigger to support more complicated features.
> 
> Marek

Using the netdev trigger would cause some problem. Most of the switch
can run in SW mode (with blink controlled by software of always on) or
be put in HW mode and they will autonomously control blinking and how
the LED will operate. So we just need to provide a way to trigger this
mode and configure it. Why having something that gets triggered and then
does nothing as it's offloaded?

The current way to configure this is very similar... Set the offload
trigger and use the deidcated subdir to set how the led will
blink/behave based on the supported trigger reported by the driver.

There is no reason to set a device_name as that would be hardcoded to
the phy (and it should not change... again in HW we can't control that
part, we can just tell the switch to blink on packet tx on that port)

So really the command is:
  $ cd /sys/class/leds/<LED>
  $ echo netdev > offload-phy-activity
  $ cd offload-phy-activity
  $ echo 1 > tx-blink

And the PHY will blink on tx packet.

I understand this should be an extension of netdev as they would do
similar task but honestly polluting the netdev trigger of if and else to
disable part of it if an offload mode can be supported seems bad and
confusionary. At this point introduce a dedicated trigger so an user can
switch between them. That way we can keep the flexibility of netdev
trigger that will always work but also permit to support the HW mode
with no load on the system.

-- 
	Ansuel

Re: [RFC PATCH 3/6] leds: add function to configure offload leds

[Randy Dunlap]

On 11/7/21 9:57 AM, Ansuel Smith wrote:
> diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
> index ab50b58d6a21..af84cce09068 100644
> --- a/Documentation/leds/leds-class.rst
> +++ b/Documentation/leds/leds-class.rst
> @@ -191,6 +191,18 @@ If the second argument (enable) to the trigger_offload() method is false, any
>   active HW offloading must be deactivated. In this case errors are not permitted
>   in the trigger_offload() method.
>   
> +The offload trigger will use the function configure_offload() provided by the driver
> +that will configure the offloaded mode for the LED.
> +This function pass as the first argument (offload_flags) a u32 flag, it's in the LED

                  passes                                           flag. It's

> +driver interest how to elaborate this flags and to declare support for a particular

                                          flag
although that sentence is not very clear.

> +offload trigger.
> +The second argument (cmd) of the configure_offload() method can be used to do various
> +operation for the specific trigger. We currently support ENABLE, DISABLE and READ to

    operations

> +enable, disable and read the state of the offload trigger for the LED driver.
> +If the driver return -ENOTSUPP on configure_offload, the trigger activation will

                  returns

> +fail as the driver doesn't support that specific offload trigger or don't know

                                                                        doesn't know

> +how to handle the provided flags.


-- 
~Randy

Re: [RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Ansuel Smith]

On Sun, Nov 07, 2021 at 02:42:37PM -0800, Randy Dunlap wrote:
> On 11/7/21 9:57 AM, Ansuel Smith wrote:
> > +config LEDS_OFFLOAD_TRIGGER_PHY_ACTIVITY
> > +	tristate "LED Offload Trigger for PHY Activity"
> > +	depends on LEDS_OFFLOAD_TRIGGERS
> > +	help
> > +	  This allows LEDs to be configured to run by HW and offloaded based
> > +	  on some rules. The LED will blink or be on based on the PHY Activity
> > +	  for example on packet receive of based on the link speed.
> 
> Cannot parse:                           of based on
>

Typo. "on packet receive or based on the link speed"
You can find right below all the modes.

> > +	  The current rules are:
> 
> 
> -- 
> ~Randy

-- 
	Ansuel

Re: [RFC PATCH 1/6] leds: trigger: add API for HW offloading of triggers

[Randy Dunlap]

Hi,

On 11/7/21 9:57 AM, Ansuel Smith wrote:
> From: Marek Behún <kabel@kernel.org>
> 
> Add method trigger_offload() and member variable `offloaded` to struct
> led_classdev. Add helper functions led_trigger_offload() and
> led_trigger_offload_stop().
> 
> The trigger_offload() method, when implemented by the LED driver, should
> be called (via led_trigger_offload() function) from trigger code wanting
> to be offloaded at the moment when configuration of the trigger changes.
> 
> If the trigger is successfully offloaded, this method returns 0 and the
> trigger does not have to blink the LED in software.
> 
> If the trigger with given configuration cannot be offloaded, the method
> should return -EOPNOTSUPP, in which case the trigger must blink the LED
> in SW.
> 
> The second argument to trigger_offload() being false means that the
> offloading is being disabled. In this case the function must return 0,
> errors are not permitted.
> 
> An additional config CONFIG_LEDS_OFFLOAD_TRIGGERS is added to add support
> for these special trigger offload driven.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   Documentation/leds/leds-class.rst | 22 +++++++++++++++++++++
>   drivers/leds/led-triggers.c       |  1 +
>   drivers/leds/trigger/Kconfig      | 10 ++++++++++
>   include/linux/leds.h              | 33 +++++++++++++++++++++++++++++++
>   4 files changed, 66 insertions(+)
> 
> diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
> index cd155ead8703..035a738afc4a 100644
> --- a/Documentation/leds/leds-class.rst
> +++ b/Documentation/leds/leds-class.rst
> @@ -169,6 +169,28 @@ Setting the brightness to zero with brightness_set() callback function
>   should completely turn off the LED and cancel the previously programmed
>   hardware blinking function, if any.
>   
> +Hardware offloading of LED triggers
> +===================================
> +
> +Some LEDs can offload SW triggers to hardware (for example a LED connected to

Better to s/SW/software/ and s/HW/hardware/ throughout the documentation file
and Kconfig file(s).

> +an ethernet PHY or an ethernet switch can be configured to blink on activity on
> +the network, which in software is done by the netdev trigger).
> +
> +To do such offloading, LED driver must support the this and a deficated offload

                                            drop:  the            dedicated

> +trigger must be used. The LED must implement the trigger_offload() method and

How does an LED implement the trigger_offload() method?
They don't have very much logic in them AFAIK.

> +the trigger code must try to call this method (via led_trigger_offload()
> +function) when configuration of the trigger (trigger_data) changes.
> +
> +The implementation of the trigger_offload() method by the LED driver must return
> +0 if the offload is successful and -EOPNOTSUPP if the requested trigger
> +configuration is not supported and the trigger should be executed in software.
> +If trigger_offload() returns negative value, the triggering will be done in
> +software, so any active offloading must also be disabled.
> +
> +If the second argument (enable) to the trigger_offload() method is false, any
> +active HW offloading must be deactivated. In this case errors are not permitted
> +in the trigger_offload() method.


> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index dc6816d36d06..c073e64e0a37 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -9,6 +9,16 @@ menuconfig LEDS_TRIGGERS
>   
>   if LEDS_TRIGGERS
>   
> +config LEDS_OFFLOAD_TRIGGERS
> +	bool "LED Offload Trigger support"
> +	help
> +	  This option enabled offload triggers support used by leds that

	                                                       LEDs

> +	  can be driven in HW by declaring some specific triggers.
> +	  A offload trigger will expose a sysfs dir to configure the
> +	  different blinking trigger and the available hw trigger.

Are the sysfs file values/meanings documented here?
I seem to have missed them.

> +
> +	  If unsure, say Y.
> +

thanks.
-- 
~Randy

Re: [RFC PATCH 1/6] leds: trigger: add API for HW offloading of triggers

[Ansuel Smith]

On Sun, Nov 07, 2021 at 02:52:12PM -0800, Randy Dunlap wrote:
> Hi,
> 
> On 11/7/21 9:57 AM, Ansuel Smith wrote:
> > From: Marek Behún <kabel@kernel.org>
> > 
> > Add method trigger_offload() and member variable `offloaded` to struct
> > led_classdev. Add helper functions led_trigger_offload() and
> > led_trigger_offload_stop().
> > 
> > The trigger_offload() method, when implemented by the LED driver, should
> > be called (via led_trigger_offload() function) from trigger code wanting
> > to be offloaded at the moment when configuration of the trigger changes.
> > 
> > If the trigger is successfully offloaded, this method returns 0 and the
> > trigger does not have to blink the LED in software.
> > 
> > If the trigger with given configuration cannot be offloaded, the method
> > should return -EOPNOTSUPP, in which case the trigger must blink the LED
> > in SW.
> > 
> > The second argument to trigger_offload() being false means that the
> > offloading is being disabled. In this case the function must return 0,
> > errors are not permitted.
> > 
> > An additional config CONFIG_LEDS_OFFLOAD_TRIGGERS is added to add support
> > for these special trigger offload driven.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   Documentation/leds/leds-class.rst | 22 +++++++++++++++++++++
> >   drivers/leds/led-triggers.c       |  1 +
> >   drivers/leds/trigger/Kconfig      | 10 ++++++++++
> >   include/linux/leds.h              | 33 +++++++++++++++++++++++++++++++
> >   4 files changed, 66 insertions(+)
> > 
> > diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
> > index cd155ead8703..035a738afc4a 100644
> > --- a/Documentation/leds/leds-class.rst
> > +++ b/Documentation/leds/leds-class.rst
> > @@ -169,6 +169,28 @@ Setting the brightness to zero with brightness_set() callback function
> >   should completely turn off the LED and cancel the previously programmed
> >   hardware blinking function, if any.
> > +Hardware offloading of LED triggers
> > +===================================
> > +
> > +Some LEDs can offload SW triggers to hardware (for example a LED connected to
> 
> Better to s/SW/software/ and s/HW/hardware/ throughout the documentation file
> and Kconfig file(s).
> 
> > +an ethernet PHY or an ethernet switch can be configured to blink on activity on
> > +the network, which in software is done by the netdev trigger).
> > +
> > +To do such offloading, LED driver must support the this and a deficated offload
> 
>                                            drop:  the            dedicated
> 
> > +trigger must be used. The LED must implement the trigger_offload() method and
> 
> How does an LED implement the trigger_offload() method?
> They don't have very much logic in them AFAIK.
>

With trigger_offload here I meand activating that mode. So the function
will just do the required operation to set the LED in that specific
mode. Nothing else.

> > +the trigger code must try to call this method (via led_trigger_offload()
> > +function) when configuration of the trigger (trigger_data) changes.
> > +
> > +The implementation of the trigger_offload() method by the LED driver must return
> > +0 if the offload is successful and -EOPNOTSUPP if the requested trigger
> > +configuration is not supported and the trigger should be executed in software.
> > +If trigger_offload() returns negative value, the triggering will be done in
> > +software, so any active offloading must also be disabled.
> > +
> > +If the second argument (enable) to the trigger_offload() method is false, any
> > +active HW offloading must be deactivated. In this case errors are not permitted
> > +in the trigger_offload() method.
> 
> 
> > diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> > index dc6816d36d06..c073e64e0a37 100644
> > --- a/drivers/leds/trigger/Kconfig
> > +++ b/drivers/leds/trigger/Kconfig
> > @@ -9,6 +9,16 @@ menuconfig LEDS_TRIGGERS
> >   if LEDS_TRIGGERS
> > +config LEDS_OFFLOAD_TRIGGERS
> > +	bool "LED Offload Trigger support"
> > +	help
> > +	  This option enabled offload triggers support used by leds that
> 
> 	                                                       LEDs
> 
> > +	  can be driven in HW by declaring some specific triggers.
> > +	  A offload trigger will expose a sysfs dir to configure the
> > +	  different blinking trigger and the available hw trigger.
> 
> Are the sysfs file values/meanings documented here?
> I seem to have missed them.
> 

The trigger should expose them and describe them. The idea is that the
LED driver need to explicitly support the trigger and the LED driver
will comunicate the trigger the supported offload trigger. And only then
the trigger will make them available via sysfs and set them
configurable. So all the sysfs stuff should be put in the trigger
Kconfig help. This is just to add support for the entire offload part.

> > +
> > +	  If unsure, say Y.
> > +
> 
> thanks.
> -- 
> ~Randy

-- 
	Ansuel

Re: [RFC PATCH 4/6] leds: trigger: add offload-phy-activity trigger

[Marek Behún]

On Sun, 7 Nov 2021 23:43:37 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:

> On Sun, Nov 07, 2021 at 11:10:09PM +0100, Marek Behún wrote:
> > On Sun,  7 Nov 2021 18:57:16 +0100
> > Ansuel Smith <ansuelsmth@gmail.com> wrote:
> >   
> > > Add Offload Trigger for PHY Activity. This special trigger is used to
> > > configure and expose the different HW trigger that are provided by the
> > > PHY. Each offload trigger can be configured by sysfs and on trigger
> > > activation the offload mode is enabled.
> > > 
> > > This currently implement these hw triggers:
> > >   - blink_tx: Blink LED on tx packet receive
> > >   - blink_rx: Blink LED on rx packet receive
> > >   - blink_collision: Blink LED on collision detection
> > >   - link_10m: Keep LED on with 10m link speed
> > >   - link_100m: Keep LED on with 100m link speed
> > >   - link_1000m: Keep LED on with 1000m link speed
> > >   - half_duplex: Keep LED on with half duplex link
> > >   - full_duplex: Keep LED on with full duplex link
> > >   - linkup_over: Keep LED on with link speed and blink on rx/tx traffic
> > >   - power_on_reset: Keep LED on with switch reset
> > >   - blink_2hz: Set blink speed at 2hz for every blink event
> > >   - blink_4hz: Set blink speed at 4hz for every blink event
> > >   - blink_8hz: Set blink speed at 8hz for every blink event
> > >   - blink_auto: Set blink speed at 2hz for 10m link speed,
> > >       4hz for 100m and 8hz for 1000m
> > > 
> > > The trigger will read the supported offload trigger in the led cdev and
> > > will expose the offload triggers in sysfs and then activate the offload
> > > mode for the led in offload mode has it configured by default. A flag is
> > > passed to configure_offload with the related rule from this trigger to
> > > active or disable.
> > > It's in the led driver interest the detection and knowing how to
> > > elaborate the passed flags.
> > > 
> > > The different hw triggers are exposed in the led sysfs dir under the
> > > offload-phy-activity subdir.  
> > 
> > NAK. The current plan is to use netdev trigger, and if it can
> > transparently offload the settings to HW, it will.
> > 
> > Yes, netdev trigger currently does not support all these settings.
> > But it supports indicating link and blinking on activity.
> > 
> > So the plan is to start with offloading the blinking on activity, i.e.
> > I the user does
> >   $ cd /sys/class/leds/<LED>
> >   $ echo netdev >trigger
> >   $ echo 1 >rx
> >   $ echo eth0 >device_name
> > 
> > this would, instead of doing blinking in software, do it in HW instead.
> > 
> > After this is implemented, we can start working on extending netdev
> > trigger to support more complicated features.
> > 
> > Marek  
> 
> Using the netdev trigger would cause some problem. Most of the switch
> can run in SW mode (with blink controlled by software of always on) or
> be put in HW mode and they will autonomously control blinking and how
> the LED will operate. So we just need to provide a way to trigger this
> mode and configure it. Why having something that gets triggered and then
> does nothing as it's offloaded?

Nothing gets triggered. In my last proposal, when the netdev trigger
gets activated, it will first try to offload() itself to the LED
controller. If the controller supports offloading, then netdev trigger
won't be blinking the LEDs.

See my last proposal at
https://lore.kernel.org/linux-leds/20210601005155.27997-1-kabel@kernel.org/

> The current way to configure this is very similar... Set the offload
> trigger and use the deidcated subdir to set how the led will
> blink/behave based on the supported trigger reported by the driver.
> 
> There is no reason to set a device_name as that would be hardcoded to
> the phy (and it should not change... again in HW we can't control that
> part, we can just tell the switch to blink on packet tx on that port)
> 
> So really the command is:
>   $ cd /sys/class/leds/<LED>
>   $ echo netdev > offload-phy-activity
>   $ cd offload-phy-activity
>   $ echo 1 > tx-blink
> 
> And the PHY will blink on tx packet.

So there are now 2 different ways to set a LED to blink on tx activity:
the first one is via standard netdev trigger, the second one is this.
That is wrong.

> I understand this should be an extension of netdev as they would do
> similar task but honestly polluting the netdev trigger of if and else to
> disable part of it if an offload mode can be supported seems bad and
> confusionary. At this point introduce a dedicated trigger so an user can
> switch between them. That way we can keep the flexibility of netdev
> trigger that will always work but also permit to support the HW mode
> with no load on the system.

The rationale behind offloading netdev trigger is that it can use
standard, existing sysfs ABI. If the driver finds out that it can
offload the blinking to HW, it will.

This is similar to how other kernel APIs do this.

For example for an ethernet switch, we have DSA. So that we don't need
a specific way to say to the switch which ports it should bridge. All
ports are exported by the switch driver as standard network devices,
and if we bridge them via standard API, the kernel, upon finding out
that the bridging can be offloading to the switch chip, does that.

Marek

Re: [RFC PATCH 2/6] leds: permit to declare supported offload triggers

[Andrew Lunn]

On Sun, Nov 07, 2021 at 11:32:56PM +0100, Ansuel Smith wrote:
> On Sun, Nov 07, 2021 at 11:06:24PM +0100, Marek Behún wrote:
> > On Sun,  7 Nov 2021 18:57:14 +0100
> > Ansuel Smith <ansuelsmth@gmail.com> wrote:
> > 
> > > With LEDs that can be offload driven, permit to declare supported triggers
> > > in the dts and add them to the cled struct to be used by the related
> > > offload trigger. This is particurally useful for phy that have support
> > > for HW blinking on tx/rx traffic or based on the speed link.
> > > 
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > 
> > NAK. The device-tree shouldn't define this, only the LED's function as
> > designated by the manufacturer of the device.
> > 
> > Marek
> 
> Sure I will add a way to ask the led driver if the trigger is supported
> and report it.

Yes, you need some way for the PHY/MAC driver to enumerate what it can
do.

I've not looked at v2 yet...

	Andrew