[net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[Ansuel Smith]

Add support for led control and led toggle.
qca8337 and qca8327 switch have various reg to control port leds.
The current implementation permit to toggle them on/off and to declare
their blink rules based on the entry in the dts.
They can also be declared in userspace by the "control_rule" entry in
the led sysfs. When hw_mode is active (set by default) the leds blink
based on the control_rule. There are 6 total control rule.
Control rule that applies to phy0-3 commonly used for lan port.
Control rule that applies to phy4 commonly used for wan port.
Each phy port (5 in total) can have a maximum of 3 different leds
attached. Each led can be turned off, blink at 4hz, off or set to
hw_mode and follow their respecitve control rule. The hw_mode can be
toggled using the sysfs entry and will be disabled on brightness or
blink set.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca8k.c | 490 +++++++++++++++++++++++++++++++++++++++-
 drivers/net/dsa/qca8k.h |  50 ++++
 2 files changed, 536 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index bda5a9bf4f52..56385a80987f 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -18,6 +18,7 @@
 #include <linux/phylink.h>
 #include <linux/gpio/consumer.h>
 #include <linux/etherdevice.h>
+#include <linux/leds.h>
 
 #include "qca8k.h"
 
@@ -950,6 +951,467 @@ qca8k_setup_of_rgmii_delay(struct qca8k_priv *priv)
 	return 0;
 }
 
