[PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Andrew Lunn]

This patchset extends the DSA core to add support for port LEDs being
controlled via sys/class/leds, and offloading blinking via
ledtrig-netdev. The core parses the device tree binding, and registers
LEDs. The DSA switch ops structure is extended with the needed
functions.

The mv88e6xxx support is partially added. Support for setting the
brightness and blinking is provided, but offloading of blinking is not
yet available. To demonstrate this, the wrt1900ac device tree is
extended with LEDs.

The existing QCA8K code is refactored to make use of this shared code.

RFC:

Linus, can you rework your code into this for offloading blinking ?
And test with ports 5 & 6.

Christian: Please test QCA8K. I would not be surprised if there is an
off-by-one.

This code can also be found in

https://github.com/lunn/ v6.7-rc2-net-next-mv88e6xxx-leds

Andrew Lunn (8):
  net: dsa: mv88e6xxx: Add helpers for 6352 LED blink and brightness
  net: dsa: mv88e6xxx: Tie the low level LED functions to device ops
  net: dsa: Plumb LED brightnes and blink into switch API
  dsa: Create port LEDs based on DT binding
  dsa: Plumb in LED calls needed for hardware offload
  dsa: mv88e6xxx: Plumb in LED offload functions
  arm: boot: dts: mvebu: linksys-mamba: Add Ethernet LEDs
  dsa: qca8k: Use DSA common code for LEDs

 .../dts/marvell/armada-xp-linksys-mamba.dts   |  66 +++++
 drivers/net/dsa/mv88e6xxx/chip.c              | 103 +++++++
 drivers/net/dsa/mv88e6xxx/chip.h              |  14 +
 drivers/net/dsa/mv88e6xxx/port.c              |  99 +++++++
 drivers/net/dsa/mv88e6xxx/port.h              |  76 +++++-
 drivers/net/dsa/qca/qca8k-8xxx.c              |  11 +-
 drivers/net/dsa/qca/qca8k-leds.c              | 255 +++---------------
 drivers/net/dsa/qca/qca8k.h                   |   9 -
 drivers/net/dsa/qca/qca8k_leds.h              |  21 +-
 include/net/dsa.h                             |  17 ++
 net/dsa/dsa.c                                 | 190 +++++++++++++
 11 files changed, 620 insertions(+), 241 deletions(-)

-- 
2.42.0

[PATCH RFC net-next 1/8] net: dsa: mv88e6xxx: Add helpers for 6352 LED blink and brightness

[Andrew Lunn]

The 6352 family has two LEDs per port for ports 0-4. Ports 5 and 6
share a couple of LEDs. Add support functions to set the brightness,
i.e. on or off, and to make the LEDs blink at a fixed rate.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/port.c | 99 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h | 76 +++++++++++++++++++++++-
 2 files changed, 174 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 5394a8cf7bf1..8dae51f0c1d4 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1723,3 +1723,102 @@ int mv88e6393x_port_set_policy(struct mv88e6xxx_chip *chip, int port,
 
 	return mv88e6393x_port_policy_write(chip, port, ptr, reg);
 }
+
+/* Offset 0x16: LED Control Register */
+
+static int mv88e6352_port_led_write(struct mv88e6xxx_chip *chip, int port,
+				    u16 pointer, u16 data)
+{
+	u16 reg = MV88E6352_PORT_LED_CTL_UPDATE | pointer | data;
+
+	return mv88e6xxx_port_write(chip, port, MV88E6352_PORT_LED_CTL, reg);
+}
+
+static int mv88e6352_port_led_read(struct mv88e6xxx_chip *chip, int port,
+				   u16 pointer, u16 *data)
+{
+	int err;
+	u16 val;
+
+	err = mv88e6xxx_port_write(chip, port, MV88E6352_PORT_LED_CTL, pointer);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6352_PORT_LED_CTL, &val);
+	if (err)
+		return err;
+
+	*data = val & MV88E6352_PORT_LED_CTL_DATA_MASK;
+
+	return 0;
+}
+
+int mv88e6352_port_led_brightness_set(struct mv88e6xxx_chip *chip, int port,
+				      u8 led, enum led_brightness value)
+{
+	int err;
+	u16 val;
+
+	if (led > 1)
+		return -EINVAL;
+
+	if (port > 5)
+		return -EOPNOTSUPP;
+
+	err = mv88e6352_port_led_read(chip, port,
+				      MV88E6352_PORT_LED_CTL_PTR_LED01,
+				      &val);
+	if (err)
+		return err;
+
+	if (led == 0) {
+		val &= ~MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_MASK;
+		if (value)
+			val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_ON;
+		else
+			val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_OFF;
+	} else {
+		val &= ~MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_MASK;
+		if (value)
+			val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_ON;
+		else
+			val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_OFF;
+	}
+	return mv88e6352_port_led_write(chip, port,
+					MV88E6352_PORT_LED_CTL_PTR_LED01,
+					val);
+}
+
+int mv88e6352_port_led_blink_set(struct mv88e6xxx_chip *chip, int port, u8 led,
+				 unsigned long *delay_on,
+				 unsigned long *delay_off)
+{
+	int err;
+	u16 val;
+
+	if (led > 1)
+		return -EINVAL;
+
+	if (port > 5)
+		return -EOPNOTSUPP;
+
+	/* Reset default is 84ms */
+	*delay_on = 84 / 2;
+	*delay_off = 84 / 2;
+	err = mv88e6352_port_led_read(chip, port,
+				      MV88E6352_PORT_LED_CTL_PTR_LED01,
+				      &val);
+	if (err)
+		return err;
+
+	if (led == 0) {
+		val &= ~MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_MASK;
+		val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_BLINK;
+	} else {
+		val &= ~MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_MASK;
+		val |= MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_BLINK;
+	}
+	return mv88e6352_port_led_write(chip, port,
+					MV88E6352_PORT_LED_CTL_PTR_LED01,
+					val);
+}
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 86deeb347cbc..72556e4d154c 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -294,6 +294,76 @@
 /* Offset 0x13: OutFiltered Counter */
 #define MV88E6XXX_PORT_OUT_FILTERED	0x13
 
+/* Offset 0x16: LED Control */
+#define MV88E6352_PORT_LED_CTL					0x16
+#define MV88E6352_PORT_LED_CTL_UPDATE				0x8000
+#define MV88E6352_PORT_LED_CTL_PTR_LED01			0x0000
+#define MV88E6352_PORT_LED_CTL_PTR_STRETCH_BLINK		0x6000
+#define MV88E6352_PORT_LED_CTL_PTR_SPECIAL			0x7000
+#define MV88E6352_PORT_LED_CTL_DATA_MASK			0x03ff
+/* Ports 0-4 */
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_P2_SPECIAL	0x0000
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_10_100_ACT	0x0010
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_1000		0x0030
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_P1_SPECIAL	0x0040
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_10_1000_ACT	0x0060
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_10_1000		0x0070
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_ACT		0x0080
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_100		0x0090
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_100_ACT		0x00A0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_10_100		0x00B0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_PTP_ACT		0x00C0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_BLINK		0x00D0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_OFF		0x00E0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_ON		0x00F0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_MASK		0x00F0
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED1_LINK_ACT		0x0000
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_100_1000_ACT	0x0001
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_1000_ACT		0x0002
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_LINK_ACT		0x0003
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_P0_SPECIAL	0x0004
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_DUPLEX_COL	0x0006
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_10_1000_ACT	0x0007
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_LINK		0x0008
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_10		0x0009
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_10_ACT		0x000A
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_100_1000		0x000B
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_PTP_ACT		0x000C
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_BLINK		0x000D
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_OFF		0x000E
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_ON		0x000F
+#define MV88E6352_PORT_LED_CTL_DATA_LED01_LED0_MASK		0x000F
+
+/* Port 5 */
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P6_ACT		0x0000
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_FIBER_1000_ACT	0x0010
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_FIBER_100_ACT   0x0020
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_FIBER		0x0030
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P5_ACT		0x0040
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P6_LINK		0x0050
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P6_DUPLEX_COL	0x0060
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P6_LINK_ACT	0x0070
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P0_SPECIAL	0x0080
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P1_SPECIAL	0x0090
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P2_SPECIAL	0x00A0
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P6_PTP_ACT	0x00C0
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_BLINK		0x00D0
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_OFF		0x00E0
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_ON		0x00F0
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED1_P5_LINK_ACT	0x0000
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_FIBER_100_ACT	0x0001
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_FIBER_1000_ACT	0x0002
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P0_SPECIAL	0x0003
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P1_SPECIAL	0x0004
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P2_SPECIAL	0x0005
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P5_DUPLEX_COL	0x0006
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P5_LINK_ACT	0x0007
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_P6_LINK_ACT	0x0008
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_BLINK		0x000D
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_OFF		0x000E
+#define MV88E6352_PORT_LED_CTL_DATA5_LED01_LED0_ON		0x000F
+/* Port 6 does not have any LEDs */
+
 /* Offset 0x18: IEEE Priority Mapping Table */
 #define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE			0x18
 #define MV88E6390_PORT_IEEE_PRIO_MAP_TABLE_UPDATE		0x8000
