Re: [PATCH net-next v6 3/3] net: dsa: motorcomm: Add LED support

[Jakub Kicinski <kuba@kernel.org>]

On Thu,  9 Jul 2026 09:47:47 +0800 David Yang wrote:
> LEDs can be described in the device tree using the same format as qca8k.
> Each port can configure up to 3 LEDs.
> 
> Currently, only parallel mode and strict 1:1 mapping are supported.

> +config NET_DSA_YT921X_LEDS
> +	bool "LED support for Motorcomm YT9215"
> +	default y
> +	depends on NET_DSA_YT921X
> +	depends on LEDS_CLASS=y || LEDS_CLASS=NET_DSA_YT921X

is it really useful to prompt the user with this option?
Shouldn't this be a hidden option which gets auto-enabled
when dependencies are met?

> +static bool
> +yt921x_led_trigger_is_supported(const struct yt921x_priv *priv, int port,
> +				int group, unsigned long flags)
> +{
> +	if (!flags)
> +		return true;
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(yt921x_led_trigger_maps); i++) {
> +		const struct yt921x_led_trigger_map *map = &yt921x_led_trigger_maps[i];
> +
> +		if ((flags & map->flags) == map->flags) {
> +			flags &= ~map->flags;
> +			if (!flags)
> +				return true;
> +		}

Isn't this conditions simply:

		if (flags == map->flags)
			return true;

?

> +	}
> +
> +	return false;


> +#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
> +
> +void yt921x_leds_remove(struct yt921x_priv *priv);
> +int yt921x_leds_setup(struct yt921x_priv *priv);
> +
> +#else
> +
> +static inline void yt921x_leds_remove(struct yt921x_priv *priv) {}
> +
> +static inline int yt921x_leds_setup(struct yt921x_priv *priv)
> +{
> +	return 0;
> +}

There are static inlines but I think you still protect the calls to
these functions with itdef?

+#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
+	res = yt921x_leds_setup(priv);
+	if (res)
+		dev_warn(dev, "Failed to setup LEDs: %d\n", res);
+#endif

> +#endif
> +
> +#endif