Re: [PATCH 5/6] arm64: dts: qcom: sdm845-oneplus-common: enable USB Type-C role switching

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing `maximum-speed = "high-speed"` misconfigures the DWC3 controller to default to SuperSpeed on a physically High-Speed-only board, leading to false capabilities advertisement and potential AXI bus hangs.
- [High] The `vdd-vbus-supply` property is missing from the `&pmi8998_typec` node, which completely breaks VBUS power delivery in USB host mode.
--

commit fd2e287ee85d01cae64bd27f78d0a7b1a4d6ea65
Author: taygoth <taygoth@gmail.com>

arm64: dts: qcom: sdm845-oneplus-common: enable USB Type-C role switching

This commit enables the PMI8998 Type-C controller and configures the dwc3
controller for OTG mode on the OnePlus 6/6T. It removes the maximum-speed
property and maps the USB endpoints for role switching.

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
[ ... ]
> +&pmi8998_vbus {
> +	regulator-min-microamp = <500000>;
> +	regulator-max-microamp = <1500000>;
> +	status = "okay";
> +};
> +
> +&pmi8998_typec {
> +	status = "okay";

Does this node need a vdd-vbus-supply property?

The qcom,pmi8998-typec driver looks up the VBUS regulator using
devm_regulator_get_optional(dev, "vdd-vbus"). Without a vdd-vbus-supply
property pointing to &pmi8998_vbus, the lookup returns -ENODEV and the
driver proceeds with a NULL vbus.

Will this result in the driver silently skipping the VBUS regulator
enablement during USB_ROLE_HOST transitions, leaving bus-powered downstream
peripherals without power?

> +
> +	connector {
> +		compatible = "usb-c-connector";
[ ... ]
> @@ -890,19 +920,18 @@ &usb_1 {
>  };
>  
>  &usb_1_dwc3 {
> -	/*
> -	 * We don't have the capability to switch modes yet.
> -	 */
> -	dr_mode = "peripheral";
> -
> -	/* fastest mode for USB 2 */
> -	maximum-speed = "high-speed";
> +	dr_mode = "otg";
> +	usb-role-switch;

Is it safe to remove the maximum-speed property here?

The commit message notes that the role switching infrastructure handles speed
negotiation through the usb-c-connector. However, Type-C CC pins typically
only negotiate power delivery and data roles, not the underlying USB protocol
speed limits.

If maximum-speed is omitted, the DWC3 core driver falls back to reading the
controller's internal hardware parameters (GHWPARAMS3). Since the SDM845 IP
is SuperSpeed-capable, it will default to USB_SPEED_SUPER.

Because the board only wires High-Speed lines and drops the SuperSpeed PHY
(phys = <&usb_1_hsphy>), will the xHCI driver register a SuperSpeed root hub
and attempt Rx.Detect polling? Could attempting to access the PIPE interface
of an unclocked QMP PHY trigger an AXI bus hang?

>  
>  	/* Remove USB3 phy as it's unused on this device. */
>  	phys = <&usb_1_hsphy>;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1779127507.git.taygoth@gmail.com?part=5