| Message ID | 1604403610-16577-1-git-send-email-jun.li@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v5,1/4] dt-bindings: usb: add documentation for typec switch simple driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
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
On Tue, Nov 03, 2020 at 07:40:10PM +0800, Li Jun wrote: > This patch adds a simple typec switch driver for cases which only > needs some simple operations but a dedicated driver is required, > current driver only supports GPIO toggle to switch the super speed > active channel according to typec orientation. > > Signed-off-by: Li Jun <jun.li@nxp.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > Changes for v5: > - A few changes address Andy's comment, remove gpio check as it's > optional, add module name for Kconfig, use correct header files, > and other minor changes. > - Remove the mutex lock as it's not required currently. > > Changes for v4: > - Change driver name to be switch simple from switch GPIO, to make it > generic for possible extention. > - Use compatiable "typec-orientation-switch" instead of bool property > for switch matching. > - Make acitve channel selection GPIO to be optional. > - Remove Andy's R-b tag since the driver changes a lot. > > Change for v3: > - Remove file name in driver description. > - Add Andy Shevchenko's Reviewed-by tag. > > Changes for v2: > - Use the correct head files for gpio api and of_device_id: > #include <linux/gpio/consumer.h> > #include <linux/mod_devicetable.h> > - Add driver dependency on GPIOLIB > > drivers/usb/typec/mux/Kconfig | 10 ++++ > drivers/usb/typec/mux/Makefile | 1 + > drivers/usb/typec/mux/switch-simple.c | 100 ++++++++++++++++++++++++++++++++++ > 3 files changed, 111 insertions(+) > > diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig > index a4dbd11..11320d7 100644 > --- a/drivers/usb/typec/mux/Kconfig > +++ b/drivers/usb/typec/mux/Kconfig > @@ -18,4 +18,14 @@ config TYPEC_MUX_INTEL_PMC > control the USB role switch and also the multiplexer/demultiplexer > switches used with USB Type-C Alternate Modes. > > +config TYPEC_SWITCH_SIMPLE > + tristate "Type-C orientation switch simple driver" > + depends on GPIOLIB > + help > + Say Y or M if your system need a simple driver for typec switch > + control, like use GPIO to select active channel. > + > + To compile this driver as a module, choose M here: the > + module will be called switch-simple. > + > endmenu > diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile > index 280a6f5..712d0ad 100644 > --- a/drivers/usb/typec/mux/Makefile > +++ b/drivers/usb/typec/mux/Makefile > @@ -2,3 +2,4 @@ > > obj-$(CONFIG_TYPEC_MUX_PI3USB30532) += pi3usb30532.o > obj-$(CONFIG_TYPEC_MUX_INTEL_PMC) += intel_pmc_mux.o > +obj-$(CONFIG_TYPEC_SWITCH_SIMPLE) += switch-simple.o > diff --git a/drivers/usb/typec/mux/switch-simple.c b/drivers/usb/typec/mux/switch-simple.c > new file mode 100644 > index 0000000..8707703 > --- /dev/null > +++ b/drivers/usb/typec/mux/switch-simple.c > @@ -0,0 +1,100 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Type-C switch simple control driver > + * > + * Copyright 2020 NXP > + * Author: Jun Li <jun.li@nxp.com> > + */ > + > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/mutex.h> > +#include <linux/platform_device.h> > +#include <linux/usb/typec_mux.h> > + > +struct typec_switch_simple { > + struct typec_switch *sw; > + struct gpio_desc *sel_gpio; > +}; > + > +static int typec_switch_simple_set(struct typec_switch *sw, > + enum typec_orientation orientation) > +{ > + struct typec_switch_simple *typec_sw = typec_switch_get_drvdata(sw); > + > + switch (orientation) { > + case TYPEC_ORIENTATION_NORMAL: > + gpiod_set_value_cansleep(typec_sw->sel_gpio, 1); > + break; > + case TYPEC_ORIENTATION_REVERSE: > + gpiod_set_value_cansleep(typec_sw->sel_gpio, 0); > + break; > + case TYPEC_ORIENTATION_NONE: > + break; > + } > + > + return 0; > +} > + > +static int typec_switch_simple_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct typec_switch_desc sw_desc; > + struct typec_switch_simple *typec_sw; > + > + typec_sw = devm_kzalloc(dev, sizeof(*typec_sw), GFP_KERNEL); > + if (!typec_sw) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, typec_sw); > + > + sw_desc.drvdata = typec_sw; > + sw_desc.fwnode = dev->fwnode; > + sw_desc.set = typec_switch_simple_set; > + > + /* Get the super speed active channel selection GPIO */ > + typec_sw->sel_gpio = devm_gpiod_get_optional(dev, "switch", GPIOD_OUT_LOW); > + if (IS_ERR(typec_sw->sel_gpio)) > + return PTR_ERR(typec_sw->sel_gpio); > + > + typec_sw->sw = typec_switch_register(dev, &sw_desc); > + if (IS_ERR(typec_sw->sw)) { > + dev_err(dev, "Error registering typec switch: %ld\n", > + PTR_ERR(typec_sw->sw)); > + return PTR_ERR(typec_sw->sw); > + } > + > + return 0; > +} > + > +static int typec_switch_simple_remove(struct platform_device *pdev) > +{ > + struct typec_switch_simple *typec_sw = platform_get_drvdata(pdev); > + > + typec_switch_unregister(typec_sw->sw); > + > + return 0; > +} > + > +static const struct of_device_id of_typec_switch_simple_match[] = { > + { .compatible = "typec-orientation-switch" }, > + { /* Sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, of_typec_switch_simple_match); > + > +static struct platform_driver typec_switch_simple_driver = { > + .probe = typec_switch_simple_probe, > + .remove = typec_switch_simple_remove, > + .driver = { > + .name = "typec-switch-simple", > + .of_match_table = of_typec_switch_simple_match, > + }, > +}; > + > +module_platform_driver(typec_switch_simple_driver); > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("TypeC Orientation Switch Simple driver"); > +MODULE_AUTHOR("Jun Li <jun.li@nxp.com>"); > -- > 2.7.4 thanks,
> -----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
On Tue, Nov 03, 2020 at 07:40:09PM +0800, Li Jun wrote: > For those need a dedicated typec switch simple solution driver, > use compatible property for matching. > > Signed-off-by: Li Jun <jun.li@nxp.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > No changes for v5 > New patch for v4 > > drivers/usb/typec/mux.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c > index 52ad277..3da17d1 100644 > --- a/drivers/usb/typec/mux.c > +++ b/drivers/usb/typec/mux.c > @@ -39,7 +39,8 @@ static void *typec_switch_match(struct device_connection *con, int ep, > { > struct device *dev; > > - if (con->id && !fwnode_property_present(con->fwnode, con->id)) > + if (con->id && !fwnode_is_compatible(con->fwnode, con->id) && > + !fwnode_property_present(con->fwnode, con->id)) > return NULL; > > dev = class_find_device(&typec_mux_class, NULL, con->fwnode, > @@ -61,8 +62,8 @@ struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode) > { > struct typec_switch *sw; > > - sw = fwnode_connection_find_match(fwnode, "orientation-switch", NULL, > - typec_switch_match); > + sw = fwnode_connection_find_match(fwnode, "typec-orientation-switch", > + NULL, typec_switch_match); > if (!IS_ERR_OR_NULL(sw)) > WARN_ON(!try_module_get(sw->dev.parent->driver->owner)); > > -- > 2.7.4 thanks,
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
> -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Monday, November 9, 2020 4:36 PM > To: Jun Li <jun.li@nxp.com> > Cc: robh+dt@kernel.org; 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 4/4] usb: typec: mux: add typec switch simple driver > > On Tue, Nov 03, 2020 at 07:40:10PM +0800, Li Jun wrote: > > This patch adds a simple typec switch driver for cases which only > > needs some simple operations but a dedicated driver is required, > > current driver only supports GPIO toggle to switch the super speed > > active channel according to typec orientation. > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> Hi, Heikki, I have to drop your A-b tag as the driver updated to use mux bindings (drivers/mux/), see v6: https://patchwork.kernel.org/project/linux-usb/patch/1606140096-1382-6-git-send-email-jun.li@nxp.com/ thanks Li Jun
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(+)