Re: [PATCH net-next v7 2/6] net: dsa: motorcomm: use max_ports in series for port bounds checking

[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

- [Medium] yt92xx_register_switch()
  (drivers/net/dsa/motorcomm/chip.c:4707-4737) calls…

--

net: dsa: motorcomm: use max_ports in series for port bounds checking

This replaces the hardcoded YT921X_PORT_NUM in the internal MDIO bus port
checks and in the phy_mask computation with priv->series->max_ports, so the
common code no longer carries a family-specific port count.

> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 1f9c0c67ad4a6..4479bb1c3c2b1 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c

[ ... ]

> @@ -390,6 +390,7 @@ yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
>  {
>  	struct device *dev = to_device(priv);
>  	struct mii_bus *mbus;
> +	u32 max_ports;
>  	int res;
>  
>  	mbus = devm_mdiobus_alloc(dev);
> @@ -402,7 +403,8 @@ yt921x_mbus_int_init(struct yt921x_priv *priv, struct device_node *mnp)
>  	mbus->read = yt921x_mbus_int_read;
>  	mbus->write = yt921x_mbus_int_write;
>  	mbus->parent = dev;
> -	mbus->phy_mask = (u32)~GENMASK(YT921X_PORT_NUM - 1, 0);
> +	max_ports = priv->series->max_ports;
> +	mbus->phy_mask = (u32)~GENMASK(max_ports - 1, 0);

[Severity: Medium]

These new users of priv->series depend on the chip-ID read that fills it in,
and that read looks like it now runs without priv->reg_lock held.

In yt92xx_register_switch():

	res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
	if (res)
		return res;

	major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
	series = yt92xx_series_lookup(major);

and in drivers/net/dsa/motorcomm/smi.c:

	int yt921x_reg_read(struct yt921x_priv *priv, u32 reg, u32 *valp)
	{
		lockdep_assert_held_once(&priv->reg_lock);

		return priv->reg_ops->read(priv->reg_ctx, reg, valp);
	}

Does this trip the lockdep assertion on every probe with
CONFIG_PROVE_LOCKING enabled?  The only caller is yt921x_mdio_probe(), which
does mutex_init(&priv->reg_lock) before yt92xx_register_switch(ds) but never
takes it, so the multi-transaction SMI access also runs outside the mutex
that chip.h describes as "protect the access to the switch registers".

Before this series the chip-ID read happened under the lock, via
yt921x_dsa_setup():

	mutex_lock(&priv->reg_lock);
	res = yt921x_chip_reset(priv);
	mutex_unlock(&priv->reg_lock);

which reached yt921x_chip_detect().  The move into the unlocked probe path
came with "net: dsa: motorcomm: initialize dsa_switch based on chipid", and
the end of the series still has no mutex_lock() in
yt92xx_register_switch().

Would wrapping the yt921x_reg_read() in yt92xx_register_switch() with
mutex_lock(&priv->reg_lock)/mutex_unlock(&priv->reg_lock) be the intended
fix here?

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