devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port
@ 2024-10-23  8:09 Macpaul Lin
  2024-10-23  8:09 ` [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205 Macpaul Lin
  2024-10-23 11:07 ` [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port AngeloGioacchino Del Regno
  0 siblings, 2 replies; 6+ messages in thread
From: Macpaul Lin @ 2024-10-23  8:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Yow-Shin Liou, Simon Sun

From: Fabien Parent <fparent@baylibre.com>

Enable USB Type-C support on MediaTek MT8395 Genio 1200 EVK by adding
configuration for TCPC Port, USB-C connector, and related settings.

Configure dual role switch capability, set up PD (Power Delivery) profiles,
and establish endpoints for SSUSB (SuperSpeed USB).

Update pinctrl configurations for U3 P0 VBus default pins and set dr_mode
to "otg" for OTG (On-The-Go) mode operation.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Yow-Shin Liou <yow-shin.liou@mediatek.com>
Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 .../dts/mediatek/mt8395-genio-1200-evk.dts    | 54 +++++++++++++++++++
 1 file changed, 54 insertions(+)

Changes for v2:
 - Drop the no need '1/2' DT Schema update patch in the 1st version.  
 - Fix intent for 'ports' node, it should under the 'connector' node.
 - Correct the index for 'port@0' and 'port@1' node.

Changes for v3:
 - Correct the order between new added nodes.

diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
index 5f16fb820580..83d520226302 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
@@ -335,6 +335,43 @@ mt6360_ldo7: ldo7 {
 				regulator-always-on;
 			};
 		};
+
+		tcpc {
+			compatible = "mediatek,mt6360-tcpc";
+			interrupts-extended = <&pio 17 IRQ_TYPE_LEVEL_LOW>;
+			interrupt-names = "PD_IRQB";
+
+			connector {
+				compatible = "usb-c-connector";
+				label = "USB-C";
+				data-role = "dual";
+				power-role = "dual";
+				try-power-role = "sink";
+				source-pdos = <PDO_FIXED(5000, 1000, \
+					       PDO_FIXED_DUAL_ROLE | \
+					       PDO_FIXED_DATA_SWAP)>;
+				sink-pdos = <PDO_FIXED(5000, 2000, \
+					     PDO_FIXED_DUAL_ROLE | \
+					     PDO_FIXED_DATA_SWAP)>;
+				op-sink-microwatt = <10000000>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+					};
+
+					port@1 {
+						reg = <1>;
+						mt6360_ssusb_ep: endpoint {
+							remote-endpoint = <&ssusb_ep>;
+						};
+					};
+				};
+			};
+		};
 	};
 };
 
@@ -770,6 +807,13 @@ pins-reset {
 		};
 	};
 
