> -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: Tuesday, December 1, 2020 5:47 AM > To: Jun Li <jun.li@xxxxxxx> > Cc: heikki.krogerus@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; > hdegoede@xxxxxxxxxx; lee.jones@xxxxxxxxxx; > mika.westerberg@xxxxxxxxxxxxxxx; dmitry.torokhov@xxxxxxxxx; > prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx; > laurent.pinchart+renesas@xxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Peter Chen > <peter.chen@xxxxxxx> > 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: > > 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 <jun.li@xxxxxxx> > > --- > > 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 > > > work.ozlabs.org%2Fpatch%2F1054342%2F&data=04%7C01%7Cjun.li%40nxp.c > > > om%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c5c3016 > > > 35%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata= > > JCV9OtFkfLv4bH2LIxfnk0lVIO6j830%2F%2Bx2OMOi%2Fs6o%3D&reserved=0 > > > > .../bindings/usb/typec-switch-simple.yaml | 122 > +++++++++++++++++++++ > > 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 > > > +cetree.org%2Fschemas%2Fusb%2Ftypec-switch-simple.yaml%23&data=04% > > > +7C01%7Cjun.li%40nxp.com%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3 > > > +bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWF > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > +Mn0%3D%7C3000&sdata=vSLcoGYWCgwpsi4LWbKo2VTyCVgN%2BA7zjHfE8QCS4rg > > +%3D&reserved=0 > > +$schema: > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cjun.li%40 > > > +nxp.com%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c > > > +5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjo > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am > > > +p;sdata=0fd3wjAtKwLCQAY3NG2Be4qNA9FiFhtO0HIZmCK7AQs%3D&reserved=0 > > + > > +title: Typec Orientation Switch Simple Solution Bindings > > + > > +maintainers: > > + - Li Jun <jun.li@xxxxxxx> > > + > > +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>; }; }; }; }; }; > > > + > > + 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 > > + 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
