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

[netdev-bot+sashiko]

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=n.

--

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

This commit moves the loop that calls disable_delayed_work_sync() on each
port's mib_read 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 c7cfaf2442749..9380d74d79ba2 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -3924,6 +3924,12 @@ 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]
Will this build with CONFIG_NET_DSA_YT921X_LEDS=n? In yt921x_dsa_teardown(),
priv is only declared inside the #if 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 sits after the #endif and uses priv->ports. With LED support
disabled, priv is undeclared and chip.c fails to compile.

That configuration looks reachable. NET_DSA_YT921X_LEDS is a user-visible
bool that depends on "LEDS_CLASS=y || LEDS_CLASS=NET_DSA_YT921X". So it is
off when the user turns it off, when LEDS_CLASS=n, or when LEDS_CLASS=m with
NET_DSA_YT921X=y.

The old loop in yt921x_mdio_remove() used a priv that is declared
unconditionally, so the failure starts with this commit. The code is still
the same in the last patch of the series ("net: dsa: motorcomm: Add support
for Motorcomm YT922x"), so the series as a whole does not build in this
configuration either.

Could priv be declared unconditionally, with only the yt921x_leds_remove()
call left under the #if? The #if could also be dropped entirely, because
leds.h already provides a no-op inline stub for yt921x_leds_remove() when
LED support is disabled.

[ ... ]

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