+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_setup_led_rules(struct qca8k_led *led, struct fwnode_handle *node)
+{
+	struct qca8k_led_pattern_en reg_info;
+	const char **rules;
+	int i, count, ret;
+	const char *rule;
+	u32 val = 0;
+
+	if (!fwnode_property_present(node, "qca,led_rules"))
+		return 0;
+
+	rules = kcalloc(QCA8K_LED_RULE_MAX, sizeof(*rules), GFP_KERNEL);
+	if (!rules)
+		return -ENOMEM;
+
+	ret = fwnode_property_read_string_array(node, "qca,led_rules", rules, QCA8K_LED_RULE_MAX);
+	if (ret < 0)
+		return ret;
+
+	count = (unsigned int)ret;
+
+	for (i = 0; i < count; i++) {
+		rule = rules[i];
+
+		if (!strcmp(rule, "tx-blink"))
+			val |= QCA8K_LED_TX_BLINK_MASK;
+
+		if (!strcmp(rule, "rx-blink"))
+			val |= QCA8K_LED_RX_BLINK_MASK;
+
+		if (!strcmp(rule, "collision-blink"))
+			val |= QCA8K_LED_COL_BLINK_MASK;
+
+		if (!strcmp(rule, "link-10M"))
+			val |= QCA8K_LED_LINK_10M_EN_MASK;
+
+		if (!strcmp(rule, "link-100M"))
+			val |= QCA8K_LED_LINK_100M_EN_MASK;
+
+		if (!strcmp(rule, "link-1000M"))
+			val |= QCA8K_LED_LINK_1000M_EN_MASK;
+
+		if (!strcmp(rule, "half-duplex"))
+			val |= QCA8K_LED_HALF_DUPLEX_MASK;
+
+		if (!strcmp(rule, "full-duplex"))
+			val |= QCA8K_LED_FULL_DUPLEX_MASK;
+
+		if (!strcmp(rule, "linkup-over"))
+			val |= QCA8K_LED_LINKUP_OVER_MASK;
+
+		if (!strcmp(rule, "power-on-reset"))
+			val |= QCA8K_LED_POWER_ON_LIGHT_MASK;
+
+		if (!(val & QCA8K_LED_BLINK_FREQ_MASK)) {
+			if (!strcmp(rule, "blink-2hz"))
+				val |= QCA8K_LED_BLINK_2HZ << QCA8K_LED_BLINK_FREQ_SHITF;
+			else if (!strcmp(rule, "blink-4hz"))
+				val |= QCA8K_LED_BLINK_4HZ << QCA8K_LED_BLINK_FREQ_SHITF;
+			else if (!strcmp(rule, "blink-8hz"))
+				val |= QCA8K_LED_BLINK_8HZ << QCA8K_LED_BLINK_FREQ_SHITF;
+			else if (!strcmp(rule, "blink-auto"))
+				val |= QCA8K_LED_BLINK_AUTO << QCA8K_LED_BLINK_FREQ_SHITF;
+		}
+	}
+
+	kfree(rules);
+
+	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+
+	ret = qca8k_rmw(led->priv, reg_info.reg,
+			QCA8K_LED_CTRL_MASK << reg_info.shift,
+			val << reg_info.shift);
+
+	return ret;
+}
+
+static ssize_t
+control_rule_show(struct device *dev, struct device_attribute *attr,
+		  char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct qca8k_led *led = container_of(led_cdev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+	u32 value;
+	int ret;
+
+	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+
+	ret = qca8k_read(led->priv, reg_info.reg, &value);
+	if (ret)
+		return sprintf(buf, "Error reading control rule\n");
+
+	value >>= reg_info.shift;
+	value &= QCA8K_LED_CTRL_MASK;
+
+	return sprintf(buf, "%x\n", value);
+}
+
+static ssize_t
+control_rule_store(struct device *dev,
+		   struct device_attribute *attr, const char *buf,
+		   size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct qca8k_led *led = container_of(led_cdev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+	ssize_t status;
+	long value;
+	int ret;
+
+	status = kstrtol(buf, 0, &value);
+	if (status)
+		return status;
+
+	if (value < 0)
+		return -EINVAL;
+
+	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+
+	value &= QCA8K_LED_CTRL_MASK;
+
+	ret = qca8k_rmw(led->priv, reg_info.reg,
+			QCA8K_LED_CTRL_MASK << reg_info.shift,
+			value << reg_info.shift);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
+static ssize_t
+hw_mode_show(struct device *dev, struct device_attribute *attr,
+	     char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct qca8k_led *led = container_of(led_cdev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+	u32 value;
+	int ret;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	ret = qca8k_read(led->priv, reg_info.reg, &value);
+	if (ret)
+		return sprintf(buf, "Error reading hw mode\n");
+
+	value >>= reg_info.shift;
+	value &= GENMASK(1, 0);
+
+	return sprintf(buf, "%x\n", value == QCA8K_LED_RULE_CONTROLLED ? 1 : 0);
+}
+
+static ssize_t
+hw_mode_store(struct device *dev,
+	      struct device_attribute *attr, const char *buf,
+	      size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct qca8k_led *led = container_of(led_cdev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+	ssize_t status;
+	long value;
+	int ret;
+
+	status = kstrtol(buf, 0, &value);
+	if (status)
+		return status;
+
+	if (value < 0)
+		return -EINVAL;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (value)
+		value = QCA8K_LED_RULE_CONTROLLED;
+
+	value &= GENMASK(1, 0);
+
+	ret = qca8k_rmw(led->priv, reg_info.reg,
+			GENMASK(1, 0) << reg_info.shift,
+			value << reg_info.shift);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
+static DEVICE_ATTR_RW(control_rule);
+static DEVICE_ATTR_RW(hw_mode);
+
+/* Each led have a enable hw_mode and optionally a way to set control rule */
+static struct attribute *qca8k_leds_attrs[] = {
+	&dev_attr_hw_mode.attr,
+	&dev_attr_control_rule.attr,
+	NULL,
+};
+
+static struct attribute_group qca8k_leds_rule_group = {
+	.attrs = qca8k_leds_attrs,
+};
+
+static const struct attribute_group *qca8k_leds_groups[] = {
+	&qca8k_leds_rule_group,
+	NULL,
+};
+
+static void
+qca8k_led_brightness_set(struct qca8k_led *led,
+			 enum led_brightness b)
+{
+	struct qca8k_led_pattern_en reg_info;
+	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(led->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;
+	u32 val;
+	int ret;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	ret = qca8k_read(led->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;
+
+	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(led->priv, reg_info.reg,
+		  GENMASK(1, 0) << reg_info.shift,
+		  QCA8K_LED_ALWAYS_BLINK_4HZ << reg_info.shift);
+
+	return 0;
+}
+
+static void
+qca8k_cled_flash_resume(struct led_classdev *ldev)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+	struct qca8k_led_pattern_en reg_info;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_rmw(led->priv, reg_info.reg,
+		  GENMASK(1, 0) << reg_info.shift,
+		  led->old_pattern << reg_info.shift);
+}
+
+static int
+qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
+{
+	struct led_init_data init_data = { };
+	struct fwnode_handle *led = NULL;
+	struct qca8k_led *port_led;
+	int led_num, port_index;
+	const char *state;
+	int ret;
+
+	fwnode_for_each_child_node(port, 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.flash_resume = qca8k_cled_flash_resume;
+		port_led->cdev.groups = qca8k_leds_groups;
+		port_led->cdev.flags |= LED_CORE_SUSPENDRESUME;
+		init_data.default_label = ":port";
+		init_data.devicename = "qca8k";
+		init_data.fwnode = led;
+
+		/* Provide control rule first lan port and wan port.
+		 * Lan port 2-3-4 follow first lan port control rule if the hw mode
+		 * is active.
+		 * The control_rule sysfs refer to the same reg for lan port (phy0-3)
+		 */
+		ret = qca8k_setup_led_rules(port_led, led);
+		if (ret)
+			dev_warn(priv->dev, "Failed to apply led control rules for %s",
+				 port_led->cdev.name);
+
+		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;
+}
+
+static int
+qca8k_setup_led_ctrl(struct qca8k_priv *priv)
+{
+	struct fwnode_handle *leds, *port;
+	int port_num;
+	int ret;
+
+	leds = device_get_named_child_node(priv->dev, "leds");
+	if (!leds) {
+		dev_info(priv->dev, "No LEDs specified in device tree!\n");
+		return 0;
+	}
+
+	fwnode_for_each_child_node(leds, 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;
+}
+
 static int
 qca8k_setup(struct dsa_switch *ds)
 {
@@ -979,6 +1441,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_led_ctrl(priv);
+	if (ret)
+		return ret;
+
 	/* Enable CPU Port */
 	ret = qca8k_reg_set(priv, QCA8K_REG_GLOBAL_FW_CTRL0,
 			    QCA8K_GLOBAL_FW_CTRL0_CPU_PORT_EN);
@@ -1890,13 +2356,29 @@ qca8k_sw_remove(struct mdio_device *mdiodev)
 static void
 qca8k_set_pm(struct qca8k_priv *priv, int enable)
 {
-	int i;
+	struct qca8k_led_pattern_en reg_info;
+	int port, led, port_index;
+	u32 val;
 
-	for (i = 0; i < QCA8K_NUM_PORTS; i++) {
-		if (!priv->port_sts[i].enabled)
+	for (port = 0; port < QCA8K_NUM_PORTS; port++) {
+		/* Save leds state for current port */
+		for (led = 0; led < 3; led++) {
+			port_index = 3 * port + led;
+			qca8k_get_enable_led_reg(port, led, &reg_info);
+
+			if (!enable) {
+				qca8k_read(priv, reg_info.reg, &val);
+				val >>= reg_info.shift;
+				val &= GENMASK(1, 0);
+
+				priv->ports_led[port_index].old_pattern = val;
+			}
+		}
+
+		if (!priv->port_sts[port].enabled)
 			continue;
 
-		qca8k_port_set_status(priv, i, enable);
+		qca8k_port_set_status(priv, port, enable);
 	}
 }
 
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index ed3b05ad6745..6c2c85a1d610 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -64,6 +64,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(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
@@ -250,6 +286,19 @@ struct qca8k_match_data {
 	u8 id;
 };
 
+struct qca8k_led_pattern_en {
+	u32 reg;
+	u8 shift;
+};
+
+struct qca8k_led {
+	u8 port_num;
+	u8 led_num;
+	u8 old_pattern;
+	struct qca8k_priv *priv;
+	struct led_classdev cdev;
+};
+
 struct qca8k_priv {
 	u8 switch_id;
 	u8 switch_revision;
@@ -265,6 +314,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 {
-- 
2.32.0

[net-next RFC PATCH 2/2] Documentation: devicetree: net: dsa: qca8k: document configurable led support

[Ansuel Smith]

Document binding for configurable led. Ports led can now be set on/off
and the blink/on rules can be configured using the "qca,led_rules"
binding. Refer to the Documentation on how to configure them.

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

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
index 8c73f67c43ca..233f02cd9e98 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
@@ -29,6 +29,45 @@ the mdio MASTER is used as communication.
 Don't use mixed external and internal mdio-bus configurations, as this is
 not supported by the hardware.
 
+A leds subnode can be declared to configure leds port behaviour.
+The leds subnode must declare the port with the mdio reg that will have the
+attached led. Each port can have a max of 3 different leds. (Refer to example)
+A led can have 4 different settings:
+- Always off
+- Always on
+- Blink at 4hz
+- Hw_mode: This special mode follow control_rule rules and blink based on switch
+event.
+A sysfs entry for control_rule and hw_mode is provided for each led.
+Control rule for phy0-3 are shared and refer to the same reg. That means that
+phy0-3 will blink based on the same rules. Phy4 have its dedicated control_rules.
+
+Each led can have the following binding:
+The binding "default-state" can be declared to set them off by default or to
+follow leds control_rule using the keep value. By default hw_mode is set as it's
+the default switch setting.
+The binding "qca,led_rules" can be used to declare the control_rule set on
+switch setup. The following rules can be applied decalred in an array of string
+in the dts:
+- tx-blink: Led blink on tx traffic for the port
+- rx-blink: Led blink on rx traffic for the port
+- collision-blink: Led blink when a collision is detected for the port
+- link-10M: Led is turned on when a link of 10M is detected for the port
+- link-100M: Led is turned on when a link of 100M is detected for the port
+- link-1000M: Led is turned on when a link of 1000M is detected for the port
+- half-duplex: Led is turned on when a half-duplex link is detected for the port
+- full-duplex: Led is turned on when a full-duplex link is detected for the port
+- linkup-over: Led blinks only when the linkup led is on, ignore blink otherwise
+- power-on-reset: Reset led on switch reset
+- One of
+	- blink-2hz: Led blinks at 2hz frequency
+	- blink-4hz: Led blinks at 4hz frequency
+	- blink-8hz: Led blinks at 8hz frequency
+	- blink-auto: Led blinks at 2hz frequency with 10M, 4hz with 100M, 8hz
+	  with 1000M
+Due to the phy0-3 limitation, multiple use of 'qca8k_led_rules' will result in
+the last defined one to be applied.
+
 The CPU port of this switch is always port 0.
 
 A CPU port node has the following optional node:
@@ -213,3 +252,213 @@ for the internal master mdio-bus configuration:
 			};
 		};
 	};
+
+for the leds declaration example:
+
+#include <dt-bindings/leds/common.h>
+
+	&mdio0 {
+		switch@10 {
+			compatible = "qca,qca8337";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
+			reg = <0x10>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					label = "cpu";
+					ethernet = <&gmac1>;
+					phy-mode = "rgmii";
+					fixed-link {
+						speed = 1000;
+						full-duplex;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					label = "lan1";
+					phy-mode = "internal";
+					phy-handle = <&phy_port1>;
+				};
+
+				port@2 {
+					reg = <2>;
+					label = "lan2";
+					phy-mode = "internal";
+					phy-handle = <&phy_port2>;
+				};
+
+				port@3 {
+					reg = <3>;
+					label = "lan3";
+					phy-mode = "internal";
+					phy-handle = <&phy_port3>;
+				};
+
+				port@4 {
+					reg = <4>;
+					label = "lan4";
+					phy-mode = "internal";
+					phy-handle = <&phy_port4>;
+				};
+
+				port@5 {
+					reg = <5>;
+					label = "wan";
+					phy-mode = "internal";
+					phy-handle = <&phy_port5>;
+				};
+			};
+
+			mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				phy_port1: phy@0 {
+					reg = <0>;
+				};
+
+				phy_port2: phy@1 {
+					reg = <1>;
+				};
+
+				phy_port3: phy@2 {
+					reg = <2>;
+				};
+
+				phy_port4: phy@3 {
+					reg = <3>;
+				};
+
+				phy_port5: phy@4 {
+					reg = <4>;
+				};
+			};
+
+			leds {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				phy@0 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_GREEN>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <1>;
+					};
+
+					led@1 {
+						reg = <1>;
+						color = <LED_COLOR_ID_AMBER>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <1>;
+					};
+				};
+
+				phy@1 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <1>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_GREEN>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <2>;
+					};
+
+					led@1 {
+						reg = <1>;
+						color = <LED_COLOR_ID_AMBER>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <2>;
+					};
+				};
+
+				phy@2 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <2>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_GREEN>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <3>;
+					};
+
+					led@1 {
+						reg = <1>;
+						color = <LED_COLOR_ID_AMBER>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <3>;
+					};
+				};
+
+				phy@3 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <3>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_GREEN>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <4>;
+					};
+
+					led@1 {
+						reg = <1>;
+						color = <LED_COLOR_ID_AMBER>;
+						default-state = "keep";
+						function = LED_FUNCTION_LAN;
+						function-enumerator = <4>;
+					};
+				};
+
+				phy@4 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					reg = <4>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_GREEN>;
+						default-state = "keep";
+						function = LED_FUNCTION_WAN;
+						qca,led_rules = "tx-blink", "rx-blink", "link-1000M", "full-duplex", "linkup-over", "blink-8hz";
+					};
+
+					led@1 {
+						reg = <1>;
+						color = <LED_COLOR_ID_AMBER>;
+						default-state = "keep";
+						function = LED_FUNCTION_WAN;
+					};
+				};
+			};
+		};
+	};
\ No newline at end of file
-- 
2.32.0

