Re: [PATCH net-next v15 03/12] net: pcs: implement Firmware node support for PCS driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Provider xlate errors are improperly masked to -ENODEV, breaking error reporting.
--

commit bda5aa3be050e19dfc3117135971734f3811b538
Author: Christian Marangi <ansuelsmth@gmail.com>

net: pcs: implement Firmware node support for PCS driver

This commit implements the foundation for firmware node support for PCS drivers.
It introduces an API for PCS drivers to add providers and for consumers to
retrieve a PCS by passing a firmware node pointer and an index.

> diff --git a/drivers/net/pcs/pcs.c b/drivers/net/pcs/pcs.c
> new file mode 100644
> index 0000000000000..39550d9a12a2a
> --- /dev/null
> +++ b/drivers/net/pcs/pcs.c
> @@ -0,0 +1,245 @@
[ ... ]
> +static struct phylink_pcs *
> +fwnode_pcs_get_from_pcsspec(struct fwnode_reference_args *pcsspec)
> +{
> +	struct fwnode_pcs_provider *provider;
> +	struct phylink_pcs *pcs = NULL;
> +
> +	if (!pcsspec)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&fwnode_pcs_mutex);
> +	list_for_each_entry(provider, &fwnode_pcs_providers, link) {
> +		pcs = __fwnode_pcs_get_from_pcsspec_provider(pcsspec, provider);
> +		if (!IS_ERR(pcs))
> +			break;
> +	}

[Severity: Medium]
What happens if a matching provider's xlate function legitimately returns
an error?

Looking at __fwnode_pcs_get_from_pcsspec_provider(), it returns -EINVAL
when the firmware node does not match. Because of this, checking
!IS_ERR(pcs) successfully skips non-matching providers.

However, if a provider does match but its xlate callback returns an error
(like -EINVAL or -EPROBE_DEFER), pcs will also be an error pointer. This
causes the loop to incorrectly continue searching instead of stopping at
the matching provider.

> +	mutex_unlock(&fwnode_pcs_mutex);
> +
> +	return !IS_ERR_OR_NULL(pcs) ? pcs : ERR_PTR(-ENODEV);

[Severity: Medium]
Does this unconditionally overwrite the error code from the xlate callback?

Because the loop does not break on xlate errors, any specific error code
returned by the provider is discarded. Even if the matching provider was
the last one in the list, its error pointer is replaced with -ENODEV here.

This causes fwnode_phylink_pcs_parse() to silently skip the PCS (treating
it as an unprobed/late PCS) instead of propagating the actual error.
Returning the actual error pointer could allow the caller to handle it
appropriately.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901083001.8044-1-ansuelsmth@gmail.com?part=3