Hi Hans > -----Original Message----- > From: Hans de Goede <hdegoede@xxxxxxxxxx> > Sent: 2019年3月11日 19:03 > To: Jun Li <jun.li@xxxxxxx>; robh+dt@xxxxxxxxxx; heikki.krogerus@xxxxxxxxxxxxxxx > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; andy.shevchenko@xxxxxxxxx; > linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; dl-linux-imx > <linux-imx@xxxxxxx> > Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch > via GPIO > > Hi, > > On 11-03-19 11:40, Jun Li wrote: > > Some typec super speed active channel switch can be controlled via a > > GPIO, this binding can be used to specify the switch node by a GPIO > > and the remote endpoint of its consumer. > > > > Signed-off-by: Li Jun <jun.li@xxxxxxx> > > --- > > .../devicetree/bindings/usb/typec-switch-gpio.txt | 30 > ++++++++++++++++++++++ > > 1 file changed, 30 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > new file mode 100644 > > index 0000000..4ef76cf > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt > > @@ -0,0 +1,30 @@ > > +Typec orientation switch via a GPIO > > +----------------------------------- > > + > > +Required properties: > > +- compatible: should be set one of following: > > + - "nxp,ptn36043" for NXP Type-C SuperSpeed active switch. > > + > > +- gpios: the GPIO used to switch the super speed active channel, > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > +- orientation-switch: must be present. > > Shouldn't this have usb-c in the propery name, e.g.: > usb-c-orientation-switch ? This is decided by drivers/usb/typec/mux.c:36 /* * With OF graph the mux node must have a boolean device property named * "orientation-switch". */ > > > + > > +Required sub-node: > > +- port: specify the remote endpoint of typec switch consumer. > > + > > +Example: > > + > > +ptn36043 { > > + compatible = "nxp,ptn36043"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_ss_sel>; > > + gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > + orientation-switch; > > + > > + port { > > + usb3_data_ss: endpoint { > > + remote-endpoint = <&typec_con_ss>; > > > Isn't this the wrong way around, shouldn't the "usb-c-connector" > compatible port be pointing to the orientation switch, rather then the other way > around? I am not sure I am getting your point, "usb-c-connector" is the user of typec switch, yes, it is pointing to the orientation switch provider(i.e, this example node). >Both will work in the end. but to me it feels more natural to group all the > info about the type-c connector together in the "usb-c-connector" compatible port > ptn36043 { compatible = "nxp,ptn36043"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_ss_sel>; gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; orientation-switch; port { usb3_data_ss: endpoint { remote-endpoint = <&typec_con_ss>; }; }; }; 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_data_ss>; }; }; }; }; Regards Jun > Regards, > > Hans