Re: [PATCH v4 3/6] 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] The essential 60MHz transceiver clock is mapped to the 'fs' clock, which is explicitly disabled by the driver during normal operation, breaking USB functionality.
- [Low] Adding a third clock to the `qcom,ci-hdrc` nodes violates the `maxItems: 2` constraint in the existing binding schema.
--

commit 453c68cf87c5de356d651306bdd24fea9db45f5a
Author: Alexandre MINETTE <contact@alex-min.fr>

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

This commit updates the clock properties 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" to ensure the fabric clock is enabled.

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";

[Severity: Low]
Does adding a third clock here violate the binding schema?

Looking at Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml, both the
clocks and clock-names properties are restricted to a maximum of 2 items:

Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml:
  clocks:
    minItems: 1
    maxItems: 2

  clock-names:
    minItems: 1
    maxItems: 2

Will this cause make dtbs_check to fail during validation?

[Severity: High]
Will mapping the transceiver clock to "fs" cause it to be permanently
disabled after probe?

Looking at ci_hdrc_msm_probe() in the MSM ChipIdea driver, the fs clock is
explicitly disabled immediately after the controller reset sequence:

drivers/usb/chipidea/ci_hdrc_msm.c:ci_hdrc_msm_probe() {
    ...
    clk_disable_unprepare(ci->fs_clk);
    ...
}

Since the 60MHz transceiver clock (USB_HS1_XCVR_CLK) is required for the
PHY and MAC to operate, won't turning it off break USB functionality on
this platform entirely?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-mainline-send-v1-sending-v4-0-83768fbf404d@alex-min.fr?part=3