-----Original Message-----
From: Rob Herring <robh@kernel.org>
Sent: Tuesday, December 1, 2020 5:47 AM
To: Jun Li <redacted>
Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org;
gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com;
hdegoede@redhat.com; lee.jones@linaro.org;
mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com;
prabhakar.mahadev-lad.rj@bp.renesas.com;
laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org;
devicetree@vger.kernel.org; dl-linux-imx [off-list ref]; Peter Chen
[off-list ref]
Subject: Re: [PATCH v6 1/6] dt-bindings: usb: add documentation for typec
switch simple driver

On Mon, Nov 23, 2020 at 10:01:31PM +0800, Li Jun wrote:

quoted

Some platforms need a simple driver to do some controls according to
typec orientation, this can be extended to be a generic driver with
compatible with "typec-orientation-switch".

Signed-off-by: Li Jun <redacted>
---
changes for v6:
- Use general mux bindings for typec switch, one typec switch
  may have 1 or 2 GPIOs for channel selection, if 1 GPIO, only
  can be used to select cc1 or cc2; if 2 GPIOs, the second GPIO
  can be used to deselect both channels or keep normal operations.
- Add one more connection to usb controller.

No changes for v5.

changes on v4:
- Use compatible instead of bool property for switch matching.
- Change switch GPIO to be switch simple.
- Change the active channel selection GPIO to be optional.

previous discussion:

https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch

quoted

work.ozlabs.org%2Fpatch%2F1054342%2F&data=04%7C01%7Cjun.li%40nxp.c

quoted

om%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c5c3016

quoted

35%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj

quoted

AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=

quoted

JCV9OtFkfLv4bH2LIxfnk0lVIO6j830%2F%2Bx2OMOi%2Fs6o%3D&reserved=0

 .../bindings/usb/typec-switch-simple.yaml          | 122

+++++++++++++++++++++

quoted

 1 file changed, 122 insertions(+)

diff --git
a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml
b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml
new file mode 100644
index 0000000..030ade5

--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml

@@ -0,0 +1,122 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
+---
+$id:

+https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi

quoted

+cetree.org%2Fschemas%2Fusb%2Ftypec-switch-simple.yaml%23&data=04%

quoted

+7C01%7Cjun.li%40nxp.com%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3

quoted

+bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWF

quoted

+pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6

quoted

+Mn0%3D%7C3000&sdata=vSLcoGYWCgwpsi4LWbKo2VTyCVgN%2BA7zjHfE8QCS4rg

quoted

+%3D&reserved=0
+$schema:

+https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi

quoted

+cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cjun.li%40

quoted

+nxp.com%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c

quoted

+5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjo

quoted

+iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am

quoted

+p;sdata=0fd3wjAtKwLCQAY3NG2Be4qNA9FiFhtO0HIZmCK7AQs%3D&reserved=0

quoted

+
+title: Typec Orientation Switch Simple Solution Bindings
+
+maintainers:
+  - Li Jun [off-list ref]
+
+description: |-
+  USB SuperSpeed (SS) lanes routing to which side of typec connector
+is
+  decided by orientation, this maybe achieved by some simple control
+like
+  GPIO toggle.
+
+properties:
+  compatible:
+    const: typec-orientation-switch
+
+  '#mux-control-cells':
+    const: 0

Who is the mux-control consumer?

Typec(controller)

       ptn5110: tcpc@50 {
                compatible = "nxp,ptn5110";
                pinctrl-names = "default";
                pinctrl-0 = <&pinctrl_typec>;
                reg = <0x50>;
                interrupt-parent = <&gpio4>;
                interrupts = <19 8>;
                mux-controls = <&typec_switch>;
                mux-control-names = "mux-typec-switch";

                usb_con: connector {
                        compatible = "usb-c-connector";
                        ... ...
                        ports {
                                #address-cells = <1>;
                                #size-cells = <0>;

                                port@1 {
                                        reg = <1>;
                                        typec_con_ss: endpoint {
                                                remote-endpoint = <&usb3_orien_sel>;
                                        };
                                };
                        };
                };
        };

