| Message ID | 1552299557-6306-1-git-send-email-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/2] dt-bindings: usb: add documentation for typec switch via GPIO | expand |
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@nxp.com> > --- > .../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 ? > + > +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
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@nxp.com> >> --- >> .../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 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. 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 >
Hi Hans > -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: 2019年3月11日 19:03 > To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; heikki.krogerus@linux.intel.com > Cc: gregkh@linuxfoundation.org; andy.shevchenko@gmail.com; > linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > 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@nxp.com> > > --- > > .../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
Hi, On Tue, Mar 12, 2019 at 10:32:00AM +0000, Jun Li wrote: > Hi Hans > > -----Original Message----- > > From: Hans de Goede <hdegoede@redhat.com> > > Sent: 2019年3月11日 19:03 > > To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; heikki.krogerus@linux.intel.com > > Cc: gregkh@linuxfoundation.org; andy.shevchenko@gmail.com; > > linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx > > <linux-imx@nxp.com> > > 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@nxp.com> > > > --- > > > .../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,
Hi, On 12-03-19 11:45, Heikki Krogerus wrote: > Hi, > > On Tue, Mar 12, 2019 at 10:32:00AM +0000, Jun Li wrote: >> Hi Hans >>> -----Original Message----- >>> From: Hans de Goede <hdegoede@redhat.com> >>> Sent: 2019年3月11日 19:03 >>> To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; heikki.krogerus@linux.intel.com >>> Cc: gregkh@linuxfoundation.org; andy.shevchenko@gmail.com; >>> linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx >>> <linux-imx@nxp.com> >>> 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@nxp.com> >>>> --- >>>> .../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. Ah I see, thank you for clarifying that. Regards, Hans
On Mon, Mar 11, 2019 at 10:40:09AM +0000, 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@nxp.com> > --- > .../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, Perhaps switch-gpios in case there are other gpios needed. > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > +- orientation-switch: must be present. Why is this needed? The compatible can't imply this? > + > +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>; > + }; > + }; > +}; > -- > 2.7.4 >
On Tue, Mar 12, 2019 at 09:45:27AM -0500, Rob Herring wrote: > On Mon, Mar 11, 2019 at 10:40:09AM +0000, 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@nxp.com> > > --- > > .../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, > > Perhaps switch-gpios in case there are other gpios needed. > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > +- orientation-switch: must be present. > > Why is this needed? The compatible can't imply this? I think Jun Li is added that based on the comment I put to drivers/usb/typec/mux.c, so I'm to blame here. If we can handle this with the compatible like I guess we can, I'm happy. thanks,
> -----Original Message----- > From: Hans de Goede <hdegoede@redhat.com> > Sent: 2019年3月11日 19:12 > To: Jun Li <jun.li@nxp.com>; robh+dt@kernel.org; heikki.krogerus@linux.intel.com > Cc: gregkh@linuxfoundation.org; andy.shevchenko@gmail.com; > linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx > <linux-imx@nxp.com> > 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@nxp.com> > >> --- > >> .../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 > >
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: 2019年3月12日 22:45 > To: Jun Li <jun.li@nxp.com> > Cc: heikki.krogerus@linux.intel.com; gregkh@linuxfoundation.org; > hdegoede@redhat.com; andy.shevchenko@gmail.com; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch > via GPIO > > On Mon, Mar 11, 2019 at 10:40:09AM +0000, 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@nxp.com> > > --- > > .../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, > > Perhaps switch-gpios in case there are other gpios needed. OK, I will change it to be switch-gpios. > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > +- orientation-switch: must be present. > > Why is this needed? The compatible can't imply this? I will try to remove this bool property and use a general compatible string. > > > + > > +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>; > > + }; > > + }; > > +}; > > -- > > 2.7.4 > >
> -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: 2019年3月13日 17:36 > To: Rob Herring <robh@kernel.org> > Cc: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org; hdegoede@redhat.com; > andy.shevchenko@gmail.com; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch > via GPIO > > On Tue, Mar 12, 2019 at 09:45:27AM -0500, Rob Herring wrote: > > On Mon, Mar 11, 2019 at 10:40:09AM +0000, 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@nxp.com> > > > --- > > > .../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, > > > > Perhaps switch-gpios in case there are other gpios needed. > > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > +- orientation-switch: must be present. > > > > Why is this needed? The compatible can't imply this? > > I think Jun Li is added that based on the comment I put to drivers/usb/typec/mux.c, > so I'm to blame here. If we can handle this with the compatible like I guess we can, > I'm happy. Hi Heikki Can I just remove the original bool property check? i.e, match OK if the remote parent node is in switch_list. Thanks Li Jun > > thanks, > > -- > heikki
On Mon, Mar 18, 2019 at 10:59:31AM +0000, Jun Li wrote: > > > > -----Original Message----- > > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Sent: 2019年3月13日 17:36 > > To: Rob Herring <robh@kernel.org> > > Cc: Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org; hdegoede@redhat.com; > > andy.shevchenko@gmail.com; linux-usb@vger.kernel.org; > > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > > Subject: Re: [PATCH v3 1/2] dt-bindings: usb: add documentation for typec switch > > via GPIO > > > > On Tue, Mar 12, 2019 at 09:45:27AM -0500, Rob Herring wrote: > > > On Mon, Mar 11, 2019 at 10:40:09AM +0000, 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@nxp.com> > > > > --- > > > > .../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, > > > > > > Perhaps switch-gpios in case there are other gpios needed. > > > > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > +- orientation-switch: must be present. > > > > > > Why is this needed? The compatible can't imply this? > > > > I think Jun Li is added that based on the comment I put to drivers/usb/typec/mux.c, > > so I'm to blame here. If we can handle this with the compatible like I guess we can, > > I'm happy. > > Hi Heikki > > Can I just remove the original bool property check? i.e, match OK if the remote > parent node is in switch_list. No. If typec_switch_get() is called before the mux device is registered, we need to return -EPROBE_DEFER. That means we need to be able to identify the mux device node. I think we should just use the compatible like Rob suggested. The Type-C muxes should probable have their own bindings file where it's defined for these muxes. thanks,
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. + +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>; + }; + }; +};
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@nxp.com> --- .../devicetree/bindings/usb/typec-switch-gpio.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+)