Re: [PATCH 2/2] ARM: dts: broadcom: model USB VBUS power with regulator-fixed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] usb: bcma: ABI breakage on older Device Trees due to missing regulator
- [High] usb: bcma: regulator enable count leaked on error paths and remove
- [Low] build: missing closing brace in bcma_hcd_probe() causes compilation failure
--

commit 2da0c63e158c66b33d97e363d8f72cf580a7db29
Author: Rosen Penev <rosenp@gmail.com>

usb: bcma: control VBUS via a regulator instead of a GPIO

This commit replaces the undocumented "vcc-gpio" property handling with the
standard regulator interface. Each core obtains its VBUS supply via
devm_regulator_get_optional() and the regulator core reference-counts the
enable/disable requests to avoid conflicts.

> diff --git a/drivers/usb/host/bcma-hcd.c b/drivers/usb/host/bcma-hcd.c
> index 519386255886c..394ba9e5a7f77 100644
> --- a/drivers/usb/host/bcma-hcd.c
> +++ b/drivers/usb/host/bcma-hcd.c

[ ... ]

> @@ -405,11 +411,14 @@ static int bcma_hcd_probe(struct bcma_device *core)
>  		return -ENOMEM;
>  	usb_dev->core = core;
>  
> -	usb_dev->gpio_desc = devm_gpiod_get_optional(&core->dev, "vcc",
> -						     GPIOD_OUT_HIGH);
> -	if (IS_ERR(usb_dev->gpio_desc))
> -		return dev_err_probe(&core->dev, PTR_ERR(usb_dev->gpio_desc),
> -				     "error obtaining VCC GPIO");
> +	usb_dev->regulator = devm_regulator_get_optional(&core->dev, "vbus");
> +	if (IS_ERR(usb_dev->regulator)) {
> +		return PTR_ERR(usb_dev->regulator);

[Severity: Critical]
Is it possible this unconditional return breaks backward compatibility with
older device trees?

Since devm_regulator_get_optional() returns -ENODEV when the regulator is
absent, returning this error will fail probe entirely on older setups rather
than proceeding without it.

[Severity: Low]
Does this code have a missing closing brace?

The if block is opened here but the closing brace is missing, incorrectly
scoping the rest of the function and breaking compilation.

> +
> +	err = regulator_enable(usb_dev->regulator);

[Severity: High]
Does this code leak the regulator enable count?

If bcma_hcd_probe() encounters an error later in the initialization sequence
after regulator_enable() has succeeded:

drivers/usb/host/bcma-hcd.c:bcma_hcd_probe() {
    ...
	switch (core->id.id) {
    ...
	}
	if (err)
		return err;
    ...
}

It returns without calling regulator_disable().

Similarly, does bcma_hcd_remove() also leak the regulator enable count?

When the module is unbound, it fails to invoke regulator_disable(), which
would leave the regulator active and trigger core warnings.

> +	if (err)
> +		return dev_err_probe(&core->dev, err,
> +				     "error enabling VCC regulator");
>  
>  	switch (core->id.id) {
>  	case BCMA_CORE_USB20_HOST:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211356.186967-1-rosenp@gmail.com?part=2