[PATCH v2 0/2] Add Devicetree support for USB controllers on QCS8300

[PATCH v2 0/2] Add Devicetree support for USB controllers on QCS8300

[Krishna Kurapati]

This series aims at enabling USB on QCS8300 which has 2 USB controllers.
The primary controller is SuperSpeed capable and secondary one is
High Speed only capable. Both the High Speed Phys are Femto phys and the
SuperSpeed Phy is a QMP Uni Phy.

Base DT Support has been added for both controllers while only one has
been enabled on Ride Platform. The primary controller has been configured
in device mode. The secondary controller will be enabled in host mode post
addition of SPMI Node which allows control over PMIC Gpios for providing
vbus to connected peripherals.

This series depends on the following series ACKed by upstream maintainers:
Base DT: https://lore.kernel.org/all/20240925-qcs8300_initial_dtsi-v2-0-494c40fa2a42@quicinc.com/

Bindings patches posted at:
https://lore.kernel.org/all/20241009195348.2649368-1-quic_kriskura@quicinc.com/

DTBS Check has been done on the patches.

Link to v1:
https://lore.kernel.org/all/20241009195636.2649952-1-quic_kriskura@quicinc.com/

Changes in v2:
Added quirk to use pipe clk as utmi clk for second controller.
Added wakeup source for second controller.
Modified commit text for DTS change.

Krishna Kurapati (2):
  arm64: dts: qcom: Add support for usb nodes on QCS8300
  arm64: dts: qcom: Enable USB controllers for QCS8300

 arch/arm64/boot/dts/qcom/qcs8300-ride.dts |  23 +++
 arch/arm64/boot/dts/qcom/qcs8300.dtsi     | 168 ++++++++++++++++++++++
 2 files changed, 191 insertions(+)

-- 
2.34.1

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

[Krishna Kurapati]

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 | 168 ++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
index 2c35f96c3f28..4e6ba9f49b95 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
@@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
 				qcom,remote-pid = <5>;
 			};
 		};