@@ -459,5 +529,9 @@ int mv88e6xxx_port_hidden_write(struct mv88e6xxx_chip *chip, int block,
 int mv88e6xxx_port_hidden_wait(struct mv88e6xxx_chip *chip);
 int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int block, int port,
 			       int reg, u16 *val);
-
+int mv88e6352_port_led_brightness_set(struct mv88e6xxx_chip *chip, int port,
+				      u8 led, enum led_brightness value);
+int mv88e6352_port_led_blink_set(struct mv88e6xxx_chip *chip, int port, u8 led,
+				 unsigned long *delay_on,
+				 unsigned long *delay_off);
 #endif /* _MV88E6XXX_PORT_H */
-- 
2.42.0

[PATCH RFC net-next 2/8] net: dsa: mv88e6xxx: Tie the low level LED functions to device ops

[Andrew Lunn]

Make the LED brightness and blink helpers available for the 6352
family via their ops structure.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 16 ++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h |  7 +++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 42b1acaca33a..6bd0a8e232a5 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4341,6 +4341,8 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
 	.phylink_get_caps = mv88e6185_phylink_get_caps,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -4395,6 +4397,8 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.phylink_get_caps = mv88e6352_phylink_get_caps,
 	.pcs_ops = &mv88e6352_pcs_ops,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -4441,6 +4445,8 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
 	.phylink_get_caps = mv88e6185_phylink_get_caps,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -4497,6 +4503,8 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.gpio_ops = &mv88e6352_gpio_ops,
 	.phylink_get_caps = mv88e6352_phylink_get_caps,
 	.pcs_ops = &mv88e6352_pcs_ops,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -4766,6 +4774,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.ptp_ops = &mv88e6352_ptp_ops,
 	.phylink_get_caps = mv88e6352_phylink_get_caps,
 	.pcs_ops = &mv88e6352_pcs_ops,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6250_ops = {
@@ -5070,6 +5080,8 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stu_getnext = mv88e6352_g1_stu_getnext,
 	.stu_loadpurge = mv88e6352_g1_stu_loadpurge,
 	.phylink_get_caps = mv88e6185_phylink_get_caps,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -5118,6 +5130,8 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.avb_ops = &mv88e6352_avb_ops,
 	.ptp_ops = &mv88e6352_ptp_ops,
 	.phylink_get_caps = mv88e6185_phylink_get_caps,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -5179,6 +5193,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.serdes_set_tx_amplitude = mv88e6352_serdes_set_tx_amplitude,
 	.phylink_get_caps = mv88e6352_phylink_get_caps,
 	.pcs_ops = &mv88e6352_pcs_ops,
+	.led_brightness_set = mv88e6352_port_led_brightness_set,
+	.led_blink_set = mv88e6352_port_led_blink_set,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 44383a03ef2f..9f6a2a7fdb1b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -649,6 +649,13 @@ struct mv88e6xxx_ops {
 
 	/* Max Frame Size */
 	int (*set_max_frame_size)(struct mv88e6xxx_chip *chip, int mtu);
+
+	/* LEDs */
+	int (*led_brightness_set)(struct mv88e6xxx_chip *chip, int port,
+				  u8 led, enum led_brightness value);
+	int (*led_blink_set)(struct mv88e6xxx_chip *chip, int port, u8 led,
+			     unsigned long *delay_on,
+			     unsigned long *delay_off);
 };
 
 struct mv88e6xxx_irq_ops {
-- 
2.42.0

[PATCH RFC net-next 3/8] net: dsa: Plumb LED brightnes and blink into switch API

[Andrew Lunn]

Allow the switch driver to expose its methods to control the LED
brightness and blinking to the DSA core.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 35 ++++++++++++++++++++++++++++++++
 include/net/dsa.h                |  8 ++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6bd0a8e232a5..38b0d8259fa0 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -6803,6 +6803,39 @@ static int mv88e6xxx_crosschip_lag_leave(struct dsa_switch *ds, int sw_index,
 	return err_sync ? : err_pvt;
 }
 
+static int mv88e6xxx_led_brightness_set(struct dsa_switch *ds, int port,
+					u8 led, enum led_brightness value)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	if (chip->info->ops->led_brightness_set) {
+		mv88e6xxx_reg_lock(chip);
+		err = chip->info->ops->led_brightness_set(chip, port, led,
+							  value);
+		mv88e6xxx_reg_unlock(chip);
+		return err;
+	}
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_led_blink_set(struct dsa_switch *ds, int port,
+				   u8 led, unsigned long *delay_on,
+				   unsigned long *delay_off)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	if (chip->info->ops->led_blink_set) {
+		mv88e6xxx_reg_lock(chip);
+		err = chip->info->ops->led_blink_set(chip, port, led,
+						     delay_on, delay_off);
+		mv88e6xxx_reg_unlock(chip);
+		return err;
+	}
+	return -EOPNOTSUPP;
+}
+
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_tag_protocol	= mv88e6xxx_get_tag_protocol,
 	.change_tag_protocol	= mv88e6xxx_change_tag_protocol,
@@ -6867,6 +6900,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.crosschip_lag_change	= mv88e6xxx_crosschip_lag_change,
 	.crosschip_lag_join	= mv88e6xxx_crosschip_lag_join,
 	.crosschip_lag_leave	= mv88e6xxx_crosschip_lag_leave,
+	.led_brightness_set	= mv88e6xxx_led_brightness_set,
+	.led_blink_set		= mv88e6xxx_led_blink_set,
 };
 
 static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 82135fbdb1e6..ae73765cd71c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1238,6 +1238,14 @@ struct dsa_switch_ops {
 	void	(*conduit_state_change)(struct dsa_switch *ds,
 					const struct net_device *conduit,
 					bool operational);
+
+	/*
+	 * LED control
+	 */
+	int (*led_brightness_set)(struct dsa_switch *ds, int port,
+				  u8 led, enum led_brightness value);
+	int (*led_blink_set)(struct dsa_switch *ds, int port, u8 led,
+			     unsigned long *delay_on, unsigned long *delay_off);
 };
 
 #define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
-- 
2.42.0

[PATCH RFC net-next 4/8] dsa: Create port LEDs based on DT binding

[Andrew Lunn]

Allow LEDs to be described in each ports DT subnode. Parse these when
setting up the ports, currently supporting brightness and link.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |   3 +
 net/dsa/dsa.c     | 138 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index ae73765cd71c..2e05e4fd0b76 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -325,6 +325,9 @@ struct dsa_port {
 		 */
 		struct list_head	user_vlans;
 	};
+
+	/* List of LEDs associated to this port */
+	struct list_head leds;
 };
 
 /* TODO: ideally DSA ports would have a single dp->link_dp member,
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index ac7be864e80d..b13748f9b519 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -34,6 +34,15 @@
 static DEFINE_MUTEX(dsa2_mutex);
 LIST_HEAD(dsa_tree_list);
 
+struct dsa_led {
+	struct list_head led_list;
+	struct dsa_port *dp;
+	struct led_classdev led_cdev;
+	u8 index;
+};
+
+#define to_dsa_led(d) container_of(d, struct dsa_led, led_cdev)
+
 static struct workqueue_struct *dsa_owq;
 
 /* Track the bridges with forwarding offload enabled */
@@ -461,6 +470,116 @@ static void dsa_tree_teardown_cpu_ports(struct dsa_switch_tree *dst)
 			dp->cpu_dp = NULL;
 }
 
