[net-next PATCH 00/11] net: Add basic LED support for switch/phy

[net-next PATCH 00/11] net: Add basic LED support for switch/phy

[Christian Marangi]

This is a continue of [1]. It was decided to take a more gradual
approach to implement LEDs support for switch and phy starting with
basic support and then implementing the hw control part when we have all
the prereq done.

This series implements only the brightness_set() and blink_set() ops.
An example of switch implementation is done with qca8k.

For PHY a more generic approach is used with implementing the LED
support in PHY core and with the user (in this case marvell) adding all
the required functions.

Currently we set the default-state as "keep" to not change the default
configuration of the declared LEDs since almost every switch have a
default configuration.

[1] https://lore.kernel.org/lkml/20230216013230.22978-1-ansuelsmth@gmail.com/

Andrew Lunn (6):
  net: phy: Add a binding for PHY LEDs
  net: phy: phy_device: Call into the PHY driver to set LED brightness.
  net: phy: marvell: Add software control of the LEDs
  net: phy: phy_device: Call into the PHY driver to set LED blinking.
  net: phy: marvell: Implement led_blink_set()
  arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

Christian Marangi (5):
  net: dsa: qca8k: add LEDs basic support
  net: dsa: qca8k: add LEDs blink_set() support
  dt-bindings: net: dsa: dsa-port: Document support for LEDs node
  dt-bindings: net: dsa: qca8k: add LEDs definition example
  dt-bindings: net: phy: Document support for LEDs node

 .../devicetree/bindings/net/dsa/dsa-port.yaml |   7 +
 .../devicetree/bindings/net/dsa/qca8k.yaml    |  24 ++
 .../devicetree/bindings/net/ethernet-phy.yaml |  22 ++
 arch/arm/boot/dts/armada-370-rd.dts           |  14 ++
 drivers/net/dsa/qca/Kconfig                   |   7 +
 drivers/net/dsa/qca/Makefile                  |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c              |   4 +
 drivers/net/dsa/qca/qca8k-leds.c              | 238 ++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h                   |  69 +++++
 drivers/net/phy/marvell.c                     |  81 +++++-
 drivers/net/phy/phy_device.c                  | 115 +++++++++
 include/linux/phy.h                           |  33 +++
 12 files changed, 610 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

-- 
2.39.2

[net-next PATCH 00/11] net: Add basic LED support for switch/phy

[Christian Marangi]

This is a continue of [1]. It was decided to take a more gradual
approach to implement LEDs support for switch and phy starting with
basic support and then implementing the hw control part when we have all
the prereq done.

This series implements only the brightness_set() and blink_set() ops.
An example of switch implementation is done with qca8k.

For PHY a more generic approach is used with implementing the LED
support in PHY core and with the user (in this case marvell) adding all
the required functions.

Currently we set the default-state as "keep" to not change the default
configuration of the declared LEDs since almost every switch have a
default configuration.

[1] https://lore.kernel.org/lkml/20230216013230.22978-1-ansuelsmth@gmail.com/

Andrew Lunn (6):
  net: phy: Add a binding for PHY LEDs
  net: phy: phy_device: Call into the PHY driver to set LED brightness.
  net: phy: marvell: Add software control of the LEDs
  net: phy: phy_device: Call into the PHY driver to set LED blinking.
  net: phy: marvell: Implement led_blink_set()
  arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

Christian Marangi (5):
  net: dsa: qca8k: add LEDs basic support
  net: dsa: qca8k: add LEDs blink_set() support
  dt-bindings: net: dsa: dsa-port: Document support for LEDs node
  dt-bindings: net: dsa: qca8k: add LEDs definition example
  dt-bindings: net: phy: Document support for LEDs node

 .../devicetree/bindings/net/dsa/dsa-port.yaml |   7 +
 .../devicetree/bindings/net/dsa/qca8k.yaml    |  24 ++
 .../devicetree/bindings/net/ethernet-phy.yaml |  22 ++
 arch/arm/boot/dts/armada-370-rd.dts           |  14 ++
 drivers/net/dsa/qca/Kconfig                   |   7 +
 drivers/net/dsa/qca/Makefile                  |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c              |   4 +
 drivers/net/dsa/qca/qca8k-leds.c              | 238 ++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h                   |  69 +++++
 drivers/net/phy/marvell.c                     |  81 +++++-
 drivers/net/phy/phy_device.c                  | 115 +++++++++
 include/linux/phy.h                           |  33 +++
 12 files changed, 610 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

Add LEDs basic support for qca8k Switch Family by adding basic
brightness_set() and brightness_get() support.

Since these LEDs refelect port status, the default label is set to
":port". DT binding should describe the color, function and number of
the leds using standard LEDs api.

These LEDs supports only blocking variant of the brightness_set()
function since they can sleep during access of the switch leds to set
the brightness.

While at it add to the qca8k header file each mode defined by the Switch
Documentation for future use.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca/Kconfig      |   7 ++
 drivers/net/dsa/qca/Makefile     |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c |   4 +
 drivers/net/dsa/qca/qca8k-leds.c | 200 +++++++++++++++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h      |  69 +++++++++++
 5 files changed, 281 insertions(+)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

diff --git a/drivers/net/dsa/qca/Kconfig b/drivers/net/dsa/qca/Kconfig
index ba339747362c..dab648f88391 100644
--- a/drivers/net/dsa/qca/Kconfig
+++ b/drivers/net/dsa/qca/Kconfig
@@ -15,3 +15,10 @@ config NET_DSA_QCA8K
 	help
 	  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"
+	depends on NET_DSA_QCA8K
+	help
+	  This enabled support for LEDs present on the Qualcomm Atheros
+	  QCA8K Ethernet switch chips.
diff --git a/drivers/net/dsa/qca/Makefile b/drivers/net/dsa/qca/Makefile
index 701f1d199e93..330ae389e489 100644
--- a/drivers/net/dsa/qca/Makefile
+++ b/drivers/net/dsa/qca/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_NET_DSA_AR9331)	+= ar9331.o
 obj-$(CONFIG_NET_DSA_QCA8K)	+= qca8k.o
 qca8k-y 			+= qca8k-common.o qca8k-8xxx.o
+obj-$(CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT) += qca8k-leds.o
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 2f224b166bbb..76fb062bef84 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1742,6 +1742,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_led_ctrl(priv);
+	if (ret)
+		return ret;
+
 	qca8k_setup_pcs(priv, &priv->pcs_port_0, 0);
 	qca8k_setup_pcs(priv, &priv->pcs_port_6, 6);
 
diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
new file mode 100644
index 000000000000..079f1b423f9a
--- /dev/null
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/regmap.h>
+#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)
+{
+	switch (port_num) {
+	case 0:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
+		break;
+	case 1:
+	case 2:
+	case 3:
+		/* Port 123 are controlled on a different reg */
+		reg_info->reg = QCA8K_LED_CTRL_REG(3);
+		reg_info->shift = QCA8K_LED_PHY123_PATTERN_EN_SHIFT(port_num, led_num);
+		break;
+	case 4:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+qca8k_led_brightness_set(struct qca8k_led *led,
+			 enum led_brightness brightness)
+{
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 mask, val = QCA8K_LED_ALWAYS_OFF;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (brightness)
+		val = QCA8K_LED_ALWAYS_ON;
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		mask = QCA8K_LED_PATTERN_EN_MASK;
+		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		mask = QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	return regmap_update_bits(priv->regmap, reg_info.reg,
+				  mask << reg_info.shift,
+				  val << reg_info.shift);
+}
+
+static int
+qca8k_cled_brightness_set_blocking(struct led_classdev *ldev,
+				   enum led_brightness brightness)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+
+	return qca8k_led_brightness_set(led, brightness);
+}
+
+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 = regmap_read(priv->regmap, reg_info.reg, &val);
+	if (ret)
+		return 0;
+
+	val >>= reg_info.shift;
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		val &= QCA8K_LED_PATTERN_EN_MASK;
+		val >>= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		val &= QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	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_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
+{
+	struct fwnode_handle *led = NULL, *leds = NULL;
+	struct led_init_data init_data = { };
+	struct qca8k_led *port_led;
+	int led_num, port_index;
+	const char *state;
+	int ret;
+
+	leds = fwnode_get_named_child_node(port, "leds");
+	if (!leds) {
+		dev_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 represent 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);
+			}
+		}
+
+		port_led->cdev.max_brightness = 1;
+		port_led->cdev.brightness_set_blocking = qca8k_cled_brightness_set_blocking;
+		port_led->cdev.brightness_get = qca8k_cled_brightness_get;
+		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 *ports, *port;
+	int port_num;
+	int ret;
+
+	ports = device_get_named_child_node(priv->dev, "ports");
+	if (!ports) {
+		dev_info(priv->dev, "No ports node specified in device tree!\n");
+		return 0;
+	}
+
+	fwnode_for_each_child_node(ports, port) {
+		struct fwnode_handle *phy_node, *reg_port_node = port;
+
+		phy_node = fwnode_find_reference(port, "phy-handle", 0);
+		if (!IS_ERR(phy_node))
+			reg_port_node = phy_node;
+
+		if (fwnode_property_read_u32(reg_port_node, "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/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 03514f7a20be..001b9daaec20 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/regmap.h>
 #include <linux/gpio.h>
+#include <linux/leds.h>
 #include <linux/dsa/tag_qca.h>
 
 #define QCA8K_ETHERNET_MDIO_PRIORITY			7
@@ -85,6 +86,50 @@
 #define   QCA8K_MDIO_MASTER_DATA(x)			FIELD_PREP(QCA8K_MDIO_MASTER_DATA_MASK, x)
 #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_PHY123_PATTERN_EN_SHIFT(_phy, _led)	((((_phy) - 1) * 6) + 8 + (2 * (_led)))
+#define QCA8K_LED_PHY123_PATTERN_EN_MASK		GENMASK(1, 0)
+
+#define QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT		0
+#define QCA8K_LED_PHY4_CONTROL_RULE_SHIFT		16
+
+#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
@@ -383,6 +428,19 @@ struct qca8k_pcs {
 	int port;
 };
 
+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;
@@ -407,6 +465,7 @@ struct qca8k_priv {
 	struct qca8k_pcs pcs_port_0;
 	struct qca8k_pcs pcs_port_6;
 	const struct qca8k_match_data *info;
+	struct qca8k_led ports_led[QCA8K_LED_COUNT];
 };
 
 struct qca8k_mib_desc {
@@ -512,4 +571,14 @@ int qca8k_port_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag,
 int qca8k_port_lag_leave(struct dsa_switch *ds, int port,
 			 struct dsa_lag lag);
 
+/* Leds Support function */
+#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.39.2

[net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

Add LEDs basic support for qca8k Switch Family by adding basic
brightness_set() and brightness_get() support.

Since these LEDs refelect port status, the default label is set to
":port". DT binding should describe the color, function and number of
the leds using standard LEDs api.

These LEDs supports only blocking variant of the brightness_set()
function since they can sleep during access of the switch leds to set
the brightness.

While at it add to the qca8k header file each mode defined by the Switch
Documentation for future use.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca/Kconfig      |   7 ++
 drivers/net/dsa/qca/Makefile     |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c |   4 +
 drivers/net/dsa/qca/qca8k-leds.c | 200 +++++++++++++++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h      |  69 +++++++++++
 5 files changed, 281 insertions(+)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

diff --git a/drivers/net/dsa/qca/Kconfig b/drivers/net/dsa/qca/Kconfig
index ba339747362c..dab648f88391 100644
--- a/drivers/net/dsa/qca/Kconfig
+++ b/drivers/net/dsa/qca/Kconfig
@@ -15,3 +15,10 @@ config NET_DSA_QCA8K
 	help
 	  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"
+	depends on NET_DSA_QCA8K
+	help
+	  This enabled support for LEDs present on the Qualcomm Atheros
+	  QCA8K Ethernet switch chips.
diff --git a/drivers/net/dsa/qca/Makefile b/drivers/net/dsa/qca/Makefile
index 701f1d199e93..330ae389e489 100644
--- a/drivers/net/dsa/qca/Makefile
+++ b/drivers/net/dsa/qca/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_NET_DSA_AR9331)	+= ar9331.o
 obj-$(CONFIG_NET_DSA_QCA8K)	+= qca8k.o
 qca8k-y 			+= qca8k-common.o qca8k-8xxx.o
+obj-$(CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT) += qca8k-leds.o
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 2f224b166bbb..76fb062bef84 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1742,6 +1742,10 @@ qca8k_setup(struct dsa_switch *ds)
 	if (ret)
 		return ret;
 
+	ret = qca8k_setup_led_ctrl(priv);
+	if (ret)
+		return ret;
+
 	qca8k_setup_pcs(priv, &priv->pcs_port_0, 0);
 	qca8k_setup_pcs(priv, &priv->pcs_port_6, 6);
 
diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
new file mode 100644
index 000000000000..079f1b423f9a
--- /dev/null
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/regmap.h>
+#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)
+{
+	switch (port_num) {
+	case 0:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
+		break;
+	case 1:
+	case 2:
+	case 3:
+		/* Port 123 are controlled on a different reg */
+		reg_info->reg = QCA8K_LED_CTRL_REG(3);
+		reg_info->shift = QCA8K_LED_PHY123_PATTERN_EN_SHIFT(port_num, led_num);
+		break;
+	case 4:
+		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
+		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+qca8k_led_brightness_set(struct qca8k_led *led,
+			 enum led_brightness brightness)
+{
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+	u32 mask, val = QCA8K_LED_ALWAYS_OFF;
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (brightness)
+		val = QCA8K_LED_ALWAYS_ON;
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		mask = QCA8K_LED_PATTERN_EN_MASK;
+		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		mask = QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	return regmap_update_bits(priv->regmap, reg_info.reg,
+				  mask << reg_info.shift,
+				  val << reg_info.shift);
+}
+
+static int
+qca8k_cled_brightness_set_blocking(struct led_classdev *ldev,
+				   enum led_brightness brightness)
+{
+	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
+
+	return qca8k_led_brightness_set(led, brightness);
+}
+
+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 = regmap_read(priv->regmap, reg_info.reg, &val);
+	if (ret)
+		return 0;
+
+	val >>= reg_info.shift;
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		val &= QCA8K_LED_PATTERN_EN_MASK;
+		val >>= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		val &= QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	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_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
+{
+	struct fwnode_handle *led = NULL, *leds = NULL;
+	struct led_init_data init_data = { };
+	struct qca8k_led *port_led;
+	int led_num, port_index;
+	const char *state;
+	int ret;
+
+	leds = fwnode_get_named_child_node(port, "leds");
+	if (!leds) {
+		dev_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 represent 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);
+			}
+		}
+
+		port_led->cdev.max_brightness = 1;
+		port_led->cdev.brightness_set_blocking = qca8k_cled_brightness_set_blocking;
+		port_led->cdev.brightness_get = qca8k_cled_brightness_get;
+		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 *ports, *port;
+	int port_num;
+	int ret;
+
+	ports = device_get_named_child_node(priv->dev, "ports");
+	if (!ports) {
+		dev_info(priv->dev, "No ports node specified in device tree!\n");
+		return 0;
+	}
+
+	fwnode_for_each_child_node(ports, port) {
+		struct fwnode_handle *phy_node, *reg_port_node = port;
+
+		phy_node = fwnode_find_reference(port, "phy-handle", 0);
+		if (!IS_ERR(phy_node))
+			reg_port_node = phy_node;
+
+		if (fwnode_property_read_u32(reg_port_node, "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/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 03514f7a20be..001b9daaec20 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/regmap.h>
 #include <linux/gpio.h>
+#include <linux/leds.h>
 #include <linux/dsa/tag_qca.h>
 
 #define QCA8K_ETHERNET_MDIO_PRIORITY			7
@@ -85,6 +86,50 @@
 #define   QCA8K_MDIO_MASTER_DATA(x)			FIELD_PREP(QCA8K_MDIO_MASTER_DATA_MASK, x)
 #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_PHY123_PATTERN_EN_SHIFT(_phy, _led)	((((_phy) - 1) * 6) + 8 + (2 * (_led)))
+#define QCA8K_LED_PHY123_PATTERN_EN_MASK		GENMASK(1, 0)
+
+#define QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT		0
+#define QCA8K_LED_PHY4_CONTROL_RULE_SHIFT		16
+
+#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
@@ -383,6 +428,19 @@ struct qca8k_pcs {
 	int port;
 };
 
+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;
@@ -407,6 +465,7 @@ struct qca8k_priv {
 	struct qca8k_pcs pcs_port_0;
 	struct qca8k_pcs pcs_port_6;
 	const struct qca8k_match_data *info;
+	struct qca8k_led ports_led[QCA8K_LED_COUNT];
 };
 
 struct qca8k_mib_desc {
@@ -512,4 +571,14 @@ int qca8k_port_lag_join(struct dsa_switch *ds, int port, struct dsa_lag lag,
 int qca8k_port_lag_leave(struct dsa_switch *ds, int port,
 			 struct dsa_lag lag);
 
+/* Leds Support function */
+#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.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

> +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> +{
> +	struct fwnode_handle *ports, *port;
> +	int port_num;
> +	int ret;
> +
> +	ports = device_get_named_child_node(priv->dev, "ports");
> +	if (!ports) {
> +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> +		return 0;
> +	}
> +
> +	fwnode_for_each_child_node(ports, port) {
> +		struct fwnode_handle *phy_node, *reg_port_node = port;
> +
> +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> +		if (!IS_ERR(phy_node))
> +			reg_port_node = phy_node;

I don't understand this bit. Why are you looking at the phy-handle?

> +
> +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> +			continue;

I would of expect port, not reg_port_node. I'm missing something
here....

	Andrew

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

> +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> +{
> +	struct fwnode_handle *ports, *port;
> +	int port_num;
> +	int ret;
> +
> +	ports = device_get_named_child_node(priv->dev, "ports");
> +	if (!ports) {
> +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> +		return 0;
> +	}
> +
> +	fwnode_for_each_child_node(ports, port) {
> +		struct fwnode_handle *phy_node, *reg_port_node = port;
> +
> +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> +		if (!IS_ERR(phy_node))
> +			reg_port_node = phy_node;

I don't understand this bit. Why are you looking at the phy-handle?

> +
> +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> +			continue;

I would of expect port, not reg_port_node. I'm missing something
here....

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > +{
> > +	struct fwnode_handle *ports, *port;
> > +	int port_num;
> > +	int ret;
> > +
> > +	ports = device_get_named_child_node(priv->dev, "ports");
> > +	if (!ports) {
> > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > +		return 0;
> > +	}
> > +
> > +	fwnode_for_each_child_node(ports, port) {
> > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > +
> > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > +		if (!IS_ERR(phy_node))
> > +			reg_port_node = phy_node;
> 
> I don't understand this bit. Why are you looking at the phy-handle?
> 
> > +
> > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > +			continue;
> 
> I would of expect port, not reg_port_node. I'm missing something
> here....
> 

It's really not to implement ugly things like "reg - 1"

On qca8k the port index goes from 0 to 6.
0 is cpu port 1
1 is port0 at mdio reg 0
2 is port1 at mdio reg 1
...
6 is cpu port 2

Each port have a phy-handle that refer to a phy node with the correct
reg and that reflect the correct port index.

Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
at times we introduced the mdio thing to describe the port - 1 directly
in DT. If needed I can drop the additional fwnode and use this function
but I would love to use what is defined in DT thatn a simple - 1.

-- 
	Ansuel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > +{
> > +	struct fwnode_handle *ports, *port;
> > +	int port_num;
> > +	int ret;
> > +
> > +	ports = device_get_named_child_node(priv->dev, "ports");
> > +	if (!ports) {
> > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > +		return 0;
> > +	}
> > +
> > +	fwnode_for_each_child_node(ports, port) {
> > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > +
> > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > +		if (!IS_ERR(phy_node))
> > +			reg_port_node = phy_node;
> 
> I don't understand this bit. Why are you looking at the phy-handle?
> 
> > +
> > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > +			continue;
> 
> I would of expect port, not reg_port_node. I'm missing something
> here....
> 

It's really not to implement ugly things like "reg - 1"

On qca8k the port index goes from 0 to 6.
0 is cpu port 1
1 is port0 at mdio reg 0
2 is port1 at mdio reg 1
...
6 is cpu port 2

Each port have a phy-handle that refer to a phy node with the correct
reg and that reflect the correct port index.

Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
at times we introduced the mdio thing to describe the port - 1 directly
in DT. If needed I can drop the additional fwnode and use this function
but I would love to use what is defined in DT thatn a simple - 1.

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:57:10PM +0100, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > > +{
> > > +	struct fwnode_handle *ports, *port;
> > > +	int port_num;
> > > +	int ret;
> > > +
> > > +	ports = device_get_named_child_node(priv->dev, "ports");
> > > +	if (!ports) {
> > > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > > +		return 0;
> > > +	}
> > > +
> > > +	fwnode_for_each_child_node(ports, port) {
> > > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > > +
> > > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > > +		if (!IS_ERR(phy_node))
> > > +			reg_port_node = phy_node;
> > 
> > I don't understand this bit. Why are you looking at the phy-handle?
> > 
> > > +
> > > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > > +			continue;
> > 
> > I would of expect port, not reg_port_node. I'm missing something
> > here....
> > 
> 
> It's really not to implement ugly things like "reg - 1"
> 
> On qca8k the port index goes from 0 to 6.
> 0 is cpu port 1
> 1 is port0 at mdio reg 0
> 2 is port1 at mdio reg 1
> ...
> 6 is cpu port 2
> 
> Each port have a phy-handle that refer to a phy node with the correct
> reg and that reflect the correct port index.
> 
> Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
> at times we introduced the mdio thing to describe the port - 1 directly
> in DT. If needed I can drop the additional fwnode and use this function
> but I would love to use what is defined in DT thatn a simple - 1.

This comes back to the off list discussion earlier today. What you
actually have here are MAC LEDs, not PHY LEDs. They are implemented in
the MAC, not the PHY. To the end user, it should not matter, they
blink when you would expect.

So your addressing should be based around the MAC port number, not the
PHY.

Also, at the moment, all we are adding are a bunch of LEDs. There is
no link to a netdev at this point. At least, i don't see one. Be once
we start using ledtrig-netdev we will need that link to a netdev. Take
a look in my git tree at the last four patch. They add an additional
call to get the device an LED is attached to.

     Andrew

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:57:10PM +0100, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > > +{
> > > +	struct fwnode_handle *ports, *port;
> > > +	int port_num;
> > > +	int ret;
> > > +
> > > +	ports = device_get_named_child_node(priv->dev, "ports");
> > > +	if (!ports) {
> > > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > > +		return 0;
> > > +	}
> > > +
> > > +	fwnode_for_each_child_node(ports, port) {
> > > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > > +
> > > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > > +		if (!IS_ERR(phy_node))
> > > +			reg_port_node = phy_node;
> > 
> > I don't understand this bit. Why are you looking at the phy-handle?
> > 
> > > +
> > > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > > +			continue;
> > 
> > I would of expect port, not reg_port_node. I'm missing something
> > here....
> > 
> 
> It's really not to implement ugly things like "reg - 1"
> 
> On qca8k the port index goes from 0 to 6.
> 0 is cpu port 1
> 1 is port0 at mdio reg 0
> 2 is port1 at mdio reg 1
> ...
> 6 is cpu port 2
> 
> Each port have a phy-handle that refer to a phy node with the correct
> reg and that reflect the correct port index.
> 
> Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
> at times we introduced the mdio thing to describe the port - 1 directly
> in DT. If needed I can drop the additional fwnode and use this function
> but I would love to use what is defined in DT thatn a simple - 1.

This comes back to the off list discussion earlier today. What you
actually have here are MAC LEDs, not PHY LEDs. They are implemented in
the MAC, not the PHY. To the end user, it should not matter, they
blink when you would expect.

So your addressing should be based around the MAC port number, not the
PHY.

Also, at the moment, all we are adding are a bunch of LEDs. There is
no link to a netdev at this point. At least, i don't see one. Be once
we start using ledtrig-netdev we will need that link to a netdev. Take
a look in my git tree at the last four patch. They add an additional
call to get the device an LED is attached to.

     Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

On Wed, Mar 08, 2023 at 01:49:55AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:57:10PM +0100, Christian Marangi wrote:
> > On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > > > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > > > +{
> > > > +	struct fwnode_handle *ports, *port;
> > > > +	int port_num;
> > > > +	int ret;
> > > > +
> > > > +	ports = device_get_named_child_node(priv->dev, "ports");
> > > > +	if (!ports) {
> > > > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	fwnode_for_each_child_node(ports, port) {
> > > > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > > > +
> > > > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > > > +		if (!IS_ERR(phy_node))
> > > > +			reg_port_node = phy_node;
> > > 
> > > I don't understand this bit. Why are you looking at the phy-handle?
> > > 
> > > > +
> > > > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > > > +			continue;
> > > 
> > > I would of expect port, not reg_port_node. I'm missing something
> > > here....
> > > 
> > 
> > It's really not to implement ugly things like "reg - 1"
> > 
> > On qca8k the port index goes from 0 to 6.
> > 0 is cpu port 1
> > 1 is port0 at mdio reg 0
> > 2 is port1 at mdio reg 1
> > ...
> > 6 is cpu port 2
> > 
> > Each port have a phy-handle that refer to a phy node with the correct
> > reg and that reflect the correct port index.
> > 
> > Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
> > at times we introduced the mdio thing to describe the port - 1 directly
> > in DT. If needed I can drop the additional fwnode and use this function
> > but I would love to use what is defined in DT thatn a simple - 1.
> 
> This comes back to the off list discussion earlier today. What you
> actually have here are MAC LEDs, not PHY LEDs. They are implemented in
> the MAC, not the PHY. To the end user, it should not matter, they
> blink when you would expect.
> 
> So your addressing should be based around the MAC port number, not the
> PHY.

Ok will drop this.

> 
> Also, at the moment, all we are adding are a bunch of LEDs. There is
> no link to a netdev at this point. At least, i don't see one. Be once
> we start using ledtrig-netdev we will need that link to a netdev. Take
> a look in my git tree at the last four patch. They add an additional
> call to get the device an LED is attached to.
> 

No currently we have no link for netdev, hence we are setting keep and
not setting a default trigger in DT.
Just checked them, interesting concept, guess we can think of something
also for the interval setting. That would effectively make all the
setting of the trigger set. Just my concern is that they may be too much
specific to netdev trigger and may be problematic for other kind of hw
control. (one main argument that was made for this feature was that some
stuff were too much specific and actually not that generic)

-- 
	Ansuel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Christian Marangi]

On Wed, Mar 08, 2023 at 01:49:55AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:57:10PM +0100, Christian Marangi wrote:
> > On Wed, Mar 08, 2023 at 12:16:13AM +0100, Andrew Lunn wrote:
> > > > +qca8k_setup_led_ctrl(struct qca8k_priv *priv)
> > > > +{
> > > > +	struct fwnode_handle *ports, *port;
> > > > +	int port_num;
> > > > +	int ret;
> > > > +
> > > > +	ports = device_get_named_child_node(priv->dev, "ports");
> > > > +	if (!ports) {
> > > > +		dev_info(priv->dev, "No ports node specified in device tree!\n");
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	fwnode_for_each_child_node(ports, port) {
> > > > +		struct fwnode_handle *phy_node, *reg_port_node = port;
> > > > +
> > > > +		phy_node = fwnode_find_reference(port, "phy-handle", 0);
> > > > +		if (!IS_ERR(phy_node))
> > > > +			reg_port_node = phy_node;
> > > 
> > > I don't understand this bit. Why are you looking at the phy-handle?
> > > 
> > > > +
> > > > +		if (fwnode_property_read_u32(reg_port_node, "reg", &port_num))
> > > > +			continue;
> > > 
> > > I would of expect port, not reg_port_node. I'm missing something
> > > here....
> > > 
> > 
> > It's really not to implement ugly things like "reg - 1"
> > 
> > On qca8k the port index goes from 0 to 6.
> > 0 is cpu port 1
> > 1 is port0 at mdio reg 0
> > 2 is port1 at mdio reg 1
> > ...
> > 6 is cpu port 2
> > 
> > Each port have a phy-handle that refer to a phy node with the correct
> > reg and that reflect the correct port index.
> > 
> > Tell me if this looks wrong, for qca8k we have qca8k_port_to_phy() and
> > at times we introduced the mdio thing to describe the port - 1 directly
> > in DT. If needed I can drop the additional fwnode and use this function
> > but I would love to use what is defined in DT thatn a simple - 1.
> 
> This comes back to the off list discussion earlier today. What you
> actually have here are MAC LEDs, not PHY LEDs. They are implemented in
> the MAC, not the PHY. To the end user, it should not matter, they
> blink when you would expect.
> 
> So your addressing should be based around the MAC port number, not the
> PHY.

Ok will drop this.

> 
> Also, at the moment, all we are adding are a bunch of LEDs. There is
> no link to a netdev at this point. At least, i don't see one. Be once
> we start using ledtrig-netdev we will need that link to a netdev. Take
> a look in my git tree at the last four patch. They add an additional
> call to get the device an LED is attached to.
> 

No currently we have no link for netdev, hence we are setting keep and
not setting a default trigger in DT.
Just checked them, interesting concept, guess we can think of something
also for the interval setting. That would effectively make all the
setting of the trigger set. Just my concern is that they may be too much
specific to netdev trigger and may be problematic for other kind of hw
control. (one main argument that was made for this feature was that some
stuff were too much specific and actually not that generic)

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

> Just checked them, interesting concept, guess we can think of something
> also for the interval setting. That would effectively make all the
> setting of the trigger set. Just my concern is that they may be too much
> specific to netdev trigger and may be problematic for other kind of hw
> control. (one main argument that was made for this feature was that some
> stuff were too much specific and actually not that generic)

I deliberately made this API return a struct device, not a struct
net_device. That should keep it generic. The LED could then be
attached to an disk device, an mtd device, or a tty device, each of
which have an ledtrig-*.c file.

      Andrew

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[Andrew Lunn]

> Just checked them, interesting concept, guess we can think of something
> also for the interval setting. That would effectively make all the
> setting of the trigger set. Just my concern is that they may be too much
> specific to netdev trigger and may be problematic for other kind of hw
> control. (one main argument that was made for this feature was that some
> stuff were too much specific and actually not that generic)

I deliberately made this API return a struct device, not a struct
net_device. That should keep it generic. The LED could then be
attached to an disk device, an mtd device, or a tty device, each of
which have an ledtrig-*.c file.

      Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[kernel test robot]

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/net-dsa-qca8k-add-LEDs-basic-support/20230308-063832
patch link:    https://lore.kernel.org/r/20230307170046.28917-2-ansuelsmth%40gmail.com
patch subject: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20230317/202303170529.8ag9rmM4-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ce1977c679b8737815636b72f4e65c2de59e8f7d
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Marangi/net-dsa-qca8k-add-LEDs-basic-support/20230308-063832
        git checkout ce1977c679b8737815636b72f4e65c2de59e8f7d
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/dsa/qca/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303170529.8ag9rmM4-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/dsa/qca/qca8k-leds.c:171:1: error: redefinition of 'qca8k_setup_led_ctrl'
     171 | qca8k_setup_led_ctrl(struct qca8k_priv *priv)
         | ^~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/dsa/qca/qca8k-leds.c:5:
   drivers/net/dsa/qca/qca8k.h:577:19: note: previous definition of 'qca8k_setup_led_ctrl' with type 'int(struct qca8k_priv *)'
     577 | static inline int qca8k_setup_led_ctrl(struct qca8k_priv *priv)
         |                   ^~~~~~~~~~~~~~~~~~~~


vim +/qca8k_setup_led_ctrl +171 drivers/net/dsa/qca/qca8k-leds.c

   169	
   170	int
 > 171	qca8k_setup_led_ctrl(struct qca8k_priv *priv)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support

[kernel test robot]

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/net-dsa-qca8k-add-LEDs-basic-support/20230308-063832
patch link:    https://lore.kernel.org/r/20230307170046.28917-2-ansuelsmth%40gmail.com
patch subject: [net-next PATCH 01/11] net: dsa: qca8k: add LEDs basic support
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20230317/202303170529.8ag9rmM4-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ce1977c679b8737815636b72f4e65c2de59e8f7d
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Marangi/net-dsa-qca8k-add-LEDs-basic-support/20230308-063832
        git checkout ce1977c679b8737815636b72f4e65c2de59e8f7d
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/dsa/qca/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303170529.8ag9rmM4-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/dsa/qca/qca8k-leds.c:171:1: error: redefinition of 'qca8k_setup_led_ctrl'
     171 | qca8k_setup_led_ctrl(struct qca8k_priv *priv)
         | ^~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/dsa/qca/qca8k-leds.c:5:
   drivers/net/dsa/qca/qca8k.h:577:19: note: previous definition of 'qca8k_setup_led_ctrl' with type 'int(struct qca8k_priv *)'
     577 | static inline int qca8k_setup_led_ctrl(struct qca8k_priv *priv)
         |                   ^~~~~~~~~~~~~~~~~~~~


vim +/qca8k_setup_led_ctrl +171 drivers/net/dsa/qca/qca8k-leds.c

   169	
   170	int
 > 171	qca8k_setup_led_ctrl(struct qca8k_priv *priv)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 02/11] net: dsa: qca8k: add LEDs blink_set() support

[Christian Marangi]

Add LEDs blink_set() support to qca8k Switch Family.
These LEDs support hw accellerated blinking at a fixed rate
of 4Hz.

Reject any other value since not supported by the LEDs switch.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca/qca8k-leds.c | 38 ++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index 079f1b423f9a..821ffa1f5b5d 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -98,6 +98,43 @@ qca8k_cled_brightness_get(struct led_classdev *ldev)
 	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);
+	u32 mask, val = QCA8K_LED_ALWAYS_BLINK_4HZ;
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 125;
+		*delay_off = 125;
+	}
+
+	if (*delay_on != 125 || *delay_off != 125) {
+		/* The hardware only supports blinking at 4Hz. Fall back
+		 * to software implementation in other cases.
+		 */
+		return -EINVAL;
+	}
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		mask = QCA8K_LED_PATTERN_EN_MASK;
+		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		mask = QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	regmap_update_bits(priv->regmap, reg_info.reg, mask << reg_info.shift,
+			   val << reg_info.shift);
+
+	return 0;
+}
+
 static int
 qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
 {
@@ -155,6 +192,7 @@ qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int p
 		port_led->cdev.max_brightness = 1;
 		port_led->cdev.brightness_set_blocking = qca8k_cled_brightness_set_blocking;
 		port_led->cdev.brightness_get = qca8k_cled_brightness_get;
+		port_led->cdev.blink_set = qca8k_cled_blink_set;
 		init_data.default_label = ":port";
 		init_data.devicename = "qca8k";
 		init_data.fwnode = led;
-- 
2.39.2

[net-next PATCH 02/11] net: dsa: qca8k: add LEDs blink_set() support

[Christian Marangi]

Add LEDs blink_set() support to qca8k Switch Family.
These LEDs support hw accellerated blinking at a fixed rate
of 4Hz.

Reject any other value since not supported by the LEDs switch.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca/qca8k-leds.c | 38 ++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index 079f1b423f9a..821ffa1f5b5d 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -98,6 +98,43 @@ qca8k_cled_brightness_get(struct led_classdev *ldev)
 	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);
+	u32 mask, val = QCA8K_LED_ALWAYS_BLINK_4HZ;
+	struct qca8k_led_pattern_en reg_info;
+	struct qca8k_priv *priv = led->priv;
+
+	if (*delay_on == 0 && *delay_off == 0) {
+		*delay_on = 125;
+		*delay_off = 125;
+	}
+
+	if (*delay_on != 125 || *delay_off != 125) {
+		/* The hardware only supports blinking at 4Hz. Fall back
+		 * to software implementation in other cases.
+		 */
+		return -EINVAL;
+	}
+
+	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+
+	if (led->port_num == 0 || led->port_num == 4) {
+		mask = QCA8K_LED_PATTERN_EN_MASK;
+		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
+	} else {
+		mask = QCA8K_LED_PHY123_PATTERN_EN_MASK;
+	}
+
+	regmap_update_bits(priv->regmap, reg_info.reg, mask << reg_info.shift,
+			   val << reg_info.shift);
+
+	return 0;
+}
+
 static int
 qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int port_num)
 {
@@ -155,6 +192,7 @@ qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int p
 		port_led->cdev.max_brightness = 1;
 		port_led->cdev.brightness_set_blocking = qca8k_cled_brightness_set_blocking;
 		port_led->cdev.brightness_get = qca8k_cled_brightness_get;
+		port_led->cdev.blink_set = qca8k_cled_blink_set;
 		init_data.default_label = ":port";
 		init_data.devicename = "qca8k";
 		init_data.fwnode = led;
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 02/11] net: dsa: qca8k: add LEDs blink_set() support

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:37PM +0100, Christian Marangi wrote:
> Add LEDs blink_set() support to qca8k Switch Family.
> These LEDs support hw accellerated blinking at a fixed rate
> of 4Hz.
> 
> Reject any other value since not supported by the LEDs switch.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next PATCH 02/11] net: dsa: qca8k: add LEDs blink_set() support

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:37PM +0100, Christian Marangi wrote:
> Add LEDs blink_set() support to qca8k Switch Family.
> These LEDs support hw accellerated blinking at a fixed rate
> of 4Hz.
> 
> Reject any other value since not supported by the LEDs switch.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Define common binding parsing for all PHY drivers with LEDs using
phylib. Parse the DT as part of the phy_probe and add LEDs to the
linux LED class infrastructure. For the moment, provide a dummy
brightness function, which will later be replaced with a call into the
PHY driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 89 ++++++++++++++++++++++++++++++++++++
 include/linux/phy.h          | 16 +++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9ba8f973f26f..8acade42615c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -19,10 +19,12 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/mii.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_led_triggers.h>
@@ -658,6 +660,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 	device_initialize(&mdiodev->dev);
 
 	dev->state = PHY_DOWN;
+	INIT_LIST_HEAD(&dev->led_list);
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
@@ -2964,6 +2967,85 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+/* Dummy implementation until calls into PHY driver are added */
+static int phy_led_set_brightness(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	return 0;
+}
+
+static int of_phy_led(struct phy_device *phydev,
+		      struct device_node *led)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_init_data init_data = {};
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+	int err;
+
+	phyled = devm_kzalloc(dev, sizeof(*phyled), GFP_KERNEL);
+	if (!phyled)
+		return -ENOMEM;
+
+	phyled->phydev = phydev;
+	cdev = &phyled->led_cdev;
+	INIT_LIST_HEAD(&phyled->led_list);
+
+	err = of_property_read_u32(led, "reg", &phyled->index);
+	if (err)
+		return err;
+
+	cdev->brightness_set_blocking = phy_led_set_brightness;
+	cdev->max_brightness = 1;
+	init_data.devicename = dev_name(&phydev->mdio.dev);
+	init_data.fwnode = of_fwnode_handle(led);
+
+	err = devm_led_classdev_register_ext(dev, cdev, &init_data);
+	if (err)
+		return err;
+
+	list_add(&phyled->led_list, &phydev->led_list);
+
+	return 0;
+}
+
+static int of_phy_leds(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct device_node *leds, *led;
+	int err;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (!node)
+		return 0;
+
+	leds = of_get_child_by_name(node, "leds");
+	if (!leds)
+		return 0;
+
+	for_each_available_child_of_node(leds, led) {
+		err = of_phy_led(phydev, led);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void phy_leds_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+
+	list_for_each_entry(phyled, &phydev->led_list, led_list) {
+		cdev = &phyled->led_cdev;
+		devm_led_classdev_unregister(dev, cdev);
+	}
+}
+
 /**
  * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
  * @fwnode: pointer to the mdio_device's fwnode
@@ -3142,6 +3224,11 @@ static int phy_probe(struct device *dev)
 	/* Set the state to READY by default */
 	phydev->state = PHY_READY;
 
+	/* Get the LEDs from the device tree, and instantiate standard
+	 * LEDs for them.
+	 */
+	of_phy_leds(phydev);
+
 out:
 	/* Assert the reset signal */
 	if (err)
@@ -3156,6 +3243,8 @@ static int phy_remove(struct device *dev)
 {
 	struct phy_device *phydev = to_phy_device(dev);
 
+	phy_leds_remove(phydev);
+
 	cancel_delayed_work_sync(&phydev->state_queue);
 
 	mutex_lock(&phydev->lock);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fbeba4fee8d4..1b1efe120f0f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -14,6 +14,7 @@
 #include <linux/compiler.h>
 #include <linux/spinlock.h>
 #include <linux/ethtool.h>
+#include <linux/leds.h>
 #include <linux/linkmode.h>
 #include <linux/netlink.h>
 #include <linux/mdio.h>
@@ -595,6 +596,7 @@ struct macsec_ops;
  * @phy_num_led_triggers: Number of triggers in @phy_led_triggers
  * @led_link_trigger: LED trigger for link up/down
  * @last_triggered: last LED trigger for link speed
+ * @led_list: list of PHY LED structures
  * @master_slave_set: User requested master/slave configuration
  * @master_slave_get: Current master/slave advertisement
  * @master_slave_state: Current master/slave configuration
@@ -690,6 +692,7 @@ struct phy_device {
 
 	struct phy_led_trigger *led_link_trigger;
 #endif
+	struct list_head led_list;
 
 	/*
 	 * Interrupt number for this PHY
@@ -825,6 +828,19 @@ struct phy_plca_status {
 	bool pst;
 };
 
+/* phy_led: An LED driven by the PHY
+ *
+ * phydev: Pointer to the PHY this LED belongs to
+ * led_cdev: Standard LED class structure
+ * index: Number of the LED
+ */
+struct phy_led {
+	struct list_head led_list;
+	struct phy_device *phydev;
+	struct led_classdev led_cdev;
+	u32 index;
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
-- 
2.39.2

[net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Define common binding parsing for all PHY drivers with LEDs using
phylib. Parse the DT as part of the phy_probe and add LEDs to the
linux LED class infrastructure. For the moment, provide a dummy
brightness function, which will later be replaced with a call into the
PHY driver.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 89 ++++++++++++++++++++++++++++++++++++
 include/linux/phy.h          | 16 +++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9ba8f973f26f..8acade42615c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -19,10 +19,12 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/mdio.h>
 #include <linux/mii.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/phy_led_triggers.h>
@@ -658,6 +660,7 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, u32 phy_id,
 	device_initialize(&mdiodev->dev);
 
 	dev->state = PHY_DOWN;
+	INIT_LIST_HEAD(&dev->led_list);
 
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
@@ -2964,6 +2967,85 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+/* Dummy implementation until calls into PHY driver are added */
+static int phy_led_set_brightness(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	return 0;
+}
+
+static int of_phy_led(struct phy_device *phydev,
+		      struct device_node *led)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_init_data init_data = {};
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+	int err;
+
+	phyled = devm_kzalloc(dev, sizeof(*phyled), GFP_KERNEL);
+	if (!phyled)
+		return -ENOMEM;
+
+	phyled->phydev = phydev;
+	cdev = &phyled->led_cdev;
+	INIT_LIST_HEAD(&phyled->led_list);
+
+	err = of_property_read_u32(led, "reg", &phyled->index);
+	if (err)
+		return err;
+
+	cdev->brightness_set_blocking = phy_led_set_brightness;
+	cdev->max_brightness = 1;
+	init_data.devicename = dev_name(&phydev->mdio.dev);
+	init_data.fwnode = of_fwnode_handle(led);
+
+	err = devm_led_classdev_register_ext(dev, cdev, &init_data);
+	if (err)
+		return err;
+
+	list_add(&phyled->led_list, &phydev->led_list);
+
+	return 0;
+}
+
+static int of_phy_leds(struct phy_device *phydev)
+{
+	struct device_node *node = phydev->mdio.dev.of_node;
+	struct device_node *leds, *led;
+	int err;
+
+	if (!IS_ENABLED(CONFIG_OF_MDIO))
+		return 0;
+
+	if (!node)
+		return 0;
+
+	leds = of_get_child_by_name(node, "leds");
+	if (!leds)
+		return 0;
+
+	for_each_available_child_of_node(leds, led) {
+		err = of_phy_led(phydev, led);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void phy_leds_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct led_classdev *cdev;
+	struct phy_led *phyled;
+
+	list_for_each_entry(phyled, &phydev->led_list, led_list) {
+		cdev = &phyled->led_cdev;
+		devm_led_classdev_unregister(dev, cdev);
+	}
+}
+
 /**
  * fwnode_mdio_find_device - Given a fwnode, find the mdio_device
  * @fwnode: pointer to the mdio_device's fwnode
@@ -3142,6 +3224,11 @@ static int phy_probe(struct device *dev)
 	/* Set the state to READY by default */
 	phydev->state = PHY_READY;
 
+	/* Get the LEDs from the device tree, and instantiate standard
+	 * LEDs for them.
+	 */
+	of_phy_leds(phydev);
+
 out:
 	/* Assert the reset signal */
 	if (err)
@@ -3156,6 +3243,8 @@ static int phy_remove(struct device *dev)
 {
 	struct phy_device *phydev = to_phy_device(dev);
 
+	phy_leds_remove(phydev);
+
 	cancel_delayed_work_sync(&phydev->state_queue);
 
 	mutex_lock(&phydev->lock);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fbeba4fee8d4..1b1efe120f0f 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -14,6 +14,7 @@
 #include <linux/compiler.h>
 #include <linux/spinlock.h>
 #include <linux/ethtool.h>
+#include <linux/leds.h>
 #include <linux/linkmode.h>
 #include <linux/netlink.h>
 #include <linux/mdio.h>
@@ -595,6 +596,7 @@ struct macsec_ops;
  * @phy_num_led_triggers: Number of triggers in @phy_led_triggers
  * @led_link_trigger: LED trigger for link up/down
  * @last_triggered: last LED trigger for link speed
+ * @led_list: list of PHY LED structures
  * @master_slave_set: User requested master/slave configuration
  * @master_slave_get: Current master/slave advertisement
  * @master_slave_state: Current master/slave configuration
@@ -690,6 +692,7 @@ struct phy_device {
 
 	struct phy_led_trigger *led_link_trigger;
 #endif
+	struct list_head led_list;
 
 	/*
 	 * Interrupt number for this PHY
@@ -825,6 +828,19 @@ struct phy_plca_status {
 	bool pst;
 };
 
+/* phy_led: An LED driven by the PHY
+ *
+ * phydev: Pointer to the PHY this LED belongs to
+ * led_cdev: Standard LED class structure
+ * index: Number of the LED
+ */
+struct phy_led {
+	struct list_head led_list;
+	struct phy_device *phydev;
+	struct led_classdev led_cdev;
+	u32 index;
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Define common binding parsing for all PHY drivers with LEDs using
> phylib. Parse the DT as part of the phy_probe and add LEDs to the
> linux LED class infrastructure. For the moment, provide a dummy
> brightness function, which will later be replaced with a call into the
> PHY driver.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Hi Christian

Since you are submitting this, you need to add your own Signed-off-by:
after mine.

      Andrew

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> Define common binding parsing for all PHY drivers with LEDs using
> phylib. Parse the DT as part of the phy_probe and add LEDs to the
> linux LED class infrastructure. For the moment, provide a dummy
> brightness function, which will later be replaced with a call into the
> PHY driver.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Hi Christian

Since you are submitting this, you need to add your own Signed-off-by:
after mine.

      Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > Define common binding parsing for all PHY drivers with LEDs using
> > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > linux LED class infrastructure. For the moment, provide a dummy
> > brightness function, which will later be replaced with a call into the
> > PHY driver.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Hi Christian
> 
> Since you are submitting this, you need to add your own Signed-off-by:
> after mine.
> 

Tought it was needed only for patch where I have put any change. (case
of 2 patch in this series where there was a whitespace error and had to
change a binding)

Think I need do to this for every other patch right?

-- 
	Ansuel

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > Define common binding parsing for all PHY drivers with LEDs using
> > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > linux LED class infrastructure. For the moment, provide a dummy
> > brightness function, which will later be replaced with a call into the
> > PHY driver.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> 
> Hi Christian
> 
> Since you are submitting this, you need to add your own Signed-off-by:
> after mine.
> 

Tought it was needed only for patch where I have put any change. (case
of 2 patch in this series where there was a whitespace error and had to
change a binding)

Think I need do to this for every other patch right?

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Andrew Lunn]

On Tue, Mar 07, 2023 at 07:00:08PM +0100, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> > On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > 
> > > Define common binding parsing for all PHY drivers with LEDs using
> > > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > > linux LED class infrastructure. For the moment, provide a dummy
> > > brightness function, which will later be replaced with a call into the
> > > PHY driver.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > Hi Christian
> > 
> > Since you are submitting this, you need to add your own Signed-off-by:
> > after mine.
> > 
> 
> Tought it was needed only for patch where I have put any change. (case
> of 2 patch in this series where there was a whitespace error and had to
> change a binding)
> 
> Think I need do to this for every other patch right?

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

says:

   Any further SoBs (Signed-off-by:’s) following the author’s SoB are
   from people handling and transporting the patch, but were not
   involved in its development. SoB chains should reflect the real
   route a patch took as it was propagated to the maintainers and
   ultimately to Linus, with the first SoB entry signalling primary
   authorship of a single author.

So yes, you need to add your Signed-off-by to all my patches,
independent of if you make changes or not.

	    Andrew

Re: [net-next PATCH 03/11] net: phy: Add a binding for PHY LEDs

[Andrew Lunn]

On Tue, Mar 07, 2023 at 07:00:08PM +0100, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 12:17:51AM +0100, Andrew Lunn wrote:
> > On Tue, Mar 07, 2023 at 06:00:38PM +0100, Christian Marangi wrote:
> > > From: Andrew Lunn <andrew@lunn.ch>
> > > 
> > > Define common binding parsing for all PHY drivers with LEDs using
> > > phylib. Parse the DT as part of the phy_probe and add LEDs to the
> > > linux LED class infrastructure. For the moment, provide a dummy
> > > brightness function, which will later be replaced with a call into the
> > > PHY driver.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > Hi Christian
> > 
> > Since you are submitting this, you need to add your own Signed-off-by:
> > after mine.
> > 
> 
> Tought it was needed only for patch where I have put any change. (case
> of 2 patch in this series where there was a whitespace error and had to
> change a binding)
> 
> Think I need do to this for every other patch right?

https://www.kernel.org/doc/html/latest/process/submitting-patches.html

says:

   Any further SoBs (Signed-off-by:’s) following the author’s SoB are
   from people handling and transporting the patch, but were not
   involved in its development. SoB chains should reflect the real
   route a patch took as it was propagated to the maintainers and
   ultimately to Linus, with the first SoB entry signalling primary
   authorship of a single author.

So yes, you need to add your Signed-off-by to all my patches,
independent of if you make changes or not.

	    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 04/11] net: phy: phy_device: Call into the PHY driver to set LED brightness.

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Linux LEDs can be software controlled via the brightness file in /sys.
LED drivers need to implement a brightness_set function which the core
will call. Implement an intermediary in phy_device, which will call
into the phy driver if it implements the necessary function.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 14 +++++++++++---
 include/linux/phy.h          |  9 +++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8acade42615c..e4df4fcb6b05 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2967,11 +2967,18 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
-/* Dummy implementation until calls into PHY driver are added */
 static int phy_led_set_brightness(struct led_classdev *led_cdev,
 				  enum led_brightness value)
 {
-	return 0;
+	struct phy_led *phyled = to_phy_led(led_cdev);
+	struct phy_device *phydev = phyled->phydev;
+	int err;
+
+	mutex_lock(&phydev->lock);
+	err = phydev->drv->led_brightness_set(phydev, phyled->index, value);
+	mutex_unlock(&phydev->lock);
+
+	return err;
 }
 
 static int of_phy_led(struct phy_device *phydev,
@@ -2995,7 +3002,8 @@ static int of_phy_led(struct phy_device *phydev,
 	if (err)
 		return err;
 
-	cdev->brightness_set_blocking = phy_led_set_brightness;
+	if (phydev->drv->led_brightness_set)
+		cdev->brightness_set_blocking = phy_led_set_brightness;
 	cdev->max_brightness = 1;
 	init_data.devicename = dev_name(&phydev->mdio.dev);
 	init_data.fwnode = of_fwnode_handle(led);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 1b1efe120f0f..83d3ed7485e0 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -841,6 +841,8 @@ struct phy_led {
 	u32 index;
 };
 
+#define to_phy_led(d) container_of(d, struct phy_led, led_cdev)
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
@@ -1063,6 +1065,13 @@ struct phy_driver {
 	/** @get_plca_status: Return the current PLCA status info */
 	int (*get_plca_status)(struct phy_device *dev,
 			       struct phy_plca_status *plca_st);
+
+	/* Set a PHY LED brightness. Index indicates which of the PHYs
+	 * led should be set. Value follows the standard LED class meaning,
+	 * e.g. LED_OFF, LED_HALF, LED_FULL.
+	 */
+	int (*led_brightness_set)(struct phy_device *dev,
+				  u32 index, enum led_brightness value);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
-- 
2.39.2

[net-next PATCH 04/11] net: phy: phy_device: Call into the PHY driver to set LED brightness.

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Linux LEDs can be software controlled via the brightness file in /sys.
LED drivers need to implement a brightness_set function which the core
will call. Implement an intermediary in phy_device, which will call
into the phy driver if it implements the necessary function.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 14 +++++++++++---
 include/linux/phy.h          |  9 +++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8acade42615c..e4df4fcb6b05 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2967,11 +2967,18 @@ static bool phy_drv_supports_irq(struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
-/* Dummy implementation until calls into PHY driver are added */
 static int phy_led_set_brightness(struct led_classdev *led_cdev,
 				  enum led_brightness value)
 {
-	return 0;
+	struct phy_led *phyled = to_phy_led(led_cdev);
+	struct phy_device *phydev = phyled->phydev;
+	int err;
+
+	mutex_lock(&phydev->lock);
+	err = phydev->drv->led_brightness_set(phydev, phyled->index, value);
+	mutex_unlock(&phydev->lock);
+
+	return err;
 }
 
 static int of_phy_led(struct phy_device *phydev,
@@ -2995,7 +3002,8 @@ static int of_phy_led(struct phy_device *phydev,
 	if (err)
 		return err;
 
-	cdev->brightness_set_blocking = phy_led_set_brightness;
+	if (phydev->drv->led_brightness_set)
+		cdev->brightness_set_blocking = phy_led_set_brightness;
 	cdev->max_brightness = 1;
 	init_data.devicename = dev_name(&phydev->mdio.dev);
 	init_data.fwnode = of_fwnode_handle(led);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 1b1efe120f0f..83d3ed7485e0 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -841,6 +841,8 @@ struct phy_led {
 	u32 index;
 };
 
+#define to_phy_led(d) container_of(d, struct phy_led, led_cdev)
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
@@ -1063,6 +1065,13 @@ struct phy_driver {
 	/** @get_plca_status: Return the current PLCA status info */
 	int (*get_plca_status)(struct phy_device *dev,
 			       struct phy_plca_status *plca_st);
+
+	/* Set a PHY LED brightness. Index indicates which of the PHYs
+	 * led should be set. Value follows the standard LED class meaning,
+	 * e.g. LED_OFF, LED_HALF, LED_FULL.
+	 */
+	int (*led_brightness_set)(struct phy_device *dev,
+				  u32 index, enum led_brightness value);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 05/11] net: phy: marvell: Add software control of the LEDs

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Add a brightness function, so the LEDs can be controlled from
software using the standard Linux LED infrastructure.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 45 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0d706ee266af..e44a4a26346a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -144,11 +144,13 @@
 /* WOL Event Interrupt Enable */
 #define MII_88E1318S_PHY_CSIER_WOL_EIE			BIT(7)
 
-/* LED Timer Control Register */
-#define MII_88E1318S_PHY_LED_TCR			0x12
-#define MII_88E1318S_PHY_LED_TCR_FORCE_INT		BIT(15)
-#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE		BIT(7)
-#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW		BIT(11)
+#define MII_88E1318S_PHY_LED_FUNC		0x10
+#define MII_88E1318S_PHY_LED_FUNC_OFF		(0x8)
+#define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
+#define MII_88E1318S_PHY_LED_TCR		0x12
+#define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
+#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
+#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW	BIT(11)
 
 /* Magic Packet MAC address registers */
 #define MII_88E1318S_PHY_MAGIC_PACKET_WORD2		0x17
@@ -2832,6 +2834,34 @@ static int marvell_hwmon_probe(struct phy_device *phydev)
 }
 #endif
 
+static int m88e1318_led_brightness_set(struct phy_device *phydev,
+				       u32 index, enum led_brightness value)
+{
+	u16 reg;
+
+	reg = phy_read_paged(phydev, MII_MARVELL_LED_PAGE,
+			     MII_88E1318S_PHY_LED_FUNC);
+	if (reg < 0)
+		return reg;
+
+	switch (index) {
+	case 0:
+	case 1:
+	case 2:
+		reg &= ~(0xf << (4 * index));
+		if (value == LED_OFF)
+			reg |= MII_88E1318S_PHY_LED_FUNC_OFF << (4 * index);
+		else
+			reg |= MII_88E1318S_PHY_LED_FUNC_ON << (4 * index);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+			       MII_88E1318S_PHY_LED_FUNC, reg);
+}
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -3081,6 +3111,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_sset_count = marvell_get_sset_count,
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1145,
@@ -3187,6 +3218,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1540,
@@ -3213,6 +3245,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1545,
@@ -3239,6 +3272,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E3016,
@@ -3380,6 +3414,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_stats = marvell_get_stats,
 		.get_tunable = m88e1540_get_tunable,
 		.set_tunable = m88e1540_set_tunable,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 };
 
-- 
2.39.2

[net-next PATCH 05/11] net: phy: marvell: Add software control of the LEDs

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Add a brightness function, so the LEDs can be controlled from
software using the standard Linux LED infrastructure.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 45 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0d706ee266af..e44a4a26346a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -144,11 +144,13 @@
 /* WOL Event Interrupt Enable */
 #define MII_88E1318S_PHY_CSIER_WOL_EIE			BIT(7)
 
-/* LED Timer Control Register */
-#define MII_88E1318S_PHY_LED_TCR			0x12
-#define MII_88E1318S_PHY_LED_TCR_FORCE_INT		BIT(15)
-#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE		BIT(7)
-#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW		BIT(11)
+#define MII_88E1318S_PHY_LED_FUNC		0x10
+#define MII_88E1318S_PHY_LED_FUNC_OFF		(0x8)
+#define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
+#define MII_88E1318S_PHY_LED_TCR		0x12
+#define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
+#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
+#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW	BIT(11)
 
 /* Magic Packet MAC address registers */
 #define MII_88E1318S_PHY_MAGIC_PACKET_WORD2		0x17
@@ -2832,6 +2834,34 @@ static int marvell_hwmon_probe(struct phy_device *phydev)
 }
 #endif
 
+static int m88e1318_led_brightness_set(struct phy_device *phydev,
+				       u32 index, enum led_brightness value)
+{
+	u16 reg;
+
+	reg = phy_read_paged(phydev, MII_MARVELL_LED_PAGE,
+			     MII_88E1318S_PHY_LED_FUNC);
+	if (reg < 0)
+		return reg;
+
+	switch (index) {
+	case 0:
+	case 1:
+	case 2:
+		reg &= ~(0xf << (4 * index));
+		if (value == LED_OFF)
+			reg |= MII_88E1318S_PHY_LED_FUNC_OFF << (4 * index);
+		else
+			reg |= MII_88E1318S_PHY_LED_FUNC_ON << (4 * index);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+			       MII_88E1318S_PHY_LED_FUNC, reg);
+}
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -3081,6 +3111,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_sset_count = marvell_get_sset_count,
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1145,
@@ -3187,6 +3218,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1540,
@@ -3213,6 +3245,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1545,
@@ -3239,6 +3272,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_start = marvell_vct7_cable_test_start,
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E3016,
@@ -3380,6 +3414,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_stats = marvell_get_stats,
 		.get_tunable = m88e1540_get_tunable,
 		.set_tunable = m88e1540_set_tunable,
+		.led_brightness_set = m88e1318_led_brightness_set,
 	},
 };
 
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 06/11] net: phy: phy_device: Call into the PHY driver to set LED blinking.

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Linux LEDs can be requested to perform hardware accelerated
blinking. Pass this to the PHY driver, if it implements the op.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
 include/linux/phy.h          |  8 ++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e4df4fcb6b05..ae8ec721353d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2981,6 +2981,22 @@ static int phy_led_set_brightness(struct led_classdev *led_cdev,
 	return err;
 }
 
+static int phy_led_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off)
+{
+	struct phy_led *phyled = to_phy_led(led_cdev);
+	struct phy_device *phydev = phyled->phydev;
+	int err;
+
+	mutex_lock(&phydev->lock);
+	err = phydev->drv->led_blink_set(phydev, phyled->index,
+					 delay_on, delay_off);
+	mutex_unlock(&phydev->lock);
+
+	return err;
+}
+
 static int of_phy_led(struct phy_device *phydev,
 		      struct device_node *led)
 {
@@ -3004,6 +3020,8 @@ static int of_phy_led(struct phy_device *phydev,
 
 	if (phydev->drv->led_brightness_set)
 		cdev->brightness_set_blocking = phy_led_set_brightness;
+	if (phydev->drv->led_blink_set)
+		cdev->blink_set = phy_led_blink_set;
 	cdev->max_brightness = 1;
 	init_data.devicename = dev_name(&phydev->mdio.dev);
 	init_data.fwnode = of_fwnode_handle(led);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 83d3ed7485e0..b1ac3c8a97e6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1072,6 +1072,14 @@ struct phy_driver {
 	 */
 	int (*led_brightness_set)(struct phy_device *dev,
 				  u32 index, enum led_brightness value);
+	/* Activate hardware accelerated blink, delays are in milliseconds
+	 * and if both are zero then a sensible default should be chosen.
+	 * The call should adjust the timings in that case and if it can't
+	 * match the values specified exactly.
+	 */
+	int (*led_blink_set)(struct phy_device *dev, u32 index,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
-- 
2.39.2

[net-next PATCH 06/11] net: phy: phy_device: Call into the PHY driver to set LED blinking.

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

Linux LEDs can be requested to perform hardware accelerated
blinking. Pass this to the PHY driver, if it implements the op.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
 include/linux/phy.h          |  8 ++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e4df4fcb6b05..ae8ec721353d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2981,6 +2981,22 @@ static int phy_led_set_brightness(struct led_classdev *led_cdev,
 	return err;
 }
 
+static int phy_led_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off)
+{
+	struct phy_led *phyled = to_phy_led(led_cdev);
+	struct phy_device *phydev = phyled->phydev;
+	int err;
+
+	mutex_lock(&phydev->lock);
+	err = phydev->drv->led_blink_set(phydev, phyled->index,
+					 delay_on, delay_off);
+	mutex_unlock(&phydev->lock);
+
+	return err;
+}
+
 static int of_phy_led(struct phy_device *phydev,
 		      struct device_node *led)
 {
@@ -3004,6 +3020,8 @@ static int of_phy_led(struct phy_device *phydev,
 
 	if (phydev->drv->led_brightness_set)
 		cdev->brightness_set_blocking = phy_led_set_brightness;
+	if (phydev->drv->led_blink_set)
+		cdev->blink_set = phy_led_blink_set;
 	cdev->max_brightness = 1;
 	init_data.devicename = dev_name(&phydev->mdio.dev);
 	init_data.fwnode = of_fwnode_handle(led);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 83d3ed7485e0..b1ac3c8a97e6 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1072,6 +1072,14 @@ struct phy_driver {
 	 */
 	int (*led_brightness_set)(struct phy_device *dev,
 				  u32 index, enum led_brightness value);
+	/* Activate hardware accelerated blink, delays are in milliseconds
+	 * and if both are zero then a sensible default should be chosen.
+	 * The call should adjust the timings in that case and if it can't
+	 * match the values specified exactly.
+	 */
+	int (*led_blink_set)(struct phy_device *dev, u32 index,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off);
 };
 #define to_phy_driver(d) container_of(to_mdio_common_driver(d),		\
 				      struct phy_driver, mdiodrv)
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 07/11] net: phy: marvell: Implement led_blink_set()

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

The Marvell PHY can blink the LEDs, simple on/off. All LEDs blink at
the same rate, and the reset default is 84ms per blink, which is
around 12Hz.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index e44a4a26346a..3252b15266e2 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -147,6 +147,8 @@
 #define MII_88E1318S_PHY_LED_FUNC		0x10
 #define MII_88E1318S_PHY_LED_FUNC_OFF		(0x8)
 #define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
+#define MII_88E1318S_PHY_LED_FUNC_HI_Z		(0xa)
+#define MII_88E1318S_PHY_LED_FUNC_BLINK		(0xb)
 #define MII_88E1318S_PHY_LED_TCR		0x12
 #define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
 #define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
@@ -2862,6 +2864,35 @@ static int m88e1318_led_brightness_set(struct phy_device *phydev,
 			       MII_88E1318S_PHY_LED_FUNC, reg);
 }
 
+static int m88e1318_led_blink_set(struct phy_device *phydev, u32 index,
+				  unsigned long *delay_on,
+				  unsigned long *delay_off)
+{
+	u16 reg;
+
+	reg = phy_read_paged(phydev, MII_MARVELL_LED_PAGE,
+			     MII_88E1318S_PHY_LED_FUNC);
+	if (reg < 0)
+		return reg;
+
+	switch (index) {
+	case 0:
+	case 1:
+	case 2:
+		reg &= ~(0xf << (4 * index));
+			reg |= MII_88E1318S_PHY_LED_FUNC_BLINK << (4 * index);
+			/* Reset default is 84ms */
+			*delay_on = 84 / 2;
+			*delay_off = 84 / 2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+			       MII_88E1318S_PHY_LED_FUNC, reg);
+}
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -3112,6 +3143,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1145,
@@ -3219,6 +3251,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1540,
@@ -3246,6 +3279,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1545,
@@ -3273,6 +3307,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E3016,
@@ -3415,6 +3450,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_tunable = m88e1540_get_tunable,
 		.set_tunable = m88e1540_set_tunable,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 };
 
-- 
2.39.2

[net-next PATCH 07/11] net: phy: marvell: Implement led_blink_set()

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

The Marvell PHY can blink the LEDs, simple on/off. All LEDs blink at
the same rate, and the reset default is 84ms per blink, which is
around 12Hz.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index e44a4a26346a..3252b15266e2 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -147,6 +147,8 @@
 #define MII_88E1318S_PHY_LED_FUNC		0x10
 #define MII_88E1318S_PHY_LED_FUNC_OFF		(0x8)
 #define MII_88E1318S_PHY_LED_FUNC_ON		(0x9)
+#define MII_88E1318S_PHY_LED_FUNC_HI_Z		(0xa)
+#define MII_88E1318S_PHY_LED_FUNC_BLINK		(0xb)
 #define MII_88E1318S_PHY_LED_TCR		0x12
 #define MII_88E1318S_PHY_LED_TCR_FORCE_INT	BIT(15)
 #define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE	BIT(7)
@@ -2862,6 +2864,35 @@ static int m88e1318_led_brightness_set(struct phy_device *phydev,
 			       MII_88E1318S_PHY_LED_FUNC, reg);
 }
 
+static int m88e1318_led_blink_set(struct phy_device *phydev, u32 index,
+				  unsigned long *delay_on,
+				  unsigned long *delay_off)
+{
+	u16 reg;
+
+	reg = phy_read_paged(phydev, MII_MARVELL_LED_PAGE,
+			     MII_88E1318S_PHY_LED_FUNC);
+	if (reg < 0)
+		return reg;
+
+	switch (index) {
+	case 0:
+	case 1:
+	case 2:
+		reg &= ~(0xf << (4 * index));
+			reg |= MII_88E1318S_PHY_LED_FUNC_BLINK << (4 * index);
+			/* Reset default is 84ms */
+			*delay_on = 84 / 2;
+			*delay_off = 84 / 2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return phy_write_paged(phydev, MII_MARVELL_LED_PAGE,
+			       MII_88E1318S_PHY_LED_FUNC, reg);
+}
+
 static int marvell_probe(struct phy_device *phydev)
 {
 	struct marvell_priv *priv;
@@ -3112,6 +3143,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1145,
@@ -3219,6 +3251,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1540,
@@ -3246,6 +3279,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1545,
@@ -3273,6 +3307,7 @@ static struct phy_driver marvell_drivers[] = {
 		.cable_test_tdr_start = marvell_vct5_cable_test_tdr_start,
 		.cable_test_get_status = marvell_vct7_cable_test_get_status,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E3016,
@@ -3415,6 +3450,7 @@ static struct phy_driver marvell_drivers[] = {
 		.get_tunable = m88e1540_get_tunable,
 		.set_tunable = m88e1540_set_tunable,
 		.led_brightness_set = m88e1318_led_brightness_set,
+		.led_blink_set = m88e1318_led_blink_set,
 	},
 };
 
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Christian Marangi]

Document support for LEDs node in dsa port.
Switch may support different LEDs that can be configured for different
operation like blinking on traffic event or port link.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 480120469953..f813e1f64f75 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -59,6 +59,13 @@ properties:
       - rtl8_4t
       - seville
 
+  leds:
+    type: object
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
 # CPU and DSA ports must have phylink-compatible link descriptions
 if:
   oneOf:
-- 
2.39.2

[net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Christian Marangi]

Document support for LEDs node in dsa port.
Switch may support different LEDs that can be configured for different
operation like blinking on traffic event or port link.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 480120469953..f813e1f64f75 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -59,6 +59,13 @@ properties:
       - rtl8_4t
       - seville
 
+  leds:
+    type: object
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
 # CPU and DSA ports must have phylink-compatible link descriptions
 if:
   oneOf:
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:43PM +0100, Christian Marangi wrote:
> Document support for LEDs node in dsa port.
> Switch may support different LEDs that can be configured for different
> operation like blinking on traffic event or port link.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..f813e1f64f75 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -59,6 +59,13 @@ properties:
>        - rtl8_4t
>        - seville
>  
> +  leds:
> +    type: object
> +
> +    patternProperties:
> +      '^led(@[a-f0-9]+)?$':
> +        $ref: /schemas/leds/common.yaml#

Please could you add a description here documenting that these are
LEDs the switch controls in its MACs. They are not LEDs in the
possibly integrated PHY, which the PHY driver controls.

We got this wrong, so i'm sure others will as well. So a bit of
documentation should help avoid this.

	Andrew

Re: [net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:43PM +0100, Christian Marangi wrote:
> Document support for LEDs node in dsa port.
> Switch may support different LEDs that can be configured for different
> operation like blinking on traffic event or port link.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..f813e1f64f75 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -59,6 +59,13 @@ properties:
>        - rtl8_4t
>        - seville
>  
> +  leds:
> +    type: object
> +
> +    patternProperties:
> +      '^led(@[a-f0-9]+)?$':
> +        $ref: /schemas/leds/common.yaml#

Please could you add a description here documenting that these are
LEDs the switch controls in its MACs. They are not LEDs in the
possibly integrated PHY, which the PHY driver controls.

We got this wrong, so i'm sure others will as well. So a bit of
documentation should help avoid this.

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in dsa port.
> Switch may support different LEDs that can be configured for different
> operation like blinking on traffic event or port link.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..f813e1f64f75 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -59,6 +59,13 @@ properties:
>        - rtl8_4t
>        - seville
>  
> +  leds:
> +    type: object

additionalProperties: false

Best regards,
Krzysztof

Re: [net-next PATCH 08/11] dt-bindings: net: dsa: dsa-port: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in dsa port.
> Switch may support different LEDs that can be configured for different
> operation like blinking on traffic event or port link.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/dsa/dsa-port.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..f813e1f64f75 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -59,6 +59,13 @@ properties:
>        - rtl8_4t
>        - seville
>  
> +  leds:
> +    type: object

additionalProperties: false

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

Add LEDs definition example for qca8k Switch Family to describe how they
should be defined for a correct usage.

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

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 389892592aac..866b3cc73216 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -18,6 +18,8 @@ description:
   PHY it is connected to. In this config, an internal mdio-bus is registered and
   the MDIO master is used for communication. Mixed external and internal
   mdio-bus configurations are not supported by the hardware.
+  Each phy have at least 3 LEDs connected and can be declared
+  using the standard LEDs structure.
 
 properties:
   compatible:
@@ -117,6 +119,7 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
 
     mdio {
         #address-cells = <1>;
@@ -226,6 +229,27 @@ examples:
                     label = "lan1";
                     phy-mode = "internal";
                     phy-handle = <&internal_phy_port1>;
+
+                    leds {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        led@0 {
+                            reg = <0>;
+                            color = <LED_COLOR_ID_WHITE>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            default-state = "keep";
+                        };
+
+                        led@1 {
+                            reg = <1>;
+                            color = <LED_COLOR_ID_AMBER>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            default-state = "keep";
+                        };
+                    };
                 };
 
                 port@2 {
-- 
2.39.2

[net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

Add LEDs definition example for qca8k Switch Family to describe how they
should be defined for a correct usage.

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

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 389892592aac..866b3cc73216 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -18,6 +18,8 @@ description:
   PHY it is connected to. In this config, an internal mdio-bus is registered and
   the MDIO master is used for communication. Mixed external and internal
   mdio-bus configurations are not supported by the hardware.
+  Each phy have at least 3 LEDs connected and can be declared
+  using the standard LEDs structure.
 
 properties:
   compatible:
@@ -117,6 +119,7 @@ unevaluatedProperties: false
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/leds/common.h>
 
     mdio {
         #address-cells = <1>;
@@ -226,6 +229,27 @@ examples:
                     label = "lan1";
                     phy-mode = "internal";
                     phy-handle = <&internal_phy_port1>;
+
+                    leds {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        led@0 {
+                            reg = <0>;
+                            color = <LED_COLOR_ID_WHITE>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            default-state = "keep";
+                        };
+
+                        led@1 {
+                            reg = <1>;
+                            color = <LED_COLOR_ID_AMBER>;
+                            function = LED_FUNCTION_LAN;
+                            function-enumerator = <1>;
+                            default-state = "keep";
+                        };
+                    };
                 };
 
                 port@2 {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Add LEDs definition example for qca8k Switch Family to describe how they
> should be defined for a correct usage.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Where is the changelog? This was v8 already! What happened with all
review, changes?

Best regards,
Krzysztof

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Add LEDs definition example for qca8k Switch Family to describe how they
> should be defined for a correct usage.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Where is the changelog? This was v8 already! What happened with all
review, changes?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Andrew Lunn]

On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
> > Add LEDs definition example for qca8k Switch Family to describe how they
> > should be defined for a correct usage.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> Where is the changelog? This was v8 already! What happened with all
> review, changes?

Did you read patch 0?

We have decided to start again, starting small and working up. This
patchset just adds plain, boring LEDs. No acceleration, on hardware
offload. Just on/off, and fixed blink.

What do you think makes the patchset is not bisectable? We are happy
to address such issues, but i did not notice anything.

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Andrew Lunn]

On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
> > Add LEDs definition example for qca8k Switch Family to describe how they
> > should be defined for a correct usage.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> Where is the changelog? This was v8 already! What happened with all
> review, changes?

Did you read patch 0?

We have decided to start again, starting small and working up. This
patchset just adds plain, boring LEDs. No acceleration, on hardware
offload. Just on/off, and fixed blink.

What do you think makes the patchset is not bisectable? We are happy
to address such issues, but i did not notice anything.

    Andrew

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 08/03/2023 14:57, Andrew Lunn wrote:
> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
>> On 07/03/2023 18:00, Christian Marangi wrote:
>>> Add LEDs definition example for qca8k Switch Family to describe how they
>>> should be defined for a correct usage.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>
>> Where is the changelog? This was v8 already! What happened with all
>> review, changes?
> 
> Did you read patch 0?
> 
> We have decided to start again, starting small and working up. This
> patchset just adds plain, boring LEDs. No acceleration, on hardware
> offload. Just on/off, and fixed blink.

Sure, but the patch is carried over. So what happened with all its
feedback? Was there or was not? How can we know?

> 
> What do you think makes the patchset is not bisectable? We are happy
> to address such issues, but i did not notice anything.

I didn't write anything like that here...

Best regards,
Krzysztof

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 08/03/2023 14:57, Andrew Lunn wrote:
> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
>> On 07/03/2023 18:00, Christian Marangi wrote:
>>> Add LEDs definition example for qca8k Switch Family to describe how they
>>> should be defined for a correct usage.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>
>> Where is the changelog? This was v8 already! What happened with all
>> review, changes?
> 
> Did you read patch 0?
> 
> We have decided to start again, starting small and working up. This
> patchset just adds plain, boring LEDs. No acceleration, on hardware
> offload. Just on/off, and fixed blink.

Sure, but the patch is carried over. So what happened with all its
feedback? Was there or was not? How can we know?

> 
> What do you think makes the patchset is not bisectable? We are happy
> to address such issues, but i did not notice anything.

I didn't write anything like that here...

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2023 14:57, Andrew Lunn wrote:
> > On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> >> On 07/03/2023 18:00, Christian Marangi wrote:
> >>> Add LEDs definition example for qca8k Switch Family to describe how they
> >>> should be defined for a correct usage.
> >>>
> >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >>
> >> Where is the changelog? This was v8 already! What happened with all
> >> review, changes?
> > 
> > Did you read patch 0?
> > 
> > We have decided to start again, starting small and working up. This
> > patchset just adds plain, boring LEDs. No acceleration, on hardware
> > offload. Just on/off, and fixed blink.
> 
> Sure, but the patch is carried over. So what happened with all its
> feedback? Was there or was not? How can we know?
>

The history of the old series is a bit sad, not enough review, another
dev asking for a different implementation and me doing an hybrid to
reach a common point (and then disappear intro oblivion)...

Short story is that this current series have nothing related to the HW
offload feature and only in v7 it was asked to put the LED nodes in
ethernet-phy.yaml

I can put in the cover letter of v2 of this series the changelog of the
previous series but they would only be related to other part that are
not related to this.

Just to give you some context and explain why the changelog was dropped.

> > 
> > What do you think makes the patchset is not bisectable? We are happy
> > to address such issues, but i did not notice anything.
> 
> I didn't write anything like that here...
> 

-- 
	Ansuel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2023 14:57, Andrew Lunn wrote:
> > On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> >> On 07/03/2023 18:00, Christian Marangi wrote:
> >>> Add LEDs definition example for qca8k Switch Family to describe how they
> >>> should be defined for a correct usage.
> >>>
> >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >>
> >> Where is the changelog? This was v8 already! What happened with all
> >> review, changes?
> > 
> > Did you read patch 0?
> > 
> > We have decided to start again, starting small and working up. This
> > patchset just adds plain, boring LEDs. No acceleration, on hardware
> > offload. Just on/off, and fixed blink.
> 
> Sure, but the patch is carried over. So what happened with all its
> feedback? Was there or was not? How can we know?
>

The history of the old series is a bit sad, not enough review, another
dev asking for a different implementation and me doing an hybrid to
reach a common point (and then disappear intro oblivion)...

Short story is that this current series have nothing related to the HW
offload feature and only in v7 it was asked to put the LED nodes in
ethernet-phy.yaml

I can put in the cover letter of v2 of this series the changelog of the
previous series but they would only be related to other part that are
not related to this.

Just to give you some context and explain why the changelog was dropped.

> > 
> > What do you think makes the patchset is not bisectable? We are happy
> > to address such issues, but i did not notice anything.
> 
> I didn't write anything like that here...
> 

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 08/03/2023 20:02, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
>> On 08/03/2023 14:57, Andrew Lunn wrote:
>>> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
>>>> On 07/03/2023 18:00, Christian Marangi wrote:
>>>>> Add LEDs definition example for qca8k Switch Family to describe how they
>>>>> should be defined for a correct usage.
>>>>>
>>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>>>
>>>> Where is the changelog? This was v8 already! What happened with all
>>>> review, changes?
>>>
>>> Did you read patch 0?
>>>
>>> We have decided to start again, starting small and working up. This
>>> patchset just adds plain, boring LEDs. No acceleration, on hardware
>>> offload. Just on/off, and fixed blink.
>>
>> Sure, but the patch is carried over. So what happened with all its
>> feedback? Was there or was not? How can we know?
>>
> 
> The history of the old series is a bit sad, not enough review, another
> dev asking for a different implementation and me doing an hybrid to
> reach a common point (and then disappear intro oblivion)...
> 
> Short story is that this current series have nothing related to the HW
> offload feature and only in v7 it was asked to put the LED nodes in
> ethernet-phy.yaml
> 
> I can put in the cover letter of v2 of this series the changelog of the
> previous series but they would only be related to other part that are
> not related to this.
> 
> Just to give you some context and explain why the changelog was dropped.

I am less interested in the changelog of entire patchset but of the
patches which are for me to review. Sending vX as v1 suggests that all
previous review work on this patch could be in some limbo state. Maybe
nothing happened in all previous version, but it's now my task to dig it?

This is why you have "---" for the patch changelog.

Best regards,
Krzysztof

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Krzysztof Kozlowski]

On 08/03/2023 20:02, Christian Marangi wrote:
> On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
>> On 08/03/2023 14:57, Andrew Lunn wrote:
>>> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
>>>> On 07/03/2023 18:00, Christian Marangi wrote:
>>>>> Add LEDs definition example for qca8k Switch Family to describe how they
>>>>> should be defined for a correct usage.
>>>>>
>>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>>>
>>>> Where is the changelog? This was v8 already! What happened with all
>>>> review, changes?
>>>
>>> Did you read patch 0?
>>>
>>> We have decided to start again, starting small and working up. This
>>> patchset just adds plain, boring LEDs. No acceleration, on hardware
>>> offload. Just on/off, and fixed blink.
>>
>> Sure, but the patch is carried over. So what happened with all its
>> feedback? Was there or was not? How can we know?
>>
> 
> The history of the old series is a bit sad, not enough review, another
> dev asking for a different implementation and me doing an hybrid to
> reach a common point (and then disappear intro oblivion)...
> 
> Short story is that this current series have nothing related to the HW
> offload feature and only in v7 it was asked to put the LED nodes in
> ethernet-phy.yaml
> 
> I can put in the cover letter of v2 of this series the changelog of the
> previous series but they would only be related to other part that are
> not related to this.
> 
> Just to give you some context and explain why the changelog was dropped.

I am less interested in the changelog of entire patchset but of the
patches which are for me to review. Sending vX as v1 suggests that all
previous review work on this patch could be in some limbo state. Maybe
nothing happened in all previous version, but it's now my task to dig it?

This is why you have "---" for the patch changelog.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

On Wed, Mar 08, 2023 at 08:09:34PM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2023 20:02, Christian Marangi wrote:
> > On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
> >> On 08/03/2023 14:57, Andrew Lunn wrote:
> >>> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 07/03/2023 18:00, Christian Marangi wrote:
> >>>>> Add LEDs definition example for qca8k Switch Family to describe how they
> >>>>> should be defined for a correct usage.
> >>>>>
> >>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >>>>
> >>>> Where is the changelog? This was v8 already! What happened with all
> >>>> review, changes?
> >>>
> >>> Did you read patch 0?
> >>>
> >>> We have decided to start again, starting small and working up. This
> >>> patchset just adds plain, boring LEDs. No acceleration, on hardware
> >>> offload. Just on/off, and fixed blink.
> >>
> >> Sure, but the patch is carried over. So what happened with all its
> >> feedback? Was there or was not? How can we know?
> >>
> > 
> > The history of the old series is a bit sad, not enough review, another
> > dev asking for a different implementation and me doing an hybrid to
> > reach a common point (and then disappear intro oblivion)...
> > 
> > Short story is that this current series have nothing related to the HW
> > offload feature and only in v7 it was asked to put the LED nodes in
> > ethernet-phy.yaml
> > 
> > I can put in the cover letter of v2 of this series the changelog of the
> > previous series but they would only be related to other part that are
> > not related to this.
> > 
> > Just to give you some context and explain why the changelog was dropped.
> 
> I am less interested in the changelog of entire patchset but of the
> patches which are for me to review. Sending vX as v1 suggests that all
> previous review work on this patch could be in some limbo state. Maybe
> nothing happened in all previous version, but it's now my task to dig it?
> 

OK, if you are intrested only on the DT changes I can list:
From v0 to v6:
- There was only qca8k Documentation addition for the leds node.
  Some extra binding were dropped due to implementation change but they
  were only specific to qca8k.

In v7:
- The default-trigger for netdev was introduced (this caused some
  warning for missing netdev trigger for that binding)
- It was suggested and asked to make the LEDs generic and add
  Documentration for phy-ethernet.yaml

In v8:
- The netdev trigger was added to the list of accepted default-trigger
- The Documentation for phy-ethernet.yaml was added

Then we decided to "reboot" the series and:
In v1:
- The default-trigger is dropped (will be introduced later when the work
  for the netdev trigger will be done)
- We use the default state to "keep"

This should be the entire changelog excluding some changed things done
from v0 to v6 with low review and implementation changed at least 3
times during the life of that series.


> This is why you have "---" for the patch changelog.

-- 
	Ansuel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Christian Marangi]

On Wed, Mar 08, 2023 at 08:09:34PM +0100, Krzysztof Kozlowski wrote:
> On 08/03/2023 20:02, Christian Marangi wrote:
> > On Wed, Mar 08, 2023 at 07:49:26PM +0100, Krzysztof Kozlowski wrote:
> >> On 08/03/2023 14:57, Andrew Lunn wrote:
> >>> On Wed, Mar 08, 2023 at 11:58:33AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 07/03/2023 18:00, Christian Marangi wrote:
> >>>>> Add LEDs definition example for qca8k Switch Family to describe how they
> >>>>> should be defined for a correct usage.
> >>>>>
> >>>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >>>>
> >>>> Where is the changelog? This was v8 already! What happened with all
> >>>> review, changes?
> >>>
> >>> Did you read patch 0?
> >>>
> >>> We have decided to start again, starting small and working up. This
> >>> patchset just adds plain, boring LEDs. No acceleration, on hardware
> >>> offload. Just on/off, and fixed blink.
> >>
> >> Sure, but the patch is carried over. So what happened with all its
> >> feedback? Was there or was not? How can we know?
> >>
> > 
> > The history of the old series is a bit sad, not enough review, another
> > dev asking for a different implementation and me doing an hybrid to
> > reach a common point (and then disappear intro oblivion)...
> > 
> > Short story is that this current series have nothing related to the HW
> > offload feature and only in v7 it was asked to put the LED nodes in
> > ethernet-phy.yaml
> > 
> > I can put in the cover letter of v2 of this series the changelog of the
> > previous series but they would only be related to other part that are
> > not related to this.
> > 
> > Just to give you some context and explain why the changelog was dropped.
> 
> I am less interested in the changelog of entire patchset but of the
> patches which are for me to review. Sending vX as v1 suggests that all
> previous review work on this patch could be in some limbo state. Maybe
> nothing happened in all previous version, but it's now my task to dig it?
> 

OK, if you are intrested only on the DT changes I can list:
From v0 to v6:
- There was only qca8k Documentation addition for the leds node.
  Some extra binding were dropped due to implementation change but they
  were only specific to qca8k.

In v7:
- The default-trigger for netdev was introduced (this caused some
  warning for missing netdev trigger for that binding)
- It was suggested and asked to make the LEDs generic and add
  Documentration for phy-ethernet.yaml

In v8:
- The netdev trigger was added to the list of accepted default-trigger
- The Documentation for phy-ethernet.yaml was added

Then we decided to "reboot" the series and:
In v1:
- The default-trigger is dropped (will be introduced later when the work
  for the netdev trigger will be done)
- We use the default state to "keep"

This should be the entire changelog excluding some changed things done
from v0 to v6 with low review and implementation changed at least 3
times during the life of that series.


> This is why you have "---" for the patch changelog.

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Andrew Lunn]

> Then we decided to "reboot" the series and:
> In v1:
> - The default-trigger is dropped (will be introduced later when the work
>   for the netdev trigger will be done)
> - We use the default state to "keep"

There is one more change. The leds {} property moved from the PHY nodes
into the port nodes, because these are MAC LEDs, not PHY LEDs.

     Andrew

Re: [net-next PATCH 09/11] dt-bindings: net: dsa: qca8k: add LEDs definition example

[Andrew Lunn]

> Then we decided to "reboot" the series and:
> In v1:
> - The default-trigger is dropped (will be introduced later when the work
>   for the netdev trigger will be done)
> - We use the default state to "keep"

There is one more change. The leds {} property moved from the PHY nodes
into the port nodes, because these are MAC LEDs, not PHY LEDs.

     Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Christian Marangi]

Document support for LEDs node in phy and add an example for it.
PHY LED will have to match led pattern and should be treated as a
generic led.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 1327b81f15a2..0ec8ef6b0d8a 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -197,6 +197,13 @@ properties:
       PHY's that have configurable TX internal delays. If this property is
       present then the PHY applies the TX delay.
 
+  leds:
+    type: object
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
 required:
   - reg
 
@@ -204,6 +211,8 @@ additionalProperties: true
 
 examples:
   - |
+    #include <dt-bindings/leds/common.h>
+
     ethernet {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -219,5 +228,18 @@ examples:
             reset-gpios = <&gpio1 4 1>;
             reset-assert-us = <1000>;
             reset-deassert-us = <2000>;
+
+            leds {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                led@0 {
+                    reg = <0>;
+                    color = <LED_COLOR_ID_WHITE>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <1>;
+                    default-state = "keep";
+                };
+            };
         };
     };
-- 
2.39.2

[net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Christian Marangi]

Document support for LEDs node in phy and add an example for it.
PHY LED will have to match led pattern and should be treated as a
generic led.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 1327b81f15a2..0ec8ef6b0d8a 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -197,6 +197,13 @@ properties:
       PHY's that have configurable TX internal delays. If this property is
       present then the PHY applies the TX delay.
 
+  leds:
+    type: object
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
 required:
   - reg
 
@@ -204,6 +211,8 @@ additionalProperties: true
 
 examples:
   - |
+    #include <dt-bindings/leds/common.h>
+
     ethernet {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -219,5 +228,18 @@ examples:
             reset-gpios = <&gpio1 4 1>;
             reset-assert-us = <1000>;
             reset-deassert-us = <2000>;
+
+            leds {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                led@0 {
+                    reg = <0>;
+                    color = <LED_COLOR_ID_WHITE>;
+                    function = LED_FUNCTION_LAN;
+                    function-enumerator = <1>;
+                    default-state = "keep";
+                };
+            };
         };
     };
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:45PM +0100, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:45PM +0100, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---

Also missing changelog, history, tags, anything. This was already v8.

Also, I have doubts that your patchset is fully bisectable. Are you sure
of this?

Best regards,
Krzysztof

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---

Also missing changelog, history, tags, anything. This was already v8.

Also, I have doubts that your patchset is fully bisectable. Are you sure
of this?

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 08/03/2023 12:00, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
>> Document support for LEDs node in phy and add an example for it.
>> PHY LED will have to match led pattern and should be treated as a
>> generic led.
>>
>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>> ---
> 
> Also missing changelog, history, tags, anything. This was already v8.
> 
> Also, I have doubts that your patchset is fully bisectable. Are you sure
> of this?

OK, so the leds in previous patch are not for phy but for port, so it is
properly bisectable.

Best regards,
Krzysztof

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 08/03/2023 12:00, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
>> Document support for LEDs node in phy and add an example for it.
>> PHY LED will have to match led pattern and should be treated as a
>> generic led.
>>
>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>> ---
> 
> Also missing changelog, history, tags, anything. This was already v8.
> 
> Also, I have doubts that your patchset is fully bisectable. Are you sure
> of this?

OK, so the leds in previous patch are not for phy but for port, so it is
properly bisectable.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 1327b81f15a2..0ec8ef6b0d8a 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -197,6 +197,13 @@ properties:
>        PHY's that have configurable TX internal delays. If this property is
>        present then the PHY applies the TX delay.
>  
> +  leds:
> +    type: object

additionalProperties: false

although maybe this was already said in one of previous ten reviews...

> +
> +    patternProperties:
> +      '^led(@[a-f0-9]+)?$':
> +        $ref: /schemas/leds/common.yaml#
> +


Best regards,
Krzysztof

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Krzysztof Kozlowski]

On 07/03/2023 18:00, Christian Marangi wrote:
> Document support for LEDs node in phy and add an example for it.
> PHY LED will have to match led pattern and should be treated as a
> generic led.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 1327b81f15a2..0ec8ef6b0d8a 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -197,6 +197,13 @@ properties:
>        PHY's that have configurable TX internal delays. If this property is
>        present then the PHY applies the TX delay.
>  
> +  leds:
> +    type: object

additionalProperties: false

although maybe this was already said in one of previous ten reviews...

> +
> +    patternProperties:
> +      '^led(@[a-f0-9]+)?$':
> +        $ref: /schemas/leds/common.yaml#
> +


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Christian Marangi]

On Wed, Mar 08, 2023 at 07:56:57PM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
> > Document support for LEDs node in phy and add an example for it.
> > PHY LED will have to match led pattern and should be treated as a
> > generic led.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 1327b81f15a2..0ec8ef6b0d8a 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -197,6 +197,13 @@ properties:
> >        PHY's that have configurable TX internal delays. If this property is
> >        present then the PHY applies the TX delay.
> >  
> > +  leds:
> > +    type: object
> 
> additionalProperties: false
> 
> although maybe this was already said in one of previous ten reviews...
>

Thanks for the review. (this is rather new from the old patch (appeared
only in v7 so sorry if I didn't see that in the old series. Will fix in
v2 of this!)

> > +
> > +    patternProperties:
> > +      '^led(@[a-f0-9]+)?$':
> > +        $ref: /schemas/leds/common.yaml#
> > +
> 

-- 
	Ansuel

Re: [net-next PATCH 10/11] dt-bindings: net: phy: Document support for LEDs node

[Christian Marangi]

On Wed, Mar 08, 2023 at 07:56:57PM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2023 18:00, Christian Marangi wrote:
> > Document support for LEDs node in phy and add an example for it.
> > PHY LED will have to match led pattern and should be treated as a
> > generic led.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/net/ethernet-phy.yaml | 22 +++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 1327b81f15a2..0ec8ef6b0d8a 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -197,6 +197,13 @@ properties:
> >        PHY's that have configurable TX internal delays. If this property is
> >        present then the PHY applies the TX delay.
> >  
> > +  leds:
> > +    type: object
> 
> additionalProperties: false
> 
> although maybe this was already said in one of previous ten reviews...
>

Thanks for the review. (this is rather new from the old patch (appeared
only in v7 so sorry if I didn't see that in the old series. Will fix in
v2 of this!)

> > +
> > +    patternProperties:
> > +      '^led(@[a-f0-9]+)?$':
> > +        $ref: /schemas/leds/common.yaml#
> > +
> 

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

The WAN port of the 370-RD has a Marvell PHY, with one LED on
the front panel. List this LED in the device tree.

Set the LED default state to "keep" to not change any blink rule
set by default.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/armada-370-rd.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-rd.dts b/arch/arm/boot/dts/armada-370-rd.dts
index be005c9f42ef..ccd4699b219f 100644
--- a/arch/arm/boot/dts/armada-370-rd.dts
+++ b/arch/arm/boot/dts/armada-370-rd.dts
@@ -20,6 +20,7 @@
 /dts-v1/;
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/gpio/gpio.h>
 #include "armada-370.dtsi"
 
@@ -135,6 +136,19 @@ &mdio {
 	pinctrl-names = "default";
 	phy0: ethernet-phy@0 {
 		reg = <0>;
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 {
+				reg = <0>;
+				label = "WAN";
+				color = <LED_COLOR_ID_WHITE>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+			};
+		};
 	};
 
 	switch: switch@10 {
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Christian Marangi]

From: Andrew Lunn <andrew@lunn.ch>

The WAN port of the 370-RD has a Marvell PHY, with one LED on
the front panel. List this LED in the device tree.

Set the LED default state to "keep" to not change any blink rule
set by default.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/armada-370-rd.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-rd.dts b/arch/arm/boot/dts/armada-370-rd.dts
index be005c9f42ef..ccd4699b219f 100644
--- a/arch/arm/boot/dts/armada-370-rd.dts
+++ b/arch/arm/boot/dts/armada-370-rd.dts
@@ -20,6 +20,7 @@
 /dts-v1/;
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/leds/common.h>
 #include <dt-bindings/gpio/gpio.h>
 #include "armada-370.dtsi"
 
@@ -135,6 +136,19 @@ &mdio {
 	pinctrl-names = "default";
 	phy0: ethernet-phy@0 {
 		reg = <0>;
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			led@0 {
+				reg = <0>;
+				label = "WAN";
+				color = <LED_COLOR_ID_WHITE>;
+				function = LED_FUNCTION_LAN;
+				function-enumerator = <1>;
+				default-state = "keep";
+			};
+		};
 	};
 
 	switch: switch@10 {
-- 
2.39.2

Re: [net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:46PM +0100, Christian Marangi wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> The WAN port of the 370-RD has a Marvell PHY, with one LED on
> the front panel. List this LED in the device tree.
> 
> Set the LED default state to "keep" to not change any blink rule
> set by default.

Hi Christian

What board are you using for testing? It would be good to patch its
.dts file to enable its LEDs. We don't normally had new code without a
user.

	Andrew

Re: [net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:46PM +0100, Christian Marangi wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> The WAN port of the 370-RD has a Marvell PHY, with one LED on
> the front panel. List this LED in the device tree.
> 
> Set the LED default state to "keep" to not change any blink rule
> set by default.

Hi Christian

What board are you using for testing? It would be good to patch its
.dts file to enable its LEDs. We don't normally had new code without a
user.

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:20:17AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:00:46PM +0100, Christian Marangi wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > The WAN port of the 370-RD has a Marvell PHY, with one LED on
> > the front panel. List this LED in the device tree.
> > 
> > Set the LED default state to "keep" to not change any blink rule
> > set by default.
> 
> Hi Christian
> 
> What board are you using for testing? It would be good to patch its
> .dts file to enable its LEDs. We don't normally had new code without a
> user.
> 

ipq806x, my specific device is not present upstream but we have a rb3011
that currently use the qca8k dsa switch upstream.
If needed I can add support there by checking the leds supported...

Also looking at the dt I think I need to use "port - 1" after all since
it does use legacy way... Sad me...

-- 
	Ansuel

Re: [net-next PATCH 11/11] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port

[Christian Marangi]

On Wed, Mar 08, 2023 at 12:20:17AM +0100, Andrew Lunn wrote:
> On Tue, Mar 07, 2023 at 06:00:46PM +0100, Christian Marangi wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > The WAN port of the 370-RD has a Marvell PHY, with one LED on
> > the front panel. List this LED in the device tree.
> > 
> > Set the LED default state to "keep" to not change any blink rule
> > set by default.
> 
> Hi Christian
> 
> What board are you using for testing? It would be good to patch its
> .dts file to enable its LEDs. We don't normally had new code without a
> user.
> 

ipq806x, my specific device is not present upstream but we have a rb3011
that currently use the qca8k dsa switch upstream.
If needed I can add support there by checking the leds supported...

Also looking at the dt I think I need to use "port - 1" after all since
it does use legacy way... Sad me...

-- 
	Ansuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [net-next PATCH 00/11] net: Add basic LED support for switch/phy

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:35PM +0100, Christian Marangi wrote:
> This is a continue of [1]. It was decided to take a more gradual
> approach to implement LEDs support for switch and phy starting with
> basic support and then implementing the hw control part when we have all
> the prereq done.

In the end, there are likely to be 3 or 4 patchsets. There are going
to be patches to both the LED subsystem and the netdev subsystem, plus
device tree bindings and some ARM DT patches.

Ideally we would like all the patches to go through one tree, so we
can keep everything together and buildable. We will cross post patches
to both major subsystems, but my guess is, merging via netdev will be
best. If not, a stable branch for the LED subsystem which can be
pulled into netdev could maybe made be to work.

       Andrew

Re: [net-next PATCH 00/11] net: Add basic LED support for switch/phy

[Andrew Lunn]

On Tue, Mar 07, 2023 at 06:00:35PM +0100, Christian Marangi wrote:
> This is a continue of [1]. It was decided to take a more gradual
> approach to implement LEDs support for switch and phy starting with
> basic support and then implementing the hw control part when we have all
> the prereq done.

In the end, there are likely to be 3 or 4 patchsets. There are going
to be patches to both the LED subsystem and the netdev subsystem, plus
device tree bindings and some ARM DT patches.

Ideally we would like all the patches to go through one tree, so we
can keep everything together and buildable. We will cross post patches
to both major subsystems, but my guess is, merging via netdev will be
best. If not, a stable branch for the LED subsystem which can be
pulled into netdev could maybe made be to work.

       Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel