Re: [PATCH v2 net-next 6/7] dt-bindings: net: dsa: mediatek,mt7530: add econet,en751221

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net: dsa: lockless child_bus access causes MDIO race condition
- [High] net: dsa: late mdio irq setup forces PHYs to poll mode
--

commit ef09d813c1932657a0f0b34029eea8c2c9403e91
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: support PLL setup on MMIO MT7530

On MT7530 and MT7531, PHYs are an integral part of the switch. However,
on MT7530, certain configuration such as PLL setup is done via special
registers on one of the PHYs. In an MMIO implementation, the MDIO bus
is part of the switch itself.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index b2344808457cb..b152b73315d50 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -77,6 +77,12 @@ core_write(struct mt7530_priv *priv, u32 reg, u32 val)
>  	struct mii_bus *bus = priv->bus;
>  	int ret;
>  
> +	if (!bus)
> +		bus = priv->child_bus;
> +
> +	if (WARN_ON_ONCE(!bus))
> +		return;
> +
>  	mt7530_mutex_lock(priv);
>  
>  	/* Write the desired MMD Devad */

[Severity: High]
When an MMIO switch (e.g. ID_EN751221) triggers a port state change,
mt7530_setup_port6() calls core_write().

The lock helper mt7530_mutex_lock() ignores child_bus:

drivers/net/dsa/mt7530.c:mt7530_mutex_lock() {
    if (priv->bus)
        mutex_lock_nested(&priv->bus->mdio_lock, MDIO_MUTEX_NESTED);
}

This allows the indirect MMD bus->write() and bus->read() operations to
execute entirely without the bus->mdio_lock synchronization.

Can this lockless child_bus access cause hardware MDIO controller state
corruption and silent transaction failures due to interleaved MDIO accesses?

commit b6de045bf619c20c22824d1e2470474ed02da7d7
Author: Caleb James DeLisle <cjd@cjdns.fr>

net: dsa: mediatek: support MDIO switch downstream of MMIO switch

The MT7530 / MT7531 can connect via either MDIO or MMIO. In MMIO mode
the switch provides its own MDIO bus which connects to its integrated
PHYs, but also can be connected to other things, such as another switch.

> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index b152b73315d50..fe93be1ece7d7 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2438,9 +2442,6 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
>  	bus->parent = dev;
>  	bus->phy_mask = ~ds->phys_mii_mask;
>  
> -	if (priv->irq_domain && !mnp)
> -		mt7530_setup_mdio_irq(priv);
> -
>  	priv->child_bus = bus;
>  
>  	ret = devm_of_mdiobus_register(dev, bus, mnp);

[ ... ]

> @@ -3285,6 +3287,9 @@ mt753x_setup(struct dsa_switch *ds)
>  	if (ret)
>  		return ret;
>  
> +	if (priv->irq_domain && ds->user_mii_bus)
> +		mt7530_setup_mdio_irq(priv);
> +

[Severity: High]
For MDIO switches, this executes after devm_of_mdiobus_register() in
mt7530_setup_mdio(), meaning the PHY devices are created and scanned before
bus->irq is populated.

For MMIO switches, ds->user_mii_bus is still NULL during mt753x_setup(),
so the IRQ setup is skipped entirely.

Will this late mdio irq setup force PHYs to fall back to polling mode,
degrading performance and delaying link state detection?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915174643.253660-1-cjd@cjdns.fr?part=6