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@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
Signed-off-by: Yow-Shin Liou <yow-shin.liou@xxxxxxxxxxxx>
Signed-off-by: Simon Sun <simon.sun@xxxxxxxxxxxxxxx>
Signed-off-by: Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>
---
.../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
