Re: [PATCH v2 05/10] ARM: dts: qcom: msm8960: add RPM clock controller and fix USB clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Mapping `USB_HS1_XCVR_CLK` to the `fs` clock slot causes the USB controller to lose its transceiver clock during normal operation, breaking USB functionality.
- [Medium] Improperly assigning an interconnect fabric clock to the `core` clock slot of the USB controller.
- [Low] The updated USB node violates the `maxItems: 2` restriction for clocks in the `qcom,ci-hdrc` device tree binding schema.
--

commit e540b9f85ba14f0ebee0eabb79c077903050c001
Author: Antony Kurniawan Soemardi <linux@smankusors.com>

ARM: dts: qcom: msm8960: add RPM clock controller and fix USB clocks

This commit adds the RPM clock controller to the MSM8960 device tree and
updates the USB controller to declare a dependency on RPM_DAYTONA_FABRIC_CLK.
It also moves USB_HS1_XCVR_CLK to the fs clock slot.

> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
> index fd28401cebb5e..0601680cd8dd1 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8960.dtsi
[ ... ]
> @@ -507,8 +519,12 @@ 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 = <&rpmcc RPM_DAYTONA_FABRIC_CLK>,
> +				 <&gcc USB_HS1_H_CLK>,
> +				 <&gcc USB_HS1_XCVR_CLK>;
> +			clock-names = "core",
> +				      "iface",
> +				      "fs";

Does adding a third clock here violate the devicetree binding schema?

The qcom,ci-hdrc binding in Documentation/devicetree/bindings/usb/ci-hdrc-usb2.yaml
restricts both clocks and clock-names to a maximum of 2 items. Applying this
without updating the schema seems to trigger dtbs_check errors.

Does mapping the transceiver clock to the fs slot inadvertently break USB
functionality during normal operation?

In the ChipIdea MSM driver in drivers/usb/chipidea/ci_hdrc_msm.c, the fs clock
is used exclusively to propagate reset signals during probe. Immediately after
the reset sequence, the driver calls clk_disable_unprepare() on it and does not
re-enable it. Since USB_HS1_XCVR_CLK is the required 60MHz ULPI transceiver
clock, turning it off would cause the USB controller to fail entirely.

Also, is it appropriate to assign the interconnect fabric clock to the core
clock slot?

The fabric clock is an interconnect bus clock rather than the operational
core clock for the controller. Usually, bus dependencies are managed through
the interconnect API or by adding dedicated support for bus clocks in the
ci_hdrc_msm driver, rather than taking over the mandatory core clock slot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514-msm8960-wifi-v2-0-7cbae45dab5e@smankusors.com?part=5