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

[RFC PATCH v4 0/8] 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 3 API (hw_control_status/start/stop).
  They are used to put the LED in hardware mode and to configure the
  various trigger.
- We have hardware triggers that are used to expose to userspace the
  supported hardware mode and set the hardware mode on trigger
  activation.
- We can also have triggers that both support hardware and software mode.
- The LED driver will declare each supported hardware blink mode and
  communicate with the trigger all the supported blink modes that will
  be available by sysfs.
- A trigger will use blink_set to configure the blink mode to active
  in hardware mode.
- On hardware trigger activation, only the hardware mode is enabled but
  the blink modes are not configured. The LED driver should reset any
  link mode active by default.

Each LED driver will have to declare explicit support for the offload
trigger (or return not supported error code) as we the trigger_data 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.

I also posted the netdev trigger expanded with the hardware support.

More polish is required but this is just to understand if I'm taking
the correct path with this implementation hoping we find a correct
implementation and we start working on the ""small details""

v4:
- Rework implementation and drop hw_configure logic.
  We now expand blink_set.
- Address even more spelling mistake. (thx a lot Randy)
- Drop blink option and use blink_set delay.
- Rework phy-activity trigger to actually make the groups dynamic.
v3:
- Rework start/stop as Andrew asked.
- Introduce more logic to permit a trigger to run in hardware mode.
- Add additional patch with netdev hardware support.
- Use test_bit API to check flag passed to hw_control_configure.
- Added a new cmd to hw_control_configure to reset any active blink_mode.
- Refactor all the patches to follow this new implementation.
v2:
- Fix spelling mistake (sorry)
- Drop patch 02 "permit to declare supported offload triggers".
  Change the logic, now the LED driver declare support for them
  using the configure_offload with the cmd TRIGGER_SUPPORTED.
- Rework code to follow this new implementation.
- Update Documentation to better describe how this offload
  implementation work.

Ansuel Smith (8):
  leds: add support for hardware driven LEDs
  leds: document additional use of blink_set for hardware control
  leds: trigger: netdev: drop NETDEV_LED_MODE_LINKUP from mode
  leds: trigger: netdev: rename and expose NETDEV trigger enum and
    struct
  leds: trigger: netdev: add hardware control support
  leds: trigger: add hardware-phy-activity trigger
  net: dsa: qca8k: add LEDs support
  dt-bindings: net: dsa: qca8k: add LEDs definition example

 .../devicetree/bindings/net/dsa/qca8k.yaml    |  20 +
 Documentation/leds/leds-class.rst             |  49 ++
 drivers/leds/Kconfig                          |  11 +
 drivers/leds/led-class.c                      |  40 ++
 drivers/leds/led-triggers.c                   |  22 +
 drivers/leds/trigger/Kconfig                  |  28 ++
 drivers/leds/trigger/Makefile                 |   1 +
 .../trigger/ledtrig-hardware-phy-activity.c   | 180 ++++++++
 drivers/leds/trigger/ledtrig-netdev.c         |  92 ++--
 drivers/net/dsa/Kconfig                       |   9 +
 drivers/net/dsa/Makefile                      |   1 +
 drivers/net/dsa/qca8k-leds.c                  | 423 ++++++++++++++++++
 drivers/net/dsa/qca8k.c                       |   8 +-
 drivers/net/dsa/qca8k.h                       |  65 +++
 include/linux/leds.h                          |  97 +++-
 15 files changed, 999 insertions(+), 47 deletions(-)
 create mode 100644 drivers/leds/trigger/ledtrig-hardware-phy-activity.c
 create mode 100644 drivers/net/dsa/qca8k-leds.c

-- 
2.32.0

[RFC PATCH v4 1/8] leds: add support for hardware driven LEDs

[Ansuel Smith]

Some LEDs can be driven by 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 this and a supported
trigger must be used.

LED driver should declare the correct control mode supported and should
set the LED_SOFTWARE_CONTROLLED or LED_HARDWARE_CONTROLLED bit in the
flags parameter.
The trigger will check these bit and fail to activate if the control mode
is not supported. By default if a LED driver doesn't declare a control
mode, bit LED_SOFTWARE_CONTROLLED is assumed and set by default.

The LED must implement 3 main API:
- hw_control_status(): This asks the LED driver if hardware mode is
    enabled or not.
- hw_control_start(): This will simply enable the hardware mode for the
    LED and the LED driver should reset any active blink_mode.
- hw_control_stop(): This will simply disable the hardware mode for the
    LED. It's advised for the driver to put the LED in the old state but
    this is not enforced and putting the LED off is also accepted.

If LED_HARDWARE_CONTROLLED bit is the only contro mode set
(LED_SOFTWARE_CONTROLLED not set) set hw_control_status/start/stop is
optional as the LED supports only hardware mode and any software only
trigger will reject activation.

On init a LED driver that support a hardware mode should reset every
blink mode set by default.

An additional config CONFIG_LEDS_HARDWARE_CONTROL is added to add support
for LEDs that can be controlled by hardware.

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

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index cd155ead8703..0175954717a3 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -169,6 +169,38 @@ 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 driven LEDs
+===================================
+
+Some LEDs can be driven by 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 this and a supported trigger must
+be used.
+
+LED driver should declare the correct control mode supported and should set
+the LED_SOFTWARE_CONTROLLED or LED_HARDWARE_CONTROLLED bit in the flags
+parameter.
+The trigger will check these bit and fail to activate if the control mode
+is not supported. By default if a LED driver doesn't declare a control mode,
+bit LED_SOFTWARE_CONTROLLED is assumed and set by default.
+
+The LED must implement 3 main API:
+- hw_control_status(): This asks the LED driver if hardware mode is enabled
+    or not.
+- hw_control_start(): This will simply enable the hardware mode for the LED
+    and the LED driver should reset any active blink_mode.
+- hw_control_stop(): This will simply disable the hardware mode for the LED.
+    It's advised to the driver to put the LED in the old state but this is not
+    enforcerd and putting the LED off is also accepted.
+
+If LED_HARDWARE_CONTROLLED bit is the only contro mode set (LED_SOFTWARE_CONTROLLED
+not set) set hw_control_status/start/stop is optional as the LED supports only
+hardware mode and any software only trigger will reject activation.
+
+On init a LED driver that support a hardware mode should reset every blink mode
+set by default.
 
 Known Issues
 ============
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index ed800f5da7d8..bd2b19cc77ec 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -49,6 +49,17 @@ config LEDS_BRIGHTNESS_HW_CHANGED
 
 	  See Documentation/ABI/testing/sysfs-class-led for details.
 
+config LEDS_HARDWARE_CONTROL
+	bool "LED Hardware Control support"
+	help
+	  This option enabled Hardware control support used by leds that
+	  can be driven in hardware by using supported triggers.
+
+	  Hardware blink modes will be exposed by sysfs class in
+	  /sys/class/leds based on the trigger currently active.
+
+	  If unsure, say Y.
+
 comment "LED drivers"
 
 config LEDS_88PM860X
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index f4bb02f6e042..98a4dc889344 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -164,6 +164,26 @@ static void led_remove_brightness_hw_changed(struct led_classdev *led_cdev)
 }
 #endif
 