quoted

+
+  switch-gpios:
+    description: |
+      GPIO specifiers to select the target channel of mux.
+      The first GPIO is for cc1 and cc2 selection, the GPIO flag use
+      GPIO_ACTIVE_HIGH if GPIO physical state high is for cc1; or use
+      GPIO_ACTIVE_LOW if GPIO physical state low is for cc1.
+      The second gpio is to deselect any channles by places all channels
+      in high-impedance state to reduce current consumption, the gpio

flag

quoted

+      use GPIO_ACTIVE_HIGH if GPIO physical state high is for
+      high-impedance state (so low for normal operations); or Use
+      GPIO_ACTIVE_LOW if GPIO physical state low is for high-impedance
+      state(so high for normal operations).
+    minItems: 1
+    maxItems: 2
+
+  idle-state:
+    description: -|
+      For TYPEC_ORIENTATION_NONE by deselect both channels if supported.
+    const: 2
+
+  # Standard properties described in
+  # Documentation/devicetree/bindings/mux/mux-controller.txt
+
+  port:
+    type: object
+    additionalProperties: false
+    description: -|
+      Connection to the remote endpoint using OF graph bindings that model
+      SS data bus to typec connector.
+
+    properties:
+      '#address-cells':
+        const: 1
+
+      '#size-cells':
+        const: 0
+
+      endpoint@0:
+        type: object
+        description: Endpoint connected to typec connector.
+        additionalProperties: false
+
+        properties:
+          reg:
+            const: 0
+          remote-endpoint: true
+
+        required:
+          - remote-endpoint
+
+      endpoint@1:
+        type: object
+        description: Endpoint connected to usb controller.
+        additionalProperties: false
+
+        properties:
+          reg:
+            const: 1
+          remote-endpoint: true
+
+        required:
+          - remote-endpoint
+
+    required:
+      - endpoint@0
+
+required:
+  - compatible
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    typec_switch: mux-controller {
+        compatible = "typec-orientation-switch";
+        #mux-control-cells = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_ss_sel>;
+        switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>,
+                       <&gpio2 20 GPIO_ACTIVE_HIGH>;
+        idle-state = <2>;
+
+        port {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                usb3_orien_sel: endpoint@0 {
+                        reg = <0>;
+                        remote-endpoint = <&typec_con_ss>;
+                };
+
+                usb3_con_data: endpoint@1 {
+                        reg = <1>;
+                        remote-endpoint = <&dwc3_0>;
+                };
+        };

This still seems unnecessarily complicated. What I had in mind is just
something like this:

mux: mux-controller {
  compatible = "gpio-mux";
  #mux-control-cells = <0>;
  mux-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>,
              <&gpio2 20 GPIO_ACTIVE_HIGH>;
  idle-state = <2>;
};

connector {
  compatible = "usb-c-connector";
  mux-controls = <&mux>;
  ...
};

Then you aren't re-implementing the gpio-mux driver.

This should work for *any* mux implementation as long as the mux states always
have the same meanings from the perspective of the connector.

Of course, I have little visibility into the possible h/w implementations
for Type-C and maybe this isn't sufficient. So input from Type-C experts
would be helpful.

From h/w point view, this can work if only consider my(GPIO) case.
But I still need register a typec switch in provider side to make it
under the unified typec switch interface:

  7 struct typec_switch {
  6         struct device dev;
  5         typec_switch_set_fn_t set;
  4         struct mux_control *mux_ctrl;
  3 };

typec_switch->dev.parent = mux_chip->dev;

So either I put the typec switch creation in exiting mux-gpio driver; or
I create a new typec switch driver with gpio mux code duplication.

If I go directly with mux control w/o(out of) typec_switch, then I don't
need any driver change as you expect, but I guess this is not preferred,
@Heikki, any comments?

Thanks
Li Jun

Rob