Re: [PATCH net-next v4 4/4] net: phy: add X-Powers AC200/AC300 EPHY driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use of device_trylock() with a fallback to -EPROBE_DEFER is an anti-pattern used to bypass lock ordering.
--

--- Patch [4]: [PATCH net-next v4 4/4] net: phy: add X-Powers AC200/AC300 EPHY driver ---
commit 6bc6a63de1c66219eb59d7933bc21eede2472534
Author: James Hilliard <james.hilliard1@gmail.com>

net: phy: add X-Powers AC200/AC300 EPHY driver

This commit introduces the Ethernet PHY driver for the X-Powers AC200 and
AC300 companion ICs. It configures the shared MDIO PHY registers and manages
the package-specific I2C or MDIO control registers. The driver also handles
clock selection, vendor initialization, and power sequencing.

> --- /dev/null
> +++ b/drivers/net/phy/xpowers-acx00-ac200.c
[ ... ]
> +struct acx00_ephy_control *
> +ac200_ephy_ctl_create(struct phy_device *phydev,
> +		      struct device_node *package_node,
> +		      bool has_calibration, u8 calibration)
> +{
[ ... ]
> +	if (!device_link_add(dev, &client->dev,
> +			     DL_FLAG_AUTOREMOVE_CONSUMER)) {
> +		ret = dev_err_probe(dev, -EINVAL,
> +				    "failed to link AC200 device\n");
> +		goto out_put_client;
> +	}
> +
> +	if (!device_trylock(&client->dev)) {
> +		ret = dev_err_probe(dev, -EPROBE_DEFER,
> +				    "AC200 driver is not ready\n");
> +		goto out_put_client;
> +	}

[Severity: High]
Is this use of device_trylock() with a fallback to return -EPROBE_DEFER safe
from causing permanent probe failures? 

The networking driver guidelines define trylock with a fallback to skip the 
work entirely as an indicator of a flawed locking design that bypasses 
proper lock ordering. If the lock happens to be held by an unrelated 
subsystem, such as userspace sysfs access or power management, at the end 
of the boot process when the deferred probe queue finishes, could this 
cause the probe to permanently fail?

Could this be replaced with a proper dependency or synchronization mechanism
that avoids bypassing lock ordering?

> +
> +	if (device_is_bound(&client->dev))
> +		priv->regmap = dev_get_regmap(&client->dev, NULL);
> +	device_unlock(&client->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-submit-acx00-of-dynamic-v1-v4-0-9bf2f7441632@gmail.com?part=4