Re: [PATCH 09/10] ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] DT Schema violation: the `compatible` property for `usb3_ss_phy` uses an array of two strings instead of a single string.

Pre-existing issues:
- [High] Data race on physical PHY control register between two uncoordinated drivers.
--

--- Patch [9]: [PATCH 09/10] ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs ---
commit 688cb2c7a84a066e54c52b8eab7f37e81311d8fb
Author: Dmitry Baryshkov <[email protected]>

ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs

This commit describes the primary USB3 controller (DWC3) and its High-Speed
and SuperSpeed PHYs for the MSM8974 SoC. It initializes both PHYs to ensure
proper upstream enumeration.

> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
> index 06c0af0aa3176..bac3c8772e0a2 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi

[ ... ]

> @@ -713,6 +713,53 @@ blsp2_i2c6: i2c@f9968000 {
>  			#size-cells = <0>;
>  		};
>  
> +		usb3: usb@f9200000 {
> +			compatible = "qcom,msm8974-dwc3", "qcom,snps-dwc3";
> +			reg = <0xf9200000 0xfc000>;

[Severity: High]
This isn't a bug introduced by this patch, but does mapping this region
expose a data race on the physical PHY control register?

The dwc3-qcom wrapper driver maps this region, calculating qscratch_base
as 0xf92f8800, and performs read-modify-write operations on offset 0x30
(physical 0xf92f8830) during role switches or gadget start/stop.

Simultaneously, the newly bound phy-qcom-ipq806x-usb driver maps the
exact same physical address bypassing exclusivity in the usb3_ss_phy
node below.

[ ... ]

> +		usb3_ss_phy: phy@f92f8830 {
> +			compatible = "qcom,msm8974-usb-phy-ss",
> +				     "qcom,ipq806x-usb-phy-ss";

[Severity: Low]
Will this compatible array cause make dtbs_check validation to fail?

The DT binding schema qcom,ipq806x-usb-phy-ss.yaml strictly defines the
compatible property as either "qcom,ipq806x-usb-phy-ss" or
"qcom,msm8974-usb-phy-ss". 

Providing a string array of both violates this definition.

> +			reg = <0xf92f8830 0x30>;

[Severity: High]
This is a pre-existing issue, but returning to the mapping overlap, the
SuperSpeed PHY driver explicitly maps the exact same physical address
(0xf92f8830) here and modifies its offset 0x00 without coordination.

Since both drivers perform uncoordinated read-modify-write operations
(sometimes with large delays like msleep(30) mid-RMW in the PHY driver),
can concurrent operations clobber the physical register state?

This could lose bits like SSUSB_CTRL_SS_PHY_EN or LANE0_PWR_PRESENT,
corrupting the power state and breaking USB enumeration during concurrent
plug/unplug events and runtime PM transitions.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy