Hi,
On Tue, Mar 12, 2019 at 10:32:00AM +0000, Jun Li wrote:
> 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".
> */
Yes, but it's still OK to change it. It's not documented anywhere yet.
> > > +
> > > +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?
Hans, in OF graph both endpoints will have a remote-endpoint pointing
to each other..
> 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>;
> };
> };
> };
> };
So like that.
thanks,
--
heikki
