Re: [PATCH v3 6/8] ARM: dts: qcom: apq8064: Fix USB controller clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Moving the transceiver clock (`USB_HSx_XCVR_CLK`) to the "fs" clock causes it to be permanently disabled during normal operation, breaking USB functionality.
- [Medium] Adding a third clock to the `qcom,ci-hdrc` nodes violates the `maxItems: 2` constraint in the `ci-hdrc-usb2.yaml` device tree binding schema.
--

commit 5a018643b67a319b143bcd44640fccd2fe85acc0
Author: Alexandre MINETTE <contact@alex-min.fr>

ARM: dts: qcom: apq8064: Fix USB controller clocks

This commit reorganizes the clock descriptions for the APQ8064 HS USB
controllers. It maps the RPM Daytona fabric clock to core, the AHB clock
to iface, and the transceiver clock to fs.

Link: https://lore.kernel.org/all/20260516-qcom-ci-hdrc-clock-fix-v2-1-aaec8d33d0aa@smankusors.com/

> diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
> index 09062b2ad8ba5..d64a162abdad4 100644
> --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi
> @@ -813,8 +813,10 @@ usb1: usb@12500000 {
>  			reg = <0x12500000 0x200>,
>  			      <0x12500200 0x200>;
>  			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&gcc USB_HS1_XCVR_CLK>, <&gcc USB_HS1_H_CLK>;
> -			clock-names = "core", "iface";
> +			clocks = <&gcc USB_HS1_H_CLK>,
> +				 <&rpmcc RPM_DAYTONA_FABRIC_CLK>,
> +				 <&gcc USB_HS1_XCVR_CLK>;
> +			clock-names = "iface", "core", "fs";

Does adding a third clock here violate the maxItems: 2 constraint in the
ci-hdrc-usb2.yaml device tree binding schema?

The qcom,ci-hdrc compatible string is validated by this schema. Expanding
the device tree node to include three clocks without updating the
corresponding YAML schema will cause the device tree to fail schema
validation.

Will mapping the transceiver clock to the fs clock cause it to be permanently
disabled during normal operation?

In drivers/usb/chipidea/ci_hdrc_msm.c, the fs clock is used exclusively for
a hardware reset sequence during probe. The driver calls
clk_prepare_enable(ci->fs_clk), toggles the reset controller, and immediately
calls clk_disable_unprepare(ci->fs_clk). It is never re-enabled.

Previously, the transceiver clock was mapped to core, which the driver keeps
continuously enabled during normal operation. Since the qcom,usb-hs-phy
driver does not independently request or enable the transceiver clock, this
change appears to guarantee the transceiver will be clock-gated while the USB
controller is actually running.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-mainline-send-v1-sending-v3-0-3dd7aa125353@alex-min.fr?part=6