Re: [PATCH 3/9] arm64: dts: marvell: Drop unused USB connector PHY supplies

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] arm64: dts: marvell: broken USB VBUS power after dropping
  phy-supply
--

commit 79bdb6a1206b7da32a160f289517e10fea0f4702
Author: Rob Herring (Arm) <robh@kernel.org>

arm64: dts: marvell: Drop unused USB connector PHY supplies

This commit removes the phy-supply properties from the usb-a-connector child
nodes of the Marvell COMPHY lane nodes across six Armada 3720, 7040, and 8040
device trees.

> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index bd00e7df87c67..1ef4689d5f18e 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -284,7 +284,6 @@ &usb2 {
>  &comphy2 {
>  	connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&exp_usb3_vbus>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-7040-db.dts b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> index 3add6506ff20e..f5a53fad66e67 100644
> --- a/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-7040-db.dts
> @@ -224,7 +224,6 @@ &cp0_utmi {
>  &cp0_comphy1 {
>  	cp0_usbh0_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&cp0_reg_usb3_0_vbus>;
>  	};
>  };
>  
> @@ -238,7 +237,6 @@ &cp0_usb3_0 {
>  &cp0_comphy4 {
>  	cp0_usbh1_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&cp0_reg_usb3_1_vbus>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
> index 90ae93274a160..4efa2a1dbda7c 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts
> @@ -599,7 +599,6 @@ flash@0 {
>  &cp1_comphy2 {
>  	cp1_usbh0_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&v_5v0_usb3_hst_vbus>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-db.dts b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> index c7102f74d4d54..89e6f35f310dd 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-db.dts
> @@ -170,7 +170,6 @@ &cp0_usb3_0 {
>  &cp0_comphy4 {
>  	cp0_usbh1_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&cp0_reg_usb3_1_vbus>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> index f2d278d171eb1..006293931c782 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dtsi
> @@ -370,7 +370,6 @@ flash@0 {
>  &cp1_comphy2 {
>  	cp1_usbh0_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&v_5v0_usb3_hst_vbus>;
>  	};
>  };
>  
> diff --git a/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
> index def25d51c4bf9..06a861f83ad87 100644
> --- a/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-8040-puzzle-m801.dts
> @@ -491,7 +491,6 @@ &cp1_uart0 {
>  &cp1_comphy2 {
>  	cp1_usbh0_con: connector {
>  		compatible = "usb-a-connector";
> -		phy-supply = <&v_5v0_usb3_hst_vbus>;
>  	};
>  };

[Severity: High]
Does dropping phy-supply here without moving the supply to the parent COMPHY
lane node (or vbus-supply on the connector) leave the USB3 VBUS regulators
disabled on these boards?

When mvebu_comphy_probe() and mvebu_a3700_comphy_probe() create a PHY for
each lane node, phy_create() calls regulator_get_optional(&phy->dev, "phy").
In of_get_regulator(), if phy-supply is not present on the lane node itself,
it falls back to searching child nodes:

drivers/regulator/of_regulator.c:of_get_regulator() {
	...
	snprintf(prop_name, 64, "%s-supply", supply);
	regnode = of_parse_phandle(node, prop_name, 0);
	if (regnode)
		return regnode;

	regnode = of_get_child_regulator(dev->of_node, prop_name);
	if (regnode)
		return regnode;
	...
}

Because of_get_child_regulator() finds phy-supply in the connector child
node, phy_power_on() currently enables these VBUS regulators when the xHCI
roothub powers on its PHYs. Removing phy-supply without moving it to the
parent lane node leaves cp0_reg_usb3_0_vbus, cp0_reg_usb3_1_vbus, and
v_5v0_usb3_hst_vbus (which are GPIO-controlled regulators without
regulator-always-on) with no consumer, so 5V VBUS power to the USB3 host
ports is never turned on.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-dtbs-marvell-v1-0-938788aff3ae@kernel.org?part=3