diff mbox series

[net-next,1/2] net: dsa: mv88e6xxx: Add LED infrastructure

Message ID 20240103103351.1188835-2-tobias@waldekranz.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: mv88e6xxx: Add LED support for 6393X | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1113 this patch: 1115
netdev/cc_maintainers warning 3 maintainers not CCed: edumazet@google.com pabeni@redhat.com linux@armlinux.org.uk
netdev/build_clang fail Errors and warnings before: 1140 this patch: 1142
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1140 this patch: 1142
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 89 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Tobias Waldekranz Jan. 3, 2024, 10:33 a.m. UTC
Parse LEDs from DT and register them with the kernel, for chips that
support it. No actual implementations exist yet, they will be added in
upcoming commits.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/Makefile |   1 +
 drivers/net/dsa/mv88e6xxx/chip.c   |   5 +
 drivers/net/dsa/mv88e6xxx/chip.h   |   4 +
 drivers/net/dsa/mv88e6xxx/leds.c   | 195 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/leds.h   |  12 ++
 5 files changed, 217 insertions(+)
 create mode 100644 drivers/net/dsa/mv88e6xxx/leds.c
 create mode 100644 drivers/net/dsa/mv88e6xxx/leds.h

Comments

Andrew Lunn Jan. 3, 2024, 2:09 p.m. UTC | #1
On Wed, Jan 03, 2024 at 11:33:50AM +0100, Tobias Waldekranz wrote:
> Parse LEDs from DT and register them with the kernel, for chips that
> support it. No actual implementations exist yet, they will be added in
> upcoming commits.

Hi Tobias

There are three of us now working on this. Linus, you and me. We all
have different implementations.

What i don't like about this is that is has code which is going to be
repeated in all DSA drivers, and even in all MAC drivers. I've already
posted one patch series which added generic DSA support for LEDs, and
some basic mv88e6xxx code. It got NACKed by Vladimir. So i'm slowly
working on making it more generic, so it can be used by any MAC
driver.

I will try to post it in the next couple of days.

    Andrew

---
pw-bot: cr
Tobias Waldekranz Jan. 3, 2024, 2:27 p.m. UTC | #2
On ons, jan 03, 2024 at 15:09, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Jan 03, 2024 at 11:33:50AM +0100, Tobias Waldekranz wrote:
>> Parse LEDs from DT and register them with the kernel, for chips that
>> support it. No actual implementations exist yet, they will be added in
>> upcoming commits.
>
> Hi Tobias
>
> There are three of us now working on this. Linus, you and me. We all
> have different implementations.

Oh, sorry about that. I should have done the proper research before
posting :)

> What i don't like about this is that is has code which is going to be
> repeated in all DSA drivers, and even in all MAC drivers. I've already
> posted one patch series which added generic DSA support for LEDs, and
> some basic mv88e6xxx code. It got NACKed by Vladimir. So i'm slowly
> working on making it more generic, so it can be used by any MAC
> driver.
>
> I will try to post it in the next couple of days.

