RE: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch via GPIO

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

 




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




[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