> -----Original Message----- > From: Hans de Goede <hdegoede@xxxxxxxxxx> > Sent: 2019年3月11日 19:12 > 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 12:02, Hans de Goede wrote: > > 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. > > Hmm, it seems that this binding should work fine with other orientation-switches as > well, so I think this needs a generic compatible string. > > >> + > >> +- 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 ? > > Also perhaps it would be better to use an additional compatible string for this, rather > then a boolean property, because what you are trying to say is that this device is > compatible with some (to be written) generic usb-c-orientation-switch binding. > > So I think you may want to use an extra compatible for this and describe the > port/graph usage linking the usb-c-connector port and the port on the > orientation-switch together in a new usb-c-orientation-switch binding document. > This patch was to only cover one kind of *typical* typec switch: done by GPIO toggle, as I don't know how other typec switch may be implemented, I will try to change this to be a *common* typec switch by using a generic compatible(type-c-orientation-switch), which will for now only support switch-gpios. > This new binding will then document the port usage which is mostly undocumented > in your typec-switch-gpio.txt binding and this port usage documentation can then be > re-used for other orientation-switch bindings. Port usage should be the same as I gave the example: https://www.spinics.net/lists/devicetree/msg278042.html thanks Li Jun > > Regards, > > Hans > > > > > >> + > >> +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? 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 > > > > Regards, > > > > Hans > >