Interesting! I'll keep an eye out for it.
kernel test robot Jan. 3, 2024, 11:03 p.m. UTC | #3
Hi Tobias,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Tobias-Waldekranz/net-dsa-mv88e6xxx-Add-LED-infrastructure/20240103-183726
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240103103351.1188835-2-tobias%40waldekranz.com
patch subject: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Add LED infrastructure
config: arm64-randconfig-001-20240104 (https://download.01.org/0day-ci/archive/20240104/202401040647.fVDMN6wS-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401040647.fVDMN6wS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401040647.fVDMN6wS-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/mv88e6xxx/leds.c:171:5: warning: no previous prototype for 'mv88e6xxx_port_setup_leds' [-Wmissing-prototypes]
     171 | int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~


vim +/mv88e6xxx_port_setup_leds +171 drivers/net/dsa/mv88e6xxx/leds.c

   170	
 > 171	int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
kernel test robot Jan. 4, 2024, 12:15 a.m. UTC | #4
Hi Tobias,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Tobias-Waldekranz/net-dsa-mv88e6xxx-Add-LED-infrastructure/20240103-183726
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240103103351.1188835-2-tobias%40waldekranz.com
patch subject: [PATCH net-next 1/2] net: dsa: mv88e6xxx: Add LED infrastructure
config: arm-orion5x_defconfig (https://download.01.org/0day-ci/archive/20240104/202401040808.PYrPvsd9-lkp@intel.com/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 7e186d366d6c7def0543acc255931f617e76dff0)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401040808.PYrPvsd9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401040808.PYrPvsd9-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/dsa/mv88e6xxx/leds.c:171:5: warning: no previous prototype for function 'mv88e6xxx_port_setup_leds' [-Wmissing-prototypes]
     171 | int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
         |     ^
   drivers/net/dsa/mv88e6xxx/leds.c:171:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     171 | int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
         | ^
         | static 
   1 warning generated.


vim +/mv88e6xxx_port_setup_leds +171 drivers/net/dsa/mv88e6xxx/leds.c

   170	
 > 171	int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index a9a9651187db..6720d9303914 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -9,6 +9,7 @@  mv88e6xxx-objs += global2.o
 mv88e6xxx-objs += global2_avb.o
 mv88e6xxx-objs += global2_scratch.o
 mv88e6xxx-$(CONFIG_NET_DSA_MV88E6XXX_PTP) += hwtstamp.o
+mv88e6xxx-objs += leds.o
 mv88e6xxx-objs += pcs-6185.o
 mv88e6xxx-objs += pcs-6352.o
 mv88e6xxx-objs += pcs-639x.o
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 383b3c4d6f59..8fab16badc9e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -37,6 +37,7 @@ 
 #include "global1.h"
 #include "global2.h"
 #include "hwtstamp.h"
+#include "leds.h"
 #include "phy.h"
 #include "port.h"
 #include "ptp.h"
@@ -4006,6 +4007,10 @@  static int mv88e6xxx_port_setup(struct dsa_switch *ds, int port)
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int err;
 
+	err = mv88e6xxx_port_setup_leds(ds, port);
+	if (err)
+		return err;
+
 	if (chip->info->ops->pcs_ops &&
 	    chip->info->ops->pcs_ops->pcs_init) {
 		err = chip->info->ops->pcs_ops->pcs_init(chip, port);
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 85eb293381a7..c229e3d6a265 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -206,6 +206,7 @@  struct mv88e6xxx_gpio_ops;
 struct mv88e6xxx_avb_ops;
 struct mv88e6xxx_ptp_ops;
 struct mv88e6xxx_pcs_ops;
+struct mv88e6xxx_led_ops;
 
 struct mv88e6xxx_irq {
 	u16 masked;
@@ -653,6 +654,9 @@  struct mv88e6xxx_ops {
 	/* Precision Time Protocol operations */
 	const struct mv88e6xxx_ptp_ops *ptp_ops;
 
+	/* LED operations */
+	const struct mv88e6xxx_led_ops *led_ops;
+
 	/* Phylink */
 	void (*phylink_get_caps)(struct mv88e6xxx_chip *chip, int port,
 				 struct phylink_config *config);
diff --git a/drivers/net/dsa/mv88e6xxx/leds.c b/drivers/net/dsa/mv88e6xxx/leds.c
new file mode 100644
index 000000000000..1f331a632065
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/leds.c
@@ -0,0 +1,195 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <net/dsa.h>
+
+#include "chip.h"
+#include "port.h"
+
+struct mv88e6xxx_led {
+	struct mv88e6xxx_chip *chip;
+	int port;
+	u8 index;
+
+	struct led_classdev ldev;
+};
+
+struct mv88e6xxx_led_ops {
+	int (*brightness_set)(struct mv88e6xxx_led *led,
+			      enum led_brightness brightness);
+	int (*blink_set)(struct mv88e6xxx_led *led,
+			 unsigned long *delay_on, unsigned long *delay_off);
+	int (*hw_control_is_supported)(struct mv88e6xxx_led *led,
+				       unsigned long flags);
+	int (*hw_control_set)(struct mv88e6xxx_led *led, unsigned long flags);
+	int (*hw_control_get)(struct mv88e6xxx_led *led, unsigned long *flags);
+};
+
+static int mv88e6xxx_led_brightness_set(struct led_classdev *ldev,
+					enum led_brightness brightness)
+{
+	const struct mv88e6xxx_led_ops *ops;
+	struct mv88e6xxx_led *led;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	ops = led->chip->info->ops->led_ops;
+
+	if (!ops->brightness_set)
+		return -EOPNOTSUPP;
+
+	return ops->brightness_set(led, brightness);
+}
+
+static int mv88e6xxx_led_blink_set(struct led_classdev *ldev,
+				   unsigned long *delay_on,
+				   unsigned long *delay_off)
+{
+	const struct mv88e6xxx_led_ops *ops;
+	struct mv88e6xxx_led *led;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	ops = led->chip->info->ops->led_ops;
+
+	if (!ops->blink_set)
+		return -EOPNOTSUPP;
+
+	return ops->blink_set(led, delay_on, delay_off);
+}
+
+static int mv88e6xxx_led_hw_control_is_supported(struct led_classdev *ldev,
+						 unsigned long flags)
+{
+	const struct mv88e6xxx_led_ops *ops;
+	struct mv88e6xxx_led *led;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	ops = led->chip->info->ops->led_ops;
+
+	if (!ops->hw_control_is_supported)
+		return -EOPNOTSUPP;
+
+	return ops->hw_control_is_supported(led, flags);
+}
+
+static int mv88e6xxx_led_hw_control_set(struct led_classdev *ldev,
+					unsigned long flags)
+{
+	const struct mv88e6xxx_led_ops *ops;
+	struct mv88e6xxx_led *led;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	ops = led->chip->info->ops->led_ops;
+
+	if (!ops->hw_control_set)
+		return -EOPNOTSUPP;
+
+	return ops->hw_control_set(led, flags);
+}
+
+static int mv88e6xxx_led_hw_control_get(struct led_classdev *ldev,
+					unsigned long *flags)
+{
+	const struct mv88e6xxx_led_ops *ops;
+	struct mv88e6xxx_led *led;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	ops = led->chip->info->ops->led_ops;
+
+	if (!ops->hw_control_get)
+		return -EOPNOTSUPP;
+
+	return ops->hw_control_get(led, flags);
+}
+
+static struct device *mv88e6xxx_led_hw_control_get_device(struct led_classdev *ldev)
+{
+	struct mv88e6xxx_led *led;
+	struct dsa_port *dp;
+
+	led = container_of(ldev, struct mv88e6xxx_led, ldev);
+	dp = dsa_to_port(led->chip->ds, led->port);
+
+	if (dp && dp->user)
+		return &dp->user->dev;
+
+	return NULL;
+}
+
+static int mv88e6xxx_port_setup_led(struct mv88e6xxx_chip *chip, int port,
+				    struct device_node *np)
+{
+	struct led_init_data init_data = {};
+	struct mv88e6xxx_led *led;
+	char *devname;
+	u32 index;
+	int err;
+
+	err = of_property_read_u32(np, "reg", &index);
+	if (err)
+		return err;
+
+	if (index >= 2)
+		return -EINVAL;
+
+	led = devm_kzalloc(chip->dev, sizeof(*led), GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	*led = (struct mv88e6xxx_led) {
+		.chip = chip,
+		.port = port,
+		.index = index,
+
+		.ldev = {
+			.max_brightness = 1,
+			.brightness_set_blocking = mv88e6xxx_led_brightness_set,
+			.blink_set = mv88e6xxx_led_blink_set,
+
+#ifdef CONFIG_LEDS_TRIGGERS
+			.hw_control_trigger = "netdev",
+			.hw_control_get_device = mv88e6xxx_led_hw_control_get_device,
+
+			.hw_control_is_supported = mv88e6xxx_led_hw_control_is_supported,
+			.hw_control_set = mv88e6xxx_led_hw_control_set,
+			.hw_control_get = mv88e6xxx_led_hw_control_get,
+#endif
+		},
+	};
+
+	devname = devm_kasprintf(chip->dev, GFP_KERNEL, "%s.%d",
+				 dev_name(chip->dev), port);
+	if (!devname)
+		return -ENOMEM;
+
+	init_data = (struct led_init_data) {
+		.fwnode = of_fwnode_handle(np),
+		.devname_mandatory = true,
+		.devicename = devname,
+	};
+
+	return devm_led_classdev_register_ext(chip->dev, &led->ldev, &init_data);
+}
+
+int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct device_node *pnp, *np;
+	int err;
+
+	if (!chip->info->ops->led_ops)
+		return 0;
+
+	if (!dp->dn)
+		return 0;
+
+	pnp = of_get_child_by_name(dp->dn, "leds");
+	if (!pnp)
+		return 0;
+
+	for_each_available_child_of_node(pnp, np) {
+		err = mv88e6xxx_port_setup_led(chip, port, np);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/dsa/mv88e6xxx/leds.h b/drivers/net/dsa/mv88e6xxx/leds.h
new file mode 100644
index 000000000000..a99d7a5ebc6d
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/leds.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/* Marvell 88E6xxx Switch LED support. */
+
+#ifndef _MV88E6XXX_LEDS_H
+#define _MV88E6XXX_LEDS_H
+
+#include "chip.h"
+
+int mv88e6xxx_port_setup_leds(struct dsa_switch *ds, int port);
+
+#endif /* _MV88E6XXX_LEDS_H */