+static int dsa_led_brightness_set(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_brightness_set(ds, dp->index, dsa_led->index,
+					   value);
+}
+
+static int dsa_led_blink_set(struct led_classdev *led_cdev,
+			     unsigned long *delay_on, unsigned long *delay_off)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_blink_set(ds, dp->index, dsa_led->index,
+				      delay_on, delay_off);
+}
+
+static int dsa_port_led_setup(struct dsa_port *dp,
+			      struct device_node *led)
+{
+	struct led_init_data init_data = {};
+	struct dsa_switch *ds = dp->ds;
+	struct led_classdev *cdev;
+	struct dsa_led *dsa_led;
+	u32 index;
+	int err;
+
+	dsa_led = devm_kzalloc(ds->dev, sizeof(*dsa_led), GFP_KERNEL);
+	if (!dsa_led)
+		return -ENOMEM;
+
+	dsa_led->dp = dp;
+	cdev = &dsa_led->led_cdev;
+
+	err = of_property_read_u32(led, "reg", &index);
+	if (err)
+		return err;
+	if (index > 255)
+		return -EINVAL;
+
+	dsa_led->index = index;
+
+	if (ds->ops->led_brightness_set)
+		cdev->brightness_set_blocking = dsa_led_brightness_set;
+	if (ds->ops->led_blink_set)
+		cdev->blink_set = dsa_led_blink_set;
+
+	cdev->max_brightness = 1;
+
+	init_data.fwnode = of_fwnode_handle(led);
+	if (dp->user) {
+		init_data.devicename = dev_name(&dp->user->dev);
+		err = devm_led_classdev_register_ext(&dp->user->dev, cdev,
+						     &init_data);
+	} else {
+		init_data.devicename = kasprintf(GFP_KERNEL, "%s:%d",
+						 dev_name(ds->dev), dp->index);
+		err = devm_led_classdev_register_ext(ds->dev, cdev, &init_data);
+	}
+	if (err)
+		return err;
+
+	INIT_LIST_HEAD(&dsa_led->led_list);
+	list_add(&dsa_led->led_list, &dp->leds);
+
+	if (!dp->user)
+		kfree(init_data.devicename);
+
+	return 0;
+}
+
+static int dsa_port_leds_setup(struct dsa_port *dp)
+{
+	struct device_node *leds, *led;
+	int err;
+
+	if (!dp->dn)
+		return 0;
+
+	leds = of_get_child_by_name(dp->dn, "leds");
+	if (!leds)
+		return 0;
+
+	for_each_available_child_of_node(leds, led) {
+		err = dsa_port_led_setup(dp, led);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void dsa_port_leds_teardown(struct dsa_port *dp)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct device *dev = ds->dev;
+	struct led_classdev *cdev;
+	struct dsa_led *dsa_led;
+
+	list_for_each_entry(dsa_led, &dp->leds, led_list) {
+		cdev = &dsa_led->led_cdev;
+		devm_led_classdev_unregister(dev, cdev);
+	}
+}
+
 static int dsa_port_setup(struct dsa_port *dp)
 {
 	bool dsa_port_link_registered = false;
@@ -494,6 +613,11 @@ static int dsa_port_setup(struct dsa_port *dp)
 		err = dsa_port_enable(dp, NULL);
 		if (err)
 			break;
+
+		err = dsa_port_leds_setup(dp);
+		if (err)
+			break;
+
 		dsa_port_enabled = true;
 
 		break;
@@ -512,12 +636,22 @@ static int dsa_port_setup(struct dsa_port *dp)
 		err = dsa_port_enable(dp, NULL);
 		if (err)
 			break;
+
+		err = dsa_port_leds_setup(dp);
+		if (err)
+			break;
+
 		dsa_port_enabled = true;
 
 		break;
 	case DSA_PORT_TYPE_USER:
 		of_get_mac_address(dp->dn, dp->mac);
 		err = dsa_user_create(dp);
+		if (err)
+			break;
+
+		err = dsa_port_leds_setup(dp);
+
 		break;
 	}
 
@@ -544,11 +678,13 @@ static void dsa_port_teardown(struct dsa_port *dp)
 	case DSA_PORT_TYPE_UNUSED:
 		break;
 	case DSA_PORT_TYPE_CPU:
+		dsa_port_leds_teardown(dp);
 		dsa_port_disable(dp);
 		if (dp->dn)
 			dsa_shared_port_link_unregister_of(dp);
 		break;
 	case DSA_PORT_TYPE_DSA:
+		dsa_port_leds_teardown(dp);
 		dsa_port_disable(dp);
 		if (dp->dn)
 			dsa_shared_port_link_unregister_of(dp);
@@ -556,6 +692,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
 	case DSA_PORT_TYPE_USER:
 		if (dp->user) {
 			dsa_user_destroy(dp->user);
+			dsa_port_leds_teardown(dp);
 			dp->user = NULL;
 		}
 		break;
@@ -1108,6 +1245,7 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
 	INIT_LIST_HEAD(&dp->mdbs);
 	INIT_LIST_HEAD(&dp->vlans); /* also initializes &dp->user_vlans */
 	INIT_LIST_HEAD(&dp->list);
+	INIT_LIST_HEAD(&dp->leds);
 	list_add_tail(&dp->list, &dst->ports);
 
 	return dp;
-- 
2.42.0

[PATCH RFC net-next 5/8] dsa: Plumb in LED calls needed for hardware offload

[Andrew Lunn]

In order to offload blinking of the LED to hardware, additional calls
are needed into the LED driver. Add them to the DSA core abstraction.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/dsa.h |  6 +++++
 net/dsa/dsa.c     | 57 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2e05e4fd0b76..19f1338ac604 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1249,6 +1249,12 @@ struct dsa_switch_ops {
 				  u8 led, enum led_brightness value);
 	int (*led_blink_set)(struct dsa_switch *ds, int port, u8 led,
 			     unsigned long *delay_on, unsigned long *delay_off);
+	int (*led_hw_control_is_supported)(struct dsa_switch *ds, int port,
+					   u8 led, unsigned long flags);
+	int (*led_hw_control_set)(struct dsa_switch *ds, int port, u8 led,
+				  unsigned long flags);
+	int (*led_hw_control_get)(struct dsa_switch *ds, int port, u8 led,
+				  unsigned long *flags);
 };
 
 #define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b13748f9b519..16e51020bc5e 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -492,6 +492,52 @@ static int dsa_led_blink_set(struct led_classdev *led_cdev,
 				      delay_on, delay_off);
 }
 
+static __maybe_unused int
+dsa_led_hw_control_is_supported(struct led_classdev *led_cdev,
+				unsigned long flags)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_hw_control_is_supported(ds, dp->index,
+						    dsa_led->index,
+						    flags);
+}
+
+static __maybe_unused int dsa_led_hw_control_set(struct led_classdev *led_cdev,
+						 unsigned long flags)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_hw_control_set(ds, dp->index, dsa_led->index,
+					   flags);
+}
+
+static __maybe_unused int dsa_led_hw_control_get(struct led_classdev *led_cdev,
+						 unsigned long *flags)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+	struct dsa_switch *ds = dp->ds;
+
+	return ds->ops->led_hw_control_get(ds, dp->index, dsa_led->index,
+					   flags);
+}
+
+static struct device *
+dsa_led_hw_control_get_device(struct led_classdev *led_cdev)
+{
+	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
+	struct dsa_port *dp = dsa_led->dp;
+
+	if (dp->user)
+		return &dp->user->dev;
+	return NULL;
+}
+
 static int dsa_port_led_setup(struct dsa_port *dp,
 			      struct device_node *led)
 {
@@ -521,7 +567,16 @@ static int dsa_port_led_setup(struct dsa_port *dp,
 		cdev->brightness_set_blocking = dsa_led_brightness_set;
 	if (ds->ops->led_blink_set)
 		cdev->blink_set = dsa_led_blink_set;
-
+#ifdef CONFIG_LEDS_TRIGGERS
+	if (ds->ops->led_hw_control_is_supported)
+		cdev->hw_control_is_supported = dsa_led_hw_control_is_supported;
+	if (ds->ops->led_hw_control_set)
+		cdev->hw_control_set = dsa_led_hw_control_set;
+	if (ds->ops->led_hw_control_get)
+		cdev->hw_control_get = dsa_led_hw_control_get;
+	cdev->hw_control_trigger = "netdev";
+#endif
+	cdev->hw_control_get_device = dsa_led_hw_control_get_device;
 	cdev->max_brightness = 1;
 
 	init_data.fwnode = of_fwnode_handle(led);
-- 
2.42.0

[PATCH RFC net-next 6/8] dsa: mv88e6xxx: Plumb in LED offload functions

[Andrew Lunn]

Add the plumbing needed for each switch family to support offloading
the LED blinking to hardware.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 52 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/chip.h |  7 +++++
 2 files changed, 59 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 38b0d8259fa0..645fba8937ee 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -6836,6 +6836,55 @@ static int mv88e6xxx_led_blink_set(struct dsa_switch *ds, int port,
 	return -EOPNOTSUPP;
 }
 
+static int mv88e6xxx_led_hw_control_is_supported(struct dsa_switch *ds,
+						 int port, u8 led,
+						 unsigned long flags)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	if (chip->info->ops->led_hw_control_is_supported) {
+		mv88e6xxx_reg_lock(chip);
+		err = chip->info->ops->led_hw_control_is_supported(chip, port,
+								   led, flags);
+		mv88e6xxx_reg_unlock(chip);
+		return err;
+	}
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_led_hw_control_set(struct dsa_switch *ds, int port,
+					u8 led, unsigned long flags)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	if (chip->info->ops->led_hw_control_set) {
+		mv88e6xxx_reg_lock(chip);
+		err = chip->info->ops->led_hw_control_set(chip, port,
+							  led, flags);
+		mv88e6xxx_reg_unlock(chip);
+		return err;
+	}
+	return -EOPNOTSUPP;
+}
+
+static int mv88e6xxx_led_hw_control_get(struct dsa_switch *ds, int port,
+					u8 led, unsigned long *flags)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	int err;
+
+	if (chip->info->ops->led_hw_control_get) {
+		mv88e6xxx_reg_lock(chip);
+		err = chip->info->ops->led_hw_control_get(chip, port,
+							  led, flags);
+		mv88e6xxx_reg_unlock(chip);
+		return err;
+	}
+	return -EOPNOTSUPP;
+}
+
 static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_tag_protocol	= mv88e6xxx_get_tag_protocol,
 	.change_tag_protocol	= mv88e6xxx_change_tag_protocol,
@@ -6902,6 +6951,9 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.crosschip_lag_leave	= mv88e6xxx_crosschip_lag_leave,
 	.led_brightness_set	= mv88e6xxx_led_brightness_set,
 	.led_blink_set		= mv88e6xxx_led_blink_set,