+
+		usb_1_hsphy: phy@8904000 {
+			compatible = "qcom,qcs8300-usb-hs-phy",
+				     "qcom,usb-snps-hs-7nm-phy";
+			reg = <0x0 0x8904000 0x0 0x400>;
+
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "ref";
+
+			resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb_2_hsphy: phy@8906000 {
+			compatible = "qcom,qcs8300-usb-hs-phy",
+				     "qcom,usb-snps-hs-7nm-phy";
+			reg = <0x0 0x08906000 0x0 0x400>;
+
+			clocks = <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "ref";
+
+			resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb_qmpphy: phy@8907000 {
+			compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
+			reg = <0x0 0x8907000 0x0 0x2000>;
+
+			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
+				 <&gcc GCC_USB_CLKREF_EN>,
+				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
+				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+			clock-names = "aux", "ref", "com_aux", "pipe";
+
+			resets = <&gcc GCC_USB3_PHY_PRIM_BCR>,
+				 <&gcc GCC_USB3PHY_PHY_PRIM_BCR>;
+			reset-names = "phy", "phy_phy";
+
+			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
+
+			#clock-cells = <0>;
+			clock-output-names = "usb3_prim_phy_pipe_clk_src";
+
+			#phy-cells = <0>;
+
+			status = "disabled";
+		};
+
+		usb_1: usb@a6f8800 {
+			compatible = "qcom,qcs8300-dwc3", "qcom,dwc3";
+			reg = <0x0 0x0a6f8800 0x0 0x400>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			clocks = <&gcc GCC_CFG_NOC_USB3_PRIM_AXI_CLK>,
+				 <&gcc GCC_USB30_PRIM_MASTER_CLK>,
+				 <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>,
+				 <&gcc GCC_USB30_PRIM_SLEEP_CLK>,
+				 <&gcc GCC_USB30_PRIM_MOCK_UTMI_CLK>;
+			clock-names = "cfg_noc",
+				      "core",
+				      "iface",
+				      "sleep",
+				      "mock_utmi";
+
+			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 287 IRQ_TYPE_LEVEL_HIGH>,
+					      <&intc GIC_SPI 261 IRQ_TYPE_LEVEL_HIGH>,
+					      <&pdc 14 IRQ_TYPE_EDGE_BOTH>,
+					      <&pdc 15 IRQ_TYPE_EDGE_BOTH>,
+					      <&pdc 12 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "pwr_event",
+					  "hs_phy_irq",
+					  "dp_hs_phy_irq",
+					  "dm_hs_phy_irq",
+					  "ss_phy_irq";
+
+			power-domains = <&gcc GCC_USB30_PRIM_GDSC>;
+			required-opps = <&rpmhpd_opp_nom>;
+
+			resets = <&gcc GCC_USB30_PRIM_BCR>;
+			interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,
+					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
+			interconnect-names = "usb-ddr", "apps-usb";
+
+			wakeup-source;
+
+			status = "disabled";
+
+			usb_1_dwc3: usb@a600000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0x0a600000 0x0 0xe000>;
+				interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
+				iommus = <&apps_smmu 0x80 0x0>;
+				phys = <&usb_1_hsphy>, <&usb_qmpphy>;
+				phy-names = "usb2-phy", "usb3-phy";
+				snps,dis_u2_susphy_quirk;
+				snps,dis_enblslpm_quirk;
+			};
+		};
+
+		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";
+
+			qcom,select-utmi-as-pipe-clk;
+			wakeup-source;
+
+			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

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

[Konrad Dybcio]

On 11.10.2024 9:46 AM, Krishna Kurapati wrote:

The commit title should include a `qcs8300: ` part, like others in
the directory (see git log --oneline arch/arm64/boot/dts/qcom).

> 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 | 168 ++++++++++++++++++++++++++
>  1 file changed, 168 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> index 2c35f96c3f28..4e6ba9f49b95 100644
> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>  				qcom,remote-pid = <5>;
>  			};
>  		};
> +
> +		usb_1_hsphy: phy@8904000 {
> +			compatible = "qcom,qcs8300-usb-hs-phy",
> +				     "qcom,usb-snps-hs-7nm-phy";
> +			reg = <0x0 0x8904000 0x0 0x400>;

Please pad the address parts to 8 hex digits with leading zeroes.

> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "ref";
> +
> +			resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_2_hsphy: phy@8906000 {
> +			compatible = "qcom,qcs8300-usb-hs-phy",
> +				     "qcom,usb-snps-hs-7nm-phy";
> +			reg = <0x0 0x08906000 0x0 0x400>;
> +
> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
> +			clock-names = "ref";
> +
> +			resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
> +
> +			#phy-cells = <0>;
> +
> +			status = "disabled";
> +		};
> +
> +		usb_qmpphy: phy@8907000 {
> +			compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
> +			reg = <0x0 0x8907000 0x0 0x2000>;
> +
> +			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> +				 <&gcc GCC_USB_CLKREF_EN>,
> +				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> +			clock-names = "aux", "ref", "com_aux", "pipe";

Please make this a vertical list like in the node below.

[...]

> +			interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,

QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi

> +					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
> +			interconnect-names = "usb-ddr", "apps-usb";
> +
> +			wakeup-source;
> +
> +			status = "disabled";
> +
> +			usb_1_dwc3: usb@a600000 {
> +				compatible = "snps,dwc3";
> +				reg = <0x0 0x0a600000 0x0 0xe000>;
> +				interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
> +				iommus = <&apps_smmu 0x80 0x0>;
> +				phys = <&usb_1_hsphy>, <&usb_qmpphy>;
> +				phy-names = "usb2-phy", "usb3-phy";
> +				snps,dis_u2_susphy_quirk;
> +				snps,dis_enblslpm_quirk;

That's a very low number of quirks.. Should we have some more?

Should it also be marked dma-coherent?

Identical comments for the second instance.

Konrad

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

[Krishna Kurapati]

On 10/25/2024 11:58 PM, Konrad Dybcio wrote:
> On 11.10.2024 9:46 AM, Krishna Kurapati wrote:
> 
> The commit title should include a `qcs8300: ` part, like others in
> the directory (see git log --oneline arch/arm64/boot/dts/qcom).
> 
>> 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 | 168 ++++++++++++++++++++++++++
>>   1 file changed, 168 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>> index 2c35f96c3f28..4e6ba9f49b95 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>>   				qcom,remote-pid = <5>;
>>   			};
>>   		};
>> +
>> +		usb_1_hsphy: phy@8904000 {
>> +			compatible = "qcom,qcs8300-usb-hs-phy",
>> +				     "qcom,usb-snps-hs-7nm-phy";
>> +			reg = <0x0 0x8904000 0x0 0x400>;
> 
> Please pad the address parts to 8 hex digits with leading zeroes.
> 
>> +
>> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
>> +			clock-names = "ref";
>> +
>> +			resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
>> +
>> +			#phy-cells = <0>;
>> +
>> +			status = "disabled";
>> +		};
>> +
>> +		usb_2_hsphy: phy@8906000 {
>> +			compatible = "qcom,qcs8300-usb-hs-phy",
>> +				     "qcom,usb-snps-hs-7nm-phy";
>> +			reg = <0x0 0x08906000 0x0 0x400>;
>> +
>> +			clocks = <&rpmhcc RPMH_CXO_CLK>;
>> +			clock-names = "ref";
>> +
>> +			resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
>> +
>> +			#phy-cells = <0>;
>> +
>> +			status = "disabled";
>> +		};
>> +
>> +		usb_qmpphy: phy@8907000 {
>> +			compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
>> +			reg = <0x0 0x8907000 0x0 0x2000>;
>> +
>> +			clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>> +				 <&gcc GCC_USB_CLKREF_EN>,
>> +				 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>> +				 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>> +			clock-names = "aux", "ref", "com_aux", "pipe";
> 
> Please make this a vertical list like in the node below.
> 
> [...]
> 
>> +			interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,
> 
> QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi
> 
>> +					<&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
>> +			interconnect-names = "usb-ddr", "apps-usb";
>> +
>> +			wakeup-source;
>> +
>> +			status = "disabled";
>> +
>> +			usb_1_dwc3: usb@a600000 {
>> +				compatible = "snps,dwc3";
>> +				reg = <0x0 0x0a600000 0x0 0xe000>;
>> +				interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
>> +				iommus = <&apps_smmu 0x80 0x0>;
>> +				phys = <&usb_1_hsphy>, <&usb_qmpphy>;
>> +				phy-names = "usb2-phy", "usb3-phy";
>> +				snps,dis_u2_susphy_quirk;
>> +				snps,dis_enblslpm_quirk;
> 
> That's a very low number of quirks.. Should we have some more?
> 

snps,dis-u1-entry-quirk;
snps,dis-u2-entry-quirk;
snps,dis_u2_susphy_quirk;
snps,ssp-u3-u0-quirk;

I would actually like to add these as well, but there is no precedent in 
upstream as to what quirks to add for usb nodes, so I kept only a couple 
of them. Ideally downstream we disable u1u2 for almost all targets 
because of some issues in the past. (atleast during tethering use cases, 
but I need to double check though).

> Should it also be marked dma-coherent?
> 

ACK.

Regards,
Krishna,

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

[Konrad Dybcio]

On 26.10.2024 6:56 PM, Krishna Kurapati wrote:
> 
> 
> On 10/25/2024 11:58 PM, Konrad Dybcio wrote:
>> On 11.10.2024 9:46 AM, Krishna Kurapati wrote:
>>
>> The commit title should include a `qcs8300: ` part, like others in
>> the directory (see git log --oneline arch/arm64/boot/dts/qcom).
>>
>>> 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 | 168 ++++++++++++++++++++++++++
>>>   1 file changed, 168 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> index 2c35f96c3f28..4e6ba9f49b95 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>>>                   qcom,remote-pid = <5>;
>>>               };
>>>           };
>>> +
>>> +        usb_1_hsphy: phy@8904000 {
>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>> +            reg = <0x0 0x8904000 0x0 0x400>;
>>
>> Please pad the address parts to 8 hex digits with leading zeroes.
>>
>>> +
>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>> +            clock-names = "ref";
>>> +
>>> +            resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
>>> +
>>> +            #phy-cells = <0>;
>>> +
>>> +            status = "disabled";
>>> +        };
>>> +
>>> +        usb_2_hsphy: phy@8906000 {
>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>> +            reg = <0x0 0x08906000 0x0 0x400>;
>>> +
>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>> +            clock-names = "ref";
>>> +
>>> +            resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
>>> +
>>> +            #phy-cells = <0>;
>>> +
>>> +            status = "disabled";
>>> +        };
>>> +
>>> +        usb_qmpphy: phy@8907000 {
>>> +            compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
>>> +            reg = <0x0 0x8907000 0x0 0x2000>;
>>> +
>>> +            clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>>> +                 <&gcc GCC_USB_CLKREF_EN>,
>>> +                 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>> +                 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>>> +            clock-names = "aux", "ref", "com_aux", "pipe";
>>
>> Please make this a vertical list like in the node below.
>>
>> [...]
>>
>>> +            interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,
>>
>> QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi
>>
>>> +                    <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
>>> +            interconnect-names = "usb-ddr", "apps-usb";
>>> +
>>> +            wakeup-source;
>>> +
>>> +            status = "disabled";
>>> +
>>> +            usb_1_dwc3: usb@a600000 {
>>> +                compatible = "snps,dwc3";
>>> +                reg = <0x0 0x0a600000 0x0 0xe000>;
>>> +                interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
>>> +                iommus = <&apps_smmu 0x80 0x0>;
>>> +                phys = <&usb_1_hsphy>, <&usb_qmpphy>;
>>> +                phy-names = "usb2-phy", "usb3-phy";
>>> +                snps,dis_u2_susphy_quirk;
>>> +                snps,dis_enblslpm_quirk;
>>
>> That's a very low number of quirks.. Should we have some more?
>>
> 
> snps,dis-u1-entry-quirk;
> snps,dis-u2-entry-quirk;
> snps,dis_u2_susphy_quirk;
> snps,ssp-u3-u0-quirk;
> 
> I would actually like to add these as well, but there is no precedent in upstream as to what quirks to add for usb nodes

Every single one that applies to the hardware ;)

> , so I kept only a couple of them. Ideally downstream we disable u1u2 for almost all targets because of some issues in the past. (atleast during tethering use cases, but I need to double check though).

Does

5b8baed4b881 ("arm64: dts: qcom: sc7180: Disable SuperSpeed instances in park mode")

apply here too?

Konrad

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

[Krishna Kurapati]

On 10/28/2024 5:08 PM, Konrad Dybcio wrote:
> On 26.10.2024 6:56 PM, Krishna Kurapati wrote:
>>
>>
>> On 10/25/2024 11:58 PM, Konrad Dybcio wrote:
>>> On 11.10.2024 9:46 AM, Krishna Kurapati wrote:
>>>
>>> The commit title should include a `qcs8300: ` part, like others in
>>> the directory (see git log --oneline arch/arm64/boot/dts/qcom).
>>>
>>>> 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 | 168 ++++++++++++++++++++++++++
>>>>    1 file changed, 168 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>>> index 2c35f96c3f28..4e6ba9f49b95 100644
>>>> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>>> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>>>>                    qcom,remote-pid = <5>;
>>>>                };
>>>>            };
>>>> +
>>>> +        usb_1_hsphy: phy@8904000 {
>>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>>> +            reg = <0x0 0x8904000 0x0 0x400>;
>>>
>>> Please pad the address parts to 8 hex digits with leading zeroes.
>>>
>>>> +
>>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>>> +            clock-names = "ref";
>>>> +
>>>> +            resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
>>>> +
>>>> +            #phy-cells = <0>;
>>>> +
>>>> +            status = "disabled";
>>>> +        };
>>>> +
>>>> +        usb_2_hsphy: phy@8906000 {
>>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>>> +            reg = <0x0 0x08906000 0x0 0x400>;
>>>> +
>>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>>> +            clock-names = "ref";
>>>> +
>>>> +            resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
>>>> +
>>>> +            #phy-cells = <0>;
>>>> +
>>>> +            status = "disabled";
>>>> +        };
>>>> +
>>>> +        usb_qmpphy: phy@8907000 {
>>>> +            compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
>>>> +            reg = <0x0 0x8907000 0x0 0x2000>;
>>>> +
>>>> +            clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>>>> +                 <&gcc GCC_USB_CLKREF_EN>,
>>>> +                 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>>> +                 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>>>> +            clock-names = "aux", "ref", "com_aux", "pipe";
>>>
>>> Please make this a vertical list like in the node below.
>>>
>>> [...]
>>>
>>>> +            interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,
>>>
>>> QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi
>>>
>>>> +                    <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
>>>> +            interconnect-names = "usb-ddr", "apps-usb";
>>>> +
>>>> +            wakeup-source;
>>>> +
>>>> +            status = "disabled";
>>>> +
>>>> +            usb_1_dwc3: usb@a600000 {
>>>> +                compatible = "snps,dwc3";
>>>> +                reg = <0x0 0x0a600000 0x0 0xe000>;
>>>> +                interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
>>>> +                iommus = <&apps_smmu 0x80 0x0>;
>>>> +                phys = <&usb_1_hsphy>, <&usb_qmpphy>;
>>>> +                phy-names = "usb2-phy", "usb3-phy";
>>>> +                snps,dis_u2_susphy_quirk;
>>>> +                snps,dis_enblslpm_quirk;
>>>
>>> That's a very low number of quirks.. Should we have some more?
>>>
>>
>> snps,dis-u1-entry-quirk;
>> snps,dis-u2-entry-quirk;
>> snps,dis_u2_susphy_quirk;
>> snps,ssp-u3-u0-quirk;
>>
>> I would actually like to add these as well, but there is no precedent in upstream as to what quirks to add for usb nodes
> 
> Every single one that applies to the hardware ;)
> 
>> , so I kept only a couple of them. Ideally downstream we disable u1u2 for almost all targets because of some issues in the past. (atleast during tethering use cases, but I need to double check though).
> 
> Does
> 
> 5b8baed4b881 ("arm64: dts: qcom: sc7180: Disable SuperSpeed instances in park mode")
> 
> apply here too?
> 

QCS8300 is Gen-2, so that quirk is not needed.

Regards,
Krishna,

[PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Krishna Kurapati]

Enable primary USB controller on QCS8300 Ride platform. The primary USB
controller is made "peripheral", as this is intended to be connected to
a host for debugging use cases.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
 arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
index 7eed19a694c3..3e925228379c 100644
--- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
+++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
@@ -265,3 +265,26 @@ &ufs_mem_phy {
 	vdda-pll-supply = <&vreg_l5a>;
 	status = "okay";
 };
+
+&usb_1_hsphy {
+	vdda-pll-supply = <&vreg_l7a>;
+	vdda18-supply = <&vreg_l7c>;
+	vdda33-supply = <&vreg_l9a>;
+
+	status = "okay";
+};
+
+&usb_qmpphy {
+	vdda-phy-supply = <&vreg_l7a>;
+	vdda-pll-supply = <&vreg_l5a>;
+
+	status = "okay";
+};
+
+&usb_1 {
+	status = "okay";
+};
+
+&usb_1_dwc3 {
+	dr_mode = "peripheral";
+};
-- 
2.34.1

Re: [PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Dmitry Baryshkov]

On Fri, Oct 11, 2024 at 01:16:19PM +0530, Krishna Kurapati wrote:
> Enable primary USB controller on QCS8300 Ride platform. The primary USB
> controller is made "peripheral", as this is intended to be connected to
> a host for debugging use cases.
> 
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> index 7eed19a694c3..3e925228379c 100644
> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> @@ -265,3 +265,26 @@ &ufs_mem_phy {
>  	vdda-pll-supply = <&vreg_l5a>;
>  	status = "okay";
>  };
> +
> +&usb_1_hsphy {
> +	vdda-pll-supply = <&vreg_l7a>;
> +	vdda18-supply = <&vreg_l7c>;
> +	vdda33-supply = <&vreg_l9a>;
> +
> +	status = "okay";
> +};
> +
> +&usb_qmpphy {
> +	vdda-phy-supply = <&vreg_l7a>;
> +	vdda-pll-supply = <&vreg_l5a>;
> +
> +	status = "okay";
> +};
> +
> +&usb_1 {
> +	status = "okay";
> +};
> +
> +&usb_1_dwc3 {
> +	dr_mode = "peripheral";
> +};

So, can it be used as a USB host controller / connector? What needs to
be done in such a case?

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Krishna Kurapati]

