Enable Host mode support for USB1 controller on EVK Platform.

For secondary USB Typec port, there is a genesys USB HUB sitting in between
SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
through the hub to the Port controller. Mark the second USB controller as
host only capable.

Added HD3SS3220 Type-C port controller along with Type-c connector for
controlling vbus supply.

Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
---
This patch depends on:
https://lore.kernel.org/all/20251128102507.3206169-1-krishna.kurapati@oss.qualcomm.com/
---
 arch/arm64/boot/dts/qcom/lemans-evk.dts | 102 ++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/lemans-evk.dts b/arch/arm64/boot/dts/qcom/lemans-evk.dts
index 2536ebe36edbfd146e07d5698779767acff87ab4..07389593b3cd4728357255d47156c6861cbabb5e 100644
--- a/arch/arm64/boot/dts/qcom/lemans-evk.dts
+++ b/arch/arm64/boot/dts/qcom/lemans-evk.dts
@@ -67,6 +67,34 @@ usb0_con_ss_ep: endpoint {
 		};
 	};
 
+	connector-1 {
+		compatible = "usb-c-connector";
+		label = "USB1-Type-C";
+		data-role = "dual";
+		power-role = "dual";
+
+		vbus-supply = <&vbus_supply_regulator_1>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				usb1_con_hs_ep: endpoint {
+				};
+			};
+			port@1 {
+				reg = <1>;
+
+				usb1_con_ss_ep: endpoint {
+					remote-endpoint = <&hd3ss3220_47_in_ep>;
+				};
+			};
+		};
+	};
+
 	edp0-connector {
 		compatible = "dp-connector";
 		label = "EDP0";
@@ -140,6 +168,16 @@ vbus_supply_regulator_0: regulator-vbus-supply-0 {
 		enable-active-high;
 	};
 
+	vbus_supply_regulator_1: vbus-supply-regulator-1 {
+		compatible = "regulator-fixed";
+		regulator-name = "vbus_supply_1";
+		gpio = <&expander1 3 GPIO_ACTIVE_HIGH>;
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-boot-on;
+		enable-active-high;
+	};
+
 	vmmc_sdc: regulator-vmmc-sdc {
 		compatible = "regulator-fixed";
 
@@ -527,6 +565,36 @@ hd3ss3220_out_ep: endpoint {
 			};
 		};
 	};
