Re: [PATCH v6 16/17] pinctrl: airoha: try to find chip scu node by phandle first

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Probe deferral and resource errors from `syscon_regmap_lookup_by_phandle()` are masked by unconditional fallback.
- [Medium] The 'airoha,chip-scu' phandle property is not documented in the device tree binding schemas.
- [Low] Unnecessary and incorrect use of `IS_ERR_OR_NULL()` for an API that exclusively returns valid pointers or `ERR_PTR()`.
--

commit 5fba429358ea0aaa62389e837020f02835843d58
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

pinctrl: airoha: try to find chip scu node by phandle first

This patch updates the probe routine to search for the chip scu regmap using
the "airoha,chip-scu" phandle before falling back to a SoC-specific compatible
string. This provides better support for newer SoCs while maintaining
compatibility.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index 543475d23fde3..e24e03b39d3d5 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -683,9 +683,12 @@ int airoha_pinctrl_probe(struct platform_device *pdev)
>  	if (IS_ERR(pinctrl->regmap))
>  		return PTR_ERR(pinctrl->regmap);
>  
> -	map = syscon_regmap_lookup_by_compatible("airoha,en7581-chip-scu");
> -	if (IS_ERR(map))
> -		return PTR_ERR(map);
> +	map = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "airoha,chip-scu");

[Severity: Medium]
Is the "airoha,chip-scu" property documented in the device tree binding
schemas? It appears this property might not have been added to the dt-binding
YAML files, which could cause device tree validation tools to emit warnings.

> +	if (IS_ERR_OR_NULL(map)) {

[Severity: Low]
Is IS_ERR_OR_NULL() necessary here? syscon_regmap_lookup_by_phandle() returns
an ERR_PTR() on failure rather than NULL, so IS_ERR() should be sufficient.

[Severity: High]
Does this logic unconditionally overwrite -EPROBE_DEFER if the primary lookup
fails because the syscon node isn't registered yet?

If syscon_regmap_lookup_by_phandle() returns -EPROBE_DEFER or -ENOMEM, the
fallback syscon_regmap_lookup_by_compatible() might fail with -ENODEV and
return that instead. This could cause a permanent driver probe failure rather
than waiting for the resource.

Should the fallback be restricted to only execute if the initial lookup
explicitly returns -ENODEV?

> +		map = syscon_regmap_lookup_by_compatible(data->chip_scu_compatible);
> +		if (IS_ERR(map))
> +			return PTR_ERR(map);
> +	}
>  
>  	pinctrl->chip_scu = map;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628143733.273651-1-mikhail.kshevetskiy@iopsys.eu?part=16