Re: [PATCH v14 2/4] phy: core: Add phy_get_by_of_node()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced `phy_get_by_of_node()` API misses the `usb-nop-xceiv` compatibility check, causing infinite probe deferrals instead of returning `-ENODEV` when called on a legacy USB PHY node.
--

commit 2369b98c7c0d0a704a02ff8f33a033c9c232ea8e
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

phy: core: Add phy_get_by_of_node()
    
Add new function phy_get_by_of_node() allowing lookup of a phy by
device_node. Separates existing logic in _of_phy_get() into an internal
helper method _of_phy_get_with_args() to allow for reuse in new method.

> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index cd9ace125567a..1d060bc22d02c 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -604,22 +604,49 @@ int phy_validate(struct phy *phy, enum phy_mode mode, int submode,
>  }
>  EXPORT_SYMBOL_GPL(phy_validate);
>  
> +/**
> + * _of_phy_get_with_args() - lookup and obtain a reference to a phy by of_phandle_args
> + * @args: of_phandle_args to the phy
> + *
> + * Returns the phy from the provider's of_xlate, -ENODEV if disabled,
> + * -EPROBE_DEFER if the provider is not yet registered.
> + */
> +static struct phy *_of_phy_get_with_args(struct of_phandle_args *args)
> +{
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +
> +	phy_provider = of_phy_provider_lookup(args->np);
> +	if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner))
> +		return ERR_PTR(-EPROBE_DEFER);

[Severity: Medium]
Will this lookup always fail for usb-nop-xceiv PHYs, incorrectly triggering
the -EPROBE_DEFER return?

[ ... ]

> @@ -997,6 +1009,41 @@ struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index);
>  
> +/**
> + * phy_get_by_of_node() - lookup and obtain a reference to a phy by device_node
> + * @np: node containing the phy
> + *
> + * Returns the phy associated with the device node or ERR_PTR.
> + */
> +struct phy *phy_get_by_of_node(struct device_node *np)
> +{
> +	struct of_phandle_args args = { .np = np, .args_count = 0 };
> +	struct phy *phy;
> +
> +	if (!np)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&phy_provider_mutex);
> +
> +	phy = _of_phy_get_with_args(&args);

[Severity: Medium]
Does this call the helper without validating if the node represents a legacy
usb-nop-xceiv PHY?

In _of_phy_get(), there is a compatibility check for usb-nop-xceiv that
returns -ENODEV before calling the lookup logic. Because that check was
left in _of_phy_get() rather than being moved to the shared
_of_phy_get_with_args() helper, calling this new generic API on a legacy
USB PHY node will result in -EPROBE_DEFER instead of -ENODEV.

This might prevent drivers from falling back to the legacy USB PHY subsystem,
potentially causing infinite probe deferrals.

> +
> +	if (IS_ERR(phy))
> +		goto out_unlock;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v14-0-01ef1eced075@linaro.org?part=2