+	.led_hw_control_is_supported = mv88e6xxx_led_hw_control_is_supported,
+	.led_hw_control_set	= mv88e6xxx_led_hw_control_set,
+	.led_hw_control_get	= mv88e6xxx_led_hw_control_get,
 };
 
 static int mv88e6xxx_register_switch(struct mv88e6xxx_chip *chip)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 9f6a2a7fdb1b..5a4daf288c17 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -656,6 +656,13 @@ struct mv88e6xxx_ops {
 	int (*led_blink_set)(struct mv88e6xxx_chip *chip, int port, u8 led,
 			     unsigned long *delay_on,
 			     unsigned long *delay_off);
+	int (*led_hw_control_is_supported)(struct mv88e6xxx_chip *chip,
+					   int port,
+					   u8 led, unsigned long flags);
+	int (*led_hw_control_set)(struct mv88e6xxx_chip *chip, int port,
+				  u8 led, unsigned long flags);
+	int (*led_hw_control_get)(struct mv88e6xxx_chip *chip, int port,
+				  u8 led, unsigned long *flags);
 };
 
 struct mv88e6xxx_irq_ops {
-- 
2.42.0

[PATCH RFC net-next 7/8] arm: boot: dts: mvebu: linksys-mamba: Add Ethernet LEDs

[Andrew Lunn]

List the front panel Ethernet LEDs in the switch section of the
device tree. They can then be controlled via /sys/class/led/

The node contains a label property to influence the name of the LED.
Without it, all the LEDs get the name lan:white, which classes, and so
some get a number appended. lan:white_1, lan:white_2, etc. Using the
label the LEDs are named lan1:front, lan2:front, lan3:front, where
lanX indicates the interface name, and front indicates they are on the
front of the box.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../dts/marvell/armada-xp-linksys-mamba.dts   | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/arch/arm/boot/dts/marvell/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/marvell/armada-xp-linksys-mamba.dts
index 7a0614fd0c93..f3949e992d56 100644
--- a/arch/arm/boot/dts/marvell/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/marvell/armada-xp-linksys-mamba.dts
@@ -19,6 +19,7 @@
 /dts-v1/;
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
 #include "armada-xp-mv78230.dtsi"
 
 / {
@@ -278,26 +279,91 @@ ports {
 			port@0 {
 				reg = <0>;
 				label = "lan4";
+
+				leds {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_WHITE>;
+						function = LED_FUNCTION_LAN;
+						label = "front";
+						default-state = "keep";
+					};
+				};
 			};
 
 			port@1 {
 				reg = <1>;
 				label = "lan3";
+
+				leds {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_WHITE>;
+						function = LED_FUNCTION_LAN;
+						label = "front";
+						default-state = "keep";
+					};
+				};
 			};
 
 			port@2 {
 				reg = <2>;
 				label = "lan2";
+
+				leds {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_WHITE>;
+						function = LED_FUNCTION_LAN;
+						label = "front";
+						default-state = "keep";
+					};
+				};
 			};
 
 			port@3 {
 				reg = <3>;
 				label = "lan1";
+
+				leds {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_WHITE>;
+						function = LED_FUNCTION_LAN;
+						label = "front";
+						default-state = "keep";
+					};
+				};
 			};
 
 			port@4 {
 				reg = <4>;
 				label = "internet";
+
+				leds {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					led@0 {
+						reg = <0>;
+						color = <LED_COLOR_ID_WHITE>;
+						function = LED_FUNCTION_LAN;
+						label = "front";
+						default-state = "keep";
+					};
+				};
 			};
 
 			port@5 {
-- 
2.42.0

[PATCH RFC net-next 8/8] dsa: qca8k: Use DSA common code for LEDs

[Andrew Lunn]

Rather than parse the device tree in the qca8k driver, make use of the
common code in the DSA core.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/qca/qca8k-8xxx.c |  11 +-
 drivers/net/dsa/qca/qca8k-leds.c | 255 +++++--------------------------
 drivers/net/dsa/qca/qca8k.h      |   9 --
 drivers/net/dsa/qca/qca8k_leds.h |  21 ++-
 4 files changed, 56 insertions(+), 240 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index ec57d9d52072..4929894a2b5d 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1838,10 +1838,6 @@ 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);
 
@@ -2018,6 +2014,13 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
 	.port_lag_leave		= qca8k_port_lag_leave,
 	.conduit_state_change	= qca8k_conduit_change,
 	.connect_tag_protocol	= qca8k_connect_tag_protocol,
+#ifdef CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT
+	.led_brightness_set	= qca8k_led_brightness_set,
+	.led_blink_set		= qca8k_led_blink_set,
+	.led_hw_control_is_supported = qca8k_led_hw_control_is_supported,
+	.led_hw_control_set	= qca8k_led_hw_control_set,
+	.led_hw_control_get	= qca8k_led_hw_control_get,
+#endif
 };
 
 static int
diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index 90e30c2909e4..febae23b65a9 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -6,18 +6,6 @@
 #include "qca8k.h"
 #include "qca8k_leds.h"
 
-static u32 qca8k_phy_to_port(int phy)
-{
-	/* Internal PHY 0 has port at index 1.
-	 * Internal PHY 1 has port at index 2.
-	 * Internal PHY 2 has port at index 3.
-	 * Internal PHY 3 has port at index 4.
-	 * Internal PHY 4 has port at index 5.
-	 */
-
-	return phy + 1;
-}
-
 static int
 qca8k_get_enable_led_reg(int port_num, int led_num, struct qca8k_led_pattern_en *reg_info)
 {
@@ -91,15 +79,15 @@ qca8k_parse_netdev(unsigned long rules, u32 *offload_trigger)
 	return 0;
 }
 
