Re: [PATCH net-next v9 5/7] net: dsa: motorcomm: move mib stop from remove() to teardown()

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Build failure when CONFIG_NET_DSA_YT921X_LEDS is off.

--

net: dsa: motorcomm: move mib stop from remove() to teardown()

This commit moves the loop that disables the per-port mib_read delayed
work out of yt921x_mdio_remove() and into yt921x_dsa_teardown(). The goal
is to match the MIB start, which already happens in setup().

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index d9d73bb7cb23..46f7b94469ea 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4233,6 +4233,11 @@ static void yt921x_dsa_teardown(struct dsa_switch *ds)
>  
>  	yt921x_leds_remove(priv);
>  #endif
> +	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
> +		struct yt921x_port *pp = &priv->ports[i];
> +
> +		disable_delayed_work_sync(&pp->mib_read);
> +	}
>  }

[Severity: Medium]
Does this build when CONFIG_NET_DSA_YT921X_LEDS is disabled?

In yt921x_dsa_teardown(), priv is only declared inside the conditional
block:

static void yt921x_dsa_teardown(struct dsa_switch *ds)
{
#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
	struct yt921x_priv *priv = to_yt921x_priv(ds);

	yt921x_leds_remove(priv);
#endif
	for (size_t i = ARRAY_SIZE(priv->ports); i-- > 0; ) {
	...

The new loop comes after the #endif and uses priv in both
ARRAY_SIZE(priv->ports) and &priv->ports[i]. With LEDS=n, priv is not
declared, so this looks like it fails with "'priv' undeclared".

NET_DSA_YT921X_LEDS is an optional bool in
drivers/net/dsa/motorcomm/Kconfig:

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

So it is off when LEDS_CLASS=n, when LEDS_CLASS=m with NET_DSA_YT921X=y,
or when a user turns it off. allmodconfig and allyesconfig still enable
it, so they would not catch this. Randconfig or minimal configs could.

The teardown code is still the same at the end of the series, in "net:
dsa: motorcomm: Add support for Motorcomm YT922x".

Could priv be declared before the #if, with only the yt921x_leds_remove()
call kept inside the conditional?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924094146.4128703-1-kyle.switch%40motor-comm.com