[PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes

[PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes

[Greg Ociepka]

The four USB3 controller nodes on Glymur (usb_0, usb_1, usb_2, usb_mp)
are missing three property groups that Glymur's own usb_hs node at
a200000 already carries, and that Hamoa (x1e80100) carries on all five
of its USB controllers:

  - assigned-clocks / assigned-clock-rates
  - required-opps
  - interconnects / interconnect-names

Without required-opps the controllers place no performance state vote
on their power domain. Measured on an Asus Zenbook A16 (UX3607OA) with
a SuperSpeed device enumerated, CX aggregates to SVS while the
controllers run their master clock at 200 MHz:

  $ grep -E "^cx " /sys/kernel/debug/pm_genpd/pm_genpd_summary
  cx    on    128

  $ grep "usb30.*master_clk " /sys/kernel/debug/clk/clk_summary
  gcc_usb30_sec_master_clk   ...  200000000
  gcc_usb30_prim_master_clk  ...  200000000
  gcc_usb30_mp_master_clk    ...  200000000

128 is RPMH_REGULATOR_LEVEL_SVS; rpmhpd_opp_nom is 256. With this
patch applied the same measurement reads 256 while a controller is
active.

Without interconnects, dwc3_qcom_interconnect_init() silently does
nothing: of_icc_get() returns NULL rather than an error pointer for a
device without the property, so probe succeeds and icc_set_bw() on a
NULL path is a no-op. No USB-DDR bandwidth is ever requested.

Without assigned-clock-rates nothing programs the master clock RCG,
which keeps whatever the bootloader left behind. On this machine that
happens to be the correct 200 MHz, but ftbl_gcc_usb30_mp_master_clk_src
also allows 66.67 and 133.33 MHz, neither of which sustains SuperSpeed.

Add all three groups to the four nodes, mirroring usb_hs. The
interconnect paths follow the provider layout in
drivers/interconnect/qcom/glymur.c: usb_0/usb_1 sit on aggre4_noc,
usb_mp on aggre3_noc and usb_2 on aggre2_noc.

Fixes: 4eee57dd4df9 ("arm64: dts: qcom: glymur: Add USB related nodes")
Assisted-by: Claude:fable-5
Signed-off-by: Greg Ociepka <greg@ferrisoft.com>
---
Note for reviewers: Hamoa additionally marks all five USB controllers
dma-coherent, while Glymur marks no peripheral DMA master as coherent
(only the SMMUs, PCIe controllers and remoteprocs). That difference is
deliberately not touched here since getting it wrong is a silent
data-corruption class of bug, but it looks like part of the same
omission. Is USB DMA coherent on Glymur?

--- a/arch/arm64/boot/dts/qcom/glymur.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
@@ -4572,6 +4572,10 @@
 				      "noc_aggr_north",
 				      "noc_aggr_south";
 
+			assigned-clocks = <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_PRIM_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <200000000>;
+
 			interrupts-extended = <&intc GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>,
 					      <&intc GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH>,
 					      <&pdc 90 IRQ_TYPE_EDGE_BOTH>,
@@ -4584,9 +4588,18 @@
 					  "ss_phy_irq";
 
 			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 			resets = <&gcc GCC_USB30_PRIM_BCR>;
 
 			iommus = <&apps_smmu 0x1420 0x0>;
+
+			interconnects = <&aggre4_noc MASTER_USB3_0 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&hsc_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_USB3_0 QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "usb-ddr",
+					     "apps-usb";
+
 			phys = <&usb_0_hsphy>,
 			       <&usb_0_qmpphy QMP_USB43DP_USB3_PHY>;
 			phy-names = "usb2-phy",
@@ -4647,6 +4660,10 @@
 				      "noc_aggr_north",
 				      "noc_aggr_south";
 
+			assigned-clocks = <&gcc GCC_USB30_SEC_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_SEC_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <200000000>;
+
 			interrupts-extended = <&intc GIC_SPI 875 IRQ_TYPE_LEVEL_HIGH>,
 					      <&intc GIC_SPI 369 IRQ_TYPE_LEVEL_HIGH>,
 					      <&pdc 88 IRQ_TYPE_EDGE_BOTH>,
@@ -4660,9 +4677,17 @@
 
 			resets = <&gcc GCC_USB30_SEC_BCR>;
 			power-domains = <&gcc GCC_USB30_SEC_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			iommus = <&apps_smmu 0x1460 0x0>;
 
+			interconnects = <&aggre4_noc MASTER_USB3_1 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&hsc_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_USB3_1 QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "usb-ddr",
+					     "apps-usb";
+
 			phys = <&usb_1_hsphy>,
 			       <&usb_1_qmpphy QMP_USB43DP_USB3_PHY>;
 			phy-names = "usb2-phy",
@@ -4723,6 +4748,10 @@
 				      "noc_aggr_north",
 				      "noc_aggr_south";
 
+			assigned-clocks = <&gcc GCC_USB30_TERT_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_TERT_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <200000000>;
+
 			interrupts-extended = <&intc GIC_SPI 871 IRQ_TYPE_LEVEL_HIGH>,
 					      <&intc GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>,
 					      <&pdc 89 IRQ_TYPE_EDGE_BOTH>,
@@ -4736,9 +4765,17 @@
 
 			resets = <&gcc GCC_USB30_TERT_BCR>;
 			power-domains = <&gcc GCC_USB30_TERT_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			iommus = <&apps_smmu 0x420 0x0>;
 
+			interconnects = <&aggre2_noc MASTER_USB3_2 QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&hsc_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_USB3_2 QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "usb-ddr",
+					     "apps-usb";
+
 			phys = <&usb_2_hsphy>,
 			       <&usb_2_qmpphy QMP_USB43DP_USB3_PHY>;
 			phy-names = "usb2-phy",
@@ -4869,6 +4906,10 @@
 				      "noc_aggr_north",
 				      "noc_aggr_south";
 
+			assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_MP_MASTER_CLK>;
+			assigned-clock-rates = <19200000>, <200000000>;
+
 			interrupts-extended = <&intc GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
 					      <&intc GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>,
 					      <&intc GIC_SPI 346 IRQ_TYPE_LEVEL_HIGH>,
@@ -4894,9 +4935,17 @@
 
 			resets = <&gcc GCC_USB30_MP_BCR>;
 			power-domains = <&gcc GCC_USB30_MP_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
 
 			iommus = <&apps_smmu 0xda0 0x0>;
 
+			interconnects = <&aggre3_noc MASTER_USB3_MP QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
+					<&hsc_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_USB3_MP QCOM_ICC_TAG_ACTIVE_ONLY>;
+			interconnect-names = "usb-ddr",
+					     "apps-usb";
+
 			phys = <&usb_mp_hsphy0>,
 			       <&usb_mp_qmpphy0>,
 			       <&usb_mp_hsphy1>,

Re: [PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes

[Abel Vesa]

On 26-08-12 10:35:22, Greg Ociepka wrote:
> The four USB3 controller nodes on Glymur (usb_0, usb_1, usb_2, usb_mp)
> are missing three property groups that Glymur's own usb_hs node at
> a200000 already carries, and that Hamoa (x1e80100) carries on all five
> of its USB controllers:
> 
>   - assigned-clocks / assigned-clock-rates
>   - required-opps
>   - interconnects / interconnect-names
> 
> Without required-opps the controllers place no performance state vote
> on their power domain. Measured on an Asus Zenbook A16 (UX3607OA) with
> a SuperSpeed device enumerated, CX aggregates to SVS while the
> controllers run their master clock at 200 MHz:
> 
>   $ grep -E "^cx " /sys/kernel/debug/pm_genpd/pm_genpd_summary
>   cx    on    128
> 
>   $ grep "usb30.*master_clk " /sys/kernel/debug/clk/clk_summary
>   gcc_usb30_sec_master_clk   ...  200000000
>   gcc_usb30_prim_master_clk  ...  200000000
>   gcc_usb30_mp_master_clk    ...  200000000
> 
> 128 is RPMH_REGULATOR_LEVEL_SVS; rpmhpd_opp_nom is 256. With this
> patch applied the same measurement reads 256 while a controller is
> active.
> 
> Without interconnects, dwc3_qcom_interconnect_init() silently does
> nothing: of_icc_get() returns NULL rather than an error pointer for a
> device without the property, so probe succeeds and icc_set_bw() on a
> NULL path is a no-op. No USB-DDR bandwidth is ever requested.
> 
> Without assigned-clock-rates nothing programs the master clock RCG,
> which keeps whatever the bootloader left behind. On this machine that
> happens to be the correct 200 MHz, but ftbl_gcc_usb30_mp_master_clk_src
> also allows 66.67 and 133.33 MHz, neither of which sustains SuperSpeed.
> 
> Add all three groups to the four nodes, mirroring usb_hs. The
> interconnect paths follow the provider layout in
> drivers/interconnect/qcom/glymur.c: usb_0/usb_1 sit on aggre4_noc,
> usb_mp on aggre3_noc and usb_2 on aggre2_noc.
> 
> Fixes: 4eee57dd4df9 ("arm64: dts: qcom: glymur: Add USB related nodes")
> Assisted-by: Claude:fable-5
> Signed-off-by: Greg Ociepka <greg@ferrisoft.com>

This looks OK to me.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>

> ---
> Note for reviewers: Hamoa additionally marks all five USB controllers
> dma-coherent, while Glymur marks no peripheral DMA master as coherent
> (only the SMMUs, PCIe controllers and remoteprocs). That difference is
> deliberately not touched here since getting it wrong is a silent
> data-corruption class of bug, but it looks like part of the same
> omission. Is USB DMA coherent on Glymur?

I think so, but lets see what Wesley thinks.

+ Wesley

Re: [PATCH] arm64: dts: qcom: glymur: Add missing USB clock, power and bandwidth votes

[Krishna Kurapati]

On 8/12/2026 3:53 PM, Abel Vesa wrote:
> On 26-08-12 10:35:22, Greg Ociepka wrote:
>> The four USB3 controller nodes on Glymur (usb_0, usb_1, usb_2, usb_mp)
>> are missing three property groups that Glymur's own usb_hs node at
>> a200000 already carries, and that Hamoa (x1e80100) carries on all five
>> of its USB controllers:
>>
>>    - assigned-clocks / assigned-clock-rates
>>    - required-opps
>>    - interconnects / interconnect-names
>>
>> Without required-opps the controllers place no performance state vote
>> on their power domain. Measured on an Asus Zenbook A16 (UX3607OA) with
>> a SuperSpeed device enumerated, CX aggregates to SVS while the
>> controllers run their master clock at 200 MHz:
>>
>>    $ grep -E "^cx " /sys/kernel/debug/pm_genpd/pm_genpd_summary
>>    cx    on    128
>>
>>    $ grep "usb30.*master_clk " /sys/kernel/debug/clk/clk_summary
>>    gcc_usb30_sec_master_clk   ...  200000000
>>    gcc_usb30_prim_master_clk  ...  200000000
>>    gcc_usb30_mp_master_clk    ...  200000000
>>
>> 128 is RPMH_REGULATOR_LEVEL_SVS; rpmhpd_opp_nom is 256. With this
>> patch applied the same measurement reads 256 while a controller is
>> active.
>>
>> Without interconnects, dwc3_qcom_interconnect_init() silently does
>> nothing: of_icc_get() returns NULL rather than an error pointer for a
>> device without the property, so probe succeeds and icc_set_bw() on a
>> NULL path is a no-op. No USB-DDR bandwidth is ever requested.
>>
>> Without assigned-clock-rates nothing programs the master clock RCG,
>> which keeps whatever the bootloader left behind. On this machine that
>> happens to be the correct 200 MHz, but ftbl_gcc_usb30_mp_master_clk_src
>> also allows 66.67 and 133.33 MHz, neither of which sustains SuperSpeed.
>>
>> Add all three groups to the four nodes, mirroring usb_hs. The
>> interconnect paths follow the provider layout in
>> drivers/interconnect/qcom/glymur.c: usb_0/usb_1 sit on aggre4_noc,
>> usb_mp on aggre3_noc and usb_2 on aggre2_noc.
>>
>> Fixes: 4eee57dd4df9 ("arm64: dts: qcom: glymur: Add USB related nodes")
>> Assisted-by: Claude:fable-5
>> Signed-off-by: Greg Ociepka <greg@ferrisoft.com>
> 
> This looks OK to me.
> 
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> 
>> ---
>> Note for reviewers: Hamoa additionally marks all five USB controllers
>> dma-coherent, while Glymur marks no peripheral DMA master as coherent
>> (only the SMMUs, PCIe controllers and remoteprocs). That difference is
>> deliberately not touched here since getting it wrong is a silent
>> data-corruption class of bug, but it looks like part of the same
>> omission. Is USB DMA coherent on Glymur?
> 
> I think so, but lets see what Wesley thinks.
> 
It is dma-coherent.

Regards,
Krishna,