linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy
@ 2023-03-09 22:35 Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
                   ` (13 more replies)
  0 siblings, 14 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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/

Changes in new series v2:
- Add LEDs node for rb3011
- Fix rb3011 switch node unevaluated properties while running 
  make dtbs_check
- Fix a copypaste error in qca8k-leds.c for port 4 required shift
- Drop phy-handle usage for qca8k and use qca8k_port_to_phy()
- Add review tag from Andrew
- Add Christian Marangi SOB in each Andrew patch
- Add extra description for dsa-port stressing that PHY have no access
  and LED are controlled by the related MAC
- Add missing additionalProperties for dsa-port.yaml and ethernet-phy.yaml

Changes from the old v8 series:
- Drop linux,default-trigger set to netdev.
- Dropped every hw control related patch and implement only
  blink_set and brightness_set
- Add default-state to "keep" for each LED node example

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 (8):
  net: dsa: qca8k: move qca8k_port_to_phy() to header
  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
  arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011
  arm: qcom: dt: Add Switch LED for each port for rb3011
  dt-bindings: net: phy: Document support for LEDs node

 .../devicetree/bindings/net/dsa/dsa-port.yaml |  21 ++
 .../devicetree/bindings/net/dsa/qca8k.yaml    |  24 ++
 .../devicetree/bindings/net/ethernet-phy.yaml |  31 +++
 arch/arm/boot/dts/armada-370-rd.dts           |  14 ++
 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts     | 124 ++++++++-
 drivers/net/dsa/qca/Kconfig                   |   7 +
 drivers/net/dsa/qca/Makefile                  |   1 +
 drivers/net/dsa/qca/qca8k-8xxx.c              |  19 +-
 drivers/net/dsa/qca/qca8k-leds.c              | 236 ++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h                   |  83 ++++++
 drivers/net/phy/marvell.c                     |  81 +++++-
 drivers/net/phy/phy_device.c                  | 115 +++++++++
 include/linux/phy.h                           |  33 +++
 13 files changed, 765 insertions(+), 24 deletions(-)
 create mode 100644 drivers/net/dsa/qca/qca8k-leds.c

-- 
2.39.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 23:58   ` Andrew Lunn
  2023-03-09 22:35 ` [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support Christian Marangi
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

Move qca8k_port_to_phy() to qca8k header as it's useful for future
reference in Switch LEDs module since the same logic is applied to get
the right index of the switch port.
Make it inline as it's simple function that just decrease the port.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 15 ---------------
 drivers/net/dsa/qca/qca8k.h      | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 2f224b166bbb..8dfc5db84700 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -716,21 +716,6 @@ qca8k_phy_eth_command(struct qca8k_priv *priv, bool read, int phy,
 	return ret;
 }
 
-static u32
-qca8k_port_to_phy(int port)
-{
-	/* From Andrew Lunn:
-	 * Port 0 has no internal phy.
-	 * Port 1 has an internal PHY at MDIO address 0.
-	 * Port 2 has an internal PHY at MDIO address 1.
-	 * ...
-	 * Port 5 has an internal PHY at MDIO address 4.
-	 * Port 6 has no internal PHY.
-	 */
-
-	return port - 1;
-}
-
 static int
 qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask)
 {
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 03514f7a20be..4e48e4dd8b0f 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -422,6 +422,20 @@ struct qca8k_fdb {
 	u8 mac[6];
 };
 
+static inline u32 qca8k_port_to_phy(int port)
+{
+	/* From Andrew Lunn:
+	 * Port 0 has no internal phy.
+	 * Port 1 has an internal PHY at MDIO address 0.
+	 * Port 2 has an internal PHY at MDIO address 1.
+	 * ...
+	 * Port 5 has an internal PHY at MDIO address 4.
+	 * Port 6 has no internal PHY.
+	 */
+
+	return port - 1;
+}
+
 /* Common setup function */
 extern const struct qca8k_mib_desc ar8327_mib[];
 extern const struct regmap_access_table qca8k_readable_table;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-10  0:12   ` Andrew Lunn
  2023-03-09 22:35 ` [net-next PATCH v2 03/14] net: dsa: qca8k: add LEDs blink_set() support Christian Marangi
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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 | 198 +++++++++++++++++++++++++++++++
 drivers/net/dsa/qca/qca8k.h      |  69 +++++++++++
 5 files changed, 279 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 8dfc5db84700..fe68ac7e4030 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1727,6 +1727,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..7ae2ea082aae
--- /dev/null
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -0,0 +1,198 @@
+// 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_PHY4_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) {
+		if (fwnode_property_read_u32(port, "reg", &port_num))
+			continue;
+
+		/* Each port can have at least 3 different leds attached.
+		 * Switch port starts from 0 to 6, but port 0 and 6 are CPU
+		 * port. The port index needs to be decreased by one to identify
+		 * the correct port for LED setup.
+		 */
+		ret = qca8k_parse_port_leds(priv, port, qca8k_port_to_phy(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 4e48e4dd8b0f..25fab385f0f7 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 {
@@ -526,4 +585,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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 03/14] net: dsa: qca8k: add LEDs blink_set() support
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 04/14] net: phy: Add a binding for PHY LEDs Christian Marangi
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
---
 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 7ae2ea082aae..df7106d498db 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 04/14] net: phy: Add a binding for PHY LEDs
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (2 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 03/14] net: dsa: qca8k: add LEDs blink_set() support Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 05/14] net: phy: phy_device: Call into the PHY driver to set LED brightness Christian Marangi
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 05/14] net: phy: phy_device: Call into the PHY driver to set LED brightness.
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (3 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 04/14] net: phy: Add a binding for PHY LEDs Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 06/14] net: phy: marvell: Add software control of the LEDs Christian Marangi
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 06/14] net: phy: marvell: Add software control of the LEDs
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (4 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 05/14] net: phy: phy_device: Call into the PHY driver to set LED brightness Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 07/14] net: phy: phy_device: Call into the PHY driver to set LED blinking Christian Marangi
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 07/14] net: phy: phy_device: Call into the PHY driver to set LED blinking.
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (5 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 06/14] net: phy: marvell: Add software control of the LEDs Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 08/14] net: phy: marvell: Implement led_blink_set() Christian Marangi
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 08/14] net: phy: marvell: Implement led_blink_set()
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (6 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 07/14] net: phy: phy_device: Call into the PHY driver to set LED blinking Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 09/14] dt-bindings: net: dsa: dsa-port: Document support for LEDs node Christian Marangi
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 09/14] dt-bindings: net: dsa: dsa-port: Document support for LEDs node
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (7 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 08/14] net: phy: marvell: Implement led_blink_set() Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 10/14] dt-bindings: net: dsa: qca8k: add LEDs definition example Christian Marangi
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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.

Also add some Documentation to describe the difference of these nodes
compared to PHY LEDs, since dsa-port LEDs are controllable by the switch
regs and the possible intergated PHY doesn't have control on them.

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

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 480120469953..1bf4151e5155 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -59,6 +59,27 @@ properties:
       - rtl8_4t
       - seville
 
+  leds:
+    type: object
+    description:
+      Describes the LEDs associated by the Switch Port and controllable
+      in its MACs. These LEDs are not integrated in the PHY and PHY
+      doesn't have any control on them. Switch regs are used to control
+      these Switch Port LEDs.
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
+    additionalProperties: false
+
 # CPU and DSA ports must have phylink-compatible link descriptions
 if:
   oneOf:
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 10/14] dt-bindings: net: dsa: qca8k: add LEDs definition example
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (8 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 09/14] dt-bindings: net: dsa: dsa-port: Document support for LEDs node Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 11/14] arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011 Christian Marangi
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 11/14] arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (9 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 10/14] dt-bindings: net: dsa: qca8k: add LEDs definition example Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 12/14] arm: qcom: dt: Add Switch LED for each port " Christian Marangi
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds
  Cc: Jonathan McDowell

IPQ8064 MikroTik RB3011UiAS-RM DT have currently unevaluted properties
in the 2 switch nodes. The bindings #address-cells and #size-cells are
redundant and cause warning for 'Unevaluated properties are not
allowed'.

Drop these bindings to mute these warning as they should not be there
from the start.

Cc: Jonathan McDowell <noodles@earth.li>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
index f908889c4f95..47a5d1849c72 100644
--- a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
+++ b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
@@ -38,8 +38,6 @@ mdio0: mdio-0 {
 
 		switch0: switch@10 {
 			compatible = "qca,qca8337";
-			#address-cells = <1>;
-			#size-cells = <0>;
 
 			dsa,member = <0 0>;
 
@@ -105,8 +103,6 @@ mdio1: mdio-1 {
 
 		switch1: switch@14 {
 			compatible = "qca,qca8337";
-			#address-cells = <1>;
-			#size-cells = <0>;
 
 			dsa,member = <1 0>;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 12/14] arm: qcom: dt: Add Switch LED for each port for rb3011
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (10 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 11/14] arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011 Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 13/14] dt-bindings: net: phy: Document support for LEDs node Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 14/14] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port Christian Marangi
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds
  Cc: Jonathan McDowell

Add Switch LED for each port for MikroTik RB3011UiAS-RM.

MikroTik RB3011UiAS-RM is a 10 port device with 2 qca8337 switch chips
connected.

It was discovered that in the hardware design all 3 Switch LED trace of
the related port is connected to the same LED. This was discovered by
setting to 'always on' the related led in the switch regs and noticing
that all 3 LED for the specific port (for example for port 1) cause the
connected LED for port 1 to turn on. As an extra test we tried enabling
2 different LED for the port resulting in the LED turned off only if
every led in the reg was off.

Aside from this funny and strange hardware implementation, the device
itself have one green LED for each port, resulting in 10 green LED one
for each of the 10 supported port.

Cc: Jonathan McDowell <noodles@earth.li>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 120 ++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
index 47a5d1849c72..472b5a2912a1 100644
--- a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
+++ b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
@@ -65,26 +65,86 @@ fixed-link {
 				port@1 {
 					reg = <1>;
 					label = "sw1";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <1>;
+						};
+					};
 				};
 
 				port@2 {
 					reg = <2>;
 					label = "sw2";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <2>;
+						};
+					};
 				};
 
 				port@3 {
 					reg = <3>;
 					label = "sw3";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <3>;
+						};
+					};
 				};
 
 				port@4 {
 					reg = <4>;
 					label = "sw4";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <4>;
+						};
+					};
 				};
 
 				port@5 {
 					reg = <5>;
 					label = "sw5";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <5>;
+						};
+					};
 				};
 			};
 		};
@@ -130,26 +190,86 @@ fixed-link {
 				port@1 {
 					reg = <1>;
 					label = "sw6";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <6>;
+						};
+					};
 				};
 
 				port@2 {
 					reg = <2>;
 					label = "sw7";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <7>;
+						};
+					};
 				};
 
 				port@3 {
 					reg = <3>;
 					label = "sw8";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <8>;
+						};
+					};
 				};
 
 				port@4 {
 					reg = <4>;
 					label = "sw9";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <9>;
+						};
+					};
 				};
 
 				port@5 {
 					reg = <5>;
 					label = "sw10";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							function-enumerator = <10>;
+						};
+					};
 				};
 			};
 		};
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 13/14] dt-bindings: net: phy: Document support for LEDs node
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (11 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 12/14] arm: qcom: dt: Add Switch LED for each port " Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  2023-03-09 22:35 ` [net-next PATCH v2 14/14] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port Christian Marangi
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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>
---
 .../devicetree/bindings/net/ethernet-phy.yaml | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 1327b81f15a2..84e15cee27c7 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -197,6 +197,22 @@ properties:
       PHY's that have configurable TX internal delays. If this property is
       present then the PHY applies the TX delay.
 