+#ifdef CONFIG_LEDS_HARDWARE_CONTROL
+static int led_classdev_check_hw_control_functions(struct led_classdev *led_cdev)
+{
+	if ((LED_SOFTWARE_CONTROLLED & led_cdev->flags) &&
+	    (LED_HARDWARE_CONTROLLED & led_cdev->flags) &&
+	    (!led_cdev->hw_control_status ||
+	    !led_cdev->hw_control_start ||
+	    !led_cdev->hw_control_stop))
+		return -EINVAL;
+
+	if ((LED_SOFTWARE_CONTROLLED & led_cdev->flags) &&
+	    (led_cdev->hw_control_status ||
+	    led_cdev->hw_control_start ||
+	    led_cdev->hw_control_stop))
+		return -EINVAL;
+
+	return 0;
+}
+#endif
+
 /**
  * led_classdev_suspend - suspend an led_classdev.
  * @led_cdev: the led_classdev to suspend.
@@ -367,6 +387,19 @@ int led_classdev_register_ext(struct device *parent,
 	if (ret < 0)
 		return ret;
 
+	/* Make sure a control mode is set.
+	 * With no control mode declared, set SOFTWARE_CONTROLLED by default.
+	 */
+	if (!(LED_SOFTWARE_CONTROLLED & led_cdev->flags) &&
+	    !(LED_HARDWARE_CONTROLLED & led_cdev->flags))
+		led_cdev->flags |= LED_SOFTWARE_CONTROLLED;
+
+#ifdef CONFIG_LEDS_HARDWARE_CONTROL
+	ret = led_classdev_check_hw_control_functions(led_cdev);
+	if (ret < 0)
+		return ret;
+#endif
+
 	mutex_init(&led_cdev->led_access);
 	mutex_lock(&led_cdev->led_access);
 	led_cdev->dev = device_create_with_groups(leds_class, parent, 0,
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 072491d3e17b..69dff5a29bea 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -154,6 +154,20 @@ ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
 }
 EXPORT_SYMBOL_GPL(led_trigger_read);
 
+static bool led_trigger_is_supported(struct led_classdev *led_cdev,
+				     struct led_trigger *trigger)
+{
+	if (trigger->supported_blink_modes == SOFTWARE_ONLY &&
+	    !(LED_SOFTWARE_CONTROLLED & led_cdev->flags))
+		return 0;
+
+	if (trigger->supported_blink_modes == HARDWARE_ONLY &&
+	    !(LED_HARDWARE_CONTROLLED & led_cdev->flags))
+		return 0;
+
+	return 1;
+}
+
 /* Caller must ensure led_cdev->trigger_lock held */
 int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 {
@@ -179,6 +193,10 @@ 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);
+		/* Disable hardware mode on trigger change if supported */
+		if ((led_cdev->flags & LED_HARDWARE_CONTROLLED) &&
+		    led_cdev->hw_control_status(led_cdev))
+			led_cdev->hw_control_stop(led_cdev);
 		if (led_cdev->trigger->deactivate)
 			led_cdev->trigger->deactivate(led_cdev);
 		device_remove_groups(led_cdev->dev, led_cdev->trigger->groups);
@@ -188,6 +206,10 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
 		led_set_brightness(led_cdev, LED_OFF);
 	}
 	if (trig) {
+		/* Make sure the trigger support the LED blink mode */
+		if (!led_trigger_is_supported(led_cdev, trig))
+			return -EINVAL;
+
 		spin_lock(&trig->leddev_list_lock);
 		list_add_tail_rcu(&led_cdev->trig_list, &trig->led_cdevs);
 		spin_unlock(&trig->leddev_list_lock);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index ba4861ec73d3..d64e5768e7ab 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -85,6 +85,11 @@ struct led_classdev {
 #define LED_BRIGHT_HW_CHANGED	BIT(21)
 #define LED_RETAIN_AT_SHUTDOWN	BIT(22)
 #define LED_INIT_DEFAULT_TRIGGER BIT(23)
+/* LED can be controlled by software. This is set by default
+ * if the LED driver doesn't report SOFTWARE or HARDWARE
+ */
+#define LED_SOFTWARE_CONTROLLED	BIT(24)
+#define LED_HARDWARE_CONTROLLED	BIT(25)
 
 	/* set_brightness_work / blink_timer flags, atomic, private. */
 	unsigned long		work_flags;
@@ -154,6 +159,20 @@ struct led_classdev {
 
 	/* LEDs that have private triggers have this set */
 	struct led_hw_trigger_type	*trigger_type;
+
+#ifdef CONFIG_LEDS_HARDWARE_CONTROL
+	/* Ask the LED driver if hardware mode is enabled or not.
+	 * Triggers will check if the hardware mode is active or not.
+	 */
+	bool			(*hw_control_status)(struct led_classdev *led_cdev);
+	/* Set LED in hardware mode and reset any blink mode active by default.
+	 */
+	int			(*hw_control_start)(struct led_classdev *led_cdev);
+	/* Disable hardware mode for LED. It's advised to the LED driver to put it to
+	 * the old status but that is not mandatory and also putting it off is accepted.
+	 */
+	int			(*hw_control_stop)(struct led_classdev *led_cdev);
+#endif
 #endif
 
 #ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED
@@ -215,7 +234,6 @@ extern struct led_classdev *of_led_get(struct device_node *np, int index);
 extern void led_put(struct led_classdev *led_cdev);
 struct led_classdev *__must_check devm_of_led_get(struct device *dev,
 						  int index);
-
 /**
  * led_blink_set - set blinking with software fallback
  * @led_cdev: the LED to start blinking
@@ -350,12 +368,26 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
 
 #define TRIG_NAME_MAX 50
 
+enum led_trigger_blink_supported_modes {
+	SOFTWARE_ONLY = BIT(0),
+	HARDWARE_ONLY = BIT(1),
+	SOFTWARE_HARDWARE = BIT(2),
+};
+
 struct led_trigger {
 	/* Trigger Properties */
 	const char	 *name;
 	int		(*activate)(struct led_classdev *led_cdev);
 	void		(*deactivate)(struct led_classdev *led_cdev);
 
+	/* Declare if the Trigger supports hardware control to
+	 * offload triggers or supports only software control.
+	 * A trigger can also declare support for hardware control
+	 * if is task is only configure LED blink modes and expose
+	 * them in sysfs.
+	 */
+	enum led_trigger_blink_supported_modes supported_blink_modes;
+
 	/* LED-private triggers have this set */
 	struct led_hw_trigger_type *trigger_type;
 
-- 
2.32.0

[RFC PATCH v4 2/8] leds: document additional use of blink_set for hardware control

[Ansuel Smith]

blink_set can now be used also to configure blink modes when hardware
control is supported and active.
Trigger will try to use blink_set and a LED driver will use the
trigger_data to configure the blink_modes once blink_set is called.
Setting a brightness to LED_OFF will reset any blink mode and disable
hardware control setting the LED off.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 Documentation/leds/leds-class.rst | 17 +++++++++++++++++
 drivers/leds/led-class.c          |  7 +++++++
 include/linux/leds.h              |  9 +++++++++
 3 files changed, 33 insertions(+)

diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
index 0175954717a3..c06a18b811de 100644
--- a/Documentation/leds/leds-class.rst
+++ b/Documentation/leds/leds-class.rst
@@ -202,6 +202,23 @@ hardware mode and any software only trigger will reject activation.
 On init a LED driver that support a hardware mode should reset every blink mode
 set by default.
 
+Once a trigger has declared support for hardware-controller blinks, it will use
+blink_set() to try to offload his trigger on activation/configuration.
+blink_set() will return 0 if the requested modes set in trigger_data can be
+controlled by hardware or an error if both the mode bitmap is not supported by
+the hardware or there was a problem in the configuration.
+
+Following blink_set logic, setting brightness to LED_OFF with hardware control active
+will reset any active blink mode and disable hardware control setting the LED to off.
+
+It's in the LED driver's interest to know how to elaborate the trigger data and report support
+for a particular set of blink modes. For this exact reason explicit support for the specific
+trigger is mandatory or the driver returns -EOPNOTSUPP if asked to enter hardware mode
+with a not supported trigger.
+If the driver returns -EOPNOTSUPP on hw_control_configure(), the trigger activation will
+fail as the driver doesn't support that specific hardware blink modes or doesn't know
+how to handle the provided trigger data.
+
 Known Issues
 ============
 
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 98a4dc889344..de39e76a7c6e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -180,6 +180,13 @@ static int led_classdev_check_hw_control_functions(struct led_classdev *led_cdev
 	    led_cdev->hw_control_stop))
 		return -EINVAL;
 
+	/* blink_set is mandatory to configure the blink modes
+	 * in hardware control.
+	 */
+	if ((LED_HARDWARE_CONTROLLED & led_cdev->flags) &&
+	    !led_cdev->blink_set)
+		return -EINVAL;
+
 	return 0;
 }
 #endif
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d64e5768e7ab..81d50269a446 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -76,6 +76,7 @@ struct led_classdev {
 	/* Lower 16 bits reflect status */
 #define LED_SUSPENDED		BIT(0)
 #define LED_UNREGISTERING	BIT(1)
+#define LED_HARDWARE_CONTROL	BIT(2)
 	/* Upper 16 bits reflect control information */
 #define LED_CORE_SUSPENDRESUME	BIT(16)
 #define LED_SYSFS_DISABLE	BIT(17)
@@ -123,6 +124,12 @@ struct led_classdev {
 	 * match the values specified exactly.
 	 * Deactivate blinking again when the brightness is set to LED_OFF
 	 * via the brightness_set() callback.
+	 * With LED_HARDWARE_CONTROL set in LED flags blink_set will also
+	 * configure blink modes requested by the current trigger if
+	 * supported by the LED driver.
+	 * Setting brightness to LED_OFF with hardware control active will
+	 * reset any active blink mode and disable hardware control setting
+	 * the LED off.
 	 */
 	int		(*blink_set)(struct led_classdev *led_cdev,
 				     unsigned long *delay_on,
@@ -166,6 +173,8 @@ struct led_classdev {
 	 */
 	bool			(*hw_control_status)(struct led_classdev *led_cdev);
 	/* Set LED in hardware mode and reset any blink mode active by default.
+	 * A trigger supporting hardware mode will have to use blink_set to configure
+	 * the modes.
 	 */
 	int			(*hw_control_start)(struct led_classdev *led_cdev);
 	/* Disable hardware mode for LED. It's advised to the LED driver to put it to
-- 
2.32.0

[RFC PATCH v4 3/8] leds: trigger: netdev: drop NETDEV_LED_MODE_LINKUP from mode

[Ansuel Smith]

NETDEV_LED_MODE_LINKUP is not strictly a blink mode but really a status
of the current situation of the carrier. It seems wrong to accomunate a
link status to the blink mode list and would cause some confusion as
someone would think that MODE_LINKUP is a separate mode that provide the
netdev trigger with the other 3 LINK, RX and TX.

Drop NETDEV_LED_MODE_LINKUP from mode list and convert to a simple bool
that will be true or false based on the carrier link. No functional
change intended.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/leds/trigger/ledtrig-netdev.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index d5e774d83021..66a81cc9b64d 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -50,10 +50,10 @@ struct led_netdev_data {
 	unsigned int last_activity;
 
 	unsigned long mode;
+	bool carrier_link_up;
 #define NETDEV_LED_LINK	0
 #define NETDEV_LED_TX	1
 #define NETDEV_LED_RX	2
-#define NETDEV_LED_MODE_LINKUP	3
 };
 
 enum netdev_led_attr {
@@ -73,9 +73,9 @@ static void set_baseline_state(struct led_netdev_data *trigger_data)
 	if (!led_cdev->blink_brightness)
 		led_cdev->blink_brightness = led_cdev->max_brightness;
 
-	if (!test_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode))
+	if (!trigger_data->carrier_link_up) {
 		led_set_brightness(led_cdev, LED_OFF);
-	else {
+	} else {
 		if (test_bit(NETDEV_LED_LINK, &trigger_data->mode))
 			led_set_brightness(led_cdev,
 					   led_cdev->blink_brightness);
@@ -131,10 +131,9 @@ static ssize_t device_name_store(struct device *dev,
 		trigger_data->net_dev =
 		    dev_get_by_name(&init_net, trigger_data->device_name);
 
-	clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode);
+	trigger_data->carrier_link_up = false;
 	if (trigger_data->net_dev != NULL)
-		if (netif_carrier_ok(trigger_data->net_dev))
-			set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode);
+		trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev);
 
 	trigger_data->last_activity = 0;
 
@@ -315,7 +314,7 @@ static int netdev_trig_notify(struct notifier_block *nb,
 
 	spin_lock_bh(&trigger_data->lock);
 
-	clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode);
+	trigger_data->carrier_link_up = false;
 	switch (evt) {
 	case NETDEV_CHANGENAME:
 	case NETDEV_REGISTER:
@@ -330,8 +329,7 @@ static int netdev_trig_notify(struct notifier_block *nb,
 		break;
 	case NETDEV_UP:
 	case NETDEV_CHANGE:
-		if (netif_carrier_ok(dev))
-			set_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode);
+		trigger_data->carrier_link_up = netif_carrier_ok(dev);
 		break;
 	}
 
-- 
2.32.0

[RFC PATCH v4 4/8] leds: trigger: netdev: rename and expose NETDEV trigger enum and struct

[Ansuel Smith]

A LED driver, to support hardware control, requires to support a specific
trigger and requires to elaborate his trigger_data struct.
Move netdev trigger data struct to leds.h to make it accessible by any
LED driver that wants to add support for hardware control for the
specific netdev trigger.
Also rename NETDEV trigger enum modes to a more symbolic and descriptive
name.

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

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 66a81cc9b64d..01e4544fa7b0 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -36,26 +36,6 @@
  *
  */
 
-struct led_netdev_data {
-	spinlock_t lock;
-
-	struct delayed_work work;
-	struct notifier_block notifier;
-
-	struct led_classdev *led_cdev;
-	struct net_device *net_dev;
-
-	char device_name[IFNAMSIZ];
-	atomic_t interval;
-	unsigned int last_activity;
-
-	unsigned long mode;
-	bool carrier_link_up;
-#define NETDEV_LED_LINK	0
-#define NETDEV_LED_TX	1
-#define NETDEV_LED_RX	2
-};
-
 enum netdev_led_attr {
 	NETDEV_ATTR_LINK,
 	NETDEV_ATTR_TX,
@@ -76,7 +56,7 @@ static void set_baseline_state(struct led_netdev_data *trigger_data)
 	if (!trigger_data->carrier_link_up) {
 		led_set_brightness(led_cdev, LED_OFF);
 	} else {
-		if (test_bit(NETDEV_LED_LINK, &trigger_data->mode))
+		if (test_bit(TRIGGER_NETDEV_LINK, &trigger_data->mode))
 			led_set_brightness(led_cdev,
 					   led_cdev->blink_brightness);
 		else
@@ -85,8 +65,8 @@ static void set_baseline_state(struct led_netdev_data *trigger_data)
 		/* If we are looking for RX/TX start periodically
 		 * checking stats
 		 */
-		if (test_bit(NETDEV_LED_TX, &trigger_data->mode) ||
-		    test_bit(NETDEV_LED_RX, &trigger_data->mode))
+		if (test_bit(TRIGGER_NETDEV_TX, &trigger_data->mode) ||
+		    test_bit(TRIGGER_NETDEV_RX, &trigger_data->mode))
 			schedule_delayed_work(&trigger_data->work, 0);
 	}
 }
@@ -153,13 +133,13 @@ static ssize_t netdev_led_attr_show(struct device *dev, char *buf,
 
 	switch (attr) {
 	case NETDEV_ATTR_LINK:
-		bit = NETDEV_LED_LINK;
+		bit = TRIGGER_NETDEV_LINK;
 		break;
 	case NETDEV_ATTR_TX:
-		bit = NETDEV_LED_TX;
+		bit = TRIGGER_NETDEV_TX;
 		break;
 	case NETDEV_ATTR_RX:
-		bit = NETDEV_LED_RX;
+		bit = TRIGGER_NETDEV_RX;
 		break;
 	default:
 		return -EINVAL;
@@ -182,13 +162,13 @@ static ssize_t netdev_led_attr_store(struct device *dev, const char *buf,
 
 	switch (attr) {
 	case NETDEV_ATTR_LINK:
-		bit = NETDEV_LED_LINK;
+		bit = TRIGGER_NETDEV_LINK;
 		break;
 	case NETDEV_ATTR_TX:
-		bit = NETDEV_LED_TX;
+		bit = TRIGGER_NETDEV_TX;
 		break;
 	case NETDEV_ATTR_RX:
-		bit = NETDEV_LED_RX;
+		bit = TRIGGER_NETDEV_RX;
 		break;
 	default:
 		return -EINVAL;
@@ -358,21 +338,21 @@ static void netdev_trig_work(struct work_struct *work)
 	}
 
 	/* If we are not looking for RX/TX then return  */
-	if (!test_bit(NETDEV_LED_TX, &trigger_data->mode) &&
-	    !test_bit(NETDEV_LED_RX, &trigger_data->mode))
+	if (!test_bit(TRIGGER_NETDEV_TX, &trigger_data->mode) &&
+	    !test_bit(TRIGGER_NETDEV_RX, &trigger_data->mode))
 		return;
 
 	dev_stats = dev_get_stats(trigger_data->net_dev, &temp);
 	new_activity =
-	    (test_bit(NETDEV_LED_TX, &trigger_data->mode) ?
+	    (test_bit(TRIGGER_NETDEV_TX, &trigger_data->mode) ?
 		dev_stats->tx_packets : 0) +
-	    (test_bit(NETDEV_LED_RX, &trigger_data->mode) ?
+	    (test_bit(TRIGGER_NETDEV_RX, &trigger_data->mode) ?
 		dev_stats->rx_packets : 0);
 
 	if (trigger_data->last_activity != new_activity) {
 		led_stop_software_blink(trigger_data->led_cdev);
 
-		invert = test_bit(NETDEV_LED_LINK, &trigger_data->mode);
+		invert = test_bit(TRIGGER_NETDEV_LINK, &trigger_data->mode);
 		interval = jiffies_to_msecs(
 				atomic_read(&trigger_data->interval));
 		/* base state is ON (link present) */
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 81d50269a446..bc3c54eb269a 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -505,6 +505,33 @@ static inline void *led_get_trigger_data(struct led_classdev *led_cdev)
 
 #endif /* CONFIG_LEDS_TRIGGERS */
 
+/* Trigger specific trigger_data used by netdev trigger */
+#ifdef CONFIG_LEDS_TRIGGER_NETDEV
+struct led_netdev_data {
+	spinlock_t lock;
+
+	struct delayed_work work;
+	struct notifier_block notifier;
+
+	struct led_classdev *led_cdev;
+	struct net_device *net_dev;
+
+	char device_name[IFNAMSIZ];
+	atomic_t interval;
+	unsigned int last_activity;
+
+	unsigned long mode;
+	bool carrier_link_up;
+};
+
+/* Trigger specific bitmap blink mode used by netdev trigger */
+enum led_trigger_netdev_modes {
+	TRIGGER_NETDEV_LINK,
+	TRIGGER_NETDEV_TX,
+	TRIGGER_NETDEV_RX,
+};
+#endif
+
 /* Trigger specific functions */
 #ifdef CONFIG_LEDS_TRIGGER_DISK
 void ledtrig_disk_activity(bool write);
-- 
2.32.0

[RFC PATCH v4 5/8] leds: trigger: netdev: add hardware control support

[Ansuel Smith]

Add hardware control support for the Netdev trigger.
The trigger on config change will check if the requested trigger can set
to blink mode using LED hardware mode and if every blink mode is supported,
the trigger will enable hardware mode with the requested configuration.
If there is at least one trigger that is not supported and can't run in
hardware mode, then software mode will be used instead.

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

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 01e4544fa7b0..74c9a6ecfbbf 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -44,9 +44,31 @@ enum netdev_led_attr {
 
 static void set_baseline_state(struct led_netdev_data *trigger_data)
 {
-	int current_brightness;
+	int current_brightness, can_offload;
 	struct led_classdev *led_cdev = trigger_data->led_cdev;
 
+	if (LED_HARDWARE_CONTROLLED & led_cdev->flags) {
+		/* Check if blink mode can he set in hardware mode.
+		 * The LED driver will chose a interval based on the trigger_data
+		 * and its implementation.
+		 */
+		can_offload = led_cdev->blink_set(led_cdev, 0, 0);
+
+		/* If blink_set doesn't return error we can run in hardware mode
+		 * So actually activate it.
+		 */
+		if (!can_offload) {
+			led_cdev->hw_control_start(led_cdev);
+			return;
+		}
+	}
+
+	/* If LED supports only hardware mode and we reach this point,
+	 * then skip any software handling.
+	 */
+	if (!(LED_SOFTWARE_CONTROLLED & led_cdev->flags))
+		return;
+
 	current_brightness = led_cdev->brightness;
 	if (current_brightness)
 		led_cdev->blink_brightness = current_brightness;
@@ -395,8 +417,11 @@ static int netdev_trig_activate(struct led_classdev *led_cdev)
 
 	rc = register_netdevice_notifier(&trigger_data->notifier);
 	if (rc)
-		kfree(trigger_data);
+		goto err;
 
+	return 0;
+err:
+	kfree(trigger_data);
 	return rc;
 }
 
@@ -416,6 +441,7 @@ static void netdev_trig_deactivate(struct led_classdev *led_cdev)
 
 static struct led_trigger netdev_led_trigger = {
 	.name = "netdev",
+	.supported_blink_modes = SOFTWARE_HARDWARE,
 	.activate = netdev_trig_activate,
 	.deactivate = netdev_trig_deactivate,
 	.groups = netdev_trig_groups,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bc3c54eb269a..dd41acd564a3 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -17,6 +17,9 @@
 #include <linux/spinlock.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
+#ifdef CONFIG_LEDS_TRIGGER_NETDEV
+#include <linux/netdevice.h>
+#endif
 
 struct device;
 struct led_pattern;
-- 
2.32.0

[RFC PATCH v4 6/8] leds: trigger: add hardware-phy-activity trigger

[Ansuel Smith]

Add Hardware Only Trigger for PHY Activity. This special trigger is used to
configure and expose the different HW trigger that are provided by the
PHY. Each blink mode can be configured by sysfs and on trigger
activation the hardware 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
  - 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
  - option_linkup_over: Ignore blink tx/rx with link keep not active
  - option_power_on_reset: Keep LED on with switch reset

The trigger will check if the LED driver support the various blink modes
and will expose the blink modes in sysfs.
It will finally enable hw mode for the LED without configuring any rule.
It's in the led driver interest the detection and knowing how to
elaborate the passed flags and should report -EOPNOTSUPP otherwise.

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

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

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index dc6816d36d06..737a8be533a3 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -154,4 +154,32 @@ config LEDS_TRIGGER_TTY
 
 	  When build as a module this driver will be called ledtrig-tty.
 
+config LEDS_TRIGGER_HARDWARE_PHY_ACTIVITY
+	tristate "LED Trigger for PHY Activity for Hardware Controlled LED"
+	depends on LEDS_HARDWARE_CONTROL
+	help
+	  This allows LEDs to be configured to run by hardware and offloaded
+	  based on some rules. The LED will blink or be on based on the PHY
+	  activity for example on packet receive or based on the link speed.
+
+	  The current supported offload triggers are:
+	  - blink_tx: Blink LED on tx packet receive
+	  - blink_rx: Blink LED on rx packet receive
+	  - keep_link_10m: Keep LED on with 10m link speed
+	  - keep_link_100m: Keep LED on with 100m link speed
+	  - keep_link_1000m: Keep LED on with 1000m link speed
+	  - keep_half_duplex: Keep LED on with half duplex link
+	  - keep_full_duplex: Keep LED on with full duplex link
+	  - option_linkup_over: Blink rules are ignored with absent link
+	  - option_power_on_reset: Power ON Led on Switch/PHY reset
+	  - option_blink_2hz: Set blink speed at 2hz for every blink event
+	  - option_blink_4hz: Set blink speed at 4hz for every blink event
+	  - option_blink_8hz: Set blink speed at 8hz for every blink event
+
+	  These blink modes are present in the LED sysfs dir under
+	  hardware-phy-activity if supported by the LED driver.
+
+	  This trigger can be used only by LEDs that support Hardware mode.
+
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 25c4db97cdd4..f5d0d0057d2b 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_TRIGGER_HARDWARE_PHY_ACTIVITY) += ledtrig-hardware-phy-activity.o
diff --git a/drivers/leds/trigger/ledtrig-hardware-phy-activity.c b/drivers/leds/trigger/ledtrig-hardware-phy-activity.c
new file mode 100644
index 000000000000..4ac51011d029
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-hardware-phy-activity.c
@@ -0,0 +1,180 @@
+// 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_BLINK_MODE 9
+
+static ssize_t blink_mode_common_show(int blink_mode, struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct hardware_phy_activity_data *trigger_data = led_cdev->trigger_data;
+	int val;
+
+	val = test_bit(blink_mode, &trigger_data->mode);
+	return sprintf(buf, "%d\n", val ? 1 : 0);
+}
+
+static ssize_t blink_mode_common_store(int blink_mode, struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct hardware_phy_activity_data *trigger_data = led_cdev->trigger_data;
+	unsigned long state;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	if (!!state)
+		set_bit(blink_mode, &trigger_data->mode);
+	else
+		clear_bit(blink_mode, &trigger_data->mode);
+
+	/* Update the configuration with every change */
+	led_cdev->blink_set(led_cdev, 0, 0);
+	return size;
+}
+
+#define DEFINE_HW_BLINK_MODE(blink_mode_name, blink_bit) \
+	static ssize_t blink_mode_name##_show(struct device *dev, \
+				struct device_attribute *attr, char *buf) \
+	{ \
+		return blink_mode_common_show(blink_bit, dev, attr, buf); \
+	} \
+	static ssize_t blink_mode_name##_store(struct device *dev, \
+					struct device_attribute *attr, \
+					const char *buf, size_t size) \
+	{ \
+		return blink_mode_common_store(blink_bit, dev, attr, buf, size); \
+	} \
+	DEVICE_ATTR_RW(blink_mode_name)
+
+/* Expose sysfs for every blink to be configurable from userspace */
+DEFINE_HW_BLINK_MODE(blink_tx, TRIGGER_PHY_ACTIVITY_BLINK_TX);
+DEFINE_HW_BLINK_MODE(blink_rx, TRIGGER_PHY_ACTIVITY_BLINK_RX);
+DEFINE_HW_BLINK_MODE(link_10M, TRIGGER_PHY_ACTIVITY_LINK_10M);
+DEFINE_HW_BLINK_MODE(link_100M, TRIGGER_PHY_ACTIVITY_LINK_100M);
+DEFINE_HW_BLINK_MODE(link_1000M, TRIGGER_PHY_ACTIVITY_LINK_1000M);
+DEFINE_HW_BLINK_MODE(half_duplex, TRIGGER_PHY_ACTIVITY_HALF_DUPLEX);
+DEFINE_HW_BLINK_MODE(full_duplex, TRIGGER_PHY_ACTIVITY_FULL_DUPLEX);
+DEFINE_HW_BLINK_MODE(option_linkup_over, TRIGGER_PHY_ACTIVITY_OPTION_LINKUP_OVER);
+DEFINE_HW_BLINK_MODE(option_power_on_reset, TRIGGER_PHY_ACTIVITY_OPTION_POWER_ON_RESET);
+
+static struct attribute *blink_mode_tbl[PHY_ACTIVITY_MAX_BLINK_MODE] = {
+	&dev_attr_blink_tx.attr,
+	&dev_attr_blink_rx.attr,
+	&dev_attr_link_10M.attr,
+	&dev_attr_link_100M.attr,
+	&dev_attr_link_1000M.attr,
+	&dev_attr_half_duplex.attr,
+	&dev_attr_full_duplex.attr,
+	&dev_attr_option_linkup_over.attr,
+	&dev_attr_option_power_on_reset.attr,
+};
+
+/* The attrs will be placed dynamically based on the supported triggers */
+// static struct attribute *phy_activity_attrs[PHY_ACTIVITY_MAX_TRIGGERS + 1];
+
+static int hardware_phy_activity_activate(struct led_classdev *led_cdev)
+{
+	struct hardware_phy_activity_data *trigger_data;
+	struct attribute_group *phy_activity_group;
+	struct attribute **phy_activity_attrs;
+	unsigned long supported_mode = 0;
+	int i, j, count = 0, ret;
+
+	trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
+	if (!trigger_data)
+		return -ENOMEM;
+
+	led_set_trigger_data(led_cdev, trigger_data);
+
+	/* Check supported blink modes
+	 * Request one mode at time and check if it can run in hardware mode
+	 */
+	for (i = 0; i < PHY_ACTIVITY_MAX_BLINK_MODE; i++) {
+		trigger_data->mode = 0;
+
+		set_bit(i, &trigger_data->mode);
+
+		if (!led_cdev->blink_set(led_cdev, 0, 0)) {
+			set_bit(i, &supported_mode);
+			count++;
+		}
+	}
+
+	if (!count) {
+		ret = -EINVAL;
+		goto fail_alloc_driver_data;
+	}
+
+	phy_activity_group = kzalloc(sizeof(phy_activity_group), GFP_KERNEL);
+	if (!phy_activity_group) {
+		ret = -ENOMEM;
+		goto fail_alloc_driver_data;
+	}
+
+	phy_activity_attrs = kcalloc(count + 1, sizeof(struct attribute *), GFP_KERNEL);
+	if (!phy_activity_attrs)
+		goto fail_alloc_attrs;
+
+	phy_activity_group->name = "hardware-phy-activity";
+	phy_activity_group->attrs = phy_activity_attrs;
+
+	for (i = 0, j = 0; i < count; i++)
+		if (test_bit(i, &supported_mode))
+			phy_activity_attrs[j++] = blink_mode_tbl[i];
+
+	ret = device_add_group(led_cdev->dev, phy_activity_group);
+	if (ret)
+		goto fail_alloc_group;
+
+	trigger_data->group = phy_activity_group;
+
+	/* Enable hardware mode. No custom configuration is applied,
+	 * the LED driver will use whatever default configuration is
+	 * currently configured.
+	 */
+	ret = led_cdev->hw_control_start(led_cdev);
+	if (ret)
+		goto fail_alloc_group;
+
+	return 0;
+fail_alloc_driver_data:
+	kfree(trigger_data);
+fail_alloc_group:
+	kfree(phy_activity_attrs);
+fail_alloc_attrs:
+	kfree(phy_activity_group);
+	return ret;
+}
+
+static void hardware_phy_activity_deactivate(struct led_classdev *led_cdev)
+{
+	struct hardware_phy_activity_data *trigger_data = led_get_trigger_data(led_cdev);
+
+	led_cdev->hw_control_stop(led_cdev);
+	device_remove_group(led_cdev->dev, trigger_data->group);
+	kfree(trigger_data->group->attrs);
+	kfree(trigger_data->group);
+	kfree(trigger_data);
+}
+
+static struct led_trigger hardware_phy_activity_trigger = {
+	.supported_blink_modes = HARDWARE_ONLY,
+	.name       = "hardware-phy-activity",
+	.activate   = hardware_phy_activity_activate,
+	.deactivate = hardware_phy_activity_deactivate,
+};
+
+static int __init hardware_phy_activity_init(void)
+{
+	return led_trigger_register(&hardware_phy_activity_trigger);
+}
+device_initcall(hardware_phy_activity_init);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index dd41acd564a3..1aeeb943290a 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -535,6 +535,30 @@ enum led_trigger_netdev_modes {
 };
 #endif
 
+#ifdef CONFIG_LEDS_TRIGGER_HARDWARE_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 hardware_phy_activity {
+	TRIGGER_PHY_ACTIVITY_BLINK_TX, /* Blink on TX traffic */
+	TRIGGER_PHY_ACTIVITY_BLINK_RX, /* Blink on RX traffic */
+	TRIGGER_PHY_ACTIVITY_LINK_10M, /* LED on with 10M link */
+	TRIGGER_PHY_ACTIVITY_LINK_100M, /* LED on with 100M link */
+	TRIGGER_PHY_ACTIVITY_LINK_1000M, /* LED on with 1000M link */
+	TRIGGER_PHY_ACTIVITY_HALF_DUPLEX, /* LED on with Half-Duplex link */
+	TRIGGER_PHY_ACTIVITY_FULL_DUPLEX, /* LED on with Full-Duplex link */
+	TRIGGER_PHY_ACTIVITY_OPTION_LINKUP_OVER, /* LED will be on with KEEP blink mode and blink on BLINK traffic */
+	TRIGGER_PHY_ACTIVITY_OPTION_POWER_ON_RESET, /* LED will be on Switch reset */
+};
+
+struct hardware_phy_activity_data {
+	unsigned long mode;
+	struct attribute_group *group;
+};
+#endif
+
 /* Trigger specific functions */
 #ifdef CONFIG_LEDS_TRIGGER_DISK
 void ledtrig_disk_activity(bool write);
-- 
2.32.0

[RFC PATCH v4 7/8] net: dsa: qca8k: add LEDs support

[Ansuel Smith]

Add LEDs support for qca8k Switch Family. This will provide the LEDs
hardware API to permit the PHY LED to support hardware mode.
Each port have at least 3 LEDs and they can HW blink, set on/off or
follow blink modes configured with the LED in hardware mode..
This adds support for 2 hardware trigger netdev and
hardware-phy-activity.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/Kconfig      |   9 +
 drivers/net/dsa/Makefile     |   1 +
 drivers/net/dsa/qca8k-leds.c | 423 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/qca8k.c      |   8 +-
 drivers/net/dsa/qca8k.h      |  65 ++++++
 5 files changed, 504 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..9f4bae5cc43d 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
+	  This enables 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..729e55dedc9d
--- /dev/null
+++ b/drivers/net/dsa/qca8k-leds.c
@@ -0,0 +1,423 @@
+// 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 void
+qca8k_check_rules(unsigned long *mode, u32 *offload_trigger,
+		  int trigger_bit, int blink_mode, bool read)
+{
+	if (read) {
+		if (*offload_trigger & blink_mode)
+			set_bit(trigger_bit, mode);
+	} else {
+		if (test_bit(trigger_bit, mode))
+			*offload_trigger |= blink_mode;
+	}
+}
+
+static int
+qca8k_parse_trigger_hardware_phy_activity(struct led_classdev *ldev, u32 *offload_trigger,
+					  u32 *mask, bool read)
+{
+	struct hardware_phy_activity_data *trigger_data = led_get_trigger_data(ldev);
+	unsigned long rules = trigger_data->mode;
+
+	/* Parse rule to hardware phy activity trigger */
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_BLINK_TX,
+			  QCA8K_LED_TX_BLINK_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_BLINK_RX,
+			  QCA8K_LED_RX_BLINK_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_LINK_10M,
+			  QCA8K_LED_LINK_10M_EN_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_LINK_100M,
+			  QCA8K_LED_LINK_100M_EN_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_LINK_1000M,
+			  QCA8K_LED_LINK_1000M_EN_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_HALF_DUPLEX,
+			  QCA8K_LED_HALF_DUPLEX_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_FULL_DUPLEX,
+			  QCA8K_LED_FULL_DUPLEX_MASK, false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_PHY_ACTIVITY_OPTION_LINKUP_OVER,
+			  QCA8K_LED_LINKUP_OVER_MASK, false);
+
+	if (!*offload_trigger)
+		return -EOPNOTSUPP;
+
+	*mask = *offload_trigger;
+
+	return 0;
+}
+
+static int
+qca8k_parse_trigger_netdev(struct led_classdev *ldev, u32 *offload_trigger,
+			   u32 *mask, bool read)
+{
+	struct led_netdev_data *trigger_data = led_get_trigger_data(ldev);
+	unsigned long rules = trigger_data->mode;
+
+	/* Parse rule to netdev trigger */
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_NETDEV_TX, QCA8K_LED_TX_BLINK_MASK,
+			  false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_NETDEV_RX, QCA8K_LED_RX_BLINK_MASK,
+			  false);
+	qca8k_check_rules(&rules, offload_trigger, TRIGGER_NETDEV_LINK,
+			  QCA8K_LED_LINK_10M_EN_MASK | QCA8K_LED_LINK_100M_EN_MASK |
+			      QCA8K_LED_LINK_1000M_EN_MASK,
+			  false);
+
+	if (!*offload_trigger)
+		return -EOPNOTSUPP;
+
+	*mask = *offload_trigger;
+
+	return 0;
+}
+
+static int
+qca8k_parse_trigger(struct led_classdev *ldev, u32 *offload_trigger, u32 *mask,
+		    bool read)
+{
+	struct led_trigger *trigger = ldev->trigger;
+	int ret;
+
+	if (strcmp(trigger->name, "hardware-phy-activity") &&
+	    strcmp(trigger->name, "netdev"))
+		return -EINVAL;
+
+	if (!strcmp(trigger->name, "hardware-phy-activity"))
+		ret = qca8k_parse_trigger_hardware_phy_activity(ldev, offload_trigger, mask, read);
+
+	if (!strcmp(trigger->name, "netdev"))
+		ret = qca8k_parse_trigger_netdev(ldev, offload_trigger, mask, read);
+
+	return ret;
+}
+
+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;
+	u32 offload_trigger = 0, mask;
+	int ret;
+
+	/* With no trigger selected, use the blink 4hz mode */
+	if (!ldev->trigger) {
+		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);
+
+		return qca8k_rmw(priv, reg_info.reg,
+				 GENMASK(1, 0) << reg_info.shift,
+				 QCA8K_LED_ALWAYS_BLINK_4HZ << reg_info.shift);
+	}
+
+	/* Check trigger compatibility
+	 * With these trigger blink_set will configure the LED pattern reg
+	 */
+	ret = qca8k_parse_trigger(ldev, &offload_trigger, &mask, false);
+	if (ret)
+		return ret;
+
+	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+
+	/* Set 4hz by default */
+	if (*delay_on == 0 && *delay_off == 0)
+		offload_trigger |= QCA8K_LED_BLINK_4HZ;
+
+	if (*delay_on == 250 || *delay_off == 250)
+		offload_trigger |= QCA8K_LED_BLINK_2HZ;
+
+	if (*delay_on == 125 || *delay_off == 125)
+		offload_trigger |= QCA8K_LED_BLINK_4HZ;
+
+	if (*delay_on == 62 || *delay_off == 62)
+		offload_trigger |= QCA8K_LED_BLINK_8HZ;
+
+	return qca8k_rmw(priv, reg_info.reg,
+			 mask << reg_info.shift,
+			 offload_trigger << reg_info.shift);
+}
+
+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_hw_control(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_cled_hw_control_start(struct led_classdev *led_cdev)
+{
+	return qca8k_cled_hw_control(led_cdev, true);
+}
+
+static int
+qca8k_cled_hw_control_stop(struct led_classdev *led_cdev)
+{
+	return qca8k_cled_hw_control(led_cdev, false);
+}
+
+static bool
+qca8k_cled_hw_control_status(struct led_classdev *ldev)
+{
+	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_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	qca8k_read(priv, reg_info.reg, &val);
+
+	val >>= reg_info.shift;
+	val &= GENMASK(1, 0);
+
+	return val == QCA8K_LED_RULE_CONTROLLED;
+}
+
+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 qca8k_led_pattern_en reg_info;
+	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_dbg(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);
+			}
+		}
+
+		qca8k_get_enable_led_reg(port_led->port_num, port_led->led_num, &reg_info);
+
+		/* Reset blink mode set by default */
+		ret = qca8k_rmw(priv, reg_info.reg, QCA8K_LED_RULE_MASK << reg_info.shift,
+				0 << reg_info.shift);
+
+		/* 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.flags |= LED_SOFTWARE_CONTROLLED;
+		port_led->cdev.flags |= LED_HARDWARE_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.hw_control_start = qca8k_cled_hw_control_start;
+		port_led->cdev.hw_control_stop = qca8k_cled_hw_control_stop;
+		port_led->cdev.hw_control_status = qca8k_cled_hw_control_status;
+		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..10b2b47b2156 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;
@@ -1109,6 +1109,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_led_ctrl(priv);
+	if (ret)
+		return ret;
+
 	/* Make sure MAC06 is disabled */
 	ret = qca8k_reg_clear(priv, QCA8K_REG_PORT0_PAD_CTRL,
 			      QCA8K_PORT0_PAD_MAC06_EXCHANGE_EN);
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 128b8cf85e08..f68f037fe909 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,43 @@
 #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_RULE_MASK				GENMASK(13, 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 +317,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 +344,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 +360,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 v4 8/8] 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    | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 48de0ace265d..106d95adc1e8 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,24 @@ 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";
+                        };
+
+                        led@1 {
+                            reg = <1>;
+                            color = <LED_COLOR_ID_AMBER>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            linux,default-trigger = "offload-phy-activity";
+                        };
+                    };
                 };
 
                 internal_phy_port2: ethernet-phy@1 {
-- 
2.32.0

Re: [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers

[Marek Behún]

On Thu, 11 Nov 2021 02:34:52 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:

> 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 3 API (hw_control_status/start/stop).
>   They are used to put the LED in hardware mode and to configure the
>   various trigger.
> - We have hardware triggers that are used to expose to userspace the
>   supported hardware mode and set the hardware mode on trigger
>   activation.
> - We can also have triggers that both support hardware and software mode.
> - The LED driver will declare each supported hardware blink mode and
>   communicate with the trigger all the supported blink modes that will
>   be available by sysfs.
> - A trigger will use blink_set to configure the blink mode to active
>   in hardware mode.
> - On hardware trigger activation, only the hardware mode is enabled but
>   the blink modes are not configured. The LED driver should reset any
>   link mode active by default.
> 
> Each LED driver will have to declare explicit support for the offload
> trigger (or return not supported error code) as we the trigger_data 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.
> 
> I also posted the netdev trigger expanded with the hardware support.
> 
> More polish is required but this is just to understand if I'm taking
> the correct path with this implementation hoping we find a correct
> implementation and we start working on the ""small details""

Hello Ansuel,

besides other things, I am still against the idea of the
`hardware-phy-activity` trigger: I think that if the user wants the LED
to indicate network device's link status or activity, it should always
be done via the existing netdev trigger, and with that trigger only.

Yes, I know that netdev trigger does not currently support indicating
different link modes, only whether the link is up (in any mode). That
should be solved by extending the netdev trigger.

I am going to try to revive my last attempt and send my proposal again.
Hope you don't mind.

Marek

Re: [RFC PATCH v4 6/8] leds: trigger: add hardware-phy-activity trigger

[Randy Dunlap]

On 11/10/21 5:34 PM, Ansuel Smith wrote:
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index dc6816d36d06..737a8be533a3 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -154,4 +154,32 @@ config LEDS_TRIGGER_TTY
>   
>   	  When build as a module this driver will be called ledtrig-tty.
>   
> +config LEDS_TRIGGER_HARDWARE_PHY_ACTIVITY
> +	tristate "LED Trigger for PHY Activity for Hardware Controlled LED"
> +	depends on LEDS_HARDWARE_CONTROL
> +	help
> +	  This allows LEDs to be configured to run by hardware and offloaded

	                                    to be run

> +	  based on some rules. The LED will blink or be on based on the PHY

	                                          or be "on" based on

> +	  activity for example on packet receive or based on the link speed.
> +
> +	  The current supported offload triggers are:
> +	  - blink_tx: Blink LED on tx packet receive
> +	  - blink_rx: Blink LED on rx packet receive
> +	  - keep_link_10m: Keep LED on with 10m link speed
> +	  - keep_link_100m: Keep LED on with 100m link speed
> +	  - keep_link_1000m: Keep LED on with 1000m link speed
> +	  - keep_half_duplex: Keep LED on with half duplex link
> +	  - keep_full_duplex: Keep LED on with full duplex link
> +	  - option_linkup_over: Blink rules are ignored with absent link
> +	  - option_power_on_reset: Power ON Led on Switch/PHY reset
> +	  - option_blink_2hz: Set blink speed at 2hz for every blink event
> +	  - option_blink_4hz: Set blink speed at 4hz for every blink event
> +	  - option_blink_8hz: Set blink speed at 8hz for every blink event
> +
> +	  These blink modes are present in the LED sysfs dir under
> +	  hardware-phy-activity if supported by the LED driver.
> +
> +	  This trigger can be used only by LEDs that support Hardware mode.


-- 
~Randy

Re: [RFC PATCH v4 7/8] net: dsa: qca8k: add LEDs support

[Randy Dunlap]

On 11/10/21 5:34 PM, Ansuel Smith wrote:
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> index 7b1457a6e327..9f4bae5cc43d 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
> +	  This enables support for LEDs present on the Qualcomm Atheros
> +	  QCA8K Ethernet switch chips. This require the LEDs offload

	                               This requires

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


-- 
~Randy

Re: [RFC PATCH v4 1/8] leds: add support for hardware driven LEDs

[Randy Dunlap]

On 11/10/21 5:34 PM, Ansuel Smith wrote:
> diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
> index cd155ead8703..0175954717a3 100644
> --- a/Documentation/leds/leds-class.rst
> +++ b/Documentation/leds/leds-class.rst
> @@ -169,6 +169,38 @@ 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 driven LEDs
> +===================================
> +
> +Some LEDs can be driven by hardware (for example a LED connected to

                                                     an LED

> +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 this and a supported trigger must
> +be used.
> +
> +LED driver should declare the correct control mode supported and should set
> +the LED_SOFTWARE_CONTROLLED or LED_HARDWARE_CONTROLLED bit in the flags
> +parameter.
> +The trigger will check these bit and fail to activate if the control mode

                                 bits

> +is not supported. By default if a LED driver doesn't declare a control mode,
> +bit LED_SOFTWARE_CONTROLLED is assumed and set by default.

drop the second:                                  by default

> +
> +The LED must implement 3 main API:

                                  APIs:

> +- hw_control_status(): This asks the LED driver if hardware mode is enabled
> +    or not.
> +- hw_control_start(): This will simply enable the hardware mode for the LED
> +    and the LED driver should reset any active blink_mode.
> +- hw_control_stop(): This will simply disable the hardware mode for the LED.
> +    It's advised to the driver to put the LED in the old state but this is not
> +    enforcerd and putting the LED off is also accepted.

        enforced

> +
> +If LED_HARDWARE_CONTROLLED bit is the only contro mode set (LED_SOFTWARE_CONTROLLED

                                               control

> +not set) set hw_control_status/start/stop is optional as the LED supports only
> +hardware mode and any software only trigger will reject activation.
> +
> +On init a LED driver that support a hardware mode should reset every blink mode

            an LED

> +set by default.
>   
>   Known Issues
>   ============
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index ed800f5da7d8..bd2b19cc77ec 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -49,6 +49,17 @@ config LEDS_BRIGHTNESS_HW_CHANGED
>   
>   	  See Documentation/ABI/testing/sysfs-class-led for details.
>   
> +config LEDS_HARDWARE_CONTROL
> +	bool "LED Hardware Control support"
> +	help
> +	  This option enabled Hardware control support used by leds that

	              enables                                  LEDs

> +	  can be driven in hardware by using supported triggers.
> +
> +	  Hardware blink modes will be exposed by sysfs class in
> +	  /sys/class/leds based on the trigger currently active.
> +
> +	  If unsure, say Y.


-- 
~Randy

Re: [RFC PATCH v4 2/8] leds: document additional use of blink_set for hardware control

[Randy Dunlap]

Hi Ansuel,

Sorry -- I screwed this one up a little bit on the previous iteration.

On 11/10/21 5:34 PM, Ansuel Smith wrote:
> diff --git a/Documentation/leds/leds-class.rst b/Documentation/leds/leds-class.rst
> index 0175954717a3..c06a18b811de 100644
> --- a/Documentation/leds/leds-class.rst
> +++ b/Documentation/leds/leds-class.rst
> @@ -202,6 +202,23 @@ hardware mode and any software only trigger will reject activation.
>   On init a LED driver that support a hardware mode should reset every blink mode
>   set by default.
>   
> +Once a trigger has declared support for hardware-controller blinks, it will use

                                            hardware-controlled

> +blink_set() to try to offload his trigger on activation/configuration.
> +blink_set() will return 0 if the requested modes set in trigger_data can be
> +controlled by hardware or an error if both the mode bitmap is not supported by

maybe:                                if both of the bitmap modes are not supported by

> +the hardware or there was a problem in the configuration.
> +
> +Following blink_set logic, setting brightness to LED_OFF with hardware control active
> +will reset any active blink mode and disable hardware control setting the LED to off.
> +
> +It's in the LED driver's interest to know how to elaborate the trigger data and report support
> +for a particular set of blink modes. For this exact reason explicit support for the specific
> +trigger is mandatory or the driver returns -EOPNOTSUPP if asked to enter hardware mode
> +with a not supported trigger.
> +If the driver returns -EOPNOTSUPP on hw_control_configure(), the trigger activation will
> +fail as the driver doesn't support that specific hardware blink modes or doesn't know

                                                                    mode

> +how to handle the provided trigger data.


thanks.
-- 
~Randy

Re: [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers

[Ansuel Smith]

On Thu, Nov 11, 2021 at 03:16:08AM +0100, Marek Behún wrote:
> On Thu, 11 Nov 2021 02:34:52 +0100
> Ansuel Smith <ansuelsmth@gmail.com> wrote:
> 
> > 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 3 API (hw_control_status/start/stop).
> >   They are used to put the LED in hardware mode and to configure the
> >   various trigger.
> > - We have hardware triggers that are used to expose to userspace the
> >   supported hardware mode and set the hardware mode on trigger
> >   activation.
> > - We can also have triggers that both support hardware and software mode.
> > - The LED driver will declare each supported hardware blink mode and
> >   communicate with the trigger all the supported blink modes that will
> >   be available by sysfs.
> > - A trigger will use blink_set to configure the blink mode to active
> >   in hardware mode.
> > - On hardware trigger activation, only the hardware mode is enabled but
> >   the blink modes are not configured. The LED driver should reset any
> >   link mode active by default.
> > 
> > Each LED driver will have to declare explicit support for the offload
> > trigger (or return not supported error code) as we the trigger_data 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.
> > 
> > I also posted the netdev trigger expanded with the hardware support.
> > 
> > More polish is required but this is just to understand if I'm taking
> > the correct path with this implementation hoping we find a correct
> > implementation and we start working on the ""small details""
> 
> Hello Ansuel,
> 
> besides other things, I am still against the idea of the
> `hardware-phy-activity` trigger: I think that if the user wants the LED
> to indicate network device's link status or activity, it should always
> be done via the existing netdev trigger, and with that trigger only.
> 
> Yes, I know that netdev trigger does not currently support indicating
> different link modes, only whether the link is up (in any mode). That
> should be solved by extending the netdev trigger.
> 
> I am going to try to revive my last attempt and send my proposal again.
> Hope you don't mind.
> 
> Marek

Honestly... It's a bit sad.
The netdev trigger have its limitation and I see introducing an
additional trigger a practical way to correctly support some
strange/specific PHY.
I implemented both idea: expand netdev and introduce a dedicated
trigger and still this is problematic.
Is having an additional trigger for the specific task that bad?

I don't care as long as the feature is implemented but again
pretty sad how this LEDs proposal went.

-- 
	Ansuel

Re: [RFC PATCH v4 0/8] Adds support for PHY LEDs with offload triggers

[Marek Behún]

On Fri, 12 Nov 2021 16:35:21 +0100
Ansuel Smith <ansuelsmth@gmail.com> wrote:

> On Thu, Nov 11, 2021 at 03:16:08AM +0100, Marek Behún wrote:
> > On Thu, 11 Nov 2021 02:34:52 +0100
> > Ansuel Smith <ansuelsmth@gmail.com> wrote:
> >   
> > > 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 3 API (hw_control_status/start/stop).
> > >   They are used to put the LED in hardware mode and to configure the
> > >   various trigger.
> > > - We have hardware triggers that are used to expose to userspace the
> > >   supported hardware mode and set the hardware mode on trigger
> > >   activation.
> > > - We can also have triggers that both support hardware and software mode.
> > > - The LED driver will declare each supported hardware blink mode and
> > >   communicate with the trigger all the supported blink modes that will
> > >   be available by sysfs.
> > > - A trigger will use blink_set to configure the blink mode to active
> > >   in hardware mode.
> > > - On hardware trigger activation, only the hardware mode is enabled but
> > >   the blink modes are not configured. The LED driver should reset any
> > >   link mode active by default.
> > > 
> > > Each LED driver will have to declare explicit support for the offload
> > > trigger (or return not supported error code) as we the trigger_data 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.
> > > 
> > > I also posted the netdev trigger expanded with the hardware support.
> > > 
> > > More polish is required but this is just to understand if I'm taking
> > > the correct path with this implementation hoping we find a correct
> > > implementation and we start working on the ""small details""  
> > 
> > Hello Ansuel,
> > 
> > besides other things, I am still against the idea of the
> > `hardware-phy-activity` trigger: I think that if the user wants the LED
> > to indicate network device's link status or activity, it should always
> > be done via the existing netdev trigger, and with that trigger only.
> > 
> > Yes, I know that netdev trigger does not currently support indicating
> > different link modes, only whether the link is up (in any mode). That
> > should be solved by extending the netdev trigger.
> > 
> > I am going to try to revive my last attempt and send my proposal again.
> > Hope you don't mind.
> > 
> > Marek  
> 
> Honestly... It's a bit sad.
> The netdev trigger have its limitation and I see introducing an
> additional trigger a practical way to correctly support some
> strange/specific PHY.
> I implemented both idea: expand netdev and introduce a dedicated
> trigger and still this is problematic.
> Is having an additional trigger for the specific task that bad?
> 
> I don't care as long as the feature is implemented but again
> pretty sad how this LEDs proposal went.

Dear Ansuel,

  Is having an additional trigger for the specific task that bad?

No, for a very specific thing it is not bad. By specific I mean
something that an existing trigger does not support and can't support
in a reasonable way. But netdev trigger already supports blinking on rx
and tx, and even setting blinking frequency, and indicating link. And
it can be reasonably extended to support indicating different link
modes and even more complex things. For example what I would like to
see is having support for indicating different links by different
colors with RGB LEDs. I have ideas about how it can be implemented.

But a very specific thing can be implemented by a separate trigger.
For an ethernet PHY chip this can be something like ethernet collision
indication, or Energy Efficient Ethernet indication. But link and
activity should be done via netdev.

Note that we even have an API for such specific triggers that can only
work with some LEDs: the LED private triggers. Look at the member
  struct led_hw_trigger_type *trigger_type;
of struct led_classdev and struct led_trigger.
With this member you can make a trigger to be only visible for some
LEDs, so that when the user does
  cd /sys/class/leds/<LED_WITH_PRIVATE_TRIGGER>
  cat trigger
it will output
  [none] netdev something something ... my-private-trigger
but for other LEDs (all those with different trigger_type) it will omit
the my-private-trigger.

Your hardware-phy trigger should in fact use this private trigger API
so that the user only sees the trigger for the LEDs that actually
support it.

Anyway, Ansuel, if you are willing, we can have a call about this where
I can explain my ideas to you and you to me and we can discuss it more
and maybe come to an understanding? I am not opposed to working on this
together.

Marek