Re: [net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[Andrew Lunn]

On Mon, Sep 20, 2021 at 08:08:50PM +0200, Ansuel Smith wrote:
> Add support for led control and led toggle.
> qca8337 and qca8327 switch have various reg to control port leds.
> The current implementation permit to toggle them on/off and to declare
> their blink rules based on the entry in the dts.
> They can also be declared in userspace by the "control_rule" entry in
> the led sysfs. When hw_mode is active (set by default) the leds blink
> based on the control_rule. There are 6 total control rule.
> Control rule that applies to phy0-3 commonly used for lan port.
> Control rule that applies to phy4 commonly used for wan port.
> Each phy port (5 in total) can have a maximum of 3 different leds
> attached. Each led can be turned off, blink at 4hz, off or set to
> hw_mode and follow their respecitve control rule. The hw_mode can be
> toggled using the sysfs entry and will be disabled on brightness or
> blink set.

Hi Ansuel

It is great you are using the LED subsystem for this. But we need to
split the code up into a generic part which can shared by any
switch/PHY and a driver specific part.

There has been a lot of discussion on the list about this. Maybe you
can help get us to a generic solution which can be used by everybody.

    Andrew

Re: [net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[Ansuel Smith]

On Mon, Sep 20, 2021 at 08:55:48PM +0200, Andrew Lunn wrote:
> On Mon, Sep 20, 2021 at 08:08:50PM +0200, Ansuel Smith wrote:
> > Add support for led control and led toggle.
> > qca8337 and qca8327 switch have various reg to control port leds.
> > The current implementation permit to toggle them on/off and to declare
> > their blink rules based on the entry in the dts.
> > They can also be declared in userspace by the "control_rule" entry in
> > the led sysfs. When hw_mode is active (set by default) the leds blink
> > based on the control_rule. There are 6 total control rule.
> > Control rule that applies to phy0-3 commonly used for lan port.
> > Control rule that applies to phy4 commonly used for wan port.
> > Each phy port (5 in total) can have a maximum of 3 different leds
> > attached. Each led can be turned off, blink at 4hz, off or set to
> > hw_mode and follow their respecitve control rule. The hw_mode can be
> > toggled using the sysfs entry and will be disabled on brightness or
> > blink set.
> 
> Hi Ansuel
> 
> It is great you are using the LED subsystem for this. But we need to
> split the code up into a generic part which can shared by any
> switch/PHY and a driver specific part.
> 
> There has been a lot of discussion on the list about this. Maybe you
> can help get us to a generic solution which can be used by everybody.
> 
>     Andrew

Yes, can you point me to the discussion?
I post this as RFC for this exact reason... I read somehwere that there
was a discussion on how to implementd leds for switch but never ever
found it. Also i'm very confused on the node structure on how to define
leds, I think my current implementation is very verbose and long.
As you can see this switch have all the leds controllable with a special
mode to apply some special rules (and actually blink by the switch)
So yes I would like to propose some idea and describe how this switch
works hoping other OEM does the same thing. (I'm very negative about
this part)

Re: [net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[Andrew Lunn]

> Yes, can you point me to the discussion?

It has gone through many cycles :-(

The linux-led list is probably the better archive to look through, it
is a lot lower volume than netdev.

https://www.spinics.net/lists/linux-leds/msg18652.html

https://www.spinics.net/lists/linux-leds/msg18527.html


> I post this as RFC for this exact reason... I read somehwere that there
> was a discussion on how to implementd leds for switch but never ever
> found it.

Most of the discussion so far has been about PHY LEDs, where the PHY
driver controls the LEDs. However some Ethernet switches also have LED
controls, which are not part of the PHY. And then there are some MAC
drivers which control the PHY in firmware, and have firmware calls for
controlling the LEDs. We need a generic solution which scales across
all this. And it needs to work without DT, or at least, not block ACPI
being added later.

But progress is slow. I hope that the PHY use case will drive things
forward, get the ABI defined. We can then scale it out to include
switches, maybe with a bit of code refactoring.

	  Andrew

Re: [net-next RFC PATCH 1/2] drivers: net: dsa: qca8k: add support for led config

[Ansuel Smith]

On Mon, Sep 20, 2021 at 09:19:13PM +0200, Andrew Lunn wrote:
> > Yes, can you point me to the discussion?
> 
> It has gone through many cycles :-(
> 
> The linux-led list is probably the better archive to look through, it
> is a lot lower volume than netdev.
> 
> https://www.spinics.net/lists/linux-leds/msg18652.html
> 
> https://www.spinics.net/lists/linux-leds/msg18527.html
> 
> 

Thanks for the links.

> > I post this as RFC for this exact reason... I read somehwere that there
> > was a discussion on how to implementd leds for switch but never ever
> > found it.
> 
> Most of the discussion so far has been about PHY LEDs, where the PHY
> driver controls the LEDs. However some Ethernet switches also have LED
> controls, which are not part of the PHY. And then there are some MAC
> drivers which control the PHY in firmware, and have firmware calls for
> controlling the LEDs. We need a generic solution which scales across
> all this. And it needs to work without DT, or at least, not block ACPI
> being added later.
> 
> But progress is slow. I hope that the PHY use case will drive things
> forward, get the ABI defined. We can then scale it out to include
> switches, maybe with a bit of code refactoring.
> 
> 	  Andrew

Wow... What a mess. Tell me if I'm wrong but it seems progress is stuck.
I can see the api proposal patch had no review from June. Should I put a
message there to try to move things up?

Re: [net-next RFC PATCH 2/2] Documentation: devicetree: net: dsa: qca8k: document configurable led support

[Rob Herring]

On Mon, Sep 20, 2021 at 08:08:51PM +0200, Ansuel Smith wrote:
> Document binding for configurable led. Ports led can now be set on/off
> and the blink/on rules can be configured using the "qca,led_rules"
> binding. Refer to the Documentation on how to configure them.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/net/dsa/qca8k.txt     | 249 ++++++++++++++++++
>  1 file changed, 249 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.txt b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> index 8c73f67c43ca..233f02cd9e98 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.txt
> @@ -29,6 +29,45 @@ the mdio MASTER is used as communication.
>  Don't use mixed external and internal mdio-bus configurations, as this is
>  not supported by the hardware.
>  
> +A leds subnode can be declared to configure leds port behaviour.
> +The leds subnode must declare the port with the mdio reg that will have the
> +attached led. Each port can have a max of 3 different leds. (Refer to example)
> +A led can have 4 different settings:
> +- Always off
> +- Always on
> +- Blink at 4hz
> +- Hw_mode: This special mode follow control_rule rules and blink based on switch
> +event.
> +A sysfs entry for control_rule and hw_mode is provided for each led.
> +Control rule for phy0-3 are shared and refer to the same reg. That means that
> +phy0-3 will blink based on the same rules. Phy4 have its dedicated control_rules.
> +
> +Each led can have the following binding:
> +The binding "default-state" can be declared to set them off by default or to
> +follow leds control_rule using the keep value. By default hw_mode is set as it's
> +the default switch setting.
> +The binding "qca,led_rules" can be used to declare the control_rule set on
> +switch setup. The following rules can be applied decalred in an array of string
> +in the dts:
> +- tx-blink: Led blink on tx traffic for the port
> +- rx-blink: Led blink on rx traffic for the port
> +- collision-blink: Led blink when a collision is detected for the port
> +- link-10M: Led is turned on when a link of 10M is detected for the port
> +- link-100M: Led is turned on when a link of 100M is detected for the port
> +- link-1000M: Led is turned on when a link of 1000M is detected for the port
> +- half-duplex: Led is turned on when a half-duplex link is detected for the port
> +- full-duplex: Led is turned on when a full-duplex link is detected for the port
> +- linkup-over: Led blinks only when the linkup led is on, ignore blink otherwise
> +- power-on-reset: Reset led on switch reset
> +- One of
> +	- blink-2hz: Led blinks at 2hz frequency
> +	- blink-4hz: Led blinks at 4hz frequency
> +	- blink-8hz: Led blinks at 8hz frequency
> +	- blink-auto: Led blinks at 2hz frequency with 10M, 4hz with 100M, 8hz
> +	  with 1000M
> +Due to the phy0-3 limitation, multiple use of 'qca8k_led_rules' will result in
> +the last defined one to be applied.
> +

Too big a change for plain text. This needs to be a schema (and also 
common most likely).

>  The CPU port of this switch is always port 0.
>  
>  A CPU port node has the following optional node:
> @@ -213,3 +252,213 @@ for the internal master mdio-bus configuration:
>  			};
>  		};
>  	};
> +
> +for the leds declaration example:
> +
> +#include <dt-bindings/leds/common.h>
> +
> +	&mdio0 {
> +		switch@10 {
> +			compatible = "qca,qca8337";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			reset-gpios = <&gpio 42 GPIO_ACTIVE_LOW>;
> +			reg = <0x10>;
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +					label = "cpu";
> +					ethernet = <&gmac1>;
> +					phy-mode = "rgmii";
> +					fixed-link {
> +						speed = 1000;
> +						full-duplex;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +					label = "lan1";
> +					phy-mode = "internal";
> +					phy-handle = <&phy_port1>;
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +					label = "lan2";
> +					phy-mode = "internal";
> +					phy-handle = <&phy_port2>;
> +				};
> +
> +				port@3 {
> +					reg = <3>;
> +					label = "lan3";
> +					phy-mode = "internal";
> +					phy-handle = <&phy_port3>;
> +				};
> +
> +				port@4 {
> +					reg = <4>;
> +					label = "lan4";
> +					phy-mode = "internal";
> +					phy-handle = <&phy_port4>;
> +				};
> +
> +				port@5 {
> +					reg = <5>;
> +					label = "wan";
> +					phy-mode = "internal";
> +					phy-handle = <&phy_port5>;
> +				};
> +			};
> +
> +			mdio {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				phy_port1: phy@0 {
> +					reg = <0>;
> +				};
> +
> +				phy_port2: phy@1 {
> +					reg = <1>;
> +				};
> +
> +				phy_port3: phy@2 {
> +					reg = <2>;
> +				};
> +
> +				phy_port4: phy@3 {
> +					reg = <3>;
> +				};
> +
> +				phy_port5: phy@4 {
> +					reg = <4>;
> +				};
> +			};
> +
> +			leds {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				phy@0 {

Duplicating the phy's here? LEDs are a function of the phy, so they 
should be under the actual phy node. So this should be a 'leds' node 
under the mdio/phy@0 node.

> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <0>;
> +
> +					led@0 {
> +						reg = <0>;
> +						color = <LED_COLOR_ID_GREEN>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <1>;
> +					};
> +
> +					led@1 {
> +						reg = <1>;
> +						color = <LED_COLOR_ID_AMBER>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <1>;
> +					};
> +				};
> +
> +				phy@1 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <1>;
> +
> +					led@0 {
> +						reg = <0>;
> +						color = <LED_COLOR_ID_GREEN>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <2>;
> +					};
> +
> +					led@1 {
> +						reg = <1>;
> +						color = <LED_COLOR_ID_AMBER>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <2>;
> +					};
> +				};
> +
> +				phy@2 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <2>;
> +
> +					led@0 {
> +						reg = <0>;
> +						color = <LED_COLOR_ID_GREEN>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <3>;
> +					};
> +
> +					led@1 {
> +						reg = <1>;
> +						color = <LED_COLOR_ID_AMBER>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <3>;
> +					};
> +				};
> +
> +				phy@3 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <3>;
> +
> +					led@0 {
> +						reg = <0>;
> +						color = <LED_COLOR_ID_GREEN>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <4>;
> +					};
> +
> +					led@1 {
> +						reg = <1>;
> +						color = <LED_COLOR_ID_AMBER>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_LAN;
> +						function-enumerator = <4>;
> +					};
> +				};
> +
> +				phy@4 {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					reg = <4>;
> +
> +					led@0 {
> +						reg = <0>;
> +						color = <LED_COLOR_ID_GREEN>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_WAN;
> +						qca,led_rules = "tx-blink", "rx-blink", "link-1000M", "full-duplex", "linkup-over", "blink-8hz";
> +					};
> +
> +					led@1 {
> +						reg = <1>;
> +						color = <LED_COLOR_ID_AMBER>;
> +						default-state = "keep";
> +						function = LED_FUNCTION_WAN;
> +					};
> +				};
> +			};
> +		};
> +	};
> \ No newline at end of file

Fix this.

> -- 
> 2.32.0
> 
>