Re: [PATCH v2 3/5] power: reset: syscon-reboot-mode: enable supplies for the next stage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] power: reset: uninitialized pointer passed to regulator_bulk_enable()
--

commit cdeebbc0731de671c753a5b987cb44186458c7fb
Author: Alexey Charkov <alchark@flipper.net>

power: reset: syscon-reboot-mode: enable supplies for the next stage

This patch adds support for enabling power supplies named in the reboot mode
node when a boot mode is written. It requests the supplies in the probe
function using of_regulator_bulk_get_all() and enables them during reboot to
ensure the PMIC configuration is suitable for early boot code on the next
startup.

Link: https://lore.kernel.org/all/20260929-regulator-get-all-v1-1-e887c66a47f1@flipper.net/

> diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c
> index e0772c9f70f7a..5ef882d818161 100644
> --- a/drivers/power/reset/syscon-reboot-mode.c
> +++ b/drivers/power/reset/syscon-reboot-mode.c
[ ... ]
> @@ -28,6 +32,20 @@ static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  
>  	syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot);
>  
> +	/*
> +	 * Whatever acts on the mode (e.g. boot ROM) runs before the operating
> +	 * system, and may need supplies that the running system had powered
> +	 * down. Enable them here, and deliberately leave them enabled: the
> +	 * system is on its way down, and what runs next may not know how to
> +	 * turn them on.
> +	 */
> +	if (syscon_rbm->num_supplies) {
> +		ret = regulator_bulk_enable(syscon_rbm->num_supplies,
> +					    syscon_rbm->supplies);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this code introduce a regression if regulator_bulk_enable() fails?

The commit message notes a separate fix for the core API issue where
of_regulator_bulk_get_all() leaves the supply names uninitialized. Because
of_regulator_bulk_get_all() uses kmalloc_objs() internally (which does not
zero memory), the 'supply' field in the returned array contains garbage
pointers:

drivers/regulator/of_regulator.c:of_regulator_bulk_get_all() {
    ...
    _consumers = kmalloc_objs(struct regulator_bulk_data, num_consumers);
    ...
}

If regulator_bulk_enable() fails, the error path in the regulator core will
attempt to print the name of the failing supply:

drivers/regulator/core.c:regulator_bulk_enable() {
    ...
    pr_err("Failed to enable %s: %pe\n", consumers[i].supply,
           ERR_PTR(consumers[i].ret));
    ...
}

Because this patch relies on an uninitialized array from
of_regulator_bulk_get_all(), applying it independently introduces a regression
where the kernel will panic if a hardware issue or misconfiguration causes
regulator_bulk_enable() to fail during reboot.

> +		if (ret < 0)
> +			dev_err(reboot->dev, "enabling reboot mode supplies failed\n");
> +	}
> +
>  	ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset,
>  				 syscon_rbm->mask, magic);
[ ... ]
> @@ -59,6 +85,21 @@ static int syscon_reboot_mode_probe(struct platform_device *pdev)
>  
>  	of_property_read_u32(pdev->dev.of_node, "mask", &syscon_rbm->mask);
>  
> +	ret = of_regulator_bulk_get_all(&pdev->dev, pdev->dev.of_node,
> +					&syscon_rbm->supplies);
> +	if (ret < 0)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "can't get reboot mode supplies\n");
> +
> +	syscon_rbm->num_supplies = ret;
> +	if (syscon_rbm->num_supplies) {
> +		ret = devm_add_action_or_reset(&pdev->dev,
> +					       syscon_reboot_mode_put_supplies,
> +					       syscon_rbm);
> +		if (ret)
> +			return ret;
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-b4-rk3576-reboot-mode-v2-0-5a34ca72065a@flipper.net?part=3