| Message ID | 1604403610-16577-1-git-send-email-jun.li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5,1/4] dt-bindings: usb: add documentation for typec switch simple driver | expand |
On Tue, Nov 03, 2020 at 07:40:07PM +0800, Li Jun wrote: > Some platforms need a simple driver to do some controls according to > typec orientation, this can be extended to be a generic driver with > compatible with "typec-orientation-switch". > > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > No changes for v5. > > changes on v4: > - Use compatible instead of bool property for switch matching. > - Change switch GPIO to be switch simple. > - Change the active channel selection GPIO to be optional. > > previous discussion: > http://patchwork.ozlabs.org/patch/1054342/ > > .../bindings/usb/typec-switch-simple.yaml | 69 ++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > new file mode 100644 > index 0000000..244162d > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > @@ -0,0 +1,69 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/usb/typec-switch-simple.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Typec Orientation Switch Simple Solution Bindings > + > +maintainers: > + - Li Jun <jun.li@nxp.com> > + > +description: |- > + USB SuperSpeed (SS) lanes routing to which side of typec connector is > + decided by orientation, this maybe achieved by some simple control like > + GPIO toggle. > + > +properties: > + compatible: > + const: typec-orientation-switch > + > + switch-gpios: > + description: | > + gpio specifier to switch the super speed active channel, > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > + GPIO_ACTIVE_LOW: GPIO state low for cc1. What does active mean? There isn't really an active and inactive state, right? It's more a mux selecting 0 or 1 input? I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's an inverter in the middle. > + maxItems: 1 > + > + port: > + type: object > + additionalProperties: false > + description: -| > + Connection to the remote endpoint using OF graph bindings that model SS > + data bus to typec connector. > + > + properties: > + endpoint: > + type: object > + additionalProperties: false > + > + properties: > + remote-endpoint: true > + > + required: > + - remote-endpoint > + > + required: > + - endpoint > + > +required: > + - compatible > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + ptn36043 { > + compatible = "typec-orientation-switch"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_ss_sel>; > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > + > + port { > + usb3_data_ss: endpoint { > + remote-endpoint = <&typec_con_ss>; The data goes from the connector to here and then where? You need a connection to the USB host controller. > + }; > + }; > + }; > -- > 2.7.4 >
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, November 6, 2020 6:26 AM > To: Jun Li <jun.li@nxp.com> > Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org; > gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com; > hdegoede@redhat.com; lee.jones@linaro.org; > mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com; > prabhakar.mahadev-lad.rj@bp.renesas.com; > laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Peter Chen > <peter.chen@nxp.com> > Subject: Re: [PATCH v5 1/4] dt-bindings: usb: add documentation for typec > switch simple driver > > On Tue, Nov 03, 2020 at 07:40:07PM +0800, Li Jun wrote: > > Some platforms need a simple driver to do some controls according to > > typec orientation, this can be extended to be a generic driver with > > compatible with "typec-orientation-switch". > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > --- > > No changes for v5. > > > > changes on v4: > > - Use compatible instead of bool property for switch matching. > > - Change switch GPIO to be switch simple. > > - Change the active channel selection GPIO to be optional. > > > > previous discussion: > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch > > > work.ozlabs.org%2Fpatch%2F1054342%2F&data=04%7C01%7Cjun.li%40nxp.c > > > om%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd99c5c3016 > > > 35%7C0%7C0%7C637402119664101856%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata= > > 8TY%2BPRIui6HhxhYE1%2BLmwWL38Vp7SY1Ceb5rGG%2B4DUo%3D&reserved=0 > > > > .../bindings/usb/typec-switch-simple.yaml | 69 > ++++++++++++++++++++++ > > 1 file changed, 69 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > new file mode 100644 > > index 0000000..244162d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > @@ -0,0 +1,69 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fschemas%2Fusb%2Ftypec-switch-simple.yaml%23&data=04% > > > +7C01%7Cjun.li%40nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1d3 > > > +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CTWF > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > +Mn0%3D%7C1000&sdata=HjWfKlDLyqb%2FKLlL6vdnyPe%2BnB8pSllhokIXQ%2Bw > > +yyw8%3D&reserved=0 > > +$schema: > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cjun.li%40 > > > +nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd99c > > > +5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CTWFpbGZsb3d8eyJWIjo > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&am > > > +p;sdata=z0bO47QVl5gw0UE%2Bx3a5E27ALgz568zp%2Bf4suGlch%2Fo%3D&rese > > +rved=0 > > + > > +title: Typec Orientation Switch Simple Solution Bindings > > + > > +maintainers: > > + - Li Jun <jun.li@nxp.com> > > + > > +description: |- > > + USB SuperSpeed (SS) lanes routing to which side of typec connector > > +is > > + decided by orientation, this maybe achieved by some simple control > > +like > > + GPIO toggle. > > + > > +properties: > > + compatible: > > + const: typec-orientation-switch > > + > > + switch-gpios: > > + description: | > > + gpio specifier to switch the super speed active channel, > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > What does active mean? There isn't really an active and inactive state, right? > It's more a mux selecting 0 or 1 input? Yes, I will change the description: gpio specifier to select the target channel of mux. > > I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's an inverter > in the middle. This depends on the switch IC design and board design, leave 2 flags (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) can cover all possible cases. NXP has 2 diff IC parts for this: 1. PTN36043(used on iMX8MQ) Output selection control When SEL=0, RX_AP_*/TX_AP_* are connected to RX_CON_2*/TX_CON_2*, and RX_CON_1*/TX_CON_1* are connected to VDD thru low ohmic resistor. When SEL=1, RX_AP_*/TX_AP_* are connected to RX_CON_1*/TX_CON_1*, and RX_CON_2*/TX_CON_2* are connected to VDD thru low ohmic resistor. Board design connects RX_CON_1*/TX_CON_1* to typec connector CC1, so GPIO_ACTIVE_HIGH 2. CBTU02043(used on iMX8MP) SEL Function -------------------------------------- Low A to B ports and vice versa High A to C ports and vice versa Board design connects B to typec connector CC1, so GPIO_ACTIVE_LOW Therefore, we need 2 flags. > > > + maxItems: 1 > > + > > + port: > > + type: object > > + additionalProperties: false > > + description: -| > > + Connection to the remote endpoint using OF graph bindings that model > SS > > + data bus to typec connector. > > + > > + properties: > > + endpoint: > > + type: object > > + additionalProperties: false > > + > > + properties: > > + remote-endpoint: true > > + > > + required: > > + - remote-endpoint > > + > > + required: > > + - endpoint > > + > > +required: > > + - compatible > > + - port > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + ptn36043 { > > + compatible = "typec-orientation-switch"; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_ss_sel>; > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > + > > + port { > > + usb3_data_ss: endpoint { > > + remote-endpoint = <&typec_con_ss>; > > The data goes from the connector to here and then where? You need a connection > to the USB host controller. The orientation switch only need interact with type-c, no any interaction with USB controller, do we still need a connection to it? Thanks Li Jun > > > + }; > > + }; > > + }; > > -- > > 2.7.4 > >
On Fri, Nov 6, 2020 at 5:07 AM Jun Li <jun.li@nxp.com> wrote: > > > > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Friday, November 6, 2020 6:26 AM > > To: Jun Li <jun.li@nxp.com> > > Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org; > > gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com; > > hdegoede@redhat.com; lee.jones@linaro.org; > > mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com; > > prabhakar.mahadev-lad.rj@bp.renesas.com; > > laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org; > > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Peter Chen > > <peter.chen@nxp.com> > > Subject: Re: [PATCH v5 1/4] dt-bindings: usb: add documentation for typec > > switch simple driver > > > > On Tue, Nov 03, 2020 at 07:40:07PM +0800, Li Jun wrote: > > > Some platforms need a simple driver to do some controls according to > > > typec orientation, this can be extended to be a generic driver with > > > compatible with "typec-orientation-switch". > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > --- > > > No changes for v5. > > > > > > changes on v4: > > > - Use compatible instead of bool property for switch matching. > > > - Change switch GPIO to be switch simple. > > > - Change the active channel selection GPIO to be optional. > > > > > > previous discussion: > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch > > > > > work.ozlabs.org%2Fpatch%2F1054342%2F&data=04%7C01%7Cjun.li%40nxp.c > > > > > om%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd99c5c3016 > > > > > 35%7C0%7C0%7C637402119664101856%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata= > > > 8TY%2BPRIui6HhxhYE1%2BLmwWL38Vp7SY1Ceb5rGG%2B4DUo%3D&reserved=0 > > > > > > .../bindings/usb/typec-switch-simple.yaml | 69 > > ++++++++++++++++++++++ > > > 1 file changed, 69 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > new file mode 100644 > > > index 0000000..244162d > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > @@ -0,0 +1,69 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > > +--- > > > +$id: > > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > > > +cetree.org%2Fschemas%2Fusb%2Ftypec-switch-simple.yaml%23&data=04% > > > > > +7C01%7Cjun.li%40nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1d3 > > > > > +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CTWF > > > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > > > +Mn0%3D%7C1000&sdata=HjWfKlDLyqb%2FKLlL6vdnyPe%2BnB8pSllhokIXQ%2Bw > > > +yyw8%3D&reserved=0 > > > +$schema: > > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi > > > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cjun.li%40 > > > > > +nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd99c > > > > > +5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CTWFpbGZsb3d8eyJWIjo > > > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&am > > > > > +p;sdata=z0bO47QVl5gw0UE%2Bx3a5E27ALgz568zp%2Bf4suGlch%2Fo%3D&rese > > > +rved=0 > > > + > > > +title: Typec Orientation Switch Simple Solution Bindings > > > + > > > +maintainers: > > > + - Li Jun <jun.li@nxp.com> > > > + > > > +description: |- > > > + USB SuperSpeed (SS) lanes routing to which side of typec connector > > > +is > > > + decided by orientation, this maybe achieved by some simple control > > > +like > > > + GPIO toggle. > > > + > > > +properties: > > > + compatible: > > > + const: typec-orientation-switch > > > + > > > + switch-gpios: > > > + description: | > > > + gpio specifier to switch the super speed active channel, > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > What does active mean? There isn't really an active and inactive state, right? > > It's more a mux selecting 0 or 1 input? > > Yes, I will change the description: > gpio specifier to select the target channel of mux. I wonder if the existing mux bindings should be used here. > > I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's an inverter > > in the middle. > > This depends on the switch IC design and board design, leave 2 flags > (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) can cover all possible cases. > > NXP has 2 diff IC parts for this: > 1. PTN36043(used on iMX8MQ) > Output selection control > When SEL=0, RX_AP_*/TX_AP_* are connected to RX_CON_2*/TX_CON_2*, and > RX_CON_1*/TX_CON_1* are connected to VDD thru low ohmic resistor. > When SEL=1, RX_AP_*/TX_AP_* are connected to RX_CON_1*/TX_CON_1*, and > RX_CON_2*/TX_CON_2* are connected to VDD thru low ohmic resistor. > > Board design connects RX_CON_1*/TX_CON_1* to typec connector CC1, > so GPIO_ACTIVE_HIGH > > 2. CBTU02043(used on iMX8MP) > SEL Function > -------------------------------------- > Low A to B ports and vice versa > High A to C ports and vice versa > > Board design connects B to typec connector CC1, so GPIO_ACTIVE_LOW > > Therefore, we need 2 flags. I'm not saying you don't. Just that the description is a bit odd. Please expand the description for how one decides how to set the flags. > > > > > > + maxItems: 1 > > > + > > > + port: > > > + type: object > > > + additionalProperties: false > > > + description: -| > > > + Connection to the remote endpoint using OF graph bindings that model > > SS > > > + data bus to typec connector. > > > + > > > + properties: > > > + endpoint: > > > + type: object > > > + additionalProperties: false > > > + > > > + properties: > > > + remote-endpoint: true > > > + > > > + required: > > > + - remote-endpoint > > > + > > > + required: > > > + - endpoint > > > + > > > +required: > > > + - compatible > > > + - port > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/gpio/gpio.h> > > > + ptn36043 { > > > + compatible = "typec-orientation-switch"; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&pinctrl_ss_sel>; > > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > > + > > > + port { > > > + usb3_data_ss: endpoint { > > > + remote-endpoint = <&typec_con_ss>; > > > > The data goes from the connector to here and then where? You need a connection > > to the USB host controller. > > The orientation switch only need interact with type-c, no any interaction > with USB controller, do we still need a connection to it? If you have 2 USB hosts and 2 connectors (and 2 muxes), how would you describe which connector goes with which host? Rob
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, November 6, 2020 10:24 PM > To: Jun Li <jun.li@nxp.com> > Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org; > gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com; > hdegoede@redhat.com; lee.jones@linaro.org; > mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com; > prabhakar.mahadev-lad.rj@bp.renesas.com; > laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Peter Chen > <peter.chen@nxp.com> > Subject: Re: [PATCH v5 1/4] dt-bindings: usb: add documentation for typec > switch simple driver > > On Fri, Nov 6, 2020 at 5:07 AM Jun Li <jun.li@nxp.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Rob Herring <robh@kernel.org> > > > Sent: Friday, November 6, 2020 6:26 AM > > > To: Jun Li <jun.li@nxp.com> > > > Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org; > > > gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com; > > > hdegoede@redhat.com; lee.jones@linaro.org; > > > mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com; > > > prabhakar.mahadev-lad.rj@bp.renesas.com; > > > laurent.pinchart+renesas@ideasonboard.com; > > > linux-usb@vger.kernel.org; devicetree@vger.kernel.org; dl-linux-imx > > > <linux-imx@nxp.com>; Peter Chen <peter.chen@nxp.com> > > > Subject: Re: [PATCH v5 1/4] dt-bindings: usb: add documentation for > > > typec switch simple driver > > > > > > On Tue, Nov 03, 2020 at 07:40:07PM +0800, Li Jun wrote: > > > > Some platforms need a simple driver to do some controls according > > > > to typec orientation, this can be extended to be a generic driver > > > > with compatible with "typec-orientation-switch". > > > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > > --- > > > > No changes for v5. > > > > > > > > changes on v4: > > > > - Use compatible instead of bool property for switch matching. > > > > - Change switch GPIO to be switch simple. > > > > - Change the active channel selection GPIO to be optional. > > > > > > > > previous discussion: > > > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpat > > > ch > > > > > > > work.ozlabs.org%2Fpatch%2F1054342%2F&data=04%7C01%7Cjun.li%40nxp > > > .c > > > > > > > om%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd99c5c30 > > > 16 > > > > > > > 35%7C0%7C0%7C637402119664101856%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w > > > Lj > > > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdat > > > a= > > > > 8TY%2BPRIui6HhxhYE1%2BLmwWL38Vp7SY1Ceb5rGG%2B4DUo%3D&reserved= > > > > 0 > > > > > > > > .../bindings/usb/typec-switch-simple.yaml | 69 > > > ++++++++++++++++++++++ > > > > 1 file changed, 69 insertions(+) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > > b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > > new file mode 100644 > > > > index 0000000..244162d > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.ya > > > > +++ ml > > > > @@ -0,0 +1,69 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML > > > > +1.2 > > > > +--- > > > > +$id: > > > > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde > > > +vi > > > > > > > +cetree.org%2Fschemas%2Fusb%2Ftypec-switch-simple.yaml%23&data=0 > > > +4% > > > > > > > +7C01%7Cjun.li%40nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1 > > > +d3 > > > > > > > +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CT > > > +WF > > > > > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVC > > > +I6 > > > > > > > +Mn0%3D%7C1000&sdata=HjWfKlDLyqb%2FKLlL6vdnyPe%2BnB8pSllhokIXQ%2 > > > +Bw > > > > +yyw8%3D&reserved=0 > > > > +$schema: > > > > > > > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde > > > +vi > > > > > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=04%7C01%7Cjun.li% > > > +40 > > > > > > > +nxp.com%7C78ca5205695149e2743208d881d9c753%7C686ea1d3bc2b4c6fa92cd9 > > > +9c > > > > > > > +5c301635%7C0%7C0%7C637402119664111854%7CUnknown%7CTWFpbGZsb3d8eyJWI > > > +jo > > > > > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000& > > > +am > > > > > > > +p;sdata=z0bO47QVl5gw0UE%2Bx3a5E27ALgz568zp%2Bf4suGlch%2Fo%3D&re > > > +se > > > > +rved=0 > > > > + > > > > +title: Typec Orientation Switch Simple Solution Bindings > > > > + > > > > +maintainers: > > > > + - Li Jun <jun.li@nxp.com> > > > > + > > > > +description: |- > > > > + USB SuperSpeed (SS) lanes routing to which side of typec > > > > +connector is > > > > + decided by orientation, this maybe achieved by some simple > > > > +control like > > > > + GPIO toggle. > > > > + > > > > +properties: > > > > + compatible: > > > > + const: typec-orientation-switch > > > > + > > > > + switch-gpios: > > > > + description: | > > > > + gpio specifier to switch the super speed active channel, > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > > > What does active mean? There isn't really an active and inactive state, > right? > > > It's more a mux selecting 0 or 1 input? > > > > Yes, I will change the description: > > gpio specifier to select the target channel of mux. > > I wonder if the existing mux bindings should be used here. If only consider typec switch via "gpio", existing "mux-gpio" binding may be used with property "mux-control-names" to be "typec-xxx" for match, but we still need create typec stuff at mux driver to hook to typec system, so little benefit, considering this binding is going to be for a generic typec orientation switch simple driver, I think a new typec binding make sense. > > > > I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's an > > > inverter in the middle. > > > > This depends on the switch IC design and board design, leave 2 flags > > (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) can cover all possible cases. > > > > NXP has 2 diff IC parts for this: > > 1. PTN36043(used on iMX8MQ) > > Output selection control > > When SEL=0, RX_AP_*/TX_AP_* are connected to RX_CON_2*/TX_CON_2*, and > > RX_CON_1*/TX_CON_1* are connected to VDD thru low ohmic resistor. > > When SEL=1, RX_AP_*/TX_AP_* are connected to RX_CON_1*/TX_CON_1*, and > > RX_CON_2*/TX_CON_2* are connected to VDD thru low ohmic resistor. > > > > Board design connects RX_CON_1*/TX_CON_1* to typec connector CC1, so > > GPIO_ACTIVE_HIGH > > > > 2. CBTU02043(used on iMX8MP) > > SEL Function > > -------------------------------------- > > Low A to B ports and vice versa > > High A to C ports and vice versa > > > > Board design connects B to typec connector CC1, so GPIO_ACTIVE_LOW > > > > Therefore, we need 2 flags. > > I'm not saying you don't. Just that the description is a bit odd. > Please expand the description for how one decides how to set the flags. Misunderstood your point, OK, I thought the "how to set the flags" was simple and clear enough: Use GPIO_ACTIVE_HIGH if GPIO physical state high is for cc1; or Use GPIO_ACTIVE_LOW if GPIO physical state low is for cc1. > > > > > > > > > > + maxItems: 1 > > > > + > > > > + port: > > > > + type: object > > > > + additionalProperties: false > > > > + description: -| > > > > + Connection to the remote endpoint using OF graph bindings > > > > + that model > > > SS > > > > + data bus to typec connector. > > > > + > > > > + properties: > > > > + endpoint: > > > > + type: object > > > > + additionalProperties: false > > > > + > > > > + properties: > > > > + remote-endpoint: true > > > > + > > > > + required: > > > > + - remote-endpoint > > > > + > > > > + required: > > > > + - endpoint > > > > + > > > > +required: > > > > + - compatible > > > > + - port > > > > + > > > > +additionalProperties: false > > > > + > > > > +examples: > > > > + - | > > > > + #include <dt-bindings/gpio/gpio.h> > > > > + ptn36043 { > > > > + compatible = "typec-orientation-switch"; > > > > + pinctrl-names = "default"; > > > > + pinctrl-0 = <&pinctrl_ss_sel>; > > > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > > > + > > > > + port { > > > > + usb3_data_ss: endpoint { > > > > + remote-endpoint = <&typec_con_ss>; > > > > > > The data goes from the connector to here and then where? You need a > > > connection to the USB host controller. > > > > The orientation switch only need interact with type-c, no any > > interaction with USB controller, do we still need a connection to it? > > If you have 2 USB hosts and 2 connectors (and 2 muxes), how would you describe > which connector goes with which host? One instance of typec orientation switch defined by this binding only for One typec connector. With that, my understanding is Whether a connection need be described depends on if the connector (typec driver) need notify the host controller driver to do something (e.g. role switch need a connection between controller node and connector node for controller driver to swap usb role). If the mux/switch control is transparent to usb host controller (e.g. my case, usb controller drivers normally don't need do anything for orientation change), there is no need to describe connection between orientation switch node and host controller node. Thanks Li Jun > > Rob
On Mon, Nov 9, 2020 at 6:24 AM Jun Li <jun.li@nxp.com> wrote: > > From: Rob Herring <robh@kernel.org> > > On Fri, Nov 6, 2020 at 5:07 AM Jun Li <jun.li@nxp.com> wrote: > > > > From: Rob Herring <robh@kernel.org> > > > > > +properties: > > > > > + compatible: > > > > > + const: typec-orientation-switch > > > > > + > > > > > + switch-gpios: > > > > > + description: | > > > > > + gpio specifier to switch the super speed active channel, > > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > > > > > What does active mean? There isn't really an active and inactive state, > > right? > > > > It's more a mux selecting 0 or 1 input? > > > > > > Yes, I will change the description: > > > gpio specifier to select the target channel of mux. > > > > I wonder if the existing mux bindings should be used here. > > If only consider typec switch via "gpio", existing "mux-gpio" > binding may be used with property "mux-control-names" to be > "typec-xxx" for match, but we still need create typec stuff at > mux driver to hook to typec system, so little benefit, considering > this binding is going to be for a generic typec orientation switch > simple driver, I think a new typec binding make sense. You can instantiate drivers without a compatible. That's just the easy way. However, using the mux binding doesn't necessarily mean you have to use 'mux-gpio'. Consider if you need to do more control than just the GPIO line. For example, the chips you mentioned may have a s/w controlled power supply or reset. Also, consider what the mux would look like with different control interfaces. That could be I2C or some sub-block in a PMIC or ??? I'm sure we already have some examples. I'm not happy with these piecemeal additions to TypeC related bindings that don't consider more than 1 h/w possibility. > > > > I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's an > > > > inverter in the middle. > > > > > > This depends on the switch IC design and board design, leave 2 flags > > > (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) can cover all possible cases. > > > > > > NXP has 2 diff IC parts for this: > > > 1. PTN36043(used on iMX8MQ) > > > Output selection control > > > When SEL=0, RX_AP_*/TX_AP_* are connected to RX_CON_2*/TX_CON_2*, and > > > RX_CON_1*/TX_CON_1* are connected to VDD thru low ohmic resistor. > > > When SEL=1, RX_AP_*/TX_AP_* are connected to RX_CON_1*/TX_CON_1*, and > > > RX_CON_2*/TX_CON_2* are connected to VDD thru low ohmic resistor. > > > > > > Board design connects RX_CON_1*/TX_CON_1* to typec connector CC1, so > > > GPIO_ACTIVE_HIGH > > > > > > 2. CBTU02043(used on iMX8MP) > > > SEL Function > > > -------------------------------------- > > > Low A to B ports and vice versa > > > High A to C ports and vice versa > > > > > > Board design connects B to typec connector CC1, so GPIO_ACTIVE_LOW > > > > > > Therefore, we need 2 flags. > > > > I'm not saying you don't. Just that the description is a bit odd. > > Please expand the description for how one decides how to set the flags. > > Misunderstood your point, OK, I thought the "how to set the flags" was > simple and clear enough: > Use GPIO_ACTIVE_HIGH if GPIO physical state high is for cc1; or > Use GPIO_ACTIVE_LOW if GPIO physical state low is for cc1. Okay. > > > > > +examples: > > > > > + - | > > > > > + #include <dt-bindings/gpio/gpio.h> > > > > > + ptn36043 { > > > > > + compatible = "typec-orientation-switch"; > > > > > + pinctrl-names = "default"; > > > > > + pinctrl-0 = <&pinctrl_ss_sel>; > > > > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > > > > + > > > > > + port { > > > > > + usb3_data_ss: endpoint { > > > > > + remote-endpoint = <&typec_con_ss>; > > > > > > > > The data goes from the connector to here and then where? You need a > > > > connection to the USB host controller. > > > > > > The orientation switch only need interact with type-c, no any > > > interaction with USB controller, do we still need a connection to it? > > > > If you have 2 USB hosts and 2 connectors (and 2 muxes), how would you describe > > which connector goes with which host? > > One instance of typec orientation switch defined by this binding only for > One typec connector. With that, my understanding is > Whether a connection need be described depends on if the connector > (typec driver) need notify the host controller driver to do something > (e.g. role switch need a connection between controller node and connector > node for controller driver to swap usb role). If the mux/switch control is > transparent to usb host controller (e.g. my case, usb controller drivers > normally don't need do anything for orientation change), there is no need > to describe connection between orientation switch node and host controller > node. There can be several reasons you need to know the association. When writing the DT you can't assume what information is or isn't needed. That may vary by h/w or can evolve in an OS and the DT shouldn't change. Rob
Hi Rob > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Monday, November 9, 2020 10:38 PM > To: Jun Li <jun.li@nxp.com> > Cc: heikki.krogerus@linux.intel.com; rafael@kernel.org; > gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com; > hdegoede@redhat.com; lee.jones@linaro.org; > mika.westerberg@linux.intel.com; dmitry.torokhov@gmail.com; > prabhakar.mahadev-lad.rj@bp.renesas.com; > laurent.pinchart+renesas@ideasonboard.com; linux-usb@vger.kernel.org; > devicetree@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Peter Chen > <peter.chen@nxp.com> > Subject: Re: [PATCH v5 1/4] dt-bindings: usb: add documentation for typec > switch simple driver > > On Mon, Nov 9, 2020 at 6:24 AM Jun Li <jun.li@nxp.com> wrote: > > > From: Rob Herring <robh@kernel.org> > > > On Fri, Nov 6, 2020 at 5:07 AM Jun Li <jun.li@nxp.com> wrote: > > > > > From: Rob Herring <robh@kernel.org> > > > > > > > +properties: > > > > > > + compatible: > > > > > > + const: typec-orientation-switch > > > > > > + > > > > > > + switch-gpios: > > > > > > + description: | > > > > > > + gpio specifier to switch the super speed active channel, > > > > > > + GPIO_ACTIVE_HIGH: GPIO state high for cc1; > > > > > > + GPIO_ACTIVE_LOW: GPIO state low for cc1. > > > > > > > > > > What does active mean? There isn't really an active and inactive > > > > > state, > > > right? > > > > > It's more a mux selecting 0 or 1 input? > > > > > > > > Yes, I will change the description: > > > > gpio specifier to select the target channel of mux. > > > > > > I wonder if the existing mux bindings should be used here. > > > > If only consider typec switch via "gpio", existing "mux-gpio" > > binding may be used with property "mux-control-names" to be > > "typec-xxx" for match, but we still need create typec stuff at mux > > driver to hook to typec system, so little benefit, considering this > > binding is going to be for a generic typec orientation switch simple > > driver, I think a new typec binding make sense. > > You can instantiate drivers without a compatible. That's just the easy way. > However, using the mux binding doesn't necessarily mean you have to use > 'mux-gpio'. Consider if you need to do more control than just the GPIO line. > For example, the chips you mentioned may have a s/w controlled power supply > or reset. > > Also, consider what the mux would look like with different control interfaces. > That could be I2C or some sub-block in a PMIC or ??? I'm sure we already > have some examples. I'm not happy with these piecemeal additions to TypeC > related bindings that don't consider more than 1 h/w possibility. As typec class sub system already has its own interface(and users) to do switch/mux control, using existing mux bindings means typec class will add switch/mux control through another approach(mux chip/controller interface), this makes the typec mux looks having 2 similar way for the same function. so I was hesitating to use it. After more check, I agree creating typec specific bindings will duplicate much existing mux bindings, the mux controller based bindings can be a good way used for various typec switch/mux, so I will try typec orientation compatible with mux controller bindings. > > > > > > I think you want flags 0 (aka GPIO_ACTIVE_HIGH) unless there's > > > > > an inverter in the middle. > > > > > > > > This depends on the switch IC design and board design, leave 2 > > > > flags (GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW) can cover all possible > cases. > > > > > > > > NXP has 2 diff IC parts for this: > > > > 1. PTN36043(used on iMX8MQ) > > > > Output selection control > > > > When SEL=0, RX_AP_*/TX_AP_* are connected to RX_CON_2*/TX_CON_2*, > > > > and > > > > RX_CON_1*/TX_CON_1* are connected to VDD thru low ohmic resistor. > > > > When SEL=1, RX_AP_*/TX_AP_* are connected to RX_CON_1*/TX_CON_1*, > > > > and > > > > RX_CON_2*/TX_CON_2* are connected to VDD thru low ohmic resistor. > > > > > > > > Board design connects RX_CON_1*/TX_CON_1* to typec connector CC1, > > > > so GPIO_ACTIVE_HIGH > > > > > > > > 2. CBTU02043(used on iMX8MP) > > > > SEL Function > > > > -------------------------------------- > > > > Low A to B ports and vice versa > > > > High A to C ports and vice versa > > > > > > > > Board design connects B to typec connector CC1, so GPIO_ACTIVE_LOW > > > > > > > > Therefore, we need 2 flags. > > > > > > I'm not saying you don't. Just that the description is a bit odd. > > > Please expand the description for how one decides how to set the flags. > > > > Misunderstood your point, OK, I thought the "how to set the flags" was > > simple and clear enough: > > Use GPIO_ACTIVE_HIGH if GPIO physical state high is for cc1; or Use > > GPIO_ACTIVE_LOW if GPIO physical state low is for cc1. > > Okay. > > > > > > > +examples: > > > > > > + - | > > > > > > + #include <dt-bindings/gpio/gpio.h> > > > > > > + ptn36043 { > > > > > > + compatible = "typec-orientation-switch"; > > > > > > + pinctrl-names = "default"; > > > > > > + pinctrl-0 = <&pinctrl_ss_sel>; > > > > > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; > > > > > > + > > > > > > + port { > > > > > > + usb3_data_ss: endpoint { > > > > > > + remote-endpoint = <&typec_con_ss>; > > > > > > > > > > The data goes from the connector to here and then where? You > > > > > need a connection to the USB host controller. > > > > > > > > The orientation switch only need interact with type-c, no any > > > > interaction with USB controller, do we still need a connection to it? > > > > > > If you have 2 USB hosts and 2 connectors (and 2 muxes), how would > > > you describe which connector goes with which host? > > > > One instance of typec orientation switch defined by this binding only > > for One typec connector. With that, my understanding is Whether a > > connection need be described depends on if the connector (typec > > driver) need notify the host controller driver to do something (e.g. > > role switch need a connection between controller node and connector > > node for controller driver to swap usb role). If the mux/switch > > control is transparent to usb host controller (e.g. my case, usb > > controller drivers normally don't need do anything for orientation > > change), there is no need to describe connection between orientation > > switch node and host controller node. > > There can be several reasons you need to know the association. When writing > the DT you can't assume what information is or isn't needed. > That may vary by h/w or can evolve in an OS and the DT shouldn't change. Okay, I will add the connection to usb controller in example. Thanks Li Jun > > Rob
diff --git a/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml new file mode 100644 index 0000000..244162d --- /dev/null +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/usb/typec-switch-simple.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Typec Orientation Switch Simple Solution Bindings + +maintainers: + - Li Jun <jun.li@nxp.com> + +description: |- + USB SuperSpeed (SS) lanes routing to which side of typec connector is + decided by orientation, this maybe achieved by some simple control like + GPIO toggle. + +properties: + compatible: + const: typec-orientation-switch + + switch-gpios: + description: | + gpio specifier to switch the super speed active channel, + GPIO_ACTIVE_HIGH: GPIO state high for cc1; + GPIO_ACTIVE_LOW: GPIO state low for cc1. + maxItems: 1 + + port: + type: object + additionalProperties: false + description: -| + Connection to the remote endpoint using OF graph bindings that model SS + data bus to typec connector. + + properties: + endpoint: + type: object + additionalProperties: false + + properties: + remote-endpoint: true + + required: + - remote-endpoint + + required: + - endpoint + +required: + - compatible + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + ptn36043 { + compatible = "typec-orientation-switch"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_ss_sel>; + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>; + + port { + usb3_data_ss: endpoint { + remote-endpoint = <&typec_con_ss>; + }; + }; + };
Some platforms need a simple driver to do some controls according to typec orientation, this can be extended to be a generic driver with compatible with "typec-orientation-switch". Signed-off-by: Li Jun <jun.li@nxp.com> --- No changes for v5. changes on v4: - Use compatible instead of bool property for switch matching. - Change switch GPIO to be switch simple. - Change the active channel selection GPIO to be optional. previous discussion: http://patchwork.ozlabs.org/patch/1054342/ .../bindings/usb/typec-switch-simple.yaml | 69 ++++++++++++++++++++++ 1 file changed, 69 insertions(+)