Re: [PATCH net-next v25 3/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: phy: motorcomm: concurrent probes bypass DT validation errors
--

commit 3e7bb92256550d445ce01db4ee78cd8ee5a8b2f1
Author: Kyle Switch <kyle.switch@motor-comm.com>
Date:   Thu Oct 15 14:50:21 2026 +0000

    net: phy: Add driver for Motorcomm Quad 2.5GbE phy
    
    Add support for Motorcomm YT8824 quad-port 2.5G PHY to the existing
    motorcomm driver, using the phy_package helpers for the shared top
    extended register space.

> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ ... @@
> +static int yt8824_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct yt8824_shared_priv *shared_priv;
> +	int ret;
> +
> +	ret = devm_of_phy_package_join(dev, phydev, sizeof(*shared_priv));
> +	if (ret)
> +		return ret;
> +
> +	/* Ensure other PHY probes wait for shared mutex initialization. */
> +	phy_package_lock(phydev);
> +	if (phy_package_probe_once(phydev)) {
> +		ret = yt8824_phy_package_probe_once(phydev);
> +		if (ret) {
> +			phy_package_unlock(phydev);
> +			return ret;
> +		}

[Severity: Medium]
Can concurrent PHY probes bypass device tree validation errors here?

If yt8824_phy_package_probe_once() fails (for example, if the
"motorcomm,package-mode" property is missing or invalid), it returns an
error and the current PHY fails to probe.

However, the phylib helper phy_package_probe_once() has already set the
'once' initialization flag. If another PHY on the same shared package is
probing concurrently, it will wait for the package lock, observe the flag
as set, and skip the package initialization block.

Because yt8824_phy_package_probe_once() initializes the shared state
defaults before validating the device tree:

yt8824_phy_package_probe_once() {
    ...
    /* Initialise shared lock for YT8824 */
    mutex_init(&priv->shared_lock);
    priv->sds_initialized = false;
    priv->package_mode = PHY_INTERFACE_MODE_INTERNAL;
    ...
}

wouldn't the concurrent PHY successfully return 0 from yt8824_probe(),
bypassing the explicit DT validation and operating inconsistently with
the fallback state (PHY_INTERFACE_MODE_INTERNAL)? Should the driver
record the initialization failure in the shared private data or clear
the flag to prevent this split state?

> +	}
> +	phy_package_unlock(phydev);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015145021.2216003-1-kyle.switch@motor-comm.com?part=3