On 10/26/2024 11:06 PM, Dmitry Baryshkov wrote:
> On Fri, Oct 11, 2024 at 01:16:19PM +0530, Krishna Kurapati wrote:
>> Enable primary USB controller on QCS8300 Ride platform. The primary USB
>> controller is made "peripheral", as this is intended to be connected to
>> a host for debugging use cases.
>>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>> index 7eed19a694c3..3e925228379c 100644
>> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>> @@ -265,3 +265,26 @@ &ufs_mem_phy {
>>   	vdda-pll-supply = <&vreg_l5a>;
>>   	status = "okay";
>>   };
>> +
>> +&usb_1_hsphy {
>> +	vdda-pll-supply = <&vreg_l7a>;
>> +	vdda18-supply = <&vreg_l7c>;
>> +	vdda33-supply = <&vreg_l9a>;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_qmpphy {
>> +	vdda-phy-supply = <&vreg_l7a>;
>> +	vdda-pll-supply = <&vreg_l5a>;
>> +
>> +	status = "okay";
>> +};
>> +
>> +&usb_1 {
>> +	status = "okay";
>> +};
>> +
>> +&usb_1_dwc3 {
>> +	dr_mode = "peripheral";
>> +};
> 
> So, can it be used as a USB host controller / connector? What needs to
> be done in such a case?
> 
Adding vbus boost pinctrl and changing dr_mode to host must be enough 
for this case.

Regards,
Krishna,

Re: [PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Dmitry Baryshkov]

On Sun, Oct 27, 2024 at 11:59:44AM +0530, Krishna Kurapati wrote:
> 
> 
> On 10/26/2024 11:06 PM, Dmitry Baryshkov wrote:
> > On Fri, Oct 11, 2024 at 01:16:19PM +0530, Krishna Kurapati wrote:
> > > Enable primary USB controller on QCS8300 Ride platform. The primary USB
> > > controller is made "peripheral", as this is intended to be connected to
> > > a host for debugging use cases.
> > > 
> > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > ---
> > >   arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
> > >   1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > index 7eed19a694c3..3e925228379c 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > @@ -265,3 +265,26 @@ &ufs_mem_phy {
> > >   	vdda-pll-supply = <&vreg_l5a>;
> > >   	status = "okay";
> > >   };
> > > +
> > > +&usb_1_hsphy {
> > > +	vdda-pll-supply = <&vreg_l7a>;
> > > +	vdda18-supply = <&vreg_l7c>;
> > > +	vdda33-supply = <&vreg_l9a>;
> > > +
> > > +	status = "okay";
> > > +};
> > > +
> > > +&usb_qmpphy {
> > > +	vdda-phy-supply = <&vreg_l7a>;
> > > +	vdda-pll-supply = <&vreg_l5a>;
> > > +
> > > +	status = "okay";
> > > +};
> > > +
> > > +&usb_1 {
> > > +	status = "okay";
> > > +};
> > > +
> > > +&usb_1_dwc3 {
> > > +	dr_mode = "peripheral";
> > > +};
> > 
> > So, can it be used as a USB host controller / connector? What needs to
> > be done in such a case?
> > 
> Adding vbus boost pinctrl and changing dr_mode to host must be enough for
> this case.

Could you please mention those either in the commie message or in the
comment before the board DT file?

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Krishna Kurapati]

On 10/27/2024 11:14 PM, Dmitry Baryshkov wrote:
> On Sun, Oct 27, 2024 at 11:59:44AM +0530, Krishna Kurapati wrote:
>>
>>
>> On 10/26/2024 11:06 PM, Dmitry Baryshkov wrote:
>>> On Fri, Oct 11, 2024 at 01:16:19PM +0530, Krishna Kurapati wrote:
>>>> Enable primary USB controller on QCS8300 Ride platform. The primary USB
>>>> controller is made "peripheral", as this is intended to be connected to
>>>> a host for debugging use cases.
>>>>
>>>> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
>>>>    1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>>>> index 7eed19a694c3..3e925228379c 100644
>>>> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
>>>> @@ -265,3 +265,26 @@ &ufs_mem_phy {
>>>>    	vdda-pll-supply = <&vreg_l5a>;
>>>>    	status = "okay";
>>>>    };
>>>> +
>>>> +&usb_1_hsphy {
>>>> +	vdda-pll-supply = <&vreg_l7a>;
>>>> +	vdda18-supply = <&vreg_l7c>;
>>>> +	vdda33-supply = <&vreg_l9a>;
>>>> +
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&usb_qmpphy {
>>>> +	vdda-phy-supply = <&vreg_l7a>;
>>>> +	vdda-pll-supply = <&vreg_l5a>;
>>>> +
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&usb_1 {
>>>> +	status = "okay";
>>>> +};
>>>> +
>>>> +&usb_1_dwc3 {
>>>> +	dr_mode = "peripheral";
>>>> +};
>>>
>>> So, can it be used as a USB host controller / connector? What needs to
>>> be done in such a case?
>>>
>> Adding vbus boost pinctrl and changing dr_mode to host must be enough for
>> this case.
> 
> Could you please mention those either in the commie message or in the
> comment before the board DT file?
> 

Sure, I can update commit text to add something like the following:

"In case first controller needs to be configured in host mode, X-GPIO to 
be enabled and dr_mode to be changed accordingly."

But when we add second controller (which I will after SPMI node is 
done), this commit text would be redundant as the same file would show 
example for host mode as well.

Regards,
Krishna,

Re: [PATCH v2 2/2] arm64: dts: qcom: Enable USB controllers for QCS8300

[Dmitry Baryshkov]

On Mon, Oct 28, 2024 at 12:32:02AM +0530, Krishna Kurapati wrote:
> 
> 
> On 10/27/2024 11:14 PM, Dmitry Baryshkov wrote:
> > On Sun, Oct 27, 2024 at 11:59:44AM +0530, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 10/26/2024 11:06 PM, Dmitry Baryshkov wrote:
> > > > On Fri, Oct 11, 2024 at 01:16:19PM +0530, Krishna Kurapati wrote:
> > > > > Enable primary USB controller on QCS8300 Ride platform. The primary USB
> > > > > controller is made "peripheral", as this is intended to be connected to
> > > > > a host for debugging use cases.
> > > > > 
> > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > > > ---
> > > > >    arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 23 +++++++++++++++++++++++
> > > > >    1 file changed, 23 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > > > index 7eed19a694c3..3e925228379c 100644
> > > > > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > > > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts
> > > > > @@ -265,3 +265,26 @@ &ufs_mem_phy {
> > > > >    	vdda-pll-supply = <&vreg_l5a>;
> > > > >    	status = "okay";
> > > > >    };
> > > > > +
> > > > > +&usb_1_hsphy {
> > > > > +	vdda-pll-supply = <&vreg_l7a>;
> > > > > +	vdda18-supply = <&vreg_l7c>;
> > > > > +	vdda33-supply = <&vreg_l9a>;
> > > > > +
> > > > > +	status = "okay";
> > > > > +};
> > > > > +
> > > > > +&usb_qmpphy {
> > > > > +	vdda-phy-supply = <&vreg_l7a>;
> > > > > +	vdda-pll-supply = <&vreg_l5a>;
> > > > > +
> > > > > +	status = "okay";
> > > > > +};
> > > > > +
> > > > > +&usb_1 {
> > > > > +	status = "okay";
> > > > > +};
> > > > > +
> > > > > +&usb_1_dwc3 {
> > > > > +	dr_mode = "peripheral";
> > > > > +};
> > > > 
> > > > So, can it be used as a USB host controller / connector? What needs to
> > > > be done in such a case?
> > > > 
> > > Adding vbus boost pinctrl and changing dr_mode to host must be enough for
> > > this case.
> > 
> > Could you please mention those either in the commie message or in the
> > comment before the board DT file?
> > 
> 
> Sure, I can update commit text to add something like the following:
> 
> "In case first controller needs to be configured in host mode, X-GPIO to be
> enabled and dr_mode to be changed accordingly."
> 
> But when we add second controller (which I will after SPMI node is done),
> this commit text would be redundant as the same file would show example for
> host mode as well.

My point was that it might be not obvious that the controller isn't
designated to be peripheral on a board level and can be easily switched
to work as a USB host.

-- 
With best wishes
Dmitry

Re: [PATCH v2 0/2] Add Devicetree support for USB controllers on QCS8300

[Krishna Kurapati]

On 10/11/2024 1:16 PM, Krishna Kurapati wrote:
> This series aims at enabling USB on QCS8300 which has 2 USB controllers.
> The primary controller is SuperSpeed capable and secondary one is
> High Speed only capable. Both the High Speed Phys are Femto phys and the
> SuperSpeed Phy is a QMP Uni Phy.
> 
> Base DT Support has been added for both controllers while only one has
> been enabled on Ride Platform. The primary controller has been configured
> in device mode. The secondary controller will be enabled in host mode post
> addition of SPMI Node which allows control over PMIC Gpios for providing
> vbus to connected peripherals.
> 
> This series depends on the following series ACKed by upstream maintainers:
> Base DT: https://lore.kernel.org/all/20240925-qcs8300_initial_dtsi-v2-0-494c40fa2a42@quicinc.com/
> 
> Bindings patches posted at:
> https://lore.kernel.org/all/20241009195348.2649368-1-quic_kriskura@quicinc.com/
> 
> DTBS Check has been done on the patches.
> 

Gentle ping for review.

Regards,
Krishna,

> Link to v1:
> https://lore.kernel.org/all/20241009195636.2649952-1-quic_kriskura@quicinc.com/
> 
> Changes in v2:
> Added quirk to use pipe clk as utmi clk for second controller.
> Added wakeup source for second controller.
> Modified commit text for DTS change.
> 
> Krishna Kurapati (2):
>    arm64: dts: qcom: Add support for usb nodes on QCS8300
>    arm64: dts: qcom: Enable USB controllers for QCS8300
> 
>   arch/arm64/boot/dts/qcom/qcs8300-ride.dts |  23 +++
>   arch/arm64/boot/dts/qcom/qcs8300.dtsi     | 168 ++++++++++++++++++++++
>   2 files changed, 191 insertions(+)
>