+
+	usb-typec@47 {
+		compatible = "ti,hd3ss3220";
+		reg = <0x47>;
+		interrupts-extended = <&pmm8654au_2_gpios 6 IRQ_TYPE_EDGE_FALLING>;
+		id-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
+		pinctrl-0 = <&usb1_id>, <&usb1_intr_state>;
+		pinctrl-names = "default";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				hd3ss3220_47_in_ep: endpoint {
+					remote-endpoint = <&usb1_con_ss_ep>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				hd3ss3220_47_out_ep: endpoint {
+				};
+			};
+		};
+	};
+
 };
 
 &i2c18 {
@@ -690,6 +758,14 @@ usb0_intr_state: usb0-intr-state {
 		bias-pull-up;
 		power-source = <0>;
 	};
+
+	usb1_intr_state: usb1-intr-state {
+		pins = "gpio6";
+		function = "normal";
+		input-enable;
+		bias-pull-up;
+		power-source = <0>;
+	};
 };
 
 &qup_i2c19_default {
@@ -849,6 +925,12 @@ usb_id: usb-id-state {
 		function = "gpio";
 		bias-pull-up;
 	};
+
+	usb1_id: usb1-id-state {
+		pins = "gpio51";
+		function = "gpio";
+		bias-pull-up;
+	};
 };
 
 &uart10 {
@@ -903,6 +985,26 @@ &usb_0_qmpphy {
 	status = "okay";
 };
 
+&usb_1 {
+	dr_mode = "host";
+	status = "okay";
+};
+
+&usb_1_hsphy {
+	vdda-pll-supply = <&vreg_l7a>;
+	vdda18-supply = <&vreg_l6c>;
+	vdda33-supply = <&vreg_l9a>;
+
+	status = "okay";
+};
+
+&usb_1_qmpphy {
+	vdda-phy-supply = <&vreg_l1c>;
+	vdda-pll-supply = <&vreg_l7a>;
+
+	status = "okay";
+};
+
 &xo_board_clk {
 	clock-frequency = <38400000>;
 };

---
base-commit: 9371fd3173173c0e128e869ab7101bd971a36b1e
change-id: 20251128-swati-65461d354617

Best regards,
-- 
Swati Agarwal <swati.agarwal@oss.qualcomm.com>

On Wed, Dec 03, 2025 at 08:12:10AM +0530, Swati Agarwal wrote:
> Enable Host mode support for USB1 controller on EVK Platform.

You don't enable a mode. You enable the USB controller.

> 
> For secondary USB Typec port, there is a genesys USB HUB sitting in between

Please describe the onboard hub in DT. See sdm850-lenovo-yoga-c630.dts
for an example on how to do it.

> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
> through the hub to the Port controller. Mark the second USB controller as
> host only capable.
> 
> Added HD3SS3220 Type-C port controller along with Type-c connector for
> controlling vbus supply.
> 
> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
> ---
> This patch depends on:
> https://lore.kernel.org/all/20251128102507.3206169-1-krishna.kurapati@oss.qualcomm.com/
> ---
>  arch/arm64/boot/dts/qcom/lemans-evk.dts | 102 ++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/lemans-evk.dts b/arch/arm64/boot/dts/qcom/lemans-evk.dts
> index 2536ebe36edbfd146e07d5698779767acff87ab4..07389593b3cd4728357255d47156c6861cbabb5e 100644
> --- a/arch/arm64/boot/dts/qcom/lemans-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/lemans-evk.dts
> @@ -67,6 +67,34 @@ usb0_con_ss_ep: endpoint {
>  		};
>  	};
>  
> +	connector-1 {
> +		compatible = "usb-c-connector";
> +		label = "USB1-Type-C";
> +		data-role = "dual";
> +		power-role = "dual";
> +
> +		vbus-supply = <&vbus_supply_regulator_1>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				usb1_con_hs_ep: endpoint {
> +				};
> +			};
> +			port@1 {
> +				reg = <1>;
> +
> +				usb1_con_ss_ep: endpoint {
> +					remote-endpoint = <&hd3ss3220_47_in_ep>;
> +				};
> +			};
> +		};
> +	};
> +
>  	edp0-connector {
>  		compatible = "dp-connector";
>  		label = "EDP0";
> @@ -140,6 +168,16 @@ vbus_supply_regulator_0: regulator-vbus-supply-0 {
>  		enable-active-high;
>  	};
>  
> +	vbus_supply_regulator_1: vbus-supply-regulator-1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vbus_supply_1";
> +		gpio = <&expander1 3 GPIO_ACTIVE_HIGH>;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-boot-on;
> +		enable-active-high;
> +	};
> +
>  	vmmc_sdc: regulator-vmmc-sdc {
>  		compatible = "regulator-fixed";
>  
> @@ -527,6 +565,36 @@ hd3ss3220_out_ep: endpoint {
>  			};
>  		};
>  	};
> +
> +	usb-typec@47 {
> +		compatible = "ti,hd3ss3220";
> +		reg = <0x47>;
> +		interrupts-extended = <&pmm8654au_2_gpios 6 IRQ_TYPE_EDGE_FALLING>;
> +		id-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&usb1_id>, <&usb1_intr_state>;
> +		pinctrl-names = "default";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				hd3ss3220_47_in_ep: endpoint {
> +					remote-endpoint = <&usb1_con_ss_ep>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +
> +				hd3ss3220_47_out_ep: endpoint {
> +				};
> +			};
> +		};
> +	};
> +
>  };
>  
>  &i2c18 {
> @@ -690,6 +758,14 @@ usb0_intr_state: usb0-intr-state {
>  		bias-pull-up;
>  		power-source = <0>;
>  	};
> +
> +	usb1_intr_state: usb1-intr-state {
> +		pins = "gpio6";
> +		function = "normal";
> +		input-enable;
> +		bias-pull-up;
> +		power-source = <0>;
> +	};
>  };
>  
>  &qup_i2c19_default {
> @@ -849,6 +925,12 @@ usb_id: usb-id-state {
>  		function = "gpio";
>  		bias-pull-up;
>  	};
> +
> +	usb1_id: usb1-id-state {

Please be consistent. One of your labels have _state suffix, while other
doesn't.

> +		pins = "gpio51";
> +		function = "gpio";
> +		bias-pull-up;
> +	};
>  };
>  
>  &uart10 {
> @@ -903,6 +985,26 @@ &usb_0_qmpphy {
>  	status = "okay";
>  };
>  
> +&usb_1 {
> +	dr_mode = "host";
> +	status = "okay";
> +};
> +
> +&usb_1_hsphy {
> +	vdda-pll-supply = <&vreg_l7a>;
> +	vdda18-supply = <&vreg_l6c>;
> +	vdda33-supply = <&vreg_l9a>;
> +
> +	status = "okay";
> +};
> +
> +&usb_1_qmpphy {
> +	vdda-phy-supply = <&vreg_l1c>;
> +	vdda-pll-supply = <&vreg_l7a>;
> +
> +	status = "okay";
> +};
> +
>  &xo_board_clk {
>  	clock-frequency = <38400000>;
>  };
> 
> ---
> base-commit: 9371fd3173173c0e128e869ab7101bd971a36b1e
> change-id: 20251128-swati-65461d354617
> 
> Best regards,
> -- 
> Swati Agarwal <swati.agarwal@oss.qualcomm.com>
> 

-- 
With best wishes
Dmitry

On 12/3/25 3:42 AM, Swati Agarwal wrote:
> Enable Host mode support for USB1 controller on EVK Platform.
> 
> For secondary USB Typec port, there is a genesys USB HUB sitting in between
> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
> through the hub to the Port controller. Mark the second USB controller as
> host only capable.
> 
> Added HD3SS3220 Type-C port controller along with Type-c connector for
> controlling vbus supply.
> 
> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
> ---
> This patch depends on:
> https://lore.kernel.org/all/20251128102507.3206169-1-krishna.kurapati@oss.qualcomm.com/

Please work on enabling both ports at the same time on the next
occasion.

> ---
>  arch/arm64/boot/dts/qcom/lemans-evk.dts | 102 ++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/lemans-evk.dts b/arch/arm64/boot/dts/qcom/lemans-evk.dts
> index 2536ebe36edbfd146e07d5698779767acff87ab4..07389593b3cd4728357255d47156c6861cbabb5e 100644
> --- a/arch/arm64/boot/dts/qcom/lemans-evk.dts
> +++ b/arch/arm64/boot/dts/qcom/lemans-evk.dts
> @@ -67,6 +67,34 @@ usb0_con_ss_ep: endpoint {
>  		};
>  	};
>  
> +	connector-1 {
> +		compatible = "usb-c-connector";
> +		label = "USB1-Type-C";
> +		data-role = "dual";
> +		power-role = "dual";
> +
> +		vbus-supply = <&vbus_supply_regulator_1>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				usb1_con_hs_ep: endpoint {
> +				};

You should really connect these endpoints somewhere.

> +			};
> +			port@1 {

Please add a \n above this line to separate the subnodes

[...]

> +	usb-typec@47 {
> +		compatible = "ti,hd3ss3220";
> +		reg = <0x47>;
> +		interrupts-extended = <&pmm8654au_2_gpios 6 IRQ_TYPE_EDGE_FALLING>;
> +		id-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +		pinctrl-0 = <&usb1_id>, <&usb1_intr_state>;
> +		pinctrl-names = "default";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				hd3ss3220_47_in_ep: endpoint {

How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
_47 doesn't really tell us anything and may re-appear if there's another
instance of this IC on another I2C bus

[...]

> +&usb_1 {
> +	dr_mode = "host";

The connector states it's a dual-role data and power device. Is there any
reason to keep this in host-only mode?

> +	status = "okay";

Please add a \n above the status line to keep consistency with all other nodes

Konrad

On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
> On 12/3/25 3:42 AM, Swati Agarwal wrote:
>> Enable Host mode support for USB1 controller on EVK Platform.
>>
>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
>> through the hub to the Port controller. Mark the second USB controller as
>> host only capable.
>>
>> Added HD3SS3220 Type-C port controller along with Type-c connector for
>> controlling vbus supply.
>>
>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
>> ---
>> This patch depends on:
>> https://lore.kernel.org/all/20251128102507.3206169-1-krishna.kurapati@oss.qualcomm.com/
> 
> Please work on enabling both ports at the same time on the next
> occasion.
> 
>> ---
>>   arch/arm64/boot/dts/qcom/lemans-evk.dts | 102 ++++++++++++++++++++++++++++++++
>>   1 file changed, 102 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/lemans-evk.dts b/arch/arm64/boot/dts/qcom/lemans-evk.dts
>> index 2536ebe36edbfd146e07d5698779767acff87ab4..07389593b3cd4728357255d47156c6861cbabb5e 100644
>> --- a/arch/arm64/boot/dts/qcom/lemans-evk.dts
>> +++ b/arch/arm64/boot/dts/qcom/lemans-evk.dts
>> @@ -67,6 +67,34 @@ usb0_con_ss_ep: endpoint {
>>   		};
>>   	};
>>   
>> +	connector-1 {
>> +		compatible = "usb-c-connector";
>> +		label = "USB1-Type-C";
>> +		data-role = "dual";
>> +		power-role = "dual";
>> +
>> +		vbus-supply = <&vbus_supply_regulator_1>;
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port@0 {
>> +				reg = <0>;
>> +
>> +				usb1_con_hs_ep: endpoint {
>> +				};
> 
> You should really connect these endpoints somewhere.
> 
>> +			};
>> +			port@1 {
> 
> Please add a \n above this line to separate the subnodes
> 
> [...]
> 
>> +	usb-typec@47 {
>> +		compatible = "ti,hd3ss3220";
>> +		reg = <0x47>;
>> +		interrupts-extended = <&pmm8654au_2_gpios 6 IRQ_TYPE_EDGE_FALLING>;
>> +		id-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>> +		pinctrl-0 = <&usb1_id>, <&usb1_intr_state>;
>> +		pinctrl-names = "default";
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port@0 {
>> +				reg = <0>;
>> +
>> +				hd3ss3220_47_in_ep: endpoint {
> 
> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
> _47 doesn't really tell us anything and may re-appear if there's another
> instance of this IC on another I2C bus
> 

ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
Krzysztof suggested to use generic names and hence we used "usb-typec" 
instead of hd3ss3220.

> [...]
> 
>> +&usb_1 {
>> +	dr_mode = "host";
> 
> The connector states it's a dual-role data and power device. Is there any
> reason to keep this in host-only mode?
> 

As mentioned in commit text, there is a onboard hub sitting between SoC 
and the HD3SS3220 port controller. Hence device mode can't be used. This 
was the reason we left the above port nodes empty since we can't connect 
them to port nodes of controller.

Can we mark the connector as host only and remove the empty endpoints ? 
Would that we sufficient ?

Regards,
Krishna,

On 12/3/25 12:41 PM, Krishna Kurapati wrote:
> 
> 
> On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
>> On 12/3/25 3:42 AM, Swati Agarwal wrote:
>>> Enable Host mode support for USB1 controller on EVK Platform.
>>>
>>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
>>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
>>> through the hub to the Port controller. Mark the second USB controller as
>>> host only capable.
>>>
>>> Added HD3SS3220 Type-C port controller along with Type-c connector for
>>> controlling vbus supply.
>>>
>>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
>>> ---

[...]

>>> +                hd3ss3220_47_in_ep: endpoint {
>>
>> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
>> _47 doesn't really tell us anything and may re-appear if there's another
>> instance of this IC on another I2C bus
>>
> 
> ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
> Krzysztof suggested to use generic names and hence we used "usb-typec" instead of hd3ss3220.

The generic names rule only applies to node names (text before '@'),
the labels are generally only expected to "make overall sense", I think

>>> +&usb_1 {
>>> +    dr_mode = "host";
>>
>> The connector states it's a dual-role data and power device. Is there any
>> reason to keep this in host-only mode?
>>
> 
> As mentioned in commit text, there is a onboard hub sitting between SoC and the HD3SS3220 port controller. Hence device mode can't be used. This was the reason we left the above port nodes empty since we can't connect them to port nodes of controller.

It would have helped if I had paid more attention to that message then..

> Can we mark the connector as host only and remove the empty endpoints ? Would that we sufficient ?

The connector should definitely be marked host-only, but the endpoints should
still reflect the physical connectivity.

If I understood your case properly, this is analogous to what &usb2_2_dwc3
does in arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts - see Commit
c02716951e66 ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: add routing for
second USB connector")

Konrad

Hi,


On 12/3/2025 5:17 PM, Konrad Dybcio wrote:
> On 12/3/25 12:41 PM, Krishna Kurapati wrote:
>>
>>
>> On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
>>> On 12/3/25 3:42 AM, Swati Agarwal wrote:
>>>> Enable Host mode support for USB1 controller on EVK Platform.
>>>>
>>>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
>>>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
>>>> through the hub to the Port controller. Mark the second USB controller as
>>>> host only capable.
>>>>
>>>> Added HD3SS3220 Type-C port controller along with Type-c connector for
>>>> controlling vbus supply.
>>>>
>>>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
>>>> ---
> 
> [...]
> 
>>>> +                hd3ss3220_47_in_ep: endpoint {
>>>
>>> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
>>> _47 doesn't really tell us anything and may re-appear if there's another
>>> instance of this IC on another I2C bus

Thanks Konrad and Dmitry for the review.
For addressing this renaming for USB0 controller, shall i keep a separate patch and should i address in same DT patch for USB1?>>>
>>
>> ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
>> Krzysztof suggested to use generic names and hence we used "usb-typec" instead of hd3ss3220.
> 
> The generic names rule only applies to node names (text before '@'),
> the labels are generally only expected to "make overall sense", I think
> 
>>>> +&usb_1 {
>>>> +    dr_mode = "host";
>>>
>>> The connector states it's a dual-role data and power device. Is there any
>>> reason to keep this in host-only mode?
>>>
>>
>> As mentioned in commit text, there is a onboard hub sitting between SoC and the HD3SS3220 port controller. Hence device mode can't be used. This was the reason we left the above port nodes empty since we can't connect them to port nodes of controller.
> 
> It would have helped if I had paid more attention to that message then..
> 
>> Can we mark the connector as host only and remove the empty endpoints ? Would that we sufficient ?
> 
> The connector should definitely be marked host-only, but the endpoints should
> still reflect the physical connectivity.
> 
> If I understood your case properly, this is analogous to what &usb2_2_dwc3
> does in arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts - see Commit
> c02716951e66 ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: add routing for
> second USB connector")

For adding, onboard hub which is GL3590 in DT, it requires adding SS hub bindings in genesys,gl850g.yaml.
My plan is to first submit the binding patch and then follow up with this DT patch on top of it.

Regards,
Swati> 
> Konrad

On 12/8/25 11:49 AM, Swati Agarwal wrote:
> Hi,
> 
> 
> On 12/3/2025 5:17 PM, Konrad Dybcio wrote:
>> On 12/3/25 12:41 PM, Krishna Kurapati wrote:
>>>
>>>
>>> On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
>>>> On 12/3/25 3:42 AM, Swati Agarwal wrote:
>>>>> Enable Host mode support for USB1 controller on EVK Platform.
>>>>>
>>>>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
>>>>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
>>>>> through the hub to the Port controller. Mark the second USB controller as
>>>>> host only capable.
>>>>>
>>>>> Added HD3SS3220 Type-C port controller along with Type-c connector for
>>>>> controlling vbus supply.
>>>>>
>>>>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
>>>>> ---
>>
>> [...]
>>
>>>>> +                hd3ss3220_47_in_ep: endpoint {
>>>>
>>>> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
>>>> _47 doesn't really tell us anything and may re-appear if there's another
>>>> instance of this IC on another I2C bus
> 
> Thanks Konrad and Dmitry for the review.
> For addressing this renaming for USB0 controller, shall i keep a separate patch and should i address in same DT patch for USB1?>>>

Separate patch, please

>>>
>>> ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
>>> Krzysztof suggested to use generic names and hence we used "usb-typec" instead of hd3ss3220.
>>
>> The generic names rule only applies to node names (text before '@'),
>> the labels are generally only expected to "make overall sense", I think
>>
>>>>> +&usb_1 {
>>>>> +    dr_mode = "host";
>>>>
>>>> The connector states it's a dual-role data and power device. Is there any
>>>> reason to keep this in host-only mode?
>>>>
>>>
>>> As mentioned in commit text, there is a onboard hub sitting between SoC and the HD3SS3220 port controller. Hence device mode can't be used. This was the reason we left the above port nodes empty since we can't connect them to port nodes of controller.
>>
>> It would have helped if I had paid more attention to that message then..
>>
>>> Can we mark the connector as host only and remove the empty endpoints ? Would that we sufficient ?
>>
>> The connector should definitely be marked host-only, but the endpoints should
>> still reflect the physical connectivity.
>>
>> If I understood your case properly, this is analogous to what &usb2_2_dwc3
>> does in arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts - see Commit
>> c02716951e66 ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: add routing for
>> second USB connector")
> 
> For adding, onboard hub which is GL3590 in DT, it requires adding SS hub bindings in genesys,gl850g.yaml.
> My plan is to first submit the binding patch and then follow up with this DT patch on top of it.

There's already *some* SS hub described in there - what needs updating?

Konrad

On 12/8/2025 10:12 PM, Konrad Dybcio wrote:
> On 12/8/25 11:49 AM, Swati Agarwal wrote:
>> Hi,
>>
>>
>> On 12/3/2025 5:17 PM, Konrad Dybcio wrote:
>>> On 12/3/25 12:41 PM, Krishna Kurapati wrote:
>>>>
>>>>
>>>> On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
>>>>> On 12/3/25 3:42 AM, Swati Agarwal wrote:
>>>>>> Enable Host mode support for USB1 controller on EVK Platform.
>>>>>>
>>>>>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
>>>>>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
>>>>>> through the hub to the Port controller. Mark the second USB controller as
>>>>>> host only capable.
>>>>>>
>>>>>> Added HD3SS3220 Type-C port controller along with Type-c connector for
>>>>>> controlling vbus supply.
>>>>>>
>>>>>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
>>>>>> ---
>>>
>>> [...]
>>>
>>>>>> +                hd3ss3220_47_in_ep: endpoint {
>>>>>
>>>>> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
>>>>> _47 doesn't really tell us anything and may re-appear if there's another
>>>>> instance of this IC on another I2C bus
>>
>> Thanks Konrad and Dmitry for the review.
>> For addressing this renaming for USB0 controller, shall i keep a separate patch and should i address in same DT patch for USB1?>>>
> 
> Separate patch, please
> 
>>>>
>>>> ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
>>>> Krzysztof suggested to use generic names and hence we used "usb-typec" instead of hd3ss3220.
>>>
>>> The generic names rule only applies to node names (text before '@'),
>>> the labels are generally only expected to "make overall sense", I think
>>>
>>>>>> +&usb_1 {
>>>>>> +    dr_mode = "host";
>>>>>
>>>>> The connector states it's a dual-role data and power device. Is there any
>>>>> reason to keep this in host-only mode?
>>>>>
>>>>
>>>> As mentioned in commit text, there is a onboard hub sitting between SoC and the HD3SS3220 port controller. Hence device mode can't be used. This was the reason we left the above port nodes empty since we can't connect them to port nodes of controller.
>>>
>>> It would have helped if I had paid more attention to that message then..
>>>
>>>> Can we mark the connector as host only and remove the empty endpoints ? Would that we sufficient ?
>>>
>>> The connector should definitely be marked host-only, but the endpoints should
>>> still reflect the physical connectivity.
>>>
>>> If I understood your case properly, this is analogous to what &usb2_2_dwc3
>>> does in arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts - see Commit
>>> c02716951e66 ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: add routing for
>>> second USB connector")
>>
>> For adding, onboard hub which is GL3590 in DT, it requires adding SS hub bindings in genesys,gl850g.yaml.
>> My plan is to first submit the binding patch and then follow up with this DT patch on top of it.
> 
> There's already *some* SS hub described in there - what needs updating?
Genesys hub used here has PID 0610 for HS hub and 0625 for SS hub. HS hub PID is available but SS hub PID not available. 

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml?h=next-20251209

Regards,
Swati

On Tue, Dec 09, 2025 at 11:33:52AM +0530, Swati Agarwal wrote:
> 
> 
> On 12/8/2025 10:12 PM, Konrad Dybcio wrote:
> > On 12/8/25 11:49 AM, Swati Agarwal wrote:
> >> Hi,
> >>
> >>
> >> On 12/3/2025 5:17 PM, Konrad Dybcio wrote:
> >>> On 12/3/25 12:41 PM, Krishna Kurapati wrote:
> >>>>
> >>>>
> >>>> On 12/3/2025 4:59 PM, Konrad Dybcio wrote:
> >>>>> On 12/3/25 3:42 AM, Swati Agarwal wrote:
> >>>>>> Enable Host mode support for USB1 controller on EVK Platform.
> >>>>>>
> >>>>>> For secondary USB Typec port, there is a genesys USB HUB sitting in between
> >>>>>> SOC and HD3SS3220 Type-C port controller and SS lines run from the SoC
> >>>>>> through the hub to the Port controller. Mark the second USB controller as
> >>>>>> host only capable.
> >>>>>>
> >>>>>> Added HD3SS3220 Type-C port controller along with Type-c connector for
> >>>>>> controlling vbus supply.
> >>>>>>
> >>>>>> Signed-off-by: Swati Agarwal <swati.agarwal@oss.qualcomm.com>
> >>>>>> ---
> >>>
> >>> [...]
> >>>
> >>>>>> +                hd3ss3220_47_in_ep: endpoint {
> >>>>>
> >>>>> How about rename the other one to hd3ss3220_0 and name this one hd3ss322_1?
> >>>>> _47 doesn't really tell us anything and may re-appear if there's another
> >>>>> instance of this IC on another I2C bus
> >>
> >> Thanks Konrad and Dmitry for the review.
> >> For addressing this renaming for USB0 controller, shall i keep a separate patch and should i address in same DT patch for USB1?>>>
> > 
> > Separate patch, please
> > 
> >>>>
> >>>> ACK. Can we rename them as "usb-typec_1" and "usb_typec_2" ?
> >>>> Krzysztof suggested to use generic names and hence we used "usb-typec" instead of hd3ss3220.
> >>>
> >>> The generic names rule only applies to node names (text before '@'),
> >>> the labels are generally only expected to "make overall sense", I think
> >>>
> >>>>>> +&usb_1 {
> >>>>>> +    dr_mode = "host";
> >>>>>
> >>>>> The connector states it's a dual-role data and power device. Is there any
> >>>>> reason to keep this in host-only mode?
> >>>>>
> >>>>
> >>>> As mentioned in commit text, there is a onboard hub sitting between SoC and the HD3SS3220 port controller. Hence device mode can't be used. This was the reason we left the above port nodes empty since we can't connect them to port nodes of controller.
> >>>
> >>> It would have helped if I had paid more attention to that message then..
> >>>
> >>>> Can we mark the connector as host only and remove the empty endpoints ? Would that we sufficient ?
> >>>
> >>> The connector should definitely be marked host-only, but the endpoints should
> >>> still reflect the physical connectivity.
> >>>
> >>> If I understood your case properly, this is analogous to what &usb2_2_dwc3
> >>> does in arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts - see Commit
> >>> c02716951e66 ("arm64: dts: qcom: sdm850-lenovo-yoga-c630: add routing for
> >>> second USB connector")
> >>
> >> For adding, onboard hub which is GL3590 in DT, it requires adding SS hub bindings in genesys,gl850g.yaml.
> >> My plan is to first submit the binding patch and then follow up with this DT patch on top of it.
> > 
> > There's already *some* SS hub described in there - what needs updating?
> Genesys hub used here has PID 0610 for HS hub and 0625 for SS hub. HS hub PID is available but SS hub PID not available. 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml?h=next-20251209

Please add it as a part of the series. There is no need to artificially
fragment the patch series.

> 
> Regards,
> Swati
> 

-- 
With best wishes
Dmitry