Re: [PATCH] arm64: dts: qcom: lemans-evk: Add host mode support for USB1 controller

[Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>]

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,