Re: [PATCHv2 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 2 potential issue(s) to consider:
- [High] usb: bcma: silent power down of USB ports due to unmanaged vbus-supply
- [Medium] usb: bcma: suspend and shutdown power savings broken by missing GPIO
--

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

ARM: dts: broadcom: model USB VBUS power with regulator-fixed

Replace the undocumented "vcc-gpio" property on the usb2/usb3 controller
nodes with the standard vbus-supply/regulator-fixed model. Boards now
declare a regulator bound to the chipcommon GPIO that bcma-hcd used to
poke directly, one per power rail.

> diff --git a/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wxr-1750dhp.dts b/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wxr-1750dhp.dts
> index f5c95c9a712ec..80414c72f9129 100644
> --- a/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wxr-1750dhp.dts
> +++ b/arch/arm/boot/dts/broadcom/bcm4708-buffalo-wxr-1750dhp.dts
> @@ -101,6 +101,16 @@ led-usb {
>  			trigger-sources = <&xhci_port1 &ehci_port1 &ohci_port1>;
>  		};
>  	};
> +
> +	usb_power: regulator-usb {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb-vcc";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-boot-on;
> +		gpios = <&chipcommon 10 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
>  };

[Severity: High]
Since this regulator lacks the regulator-always-on property, and no driver
claims it, will the regulator core forcibly disable it shortly after boot?

Looking at bcma_hcd_probe() in drivers/usb/host/bcma-hcd.c, the driver
still expects a vcc-gpio property and lacks the logic to request or consume
a vbus-supply regulator:

drivers/usb/host/bcma-hcd.c:bcma_hcd_probe() {
	...
	usb_dev->gpio_desc = devm_gpiod_get_optional(&core->dev, "vcc",
						     GPIOD_OUT_HIGH);
	...
}

Because the regulator is never claimed, it will be automatically disabled by
the regulator core via regulator_late_cleanup(), which would cut off power
to all USB ports and break attached devices.

[ ... ]

> @@ -130,7 +140,7 @@ port@4 {
>  };
>  
>  &usb3 {
> -	vcc-gpio = <&chipcommon 10 GPIO_ACTIVE_HIGH>;
> +	vbus-supply = <&usb_power>;
>  };

[Severity: Medium]
Since the bcma-hcd driver was not updated to handle vbus-supply and still
looks for vcc-gpio, does this break USB power toggling during system suspend
and shutdown?

Because usb_dev->gpio_desc now evaluates to NULL, the power-toggling
function silently returns without asserting the hardware state:

drivers/usb/host/bcma-hcd.c:bcma_hci_platform_power_gpio() {
	...
	if (!usb_dev->gpio_desc)
		return;

	gpiod_set_value(usb_dev->gpio_desc, val);
	...
}

This means when system suspend calls bcma_hcd_suspend(), which subsequently
calls bcma_hci_platform_power_gpio(dev, false), VBUS power won't actually
be turned off, resulting in a loss of power savings.

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