+  leds:
+    type: object
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      '^led(@[a-f0-9]+)?$':
+        $ref: /schemas/leds/common.yaml#
+
+    additionalProperties: false
+
 required:
   - reg
 
@@ -204,6 +220,8 @@ additionalProperties: true
 
 examples:
   - |
+    #include <dt-bindings/leds/common.h>
+
     ethernet {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -219,5 +237,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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net-next PATCH v2 14/14] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port
  2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
                   ` (12 preceding siblings ...)
  2023-03-09 22:35 ` [net-next PATCH v2 13/14] dt-bindings: net: phy: Document support for LEDs node Christian Marangi
@ 2023-03-09 22:35 ` Christian Marangi
  13 siblings, 0 replies; 19+ messages in thread
From: Christian Marangi @ 2023-03-09 22:35 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Heiner Kallweit, Russell King,
	Gregory Clement, Sebastian Hesselbarth, Andy Gross,
	Bjorn Andersson, Konrad Dybcio, Christian Marangi, John Crispin,
	netdev, devicetree, linux-kernel, linux-arm-kernel,
	linux-arm-msm, Lee Jones, linux-leds

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header
  2023-03-09 22:35 ` [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
@ 2023-03-09 23:58   ` Andrew Lunn
  0 siblings, 0 replies; 19+ messages in thread
From: Andrew Lunn @ 2023-03-09 23:58 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Heiner Kallweit, Russell King, Gregory Clement,
	Sebastian Hesselbarth, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, John Crispin, netdev, devicetree, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Lee Jones, linux-leds

On Thu, Mar 09, 2023 at 11:35:11PM +0100, Christian Marangi wrote:
> Move qca8k_port_to_phy() to qca8k header as it's useful for future
> reference in Switch LEDs module since the same logic is applied to get
> the right index of the switch port.
> Make it inline as it's simple function that just decrease the port.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

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

    Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support
  2023-03-09 22:35 ` [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support Christian Marangi
@ 2023-03-10  0:12   ` Andrew Lunn
  2023-03-10  0:18     ` Christian Marangi
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Lunn @ 2023-03-10  0:12 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Heiner Kallweit, Russell King, Gregory Clement,
	Sebastian Hesselbarth, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, John Crispin, netdev, devicetree, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Lee Jones, linux-leds

> +config NET_DSA_QCA8K_LEDS_SUPPORT
> +	tristate "Qualcomm Atheros QCA8K Ethernet switch family LEDs support"

Is tristate correct here? That means the code can either be built in,
a module, or not built at all. Is that what you want?

It seems more normal to use a bool, not a tristate.

> +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;
> +}

What will this return when in the future you add hardware offload, and
the LED is actually blinking because of frames being sent etc?

Is it better to not implement _get() when it is unclear what it should
return when offload is in operation?

       Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support
  2023-03-10  0:12   ` Andrew Lunn
@ 2023-03-10  0:18     ` Christian Marangi
  2023-03-10  0:58       ` Andrew Lunn
  0 siblings, 1 reply; 19+ messages in thread
From: Christian Marangi @ 2023-03-10  0:18 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Heiner Kallweit, Russell King, Gregory Clement,
	Sebastian Hesselbarth, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, John Crispin, netdev, devicetree, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Lee Jones, linux-leds

On Fri, Mar 10, 2023 at 01:12:03AM +0100, Andrew Lunn wrote:
> > +config NET_DSA_QCA8K_LEDS_SUPPORT
> > +	tristate "Qualcomm Atheros QCA8K Ethernet switch family LEDs support"
> 
> Is tristate correct here? That means the code can either be built in,
> a module, or not built at all. Is that what you want?
> 
> It seems more normal to use a bool, not a tristate.
>

Think you are right, can't really be a module.

> > +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;
> > +}
> 
> What will this return when in the future you add hardware offload, and
> the LED is actually blinking because of frames being sent etc?
> 
> Is it better to not implement _get() when it is unclear what it should
> return when offload is in operation?
> 
>        Andrew

My idea was that anything that is not 'always off' will have brightness
1. So also in accelerated blink brightness should be 1.

My idea of get was that it should reflect if the led is active or always
off. Is it wrong?

-- 
	Ansuel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support
  2023-03-10  0:18     ` Christian Marangi
@ 2023-03-10  0:58       ` Andrew Lunn
  0 siblings, 0 replies; 19+ messages in thread
From: Andrew Lunn @ 2023-03-10  0:58 UTC (permalink / raw)
  To: Christian Marangi
  Cc: Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Heiner Kallweit, Russell King, Gregory Clement,
	Sebastian Hesselbarth, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, John Crispin, netdev, devicetree, linux-kernel,
	linux-arm-kernel, linux-arm-msm, Lee Jones, linux-leds

> > > +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;
> > > +}
> > 
> > What will this return when in the future you add hardware offload, and
> > the LED is actually blinking because of frames being sent etc?
> > 
> > Is it better to not implement _get() when it is unclear what it should
> > return when offload is in operation?
> > 
> 
> My idea was that anything that is not 'always off' will have brightness
> 1. So also in accelerated blink brightness should be 1.
> 
> My idea of get was that it should reflect if the led is active or always
> off. Is it wrong?
 
brigntness_get seems to be used in two situations:

When the LED is first registered, it can be called to get the current
state of the LED. This then initialized cdev->brightness.

When the brightness sysfs file is read, there is first a call to
brightness_get to allow it to update the value in cdev->brightness
before returning the value in the read.

I think always returning 1 could be confusing. Take the example that
the LED is indicating link, there is no link, so it is off. Yet a read
of the brightness sysfs file will return 1?

I would say, it either needs to return the instantaneous brightness,
or it should not be implemented at all. When we come to implement
offloading, we might want to consider hiding the brightness sysfs
file. But we can solve that later.

      Andrew

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-03-10  0:58 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-09 22:35 [net-next PATCH v2 00/14] net: Add basic LED support for switch/phy Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 01/14] net: dsa: qca8k: move qca8k_port_to_phy() to header Christian Marangi
2023-03-09 23:58   ` Andrew Lunn
2023-03-09 22:35 ` [net-next PATCH v2 02/14] net: dsa: qca8k: add LEDs basic support Christian Marangi
2023-03-10  0:12   ` Andrew Lunn
2023-03-10  0:18     ` Christian Marangi
2023-03-10  0:58       ` Andrew Lunn
2023-03-09 22:35 ` [net-next PATCH v2 03/14] net: dsa: qca8k: add LEDs blink_set() support Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 04/14] net: phy: Add a binding for PHY LEDs Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 05/14] net: phy: phy_device: Call into the PHY driver to set LED brightness Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 06/14] net: phy: marvell: Add software control of the LEDs Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 07/14] net: phy: phy_device: Call into the PHY driver to set LED blinking Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 08/14] net: phy: marvell: Implement led_blink_set() Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 09/14] dt-bindings: net: dsa: dsa-port: Document support for LEDs node Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 10/14] dt-bindings: net: dsa: qca8k: add LEDs definition example Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 11/14] arm: qcom: dt: Drop unevaluated properties in switch nodes for rb3011 Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 12/14] arm: qcom: dt: Add Switch LED for each port " Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 13/14] dt-bindings: net: phy: Document support for LEDs node Christian Marangi
2023-03-09 22:35 ` [net-next PATCH v2 14/14] arm: mvebu: dt: Add PHY LED support for 370-rd WAN port Christian Marangi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).