Re: [PATCH v17 2/5] phy: core: Add phy_get_by_of_node()

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 06/09/2026 15:31, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] phy_get_by_of_node() misses the legacy usb-nop-xceiv workaround, causing infinite -EPROBE_DEFER loops.
> --
> 
> commit a021b561c8a9bfb36addca720d49ea935b34a131
> Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> 
> phy: core: Add phy_get_by_of_node()
> 
> This commit introduces phy_get_by_of_node() to allow looking up a phy
> directly by its device node. It refactors the existing lookup logic in
> _of_phy_get() into a new shared helper function called
> _of_phy_get_with_args().
> 
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 89addd732bff3..490a7cde6d40a 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
> [ ... ]
>> @@ -1001,6 +1015,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 new API miss the legacy compatibility check for usb-nop-xceiv?
> 
> In _of_phy_get(), there is a workaround that returns -ENODEV if the
> device is compatible with "usb-nop-xceiv". Because this workaround was
> left in _of_phy_get() rather than being relocated into the shared
> _of_phy_get_with_args() helper, phy_get_by_of_node() bypasses it
> completely.
> 
> Could this cause drivers looking up a "usb-nop-xceiv" node via this new
> API to receive -EPROBE_DEFER instead of -ENODEV, leading to infinite
> probe deferrals and breaking fallback to the usb-phy subsystem?

I don't agree with this.

The compat string workaround should live where it is used, not be pushed 
down into generic layers where it is not.

No media-graph lookup needs this hack.
> 
>> +
>> +	if (IS_ERR(phy))
>> +		goto out_unlock;
>> +
>> +	if (!try_module_get(phy->ops->owner)) {
>> +		phy = ERR_PTR(-EPROBE_DEFER);
>> +		goto out_unlock;
>> +	}
>> +
>> +	get_device(&phy->dev);
>> +
>> +out_unlock:
>> +	mutex_unlock(&phy_provider_mutex);
>> +
>> +	return phy;
>> +}
>> +EXPORT_SYMBOL_GPL(phy_get_by_of_node);
>