Re: [PATCH] pinctrl: sunxi: skip unused per-bank supply on controllers without IO bias

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pinctrl: sunxi: sunxi_pmx_request() skips enabling per-bank power supplies
--

commit 9182c2f5356e50617749f1a17d156851c7683cd0
Author: Yang Siyuan <hsyysy@163.com>

pinctrl: sunxi: skip unused per-bank supply on controllers without IO bias

This commit skips requesting the per-bank "vcc-pX" regulator in
sunxi_pmx_request() and freeing it in sunxi_pmx_free() when
pctl->desc->io_bias_cfg_variant is not set, in order to silence missing
supply warnings on controllers without IO bias configuration such as the
H616 R_PIO.

> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 31cd142ce0f79..4b877ec1e1836 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -924,6 +924,15 @@ static int sunxi_pmx_request(struct pinctrl_dev *pctldev, unsigned offset)
>  		return 0;
>  	}
>  
> +	/*
> +	 * The per-bank supply is only used to configure the IO bias voltage.
> +	 * Controllers without bias configuration (e.g. the H616 R_PIO) have no
> +	 * supply to request, and asking for one only makes the regulator core
> +	 * warn about the missing regulator for every bank in use.
> +	 */
> +	if (!pctl->desc->io_bias_cfg_variant)
> +		return 0;
> +
>  	snprintf(supply, sizeof(supply), "vcc-p%c", 'a' + bank);
>  	reg = regulator_get(pctl->dev, supply);
>  	if (IS_ERR(reg))

[Severity: High]
Does returning early in sunxi_pmx_request() when
!pctl->desc->io_bias_cfg_variant break per-bank power supply enabling on
SoCs without software-configurable IO bias registers?

In sunxi_pmx_request(), the per-bank "vcc-pX" supply is not only passed to
sunxi_pinctrl_set_io_bias_cfg(), but is also enabled via regulator_enable()
to power the physical pin bank while its pins are requested (originally
added in commit 9a2a566adb00 ("pinctrl: sunxi: Deal with per-bank
regulators") before IO bias configuration support in commit 402bfb3c1352):

drivers/pinctrl/sunxi/pinctrl-sunxi.c:sunxi_pmx_request() {
	...
	snprintf(supply, sizeof(supply), "vcc-p%c", 'a' + bank);
	reg = regulator_get(pctl->dev, supply);
	if (IS_ERR(reg))
		return dev_err_probe(pctl->dev, PTR_ERR(reg),
				     "Couldn't get bank P%c regulator\n",
				     'A' + bank);

	ret = regulator_enable(reg);
	if (ret) {
		dev_err(pctl->dev,
			"Couldn't enable bank P%c regulator\n", 'A' + bank);
		goto out;
	}

	sunxi_pinctrl_set_io_bias_cfg(pctl, offset, reg);
	...
}

Most Allwinner pinctrl descriptors leave io_bias_cfg_variant as 0
(BIAS_VOLTAGE_NONE), including sun4i_a10_pinctrl_data in
drivers/pinctrl/sunxi/pinctrl-sun4i-a10.c (used for A10, A20, R40, V40),
a64_pinctrl_data in drivers/pinctrl/sunxi/pinctrl-sun50i-a64.c, and
sun8i_v3s_pinctrl_data in drivers/pinctrl/sunxi/pinctrl-sun8i-v3s.c:

drivers/pinctrl/sunxi/pinctrl-sun50i-a64.c:
static const struct sunxi_pinctrl_desc a64_pinctrl_data = {
	.pins = a64_pins,
	.npins = ARRAY_SIZE(a64_pins),
	.irq_banks = 3,
};

However, boards using those SoCs wire physical per-bank VCC-Px rails to
switchable, non-always-on PMIC regulators via vcc-pX-supply in Device Tree,
such as arch/arm/boot/dts/allwinner/sun8i-r40-bananapi-m2-ultra.dts:

&pio {
	pinctrl-names = "default";
	pinctrl-0 = <&clk_out_a_pin>;
	vcc-pa-supply = <&reg_aldo2>;
	vcc-pc-supply = <&reg_dcdc1>;
	vcc-pd-supply = <&reg_dcdc1>;
	vcc-pe-supply = <&reg_eldo1>;
	vcc-pf-supply = <&reg_dcdc1>;
	vcc-pg-supply = <&reg_dldo1>;
};

&reg_aldo2 {
	regulator-min-microvolt = <2500000>;
	regulator-max-microvolt = <2500000>;
	regulator-name = "vcc-pa";
};

Similar non-always-on per-bank supplies are used on A64 boards (such as
sun50i-a64-pinephone.dtsi, sun50i-a64-pinebook.dts,
sun50i-a64-olinuxino.dts, and sun50i-a64-teres-i.dts for vcc-pd-supply,
vcc-pe-supply, and vcc-pg-supply), A20 boards
(sun7i-a20-olinuxino-lime2.dts), and S3 boards (sun8i-s3-pinecube.dts).

If sunxi_pmx_request() skips regulator_get() and regulator_enable() when
!pctl->desc->io_bias_cfg_variant, won't these per-bank regulators remain
unenabled (or be disabled as unused at late_initcall by
regulator_init_complete()), cutting power to active pin banks and attached
peripherals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026144751.1392715-1-hsyysy@163.com?part=1