2023-07-24 12:16:49

by Ravi Gunasekaran

[permalink] [raw]
Subject: [PATCH] arm64: dts: ti: k3-am62a7-sk: Enable dual role support for Type-C port

USB0 is interfaced with a Type-C DRP connector and is managed via a
USB PD controller. Add support for the Type-C port with dual data
and power sink role.

Signed-off-by: Ravi Gunasekaran <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 33 +++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
index d2cca6182738..b478b794de00 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
@@ -226,6 +226,24 @@
pinctrl-names = "default";
pinctrl-0 = <&main_i2c0_pins_default>;
clock-frequency = <400000>;
+
+ typec_pd0:tps6598x@3f {
+ compatible = "ti,tps6598x";
+ reg = <0x3f>;
+
+ connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ self-powered;
+ data-role = "dual";
+ power-role = "sink";
+ port {
+ usb_con_hs: endpoint {
+ remote-endpoint = <&usb0_hs_ep>;
+ };
+ };
+ };
+ };
};

&main_i2c1 {
@@ -290,6 +308,21 @@
status = "reserved";
};

+&usbss0 {
+ status = "okay";
+ ti,vbus-divider;
+};
+
+&usb0 {
+ usb-role-switch;
+
+ port {
+ usb0_hs_ep: endpoint {
+ remote-endpoint = <&usb_con_hs>;
+ };
+ };
+};
+
&usbss1 {
status = "okay";
};

base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36
--
2.17.1



2023-07-24 14:21:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-am62a7-sk: Enable dual role support for Type-C port

On 24/07/2023 13:51, Ravi Gunasekaran wrote:
> USB0 is interfaced with a Type-C DRP connector and is managed via a
> USB PD controller. Add support for the Type-C port with dual data
> and power sink role.
>
> Signed-off-by: Ravi Gunasekaran <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 33 +++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
> index d2cca6182738..b478b794de00 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
> @@ -226,6 +226,24 @@
> pinctrl-names = "default";
> pinctrl-0 = <&main_i2c0_pins_default>;
> clock-frequency = <400000>;
> +
> + typec_pd0:tps6598x@3f {

Missing space after:

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


Best regards,
Krzysztof


2023-07-25 04:23:17

by Ravi Gunasekaran

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-am62a7-sk: Enable dual role support for Type-C port



On 7/24/2023 7:27 PM, Krzysztof Kozlowski wrote:
> On 24/07/2023 13:51, Ravi Gunasekaran wrote:
>> USB0 is interfaced with a Type-C DRP connector and is managed via a
>> USB PD controller. Add support for the Type-C port with dual data
>> and power sink role.
>>
>> Signed-off-by: Ravi Gunasekaran <[email protected]>
>> ---
>> arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 33 +++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> index d2cca6182738..b478b794de00 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> @@ -226,6 +226,24 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&main_i2c0_pins_default>;
>> clock-frequency = <400000>;
>> +
>> + typec_pd0:tps6598x@3f {

Thanks for reviewing the patch.

> Missing space after:

I will fix this in v2.

>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Thanks for pointing to the section. I checked it and also few other node names
in the existing DTs.
TPS6598 is a USB Type C and Power Delivery Controller. So does a node name
"type-c-pd-controller" sound fine?

>
> Best regards,
> Krzysztof
>

Regards,
Ravi

2023-07-25 07:46:57

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-am62a7-sk: Enable dual role support for Type-C port



On 25/07/2023 07:19, Ravi Gunasekaran wrote:
>
>
> On 7/24/2023 7:27 PM, Krzysztof Kozlowski wrote:
>> On 24/07/2023 13:51, Ravi Gunasekaran wrote:
>>> USB0 is interfaced with a Type-C DRP connector and is managed via a
>>> USB PD controller. Add support for the Type-C port with dual data
>>> and power sink role.
>>>
>>> Signed-off-by: Ravi Gunasekaran <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 33 +++++++++++++++++++++++++
>>> 1 file changed, 33 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>> index d2cca6182738..b478b794de00 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>> @@ -226,6 +226,24 @@
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&main_i2c0_pins_default>;
>>> clock-frequency = <400000>;
>>> +
>>> + typec_pd0:tps6598x@3f {
>
> Thanks for reviewing the patch.
>
>> Missing space after:
>
> I will fix this in v2.
>
>>
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> Thanks for pointing to the section. I checked it and also few other node names
> in the existing DTs.
> TPS6598 is a USB Type C and Power Delivery Controller. So does a node name
> "type-c-pd-controller" sound fine?

Type-c is irrelevant in node name.
The name needs to indicate it has something to do with USB, Power Control and Role control.

e.g.
usb-power-controller
or
usb-role-controller
?

>
>>
>> Best regards,
>> Krzysztof
>>
>
> Regards,
> Ravi

--
cheers,
-roger

2023-07-25 08:45:54

by Ravi Gunasekaran

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ti: k3-am62a7-sk: Enable dual role support for Type-C port



On 7/25/2023 1:13 PM, Roger Quadros wrote:
>
> On 25/07/2023 07:19, Ravi Gunasekaran wrote:
>>
>> On 7/24/2023 7:27 PM, Krzysztof Kozlowski wrote:
>>> On 24/07/2023 13:51, Ravi Gunasekaran wrote:
>>>> USB0 is interfaced with a Type-C DRP connector and is managed via a
>>>> USB PD controller. Add support for the Type-C port with dual data
>>>> and power sink role.
>>>>
>>>> Signed-off-by: Ravi Gunasekaran <[email protected]>
>>>> ---
>>>> arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 33 +++++++++++++++++++++++++
>>>> 1 file changed, 33 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>> index d2cca6182738..b478b794de00 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>>>> @@ -226,6 +226,24 @@
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&main_i2c0_pins_default>;
>>>> clock-frequency = <400000>;
>>>> +
>>>> + typec_pd0:tps6598x@3f {
>> Thanks for reviewing the patch.
>>
>>> Missing space after:
>> I will fix this in v2.
>>
>>> Node names should be generic. See also an explanation and list of
>>> examples (not exhaustive) in DT specification:
>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>> Thanks for pointing to the section. I checked it and also few other node names
>> in the existing DTs.
>> TPS6598 is a USB Type C and Power Delivery Controller. So does a node name
>> "type-c-pd-controller" sound fine?
> Type-c is irrelevant in node name.
> The name needs to indicate it has something to do with USB, Power Control and Role control.
>
> e.g.
> usb-power-controller
> or
> usb-role-controller
> ?

I will use the node name "usb-power-controller'.

>
>>> Best regards,
>>> Krzysztof
>>>
>> Regards,
>> Ravi