Re: [PATCH RFC] net: dsa: mv88e6xxx: Support LED control

From: Andrew Lunn
Date: Thu Nov 23 2023 - 11:14:04 EST


> This DT config is not yet configuring everything: the netdev
> default trigger is assigned by the hw acceleration callbacks are
> not called, and there is no way to set the netdev sub-trigger
> type from the device tree, such as if you want a gigabit link
> indicator. This has to be done from userspace at this point.

Yes, part of this is a known problem, and somewhere i have some code i
was working on to fix some of these issues.

What i would really like to see happen is that the DSA core handles
the registration of the LEDs, similar to how phylib does. The DT
binding should be identical for all DSA devices, so there is no need
for each driver to do its own parsing.

There are some WIP patches at

https://github.com/lunn/linux.git leds-offload-support-reduced-auto-netdev

which implement this. Feel free to make use of them.

> +/* The following is a lookup table to check what rules we can support on a
> + * certain LED given restrictions such as that some rules only work with fiber
> + * (SFP) connections and some blink on activity by default.
> + */
> +#define MV88E6XXX_PORTS_0_3 (BIT(0)|BIT(1)|BIT(2)|BIT(3))
> +#define MV88E6XXX_PORTS_4_5 (BIT(4)|BIT(5))
> +#define MV88E6XXX_PORT_4 BIT(4)
> +#define MV88E6XXX_PORT_5 BIT(5)
> +
> +/* Entries are listed in selector order */
> +static const struct mv88e6xxx_led_hwconfig mv88e6xxx_led_hwconfigs[] = {

You need to be careful with naming. These are probably specific to the
6352. Different switches probably have different capabilities. So it
would be good to have the names reflect the switch family they are
valid for.

When we come to add support for other switch families, i wounder how
tables like this scale. Is there some things which can be shared, if
we break the table up? I need to check the data sheets.

Andrew