-static int
-qca8k_led_brightness_set(struct qca8k_led *led,
-			 enum led_brightness brightness)
+int
+qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
+			 u8 led_num, enum led_brightness brightness)
 {
 	struct qca8k_led_pattern_en reg_info;
-	struct qca8k_priv *priv = led->priv;
+	struct qca8k_priv *priv = ds->priv;
 	u32 mask, val;
 
-	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_enable_led_reg(port_num, led_num, &reg_info);
 
 	val = QCA8K_LED_ALWAYS_OFF;
 	if (brightness)
@@ -139,7 +127,7 @@ qca8k_led_brightness_set(struct qca8k_led *led,
 	 * to calculate the shift and the correct reg due to this problem of
 	 * not having a 1:1 map of LED with the regs.
 	 */
-	if (led->port_num == 0 || led->port_num == 4) {
+	if (port_num == 0 || port_num == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -151,51 +139,14 @@ qca8k_led_brightness_set(struct qca8k_led *led,
 				  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;
-	}
-
-	/* Assume brightness ON only when the LED is set to always ON */
-	return val == QCA8K_LED_ALWAYS_ON;
-}
-
-static int
-qca8k_cled_blink_set(struct led_classdev *ldev,
-		     unsigned long *delay_on,
-		     unsigned long *delay_off)
+int
+qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
+		    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;
+	struct qca8k_priv *priv = ds->priv;
 
 	if (*delay_on == 0 && *delay_off == 0) {
 		*delay_on = 125;
@@ -209,9 +160,9 @@ qca8k_cled_blink_set(struct led_classdev *ldev,
 		return -EINVAL;
 	}
 
-	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_enable_led_reg(port_num, led_num, &reg_info);
 
-	if (led->port_num == 0 || led->port_num == 4) {
+	if (port_num == 0 || port_num == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -225,20 +176,18 @@ qca8k_cled_blink_set(struct led_classdev *ldev,
 }
 
 static int
-qca8k_cled_trigger_offload(struct led_classdev *ldev, bool enable)
+qca8k_led_trigger_offload(struct qca8k_priv *priv, int port_num, u8 led_num,
+			  bool enable)
 {
-	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
-
 	struct qca8k_led_pattern_en reg_info;
-	struct qca8k_priv *priv = led->priv;
 	u32 mask, val = QCA8K_LED_ALWAYS_OFF;
 
-	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_enable_led_reg(port_num, led_num, &reg_info);
 
 	if (enable)
 		val = QCA8K_LED_RULE_CONTROLLED;
 
-	if (led->port_num == 0 || led->port_num == 4) {
+	if (port_num == 0 || port_num == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -250,21 +199,18 @@ qca8k_cled_trigger_offload(struct led_classdev *ldev, bool enable)
 }
 
 static bool
-qca8k_cled_hw_control_status(struct led_classdev *ldev)
+qca8k_led_hw_control_status(struct qca8k_priv *priv, int port_num, u8 led_num)
 {
-	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
-
 	struct qca8k_led_pattern_en reg_info;
-	struct qca8k_priv *priv = led->priv;
 	u32 val;
 
-	qca8k_get_enable_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_enable_led_reg(port_num, led_num, &reg_info);
 
 	regmap_read(priv->regmap, reg_info.reg, &val);
 
 	val >>= reg_info.shift;
 
-	if (led->port_num == 0 || led->port_num == 4) {
+	if (port_num == 0 || port_num == 4) {
 		val &= QCA8K_LED_PATTERN_EN_MASK;
 		val >>= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -274,20 +220,22 @@ qca8k_cled_hw_control_status(struct led_classdev *ldev)
 	return val == QCA8K_LED_RULE_CONTROLLED;
 }
 
-static int
-qca8k_cled_hw_control_is_supported(struct led_classdev *ldev, unsigned long rules)
+int
+qca8k_led_hw_control_is_supported(struct dsa_switch *ds,
+				  int port, u8 led,
+				  unsigned long rules)
 {
 	u32 offload_trigger = 0;
 
 	return qca8k_parse_netdev(rules, &offload_trigger);
 }
 
-static int
-qca8k_cled_hw_control_set(struct led_classdev *ldev, unsigned long rules)
+int
+qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
+			 unsigned long rules)
 {
-	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
 	struct qca8k_led_pattern_en reg_info;
-	struct qca8k_priv *priv = led->priv;
+	struct qca8k_priv *priv = ds->priv;
 	u32 offload_trigger = 0;
 	int ret;
 
@@ -295,31 +243,31 @@ qca8k_cled_hw_control_set(struct led_classdev *ldev, unsigned long rules)
 	if (ret)
 		return ret;
 
-	ret = qca8k_cled_trigger_offload(ldev, true);
+	ret = qca8k_led_trigger_offload(priv, port_num, led_num, true);
 	if (ret)
 		return ret;
 
-	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_control_led_reg(port_num, led_num, &reg_info);
 
 	return regmap_update_bits(priv->regmap, reg_info.reg,
 				  QCA8K_LED_RULE_MASK << reg_info.shift,
 				  offload_trigger << reg_info.shift);
 }
 
-static int
-qca8k_cled_hw_control_get(struct led_classdev *ldev, unsigned long *rules)
+int
+qca8k_led_hw_control_get(struct dsa_switch *ds, int port_num, u8 led_num,
+			 unsigned long *rules)
 {
-	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
 	struct qca8k_led_pattern_en reg_info;
-	struct qca8k_priv *priv = led->priv;
+	struct qca8k_priv *priv = ds->priv;
 	u32 val;
 	int ret;
 
 	/* With hw control not active return err */
-	if (!qca8k_cled_hw_control_status(ldev))
+	if (!qca8k_led_hw_control_status(priv, port_num, led_num))
 		return -EINVAL;
 
-	qca8k_get_control_led_reg(led->port_num, led->led_num, &reg_info);
+	qca8k_get_control_led_reg(port_num, led_num, &reg_info);
 
 	ret = regmap_read(priv->regmap, reg_info.reg, &val);
 	if (ret)
@@ -346,134 +294,3 @@ qca8k_cled_hw_control_get(struct led_classdev *ldev, unsigned long *rules)
 
 	return 0;
 }
-
-static struct device *qca8k_cled_hw_control_get_device(struct led_classdev *ldev)
-{
-	struct qca8k_led *led = container_of(ldev, struct qca8k_led, cdev);
-	struct qca8k_priv *priv = led->priv;
-	struct dsa_port *dp;
-
-	dp = dsa_to_port(priv->ds, qca8k_phy_to_port(led->port_num));
-	if (!dp)
-		return NULL;
-	if (dp->user)
-		return &dp->user->dev;
-	return NULL;
-}
-
-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 dsa_switch *ds = priv->ds;
-	enum led_default_state state;
-	struct qca8k_led *port_led;
-	int led_num, led_index;
-	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 most 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 %d defined for port %d",
-				 led_num, port_num);
-			continue;
-		}
-
-		led_index = QCA8K_LED_PORT_INDEX(port_num, led_num);
-
-		port_led = &priv->ports_led[led_index];
-		port_led->port_num = port_num;
-		port_led->led_num = led_num;
-		port_led->priv = priv;
-
-		state = led_init_default_state_get(led);
-		switch (state) {
-		case LEDS_DEFSTATE_ON:
-			port_led->cdev.brightness = 1;
-			qca8k_led_brightness_set(port_led, 1);
-			break;
-		case LEDS_DEFSTATE_KEEP:
-			port_led->cdev.brightness =
-					qca8k_led_brightness_get(port_led);
-			break;
-		default:
-			port_led->cdev.brightness = 0;
-			qca8k_led_brightness_set(port_led, 0);
-		}
-
-		port_led->cdev.max_brightness = 1;
-		port_led->cdev.brightness_set_blocking = qca8k_cled_brightness_set_blocking;
-		port_led->cdev.blink_set = qca8k_cled_blink_set;
-		port_led->cdev.hw_control_is_supported = qca8k_cled_hw_control_is_supported;
-		port_led->cdev.hw_control_set = qca8k_cled_hw_control_set;
-		port_led->cdev.hw_control_get = qca8k_cled_hw_control_get;
-		port_led->cdev.hw_control_get_device = qca8k_cled_hw_control_get_device;
-		port_led->cdev.hw_control_trigger = "netdev";
-		init_data.default_label = ":port";
-		init_data.fwnode = led;
-		init_data.devname_mandatory = true;
-		init_data.devicename = kasprintf(GFP_KERNEL, "%s:0%d", ds->user_mii_bus->id,
-						 port_num);
-		if (!init_data.devicename)
-			return -ENOMEM;
-
-		ret = devm_led_classdev_register_ext(priv->dev, &port_led->cdev, &init_data);
-		if (ret)
-			dev_warn(priv->dev, "Failed to init LED %d for port %d", led_num, port_num);
-
-		kfree(init_data.devicename);
-	}
-
-	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!");
-		return 0;
-	}
-
-	fwnode_for_each_child_node(ports, port) {
-		if (fwnode_property_read_u32(port, "reg", &port_num))
-			continue;
-
-		/* Skip checking for CPU port 0 and CPU port 6 as not supported */
-		if (port_num == 0 || port_num == 6)
-			continue;
-
-		/* Each port can have at most 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 2ac7e88f8da5..bf0f78f5390d 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -433,14 +433,6 @@ struct qca8k_led_pattern_en {
 	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;
@@ -465,7 +457,6 @@ 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 {
diff --git a/drivers/net/dsa/qca/qca8k_leds.h b/drivers/net/dsa/qca/qca8k_leds.h
index ab367f05b173..1c020d0f2fdc 100644
--- a/drivers/net/dsa/qca/qca8k_leds.h
+++ b/drivers/net/dsa/qca/qca8k_leds.h
@@ -5,12 +5,17 @@
 
 /* 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
-
+int qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
+			     u8 led_num, enum led_brightness brightness);
+int qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
+			unsigned long *delay_on,
+			unsigned long *delay_off);
+int qca8k_led_hw_control_is_supported(struct dsa_switch *ds,
+				      int port, u8 led,
+				      unsigned long rules);
+int qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
+			     unsigned long rules);
+int qca8k_led_hw_control_get(struct dsa_switch *ds, int port_num, u8 led_num,
+			     unsigned long *rules);
+#endif /* CONFIG_NET_DSA_QCA8K_LEDS_SUPPORT */
 #endif /* __QCA8K_LEDS_H */
-- 
2.42.0

Re: [PATCH RFC net-next 4/8] dsa: Create port LEDs based on DT binding

[Christian Marangi]

On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:
> Allow LEDs to be described in each ports DT subnode. Parse these when
> setting up the ports, currently supporting brightness and link.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/dsa.h |   3 +
>  net/dsa/dsa.c     | 138 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 141 insertions(+)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index ae73765cd71c..2e05e4fd0b76 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -325,6 +325,9 @@ struct dsa_port {
>  		 */
>  		struct list_head	user_vlans;
>  	};
> +
> +	/* List of LEDs associated to this port */
> +	struct list_head leds;
>  };
>  
>  /* TODO: ideally DSA ports would have a single dp->link_dp member,
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index ac7be864e80d..b13748f9b519 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -34,6 +34,15 @@
>  static DEFINE_MUTEX(dsa2_mutex);
>  LIST_HEAD(dsa_tree_list);
>  
> +struct dsa_led {
> +	struct list_head led_list;
> +	struct dsa_port *dp;
> +	struct led_classdev led_cdev;
> +	u8 index;
> +};
> +
> +#define to_dsa_led(d) container_of(d, struct dsa_led, led_cdev)
> +
>  static struct workqueue_struct *dsa_owq;
>  
>  /* Track the bridges with forwarding offload enabled */
> @@ -461,6 +470,116 @@ static void dsa_tree_teardown_cpu_ports(struct dsa_switch_tree *dst)
>  			dp->cpu_dp = NULL;
>  }
>  
> +static int dsa_led_brightness_set(struct led_classdev *led_cdev,
> +				  enum led_brightness value)
> +{
> +	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
> +	struct dsa_port *dp = dsa_led->dp;
> +	struct dsa_switch *ds = dp->ds;
> +
> +	return ds->ops->led_brightness_set(ds, dp->index, dsa_led->index,
> +					   value);
> +}
> +
> +static int dsa_led_blink_set(struct led_classdev *led_cdev,
> +			     unsigned long *delay_on, unsigned long *delay_off)
> +{
> +	struct dsa_led *dsa_led = to_dsa_led(led_cdev);
> +	struct dsa_port *dp = dsa_led->dp;
> +	struct dsa_switch *ds = dp->ds;
> +
> +	return ds->ops->led_blink_set(ds, dp->index, dsa_led->index,
> +				      delay_on, delay_off);
> +}
> +
> +static int dsa_port_led_setup(struct dsa_port *dp,
> +			      struct device_node *led)
> +{
> +	struct led_init_data init_data = {};
> +	struct dsa_switch *ds = dp->ds;
> +	struct led_classdev *cdev;
> +	struct dsa_led *dsa_led;
> +	u32 index;
> +	int err;
> +
> +	dsa_led = devm_kzalloc(ds->dev, sizeof(*dsa_led), GFP_KERNEL);
> +	if (!dsa_led)
> +		return -ENOMEM;
> +
> +	dsa_led->dp = dp;
> +	cdev = &dsa_led->led_cdev;
> +
> +	err = of_property_read_u32(led, "reg", &index);
> +	if (err)
> +		return err;
> +	if (index > 255)
> +		return -EINVAL;
> +
> +	dsa_led->index = index;
> +
> +	if (ds->ops->led_brightness_set)
> +		cdev->brightness_set_blocking = dsa_led_brightness_set;
> +	if (ds->ops->led_blink_set)
> +		cdev->blink_set = dsa_led_blink_set;
> +
> +	cdev->max_brightness = 1;
> +
> +	init_data.fwnode = of_fwnode_handle(led);

Please add

init_data.devname_mandatory = true;

cled will derive the name based on color action and won't include the
devname resulting in LEDs having the same name.

> +	if (dp->user) {
> +		init_data.devicename = dev_name(&dp->user->dev);
> +		err = devm_led_classdev_register_ext(&dp->user->dev, cdev,
> +						     &init_data);
> +	} else {
> +		init_data.devicename = kasprintf(GFP_KERNEL, "%s:%d",
> +						 dev_name(ds->dev), dp->index);
> +		err = devm_led_classdev_register_ext(ds->dev, cdev, &init_data);
> +	}
> +	if (err)
> +		return err;
> +
> +	INIT_LIST_HEAD(&dsa_led->led_list);
> +	list_add(&dsa_led->led_list, &dp->leds);
> +
> +	if (!dp->user)
> +		kfree(init_data.devicename);
> +
> +	return 0;
> +}
> +
> +static int dsa_port_leds_setup(struct dsa_port *dp)
> +{
> +	struct device_node *leds, *led;
> +	int err;
> +
> +	if (!dp->dn)
> +		return 0;
> +
> +	leds = of_get_child_by_name(dp->dn, "leds");
> +	if (!leds)
> +		return 0;
> +
> +	for_each_available_child_of_node(leds, led) {
> +		err = dsa_port_led_setup(dp, led);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void dsa_port_leds_teardown(struct dsa_port *dp)
> +{
> +	struct dsa_switch *ds = dp->ds;
> +	struct device *dev = ds->dev;
> +	struct led_classdev *cdev;
> +	struct dsa_led *dsa_led;
> +
> +	list_for_each_entry(dsa_led, &dp->leds, led_list) {
> +		cdev = &dsa_led->led_cdev;
> +		devm_led_classdev_unregister(dev, cdev);
> +	}
> +}
> +
>  static int dsa_port_setup(struct dsa_port *dp)
>  {
>  	bool dsa_port_link_registered = false;
> @@ -494,6 +613,11 @@ static int dsa_port_setup(struct dsa_port *dp)
>  		err = dsa_port_enable(dp, NULL);
>  		if (err)
>  			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +		if (err)
> +			break;
> +
>  		dsa_port_enabled = true;
>  
>  		break;
> @@ -512,12 +636,22 @@ static int dsa_port_setup(struct dsa_port *dp)
>  		err = dsa_port_enable(dp, NULL);
>  		if (err)
>  			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +		if (err)
> +			break;
> +
>  		dsa_port_enabled = true;
>  
>  		break;
>  	case DSA_PORT_TYPE_USER:
>  		of_get_mac_address(dp->dn, dp->mac);
>  		err = dsa_user_create(dp);
> +		if (err)
> +			break;
> +
> +		err = dsa_port_leds_setup(dp);
> +
>  		break;
>  	}
>  
> @@ -544,11 +678,13 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  	case DSA_PORT_TYPE_UNUSED:
>  		break;
>  	case DSA_PORT_TYPE_CPU:
> +		dsa_port_leds_teardown(dp);
>  		dsa_port_disable(dp);
>  		if (dp->dn)
>  			dsa_shared_port_link_unregister_of(dp);
>  		break;
>  	case DSA_PORT_TYPE_DSA:
> +		dsa_port_leds_teardown(dp);
>  		dsa_port_disable(dp);
>  		if (dp->dn)
>  			dsa_shared_port_link_unregister_of(dp);
> @@ -556,6 +692,7 @@ static void dsa_port_teardown(struct dsa_port *dp)
>  	case DSA_PORT_TYPE_USER:
>  		if (dp->user) {
>  			dsa_user_destroy(dp->user);
> +			dsa_port_leds_teardown(dp);
>  			dp->user = NULL;
>  		}
>  		break;
> @@ -1108,6 +1245,7 @@ static struct dsa_port *dsa_port_touch(struct dsa_switch *ds, int index)
>  	INIT_LIST_HEAD(&dp->mdbs);
>  	INIT_LIST_HEAD(&dp->vlans); /* also initializes &dp->user_vlans */
>  	INIT_LIST_HEAD(&dp->list);
> +	INIT_LIST_HEAD(&dp->leds);
>  	list_add_tail(&dp->list, &dst->ports);
>  
>  	return dp;
> -- 
> 2.42.0
> 

-- 
	Ansuel

Re: [PATCH RFC net-next 8/8] dsa: qca8k: Use DSA common code for LEDs

[Christian Marangi]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Wed, Nov 29, 2023 at 12:21:35AM +0100, Andrew Lunn wrote:
> Rather than parse the device tree in the qca8k driver, make use of the
> common code in the DSA core.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Hi,

I attached a fixup patch for this to correctly work. Since we are using
port_num the thing needs to be decrememted by one.

-- 
	Ansuel

[-- Attachment #2: 0001-fixup-dsa-qca8k-Use-DSA-common-code-for-LEDs.patch --]
[-- Type: text/x-diff, Size: 2802 bytes --]

From 95598e6892cb0d392249f099587bd81ea3b664de Mon Sep 17 00:00:00 2001
From: Christian Marangi <ansuelsmth@gmail.com>
Date: Wed, 29 Nov 2023 02:50:09 +0100
Subject: [PATCH] fixup! dsa: qca8k: Use DSA common code for LEDs

Fix off-by-one from port_num to phy.

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

diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index febae23b65a9..67f42a0dbbd8 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -9,7 +9,7 @@
 static int
 qca8k_get_enable_led_reg(int port_num, int led_num, struct qca8k_led_pattern_en *reg_info)
 {
-	switch (port_num) {
+	switch (qca8k_port_to_phy(port_num)) {
 	case 0:
 		reg_info->reg = QCA8K_LED_CTRL_REG(led_num);
 		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
@@ -41,7 +41,7 @@ qca8k_get_control_led_reg(int port_num, int led_num, struct qca8k_led_pattern_en
 	 * 3 control rules for phy0-3 that applies to all their leds
 	 * 3 control rules for phy4
 	 */
-	if (port_num == 4)
+	if (qca8k_port_to_phy(port_num) == 4)
 		reg_info->shift = QCA8K_LED_PHY4_CONTROL_RULE_SHIFT;
 	else
 		reg_info->shift = QCA8K_LED_PHY0123_CONTROL_RULE_SHIFT;
@@ -127,7 +127,8 @@ qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
 	 * to calculate the shift and the correct reg due to this problem of
 	 * not having a 1:1 map of LED with the regs.
 	 */
-	if (port_num == 0 || port_num == 4) {
+	if (qca8k_port_to_phy(port_num) == 0 ||
+	    qca8k_port_to_phy(port_num) == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -162,7 +163,8 @@ qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
 
 	qca8k_get_enable_led_reg(port_num, led_num, &reg_info);
 
-	if (port_num == 0 || port_num == 4) {
+	if (qca8k_port_to_phy(port_num) == 0 ||
+	    qca8k_port_to_phy(port_num) == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -187,7 +189,8 @@ qca8k_led_trigger_offload(struct qca8k_priv *priv, int port_num, u8 led_num,
 	if (enable)
 		val = QCA8K_LED_RULE_CONTROLLED;
 
-	if (port_num == 0 || port_num == 4) {
+	if (qca8k_port_to_phy(port_num) == 0 ||
+	    qca8k_port_to_phy(port_num) == 4) {
 		mask = QCA8K_LED_PATTERN_EN_MASK;
 		val <<= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
@@ -210,7 +213,8 @@ qca8k_led_hw_control_status(struct qca8k_priv *priv, int port_num, u8 led_num)
 
 	val >>= reg_info.shift;
 
-	if (port_num == 0 || port_num == 4) {
+	if (qca8k_port_to_phy(port_num) == 0 ||
+	    qca8k_port_to_phy(port_num) == 4) {
 		val &= QCA8K_LED_PATTERN_EN_MASK;
 		val >>= QCA8K_LED_PATTERN_EN_SHIFT;
 	} else {
-- 
2.40.1

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Christian Marangi]

On Wed, Nov 29, 2023 at 12:21:27AM +0100, Andrew Lunn wrote:
> This patchset extends the DSA core to add support for port LEDs being
> controlled via sys/class/leds, and offloading blinking via
> ledtrig-netdev. The core parses the device tree binding, and registers
> LEDs. The DSA switch ops structure is extended with the needed
> functions.
> 
> The mv88e6xxx support is partially added. Support for setting the
> brightness and blinking is provided, but offloading of blinking is not
> yet available. To demonstrate this, the wrt1900ac device tree is
> extended with LEDs.
> 
> The existing QCA8K code is refactored to make use of this shared code.
> 
> RFC:
> 
> Linus, can you rework your code into this for offloading blinking ?
> And test with ports 5 & 6.
> 
> Christian: Please test QCA8K. I would not be surprised if there is an
> off-by-one.

Good news! I tested this and with the requested change, the thing works
correctly.

> 
> This code can also be found in
> 
> https://github.com/lunn/ v6.7-rc2-net-next-mv88e6xxx-leds
> 
> Andrew Lunn (8):
>   net: dsa: mv88e6xxx: Add helpers for 6352 LED blink and brightness
>   net: dsa: mv88e6xxx: Tie the low level LED functions to device ops
>   net: dsa: Plumb LED brightnes and blink into switch API
>   dsa: Create port LEDs based on DT binding
>   dsa: Plumb in LED calls needed for hardware offload
>   dsa: mv88e6xxx: Plumb in LED offload functions
>   arm: boot: dts: mvebu: linksys-mamba: Add Ethernet LEDs
>   dsa: qca8k: Use DSA common code for LEDs
> 
>  .../dts/marvell/armada-xp-linksys-mamba.dts   |  66 +++++
>  drivers/net/dsa/mv88e6xxx/chip.c              | 103 +++++++
>  drivers/net/dsa/mv88e6xxx/chip.h              |  14 +
>  drivers/net/dsa/mv88e6xxx/port.c              |  99 +++++++
>  drivers/net/dsa/mv88e6xxx/port.h              |  76 +++++-
>  drivers/net/dsa/qca/qca8k-8xxx.c              |  11 +-
>  drivers/net/dsa/qca/qca8k-leds.c              | 255 +++---------------
>  drivers/net/dsa/qca/qca8k.h                   |   9 -
>  drivers/net/dsa/qca/qca8k_leds.h              |  21 +-
>  include/net/dsa.h                             |  17 ++
>  net/dsa/dsa.c                                 | 190 +++++++++++++
>  11 files changed, 620 insertions(+), 241 deletions(-)
> 
> -- 
> 2.42.0
> 

-- 
	Ansuel

Re: [PATCH RFC net-next 8/8] dsa: qca8k: Use DSA common code for LEDs

[Andrew Lunn]

> Hi,
> 
> I attached a fixup patch for this to correctly work. Since we are using
> port_num the thing needs to be decrememted by one.

I thought this might happen.

How about this fix instead? It fits better with the naming of
parameters, and just does the offset once for each API function.

	    Andrew

From 0789b95345bfa5086365051f95531fdb3d053e3e Mon Sep 17 00:00:00 2001
From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 28 Nov 2023 20:11:42 -0600
Subject: [PATCH] dsa: qca8k: Fix off-by-one for LEDs.

---
 drivers/net/dsa/qca/qca8k-leds.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index febae23b65a9..0aa209b84251 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -80,9 +80,10 @@ qca8k_parse_netdev(unsigned long rules, u32 *offload_trigger)
 }
 
 int
-qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
+qca8k_led_brightness_set(struct dsa_switch *ds, int port,
 			 u8 led_num, enum led_brightness brightness)
 {
+	int port_num = qca8k_port_to_phy(port);
 	struct qca8k_led_pattern_en reg_info;
 	struct qca8k_priv *priv = ds->priv;
 	u32 mask, val;
@@ -140,11 +141,12 @@ qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
 }
 
 int
-qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
+qca8k_led_blink_set(struct dsa_switch *ds, int port, u8 led_num,
 		    unsigned long *delay_on,
 		    unsigned long *delay_off)
 {
 	u32 mask, val = QCA8K_LED_ALWAYS_BLINK_4HZ;
+	int port_num = qca8k_port_to_phy(port);
 	struct qca8k_led_pattern_en reg_info;
 	struct qca8k_priv *priv = ds->priv;
 
@@ -231,9 +233,10 @@ qca8k_led_hw_control_is_supported(struct dsa_switch *ds,
 }
 
 int
-qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
+qca8k_led_hw_control_set(struct dsa_switch *ds, int port, u8 led_num,
 			 unsigned long rules)
 {
+	int port_num = qca8k_port_to_phy(port);
 	struct qca8k_led_pattern_en reg_info;
 	struct qca8k_priv *priv = ds->priv;
 	u32 offload_trigger = 0;
@@ -255,9 +258,10 @@ qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
 }
 
 int
-qca8k_led_hw_control_get(struct dsa_switch *ds, int port_num, u8 led_num,
+qca8k_led_hw_control_get(struct dsa_switch *ds, int port, u8 led_num,
 			 unsigned long *rules)
 {
+	int port_num = qca8k_port_to_phy(port);
 	struct qca8k_led_pattern_en reg_info;
 	struct qca8k_priv *priv = ds->priv;
 	u32 val;
-- 
2.42.0

Re: [PATCH RFC net-next 8/8] dsa: qca8k: Use DSA common code for LEDs

[Christian Marangi]

On Wed, Nov 29, 2023 at 03:16:16AM +0100, Andrew Lunn wrote:
> > Hi,
> > 
> > I attached a fixup patch for this to correctly work. Since we are using
> > port_num the thing needs to be decrememted by one.
> 
> I thought this might happen.
> 
> How about this fix instead? It fits better with the naming of
> parameters, and just does the offset once for each API function.
>

Yep also works. My idea was to act on the function that parse the
port_num and gives regs and mask to prevent problem in the future but I
don't think it will change that much.

With your proposed fix,

Reviewed-by: Christian Marangi <ansuelsmth@gmail.com>

> 
> From 0789b95345bfa5086365051f95531fdb3d053e3e Mon Sep 17 00:00:00 2001
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Tue, 28 Nov 2023 20:11:42 -0600
> Subject: [PATCH] dsa: qca8k: Fix off-by-one for LEDs.
> 
> ---
>  drivers/net/dsa/qca/qca8k-leds.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
> index febae23b65a9..0aa209b84251 100644
> --- a/drivers/net/dsa/qca/qca8k-leds.c
> +++ b/drivers/net/dsa/qca/qca8k-leds.c
> @@ -80,9 +80,10 @@ qca8k_parse_netdev(unsigned long rules, u32 *offload_trigger)
>  }
>  
>  int
> -qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
> +qca8k_led_brightness_set(struct dsa_switch *ds, int port,
>  			 u8 led_num, enum led_brightness brightness)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 mask, val;
> @@ -140,11 +141,12 @@ qca8k_led_brightness_set(struct dsa_switch *ds, int port_num,
>  }
>  
>  int
> -qca8k_led_blink_set(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_blink_set(struct dsa_switch *ds, int port, u8 led_num,
>  		    unsigned long *delay_on,
>  		    unsigned long *delay_off)
>  {
>  	u32 mask, val = QCA8K_LED_ALWAYS_BLINK_4HZ;
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  
> @@ -231,9 +233,10 @@ qca8k_led_hw_control_is_supported(struct dsa_switch *ds,
>  }
>  
>  int
> -qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_hw_control_set(struct dsa_switch *ds, int port, u8 led_num,
>  			 unsigned long rules)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 offload_trigger = 0;
> @@ -255,9 +258,10 @@ qca8k_led_hw_control_set(struct dsa_switch *ds, int port_num, u8 led_num,
>  }
>  
>  int
> -qca8k_led_hw_control_get(struct dsa_switch *ds, int port_num, u8 led_num,
> +qca8k_led_hw_control_get(struct dsa_switch *ds, int port, u8 led_num,
>  			 unsigned long *rules)
>  {
> +	int port_num = qca8k_port_to_phy(port);
>  	struct qca8k_led_pattern_en reg_info;
>  	struct qca8k_priv *priv = ds->priv;
>  	u32 val;
> -- 
> 2.42.0
> 

-- 
	Ansuel

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Vladimir Oltean]

Hi Andrew,

On Wed, Nov 29, 2023 at 12:21:27AM +0100, Andrew Lunn wrote:
> This patchset extends the DSA core to add support for port LEDs being
> controlled via sys/class/leds, and offloading blinking via
> ledtrig-netdev. The core parses the device tree binding, and registers
> LEDs. The DSA switch ops structure is extended with the needed
> functions.
> 
> The mv88e6xxx support is partially added. Support for setting the
> brightness and blinking is provided, but offloading of blinking is not
> yet available. To demonstrate this, the wrt1900ac device tree is
> extended with LEDs.
> 
> The existing QCA8K code is refactored to make use of this shared code.
> 
> RFC:
> 
> Linus, can you rework your code into this for offloading blinking ?
> And test with ports 5 & 6.
> 
> Christian: Please test QCA8K. I would not be surprised if there is an
> off-by-one.
> 
> This code can also be found in
> 
> https://github.com/lunn/ v6.7-rc2-net-next-mv88e6xxx-leds

I am disappointed to see the dsa_switch_ops API polluted with odds and
ends which have nothing to do with Ethernet-connected Ethernet switches
(DSA's focus).

Looking at the code, I don't see why dsa_port_leds_setup() cannot be
rebranded as library code usable by any netdev driver and which bypasses DSA.
Individual DSA switch drivers could call it directly while providing
their struct device for the port, and a smaller ops structure for the
cdev. But more importantly, other non-DSA drivers could do the same.

I think it comes as no surprise that driver authors prefer using the DSA
API as their first choice even for technically non-DSA switches, seeing
how we tend to cram all sorts of unrelated stuff into the monolithic
struct dsa_switch_ops, and how that makes the API attractive. But then
we push them away from DSA for valid reasons, and they end up copying
its support code word for word.

Maybe this sounds a bit harsh, but NACK from me for the approach.

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Andrew Lunn]

> I am disappointed to see the dsa_switch_ops API polluted with odds and
> ends which have nothing to do with Ethernet-connected Ethernet switches
> (DSA's focus).
> 
> Looking at the code, I don't see why dsa_port_leds_setup() cannot be
> rebranded as library code usable by any netdev driver and which bypasses DSA.
> Individual DSA switch drivers could call it directly while providing
> their struct device for the port, and a smaller ops structure for the
> cdev. But more importantly, other non-DSA drivers could do the same.
> 
> I think it comes as no surprise that driver authors prefer using the DSA
> API as their first choice even for technically non-DSA switches, seeing
> how we tend to cram all sorts of unrelated stuff into the monolithic
> struct dsa_switch_ops, and how that makes the API attractive. But then
> we push them away from DSA for valid reasons, and they end up copying
> its support code word for word.

O.K, i need to think about this.

What is not obvious to me at the moment is how we glue the bits
together. I don't want each DSA driver having to parse the DSA part of
the DT representation. So the DSA core needs to call into this library
while parsing the DT to create the LEDs. We also need an ops structure
in the DSA driver which this library can use. We then need to
associate the ops structure the driver has with the LEDs the DSA core
creates in the library. Maybe we can use ds->dev as a cookie.

Before i get too deep into code, i will post the basic API idea for a
quick review.

	Andrew

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Vladimir Oltean]

On Wed, Nov 29, 2023 at 04:13:00PM +0100, Andrew Lunn wrote:
> O.K, i need to think about this.
> 
> What is not obvious to me at the moment is how we glue the bits
> together. I don't want each DSA driver having to parse the DSA part of
> the DT representation. So the DSA core needs to call into this library
> while parsing the DT to create the LEDs. We also need an ops structure
> in the DSA driver which this library can use. We then need to
> associate the ops structure the driver has with the LEDs the DSA core
> creates in the library. Maybe we can use ds->dev as a cookie.
> 
> Before i get too deep into code, i will post the basic API idea for a
> quick review.

What is the DSA portion of the DT representation? I see "leds" goes
under the generic ethernet-controller.yaml.

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Andrew Lunn]

On Wed, Nov 29, 2023 at 05:43:36PM +0200, Vladimir Oltean wrote:
> On Wed, Nov 29, 2023 at 04:13:00PM +0100, Andrew Lunn wrote:
> > O.K, i need to think about this.
> > 
> > What is not obvious to me at the moment is how we glue the bits
> > together. I don't want each DSA driver having to parse the DSA part of
> > the DT representation. So the DSA core needs to call into this library
> > while parsing the DT to create the LEDs. We also need an ops structure
> > in the DSA driver which this library can use. We then need to
> > associate the ops structure the driver has with the LEDs the DSA core
> > creates in the library. Maybe we can use ds->dev as a cookie.
> > 
> > Before i get too deep into code, i will post the basic API idea for a
> > quick review.
> 
> What is the DSA portion of the DT representation? I see "leds" goes
> under the generic ethernet-controller.yaml.

I agree the properties are well defined. The problem is finding them.

       switch@0 {
                compatible = "marvell,mv88e6085";
                #address-cells = <1>;
                #size-cells = <0>;
                reg = <0>;

                ports {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        port@0 {
                                reg = <0>;
                                label = "lan4";

                                leds {
                                        #address-cells = <1>;
                                        #size-cells = <0>;

                                        led@0 {
                                                reg = <0>;
                                                color = <LED_COLOR_ID_WHITE>;
                                                function = LED_FUNCTION_LAN;
                                                label = "front";
                                                default-state = "keep";
                                        };
                                };
                        };

                        port@1 {
                                reg = <1>;
                                label = "lan3";

                                leds {
                                        #address-cells = <1>;
                                        #size-cells = <0>;

                                        led@0 {
                                                reg = <0>;
                                                color = <LED_COLOR_ID_WHITE>;
                                                function = LED_FUNCTION_LAN;
                                                label = "front";
                                                default-state = "keep";
                                        };
                                };
                        };


I don't want each DSA driver having to walk this tree to find the leds
node to pass it to a library to create the LEDs. We already have code
do to this walk in the DSA core. So one option would be the DSA core
does the call to the library as it performs the walk.

Now that i've looked at the code, the core does set dp->dn to point to
the port node. So setup_port() could do the call into the library to
create the LEDs, and pass it the ops structure. That seems clean, and
should avoid DSA core changes you don't like.

       Andrew

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Vladimir Oltean]

On Wed, Nov 29, 2023 at 05:27:00PM +0100, Andrew Lunn wrote:
> I don't want each DSA driver having to walk this tree to find the leds
> node to pass it to a library to create the LEDs. We already have code
> do to this walk in the DSA core. So one option would be the DSA core
> does the call to the library as it performs the walk.
> 
> Now that i've looked at the code, the core does set dp->dn to point to
> the port node. So setup_port() could do the call into the library to
> create the LEDs, and pass it the ops structure. That seems clean, and
> should avoid DSA core changes you don't like.
> 
>        Andrew

Yeah, there's nothing to find, they're already found, and available in
of_fwnode_handle(dp->dn) during any ds->ops method you wish. The library
code for netdev LEDs can take a reference on this fwnode for as long as
it wants. Absolutely not a reason to call back into the DSA framework.

Re: [PATCH RFC net-next 4/8] dsa: Create port LEDs based on DT binding

[Simon Horman]

On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:

...

> +static int dsa_port_leds_setup(struct dsa_port *dp)
> +{
> +	struct device_node *leds, *led;
> +	int err;
> +
> +	if (!dp->dn)
> +		return 0;
> +
> +	leds = of_get_child_by_name(dp->dn, "leds");
> +	if (!leds)
> +		return 0;
> +
> +	for_each_available_child_of_node(leds, led) {
> +		err = dsa_port_led_setup(dp, led);
> +		if (err)
> +			return err;

Hi Andrew,

I realise this is an RFC, but Coccinelle tells me that a call to
of_node_put() is needed here.

> +	}
> +
> +	return 0;
> +}

...

Re: [PATCH RFC net-next 4/8] dsa: Create port LEDs based on DT binding

[Andrew Lunn]

On Wed, Nov 29, 2023 at 07:40:28PM +0000, Simon Horman wrote:
> On Wed, Nov 29, 2023 at 12:21:31AM +0100, Andrew Lunn wrote:
> 
> ...
> 
> > +static int dsa_port_leds_setup(struct dsa_port *dp)
> > +{
> > +	struct device_node *leds, *led;
> > +	int err;
> > +
> > +	if (!dp->dn)
> > +		return 0;
> > +
> > +	leds = of_get_child_by_name(dp->dn, "leds");
> > +	if (!leds)
> > +		return 0;
> > +
> > +	for_each_available_child_of_node(leds, led) {
> > +		err = dsa_port_led_setup(dp, led);
> > +		if (err)
> > +			return err;
> 
> Hi Andrew,
> 
> I realise this is an RFC, but Coccinelle tells me that a call to
> of_node_put() is needed here.

Thanks. If you had not pointed it out, i would probably get it wrong
in the next version as well.

   Andrew

Re: [PATCH RFC net-next 0/8] DSA LED infrastructure, mv88e6xxx and QCA8K

[Linus Walleij]

On Wed, Nov 29, 2023 at 12:21 AM Andrew Lunn <andrew@lunn.ch> wrote:

> Linus, can you rework your code into this for offloading blinking ?
> And test with ports 5 & 6.

I see Vladimir wants some reworking of it into a stand-alone library,
so I will wait a while until it he seems happy, then I will surely do this :)

Yours,
Linus Walleij