Re: [PATCH 1/2] arm64: dts: qcom: Add support for usb nodes on QCS8300

[Krishna Kurapati <quic_kriskura@quicinc.com>]

On 10/10/2024 7:52 PM, Dmitry Baryshkov wrote:
> On Thu, Oct 10, 2024 at 01:26:35AM GMT, Krishna Kurapati wrote:
>> Add support for USB controllers on QCS8300. The second
>> controller is only High Speed capable.
>>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs8300.dtsi | 165 ++++++++++++++++++++++++++
>>   1 file changed, 165 insertions(+)
> 
> [...]
> 
>> +
>> +		usb_2: usb@a4f8800 {
>> +			compatible = "qcom,qcs8300-dwc3", "qcom,dwc3";
>> +			reg = <0x0 0x0a4f8800 0x0 0x400>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +
>> +			clocks = <&gcc GCC_CFG_NOC_USB2_PRIM_AXI_CLK>,
>> +				 <&gcc GCC_USB20_MASTER_CLK>,
>> +				 <&gcc GCC_AGGRE_USB2_PRIM_AXI_CLK>,
>> +				 <&gcc GCC_USB20_SLEEP_CLK>,
>> +				 <&gcc GCC_USB20_MOCK_UTMI_CLK>;
>> +			clock-names = "cfg_noc",
>> +				      "core",
>> +				      "iface",
>> +				      "sleep",
>> +				      "mock_utmi";
>> +
>> +			assigned-clocks = <&gcc GCC_USB20_MOCK_UTMI_CLK>,
>> +					  <&gcc GCC_USB20_MASTER_CLK>;
>> +			assigned-clock-rates = <19200000>, <120000000>;
>> +
>> +			interrupts-extended = <&intc GIC_SPI 444 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&intc GIC_SPI 443 IRQ_TYPE_LEVEL_HIGH>,
>> +					      <&pdc 10 IRQ_TYPE_EDGE_BOTH>,
>> +					      <&pdc 9 IRQ_TYPE_EDGE_BOTH>;
>> +			interrupt-names = "pwr_event",
>> +					  "hs_phy_irq",
>> +					  "dp_hs_phy_irq",
>> +					  "dm_hs_phy_irq";
>> +
>> +			power-domains = <&gcc GCC_USB20_PRIM_GDSC>;
>> +			required-opps = <&rpmhpd_opp_nom>;
>> +
>> +			resets = <&gcc GCC_USB20_PRIM_BCR>;
>> +
>> +			interconnects = <&aggre1_noc MASTER_USB2 0 &mc_virt SLAVE_EBI1 0>,
>> +					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB2 0>;
>> +			interconnect-names = "usb-ddr", "apps-usb";
> 
> As this is a USB2-only host, shouldn't it also have qcom,select-utmi-as-pipe-clk ?
> 

Hi Dmitry,

Thanks for the catch. You are right, it needs to be added according to 
bindings. Since I would be sending another patch after this series to 
enable the second controller, would it be fine if I add it in that patch 
or do you suggest updating this one.

Also I see some others are not using it as well, like sc7280 which also 
might need to be updated.

Regards,
Krishna,

>> +
>> +			status = "disabled";
>> +
>> +			usb_2_dwc3: usb@a400000 {
>> +				compatible = "snps,dwc3";
>> +				reg = <0x0 0x0a400000 0x0 0xe000>;
>> +				interrupts = <GIC_SPI 442 IRQ_TYPE_LEVEL_HIGH>;
>> +				iommus = <&apps_smmu 0x20 0x0>;
>> +				phys = <&usb_2_hsphy>;
>> +				phy-names = "usb2-phy";
>> +				snps,dis_u2_susphy_quirk;
>> +				snps,dis_enblslpm_quirk;
>> +			};
>> +		};
>>   	};
>>   
>>   	arch_timer: timer {
>> -- 
>> 2.34.1
>>
>