+	u3_p0_vbus: u3-p0-vbus-default-pins {
+		pins-cmd-dat {
+			pinmux = <PINMUX_GPIO63__FUNC_VBUSVALID>;
+			input-enable;
+		};
+	};
+
 	uart0_pins: uart0-pins {
 		pins {
 			pinmux = <PINMUX_GPIO98__FUNC_UTXD0>,
@@ -900,8 +944,18 @@ &ufsphy {
 };
 
 &ssusb0 {
+	dr_mode = "otg";
+	pinctrl-names = "default";
+	pinctrl-0 = <&u3_p0_vbus>;
+	usb-role-switch;
 	vusb33-supply = <&mt6359_vusb_ldo_reg>;
 	status = "okay";
+
+	port {
+		ssusb_ep: endpoint {
+			remote-endpoint = <&mt6360_ssusb_ep>;
+		};
+	};
 };
 
 &ssusb2 {
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205
  2024-10-23  8:09 [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port Macpaul Lin
@ 2024-10-23  8:09 ` Macpaul Lin
  2024-10-23 11:07   ` AngeloGioacchino Del Regno
  2024-10-23 11:07 ` [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port AngeloGioacchino Del Regno
  1 sibling, 1 reply; 6+ messages in thread
From: Macpaul Lin @ 2024-10-23  8:09 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
	AngeloGioacchino Del Regno, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Simon Sun

Add ITE IT5205FN (TYPEC MUX) under I2C2 bus and configure its properties;
also add references to it5205fn from MT6360 TYPE-C connector for TYPEC
configuration.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 .../dts/mediatek/mt8395-genio-1200-evk.dts    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

Changes for v2:
 - This is a new patch in the v2 patch.

Changes for v3:
 - No change.

diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
index 83d520226302..4c11c100e7b6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
@@ -229,6 +229,21 @@ &i2c2 {
 	pinctrl-0 = <&i2c2_pins>;
 	pinctrl-names = "default";
 	status = "okay";
+
+	it5205fn: typec-mux@48 {
+		compatible = "ite,it5205";
+		reg = <0x48>;
+		vcc-supply = <&mt6359_vibr_ldo_reg>;
+		mode-switch;
+		orientation-switch;
+		status = "okay";
+
+		port {
+			it5205_sbu_ep: endpoint {
+				remote-endpoint = <&mt6360_ssusb_sbu_ep>;
+			};
+		};
+	};
 };
 
 &i2c6 {
@@ -369,6 +384,13 @@ mt6360_ssusb_ep: endpoint {
 							remote-endpoint = <&ssusb_ep>;
 						};
 					};
+
+					port@2 {
+						reg = <2>;
+						mt6360_ssusb_sbu_ep: endpoint {
+							remote-endpoint = <&it5205_sbu_ep>;
+						};
+					};
 				};
 			};
 		};
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205
  2024-10-23  8:09 ` [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205 Macpaul Lin
@ 2024-10-23 11:07   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-10-23 11:07 UTC (permalink / raw)
  To: Macpaul Lin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Simon Sun

Il 23/10/24 10:09, Macpaul Lin ha scritto:
> Add ITE IT5205FN (TYPEC MUX) under I2C2 bus and configure its properties;
> also add references to it5205fn from MT6360 TYPE-C connector for TYPEC
> configuration.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>   .../dts/mediatek/mt8395-genio-1200-evk.dts    | 22 +++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> Changes for v2:
>   - This is a new patch in the v2 patch.
> 
> Changes for v3:
>   - No change.
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> index 83d520226302..4c11c100e7b6 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> @@ -229,6 +229,21 @@ &i2c2 {
>   	pinctrl-0 = <&i2c2_pins>;
>   	pinctrl-names = "default";
>   	status = "okay";
> +
> +	it5205fn: typec-mux@48 {

You don't need the it5205fn phandle, please drop.

> +		compatible = "ite,it5205";
> +		reg = <0x48>;
> +		vcc-supply = <&mt6359_vibr_ldo_reg>;
> +		mode-switch;
> +		orientation-switch;

compatible
reg
mode-switch
orientation-switch
vcc-supply

Please reorder.

After which:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> +		status = "okay";
> +
> +		port {
> +			it5205_sbu_ep: endpoint {
> +				remote-endpoint = <&mt6360_ssusb_sbu_ep>;
> +			};
> +		};
> +	};
>   };
>   
>   &i2c6 {
> @@ -369,6 +384,13 @@ mt6360_ssusb_ep: endpoint {
>   							remote-endpoint = <&ssusb_ep>;
>   						};
>   					};
> +
> +					port@2 {
> +						reg = <2>;
> +						mt6360_ssusb_sbu_ep: endpoint {
> +							remote-endpoint = <&it5205_sbu_ep>;
> +						};
> +					};
>   				};
>   			};
>   		};



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port
  2024-10-23  8:09 [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port Macpaul Lin
  2024-10-23  8:09 ` [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205 Macpaul Lin
@ 2024-10-23 11:07 ` AngeloGioacchino Del Regno
  2024-10-23 12:00   ` AngeloGioacchino Del Regno
  1 sibling, 1 reply; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-10-23 11:07 UTC (permalink / raw)
  To: Macpaul Lin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Yow-Shin Liou, Simon Sun

Il 23/10/24 10:09, Macpaul Lin ha scritto:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Enable USB Type-C support on MediaTek MT8395 Genio 1200 EVK by adding
> configuration for TCPC Port, USB-C connector, and related settings.
> 
> Configure dual role switch capability, set up PD (Power Delivery) profiles,
> and establish endpoints for SSUSB (SuperSpeed USB).
> 
> Update pinctrl configurations for U3 P0 VBus default pins and set dr_mode
> to "otg" for OTG (On-The-Go) mode operation.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Yow-Shin Liou <yow-shin.liou@mediatek.com>
> Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>   .../dts/mediatek/mt8395-genio-1200-evk.dts    | 54 +++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> Changes for v2:
>   - Drop the no need '1/2' DT Schema update patch in the 1st version.
>   - Fix intent for 'ports' node, it should under the 'connector' node.
>   - Correct the index for 'port@0' and 'port@1' node.
> 
> Changes for v3:
>   - Correct the order between new added nodes.
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> index 5f16fb820580..83d520226302 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
> @@ -335,6 +335,43 @@ mt6360_ldo7: ldo7 {
>   				regulator-always-on;
>   			};
>   		};
> +
> +		tcpc {
> +			compatible = "mediatek,mt6360-tcpc";
> +			interrupts-extended = <&pio 17 IRQ_TYPE_LEVEL_LOW>;
> +			interrupt-names = "PD_IRQB";
> +
> +			connector {
> +				compatible = "usb-c-connector";
> +				label = "USB-C";
> +				data-role = "dual";

op-sink-microwatt goes here

> +				power-role = "dual";
> +				try-power-role = "sink";
> +				source-pdos = <PDO_FIXED(5000, 1000, \
> +					       PDO_FIXED_DUAL_ROLE | \
> +					       PDO_FIXED_DATA_SWAP)>;

Please fix the indentation (and also you don't need the escaping)

				source-pdos = <PDO_FIXED(5000, 1000,
							 PDO_FIXED_DUAL_ROLE |
							 PDO_FIXED_DATA_SWAP)>;

> +				sink-pdos = <PDO_FIXED(5000, 2000, \
> +					     PDO_FIXED_DUAL_ROLE | \
> +					     PDO_FIXED_DATA_SWAP)>;
> +				op-sink-microwatt = <10000000>;
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;

Just to make sure that this is ok: are you sure that this port supports
SuperSpeed (physical connector too) and that it's not limited to HighSpeed?

I have seen Rob's comment stating that ssusb_ep goes to port@1, but I think
that his comment came after reading "ss" in "ssusb": while the controller
does surely support SS, if the port does not, this should still go to port@0.

P.S.: I didn't check the schematics - just please make sure it's correct, and
       that this actually works.

> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +						mt6360_ssusb_ep: endpoint {
> +							remote-endpoint = <&ssusb_ep>;
> +						};
> +					};
> +				};
> +			};
> +		};
>   	};
>   };
>   
> @@ -770,6 +807,13 @@ pins-reset {
>   		};
>   	};
>   
> +	u3_p0_vbus: u3-p0-vbus-default-pins {
> +		pins-cmd-dat {

That's not a command nor data pin.

pins-vbus {

> +			pinmux = <PINMUX_GPIO63__FUNC_VBUSVALID>;
> +			input-enable;
> +		};
> +	};
> +
>   	uart0_pins: uart0-pins {
>   		pins {
>   			pinmux = <PINMUX_GPIO98__FUNC_UTXD0>,
> @@ -900,8 +944,18 @@ &ufsphy {
>   };
>   
>   &ssusb0 {
> +	dr_mode = "otg";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&u3_p0_vbus>;

Is this port usb host by default? If it is:

	role-switch-default-mode = "host";

Cheers,
Angelo

> +	usb-role-switch;
>   	vusb33-supply = <&mt6359_vusb_ldo_reg>;
>   	status = "okay";
> +
> +	port {
> +		ssusb_ep: endpoint {
> +			remote-endpoint = <&mt6360_ssusb_ep>;
> +		};
> +	};
>   };
>   
>   &ssusb2 {


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port
  2024-10-23 11:07 ` [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port AngeloGioacchino Del Regno
@ 2024-10-23 12:00   ` AngeloGioacchino Del Regno
  2024-10-24  7:44     ` Macpaul Lin
  0 siblings, 1 reply; 6+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-10-23 12:00 UTC (permalink / raw)
  To: Macpaul Lin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Matthias Brugger, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Yow-Shin Liou, Simon Sun

Il 23/10/24 13:07, AngeloGioacchino Del Regno ha scritto:
> Il 23/10/24 10:09, Macpaul Lin ha scritto:
>> From: Fabien Parent <fparent@baylibre.com>
>>
>> Enable USB Type-C support on MediaTek MT8395 Genio 1200 EVK by adding
>> configuration for TCPC Port, USB-C connector, and related settings.
>>
>> Configure dual role switch capability, set up PD (Power Delivery) profiles,
>> and establish endpoints for SSUSB (SuperSpeed USB).
>>
>> Update pinctrl configurations for U3 P0 VBus default pins and set dr_mode
>> to "otg" for OTG (On-The-Go) mode operation.
>>
>> Signed-off-by: Fabien Parent <fparent@baylibre.com>
>> Signed-off-by: Yow-Shin Liou <yow-shin.liou@mediatek.com>
>> Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
>> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
>> ---
>>   .../dts/mediatek/mt8395-genio-1200-evk.dts    | 54 +++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> Changes for v2:
>>   - Drop the no need '1/2' DT Schema update patch in the 1st version.
>>   - Fix intent for 'ports' node, it should under the 'connector' node.
>>   - Correct the index for 'port@0' and 'port@1' node.
>>
>> Changes for v3:
>>   - Correct the order between new added nodes.
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts b/arch/arm64/ 
>> boot/dts/mediatek/mt8395-genio-1200-evk.dts
>> index 5f16fb820580..83d520226302 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
>> +++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
>> @@ -335,6 +335,43 @@ mt6360_ldo7: ldo7 {
>>                   regulator-always-on;
>>               };
>>           };
>> +
>> +        tcpc {
>> +            compatible = "mediatek,mt6360-tcpc";
>> +            interrupts-extended = <&pio 17 IRQ_TYPE_LEVEL_LOW>;
>> +            interrupt-names = "PD_IRQB";
>> +
>> +            connector {
>> +                compatible = "usb-c-connector";
>> +                label = "USB-C";
>> +                data-role = "dual";
> 
> op-sink-microwatt goes here
> 
>> +                power-role = "dual";
>> +                try-power-role = "sink";
>> +                source-pdos = <PDO_FIXED(5000, 1000, \
>> +                           PDO_FIXED_DUAL_ROLE | \
>> +                           PDO_FIXED_DATA_SWAP)>;
> 
> Please fix the indentation (and also you don't need the escaping)
> 
>                  source-pdos = <PDO_FIXED(5000, 1000,
>                               PDO_FIXED_DUAL_ROLE |
>                               PDO_FIXED_DATA_SWAP)>;
> 
>> +                sink-pdos = <PDO_FIXED(5000, 2000, \
>> +                         PDO_FIXED_DUAL_ROLE | \
>> +                         PDO_FIXED_DATA_SWAP)>;
>> +                op-sink-microwatt = <10000000>;
>> +
>> +                ports {
>> +                    #address-cells = <1>;
>> +                    #size-cells = <0>;
>> +
>> +                    port@0 {
>> +                        reg = <0>;
> 
> Just to make sure that this is ok: are you sure that this port supports
> SuperSpeed (physical connector too) and that it's not limited to HighSpeed?
> 
> I have seen Rob's comment stating that ssusb_ep goes to port@1, but I think
> that his comment came after reading "ss" in "ssusb": while the controller
> does surely support SS, if the port does not, this should still go to port@0.
> 
> P.S.: I didn't check the schematics - just please make sure it's correct, and
>        that this actually works.
> 

Sorry for the double email, but I've done some fast research on this as something
came to mind right after sending this review.

usb-connector.yaml says that the `ports` property models a data bus to the USB
connector, and that a single connector can have multiple data buses, of course.

The last statement doesn't come as a surprise, and actually makes me think about
what MTU3 actually provides: a High Speed data bus, and a SuperSpeed data bus.

Now, I see in MTU3 that only `port` is allowed and that only the HS part is
is exposed... so to resolve this, you want to add a binding to connect the
data bus of the MTU3 controller to the usb-c-connector, and obviously only then
you should use it here.

That should look like this:

mediatek,mtu3.yaml:
   ports:
     $ref: /schemas/graph.yaml#/properties/ports

     properties:
       port@0:
         $ref: /schemas/graph.yaml#/properties/port
         description: High Speed (HS) data bus.

       port@1:
         $ref: /schemas/graph.yaml#/properties/port
         description: Super Speed (SS) data bus.

some_devicetree.dts
&ssusb0 {
	ports {
		port@0 {
			reg = <0>;

			/* High Speed data bus */
			mtu3_hs0_ep: endpoint {
				/* connects to port@0 (HS) of usb-c-connector */
				remote-endpoint = <&typec_con_hs>;
			}
		};

		port@1 {
			reg = <1>;

			/* SuperSpeed data bus */
			mtu3_ss0_ep: endpoint {
				/* connects to port@1 (SS) of usb-c-connector */
				remote-endpoint = <&typec_con_sss>;
			}
		};
	};
};

I don't have time to do any testing in this precise moment, so, if you want
to go on and test - cool; otherwise, I can check that at a later time, but
on Genio 700 EVK instead (which is the same thing for this specific topic anyway).

Cheers,
Angelo

>> +                    };
>> +
>> +                    port@1 {
>> +                        reg = <1>;
>> +                        mt6360_ssusb_ep: endpoint {
>> +                            remote-endpoint = <&ssusb_ep>;
>> +                        };
>> +                    };
>> +                };
>> +            };
>> +        };
>>       };
>>   };
>> @@ -770,6 +807,13 @@ pins-reset {
>>           };
>>       };
>> +    u3_p0_vbus: u3-p0-vbus-default-pins {
>> +        pins-cmd-dat {
> 
> That's not a command nor data pin.
> 
> pins-vbus {
> 
>> +            pinmux = <PINMUX_GPIO63__FUNC_VBUSVALID>;
>> +            input-enable;
>> +        };
>> +    };
>> +
>>       uart0_pins: uart0-pins {
>>           pins {
>>               pinmux = <PINMUX_GPIO98__FUNC_UTXD0>,
>> @@ -900,8 +944,18 @@ &ufsphy {
>>   };
>>   &ssusb0 {
>> +    dr_mode = "otg";
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&u3_p0_vbus>;
> 
> Is this port usb host by default? If it is:
> 
>      role-switch-default-mode = "host";
> 
> Cheers,
> Angelo
> 
>> +    usb-role-switch;
>>       vusb33-supply = <&mt6359_vusb_ldo_reg>;
>>       status = "okay";
>> +
>> +    port {
>> +        ssusb_ep: endpoint {
>> +            remote-endpoint = <&mt6360_ssusb_ep>;
>> +        };
>> +    };
>>   };
>>   &ssusb2 {
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port
  2024-10-23 12:00   ` AngeloGioacchino Del Regno
@ 2024-10-24  7:44     ` Macpaul Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Macpaul Lin @ 2024-10-24  7:44 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, devicetree, linux-kernel,
	linux-arm-kernel, linux-mediatek, Alexandre Mergnat
  Cc: Bear Wang, Pablo Sun, Macpaul Lin,
	Project_Global_Chrome_Upstream_Group, linux-usb, Chris-qj chen,
	Fabien Parent, Yow-Shin Liou, Simon Sun



On 10/23/24 20:00, AngeloGioacchino Del Regno wrote:
> Il 23/10/24 13:07, AngeloGioacchino Del Regno ha scritto:
>> Il 23/10/24 10:09, Macpaul Lin ha scritto:
>>> From: Fabien Parent <fparent@baylibre.com>
>>>
>>> Enable USB Type-C support on MediaTek MT8395 Genio 1200 EVK by adding
>>> configuration for TCPC Port, USB-C connector, and related settings.
>>>
>>> Configure dual role switch capability, set up PD (Power Delivery) 
>>> profiles,
>>> and establish endpoints for SSUSB (SuperSpeed USB).
>>>
>>> Update pinctrl configurations for U3 P0 VBus default pins and set 
>>> dr_mode
>>> to "otg" for OTG (On-The-Go) mode operation.
>>>
>>> Signed-off-by: Fabien Parent <fparent@baylibre.com>
>>> Signed-off-by: Yow-Shin Liou <yow-shin.liou@mediatek.com>
>>> Signed-off-by: Simon Sun <simon.sun@yunjingtech.com>
>>> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
>>> ---
>>>   .../dts/mediatek/mt8395-genio-1200-evk.dts    | 54 +++++++++++++++++++
>>>   1 file changed, 54 insertions(+)
>>>
>>> Changes for v2:
>>>   - Drop the no need '1/2' DT Schema update patch in the 1st version.
>>>   - Fix intent for 'ports' node, it should under the 'connector' node.
>>>   - Correct the index for 'port@0' and 'port@1' node.
>>>
>>> Changes for v3:
>>>   - Correct the order between new added nodes.
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts 
>>> b/arch/arm64/ boot/dts/mediatek/mt8395-genio-1200-evk.dts
>>> index 5f16fb820580..83d520226302 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8395-genio-1200-evk.dts
>>> @@ -335,6 +335,43 @@ mt6360_ldo7: ldo7 {
>>>                   regulator-always-on;
>>>               };
>>>           };
>>> +
>>> +        tcpc {
>>> +            compatible = "mediatek,mt6360-tcpc";
>>> +            interrupts-extended = <&pio 17 IRQ_TYPE_LEVEL_LOW>;
>>> +            interrupt-names = "PD_IRQB";
>>> +
>>> +            connector {
>>> +                compatible = "usb-c-connector";
>>> +                label = "USB-C";
>>> +                data-role = "dual";
>>
>> op-sink-microwatt goes here

Okay, will fix it.

>>> +                power-role = "dual";
>>> +                try-power-role = "sink";
>>> +                source-pdos = <PDO_FIXED(5000, 1000, \
>>> +                           PDO_FIXED_DUAL_ROLE | \
>>> +                           PDO_FIXED_DATA_SWAP)>;
>>
>> Please fix the indentation (and also you don't need the escaping)

Okay, it has been complained by checkpatch about this line is too long.
Will fix it in next version.

>>                  source-pdos = <PDO_FIXED(5000, 1000,
>>                               PDO_FIXED_DUAL_ROLE |
>>                               PDO_FIXED_DATA_SWAP)>;
>>
>>> +                sink-pdos = <PDO_FIXED(5000, 2000, \
>>> +                         PDO_FIXED_DUAL_ROLE | \
>>> +                         PDO_FIXED_DATA_SWAP)>;
>>> +                op-sink-microwatt = <10000000>;
>>> +
>>> +                ports {
>>> +                    #address-cells = <1>;
>>> +                    #size-cells = <0>;
>>> +
>>> +                    port@0 {
>>> +                        reg = <0>;
>>
>> Just to make sure that this is ok: are you sure that this port supports
>> SuperSpeed (physical connector too) and that it's not limited to 
>> HighSpeed?

This port should be able to do both HighSpeed and SuperSpeed.
However our internal reference code set SuperSpeed at port@0 and seems
just work for both high and supoer speeds.
See more detail discussion below.

>>
>> I have seen Rob's comment stating that ssusb_ep goes to port@1, but I 
>> think
>> that his comment came after reading "ss" in "ssusb": while the controller
>> does surely support SS, if the port does not, this should still go to 
>> port@0.
>>
>> P.S.: I didn't check the schematics - just please make sure it's 
>> correct, and
>>        that this actually works.
>>
> Sorry for the double email, but I've done some fast research on this as 
> something
> came to mind right after sending this review.
> 
> usb-connector.yaml says that the `ports` property models a data bus to 
> the USB
> connector, and that a single connector can have multiple data buses, of 
> course.
> 
> The last statement doesn't come as a surprise, and actually makes me 
> think about
> what MTU3 actually provides: a High Speed data bus, and a SuperSpeed 
> data bus.

Ya. That's the question I was wondering, too.
Current DT bindings of MTU3 support only 1 port which is also a required
property. For the SSUSB0 port on MT8195, it indeed provides a High Speed
data bus, and a SuperSpeed data bus.

> Now, I see in MTU3 that only `port` is allowed and that only the HS part is
> is exposed... so to resolve this, you want to add a binding to connect the
> data bus of the MTU3 controller to the usb-c-connector, and obviously 
> only then
> you should use it here.
> 
> That should look like this:
> 
> mediatek,mtu3.yaml:
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> 
>      properties:
>        port@0:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: High Speed (HS) data bus.
> 
>        port@1:
>          $ref: /schemas/graph.yaml#/properties/port
>          description: Super Speed (SS) data bus.
> 
> some_devicetree.dts
> &ssusb0 {
>      ports {
>          port@0 {
>              reg = <0>;
> 
>              /* High Speed data bus */
>              mtu3_hs0_ep: endpoint {
>                  /* connects to port@0 (HS) of usb-c-connector */
>                  remote-endpoint = <&typec_con_hs>;
>              }
>          };
> 
>          port@1 {
>              reg = <1>;
> 
>              /* SuperSpeed data bus */
>              mtu3_ss0_ep: endpoint {
>                  /* connects to port@1 (SS) of usb-c-connector */
>                  remote-endpoint = <&typec_con_sss>;
>              }
>          };
>      };
> };

Sure, but after adding that DT schema before the v3 patch has been send.
The dt_binding_check seems show 'port' is a required property for MTU3.
However I didn't found where it has been defined.
I think to add the 'ports' property in MTU3's DT Schema should be better.

> I don't have time to do any testing in this precise moment, so, if you want
> to go on and test - cool; otherwise, I can check that at a later time, but
> on Genio 700 EVK instead (which is the same thing for this specific 
> topic anyway).
> 
> Cheers,
> Angelo
> 
>>> +                    };
>>> +
>>> +                    port@1 {
>>> +                        reg = <1>;
>>> +                        mt6360_ssusb_ep: endpoint {
>>> +                            remote-endpoint = <&ssusb_ep>;
>>> +                        };
>>> +                    };
>>> +                };
>>> +            };
>>> +        };
>>>       };
>>>   };
>>> @@ -770,6 +807,13 @@ pins-reset {
>>>           };
>>>       };
>>> +    u3_p0_vbus: u3-p0-vbus-default-pins {
>>> +        pins-cmd-dat {
>>
>> That's not a command nor data pin.

Oh! Will fix it.

>> pins-vbus {
>>
>>> +            pinmux = <PINMUX_GPIO63__FUNC_VBUSVALID>;
>>> +            input-enable;
>>> +        };
>>> +    };
>>> +
>>>       uart0_pins: uart0-pins {
>>>           pins {
>>>               pinmux = <PINMUX_GPIO98__FUNC_UTXD0>,
>>> @@ -900,8 +944,18 @@ &ufsphy {
>>>   };
>>>   &ssusb0 {
>>> +    dr_mode = "otg";
>>> +    pinctrl-names = "default";
>>> +    pinctrl-0 = <&u3_p0_vbus>;
>>
>> Is this port usb host by default? If it is:
>>
>>      role-switch-default-mode = "host";

This port0 is exactly a dual-role port for genio-1200-evk.
It is usually used as gadget device port (like ADB) and
sometimes can do OTG function with xhci0.

>> Cheers,
>> Angelo
>>
>>> +    usb-role-switch;
>>>       vusb33-supply = <&mt6359_vusb_ldo_reg>;
>>>       status = "okay";
>>> +
>>> +    port {
>>> +        ssusb_ep: endpoint {
>>> +            remote-endpoint = <&mt6360_ssusb_ep>;
>>> +        };
>>> +    };
>>>   };
>>>   &ssusb2 {
>>

Thanks
Macpaul Lin

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-10-24  7:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-23  8:09 [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port Macpaul Lin
2024-10-23  8:09 ` [PATCH v3 2/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for MUX IT5205 Macpaul Lin
2024-10-23 11:07   ` AngeloGioacchino Del Regno
2024-10-23 11:07 ` [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port AngeloGioacchino Del Regno
2024-10-23 12:00   ` AngeloGioacchino Del Regno
2024-10-24  7:44     ` Macpaul Lin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).