Re: [PATCH v3 1/2] arm64: dts: mediatek: mt8395-genio-1200-evk: add support for TCPC port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

+                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 {







[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux