| Message ID | 1606140096-1382-1-git-send-email-jun.li@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v6,1/6] dt-bindings: usb: add documentation for typec switch simple driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
On Mon, Nov 23, 2020 at 10:01:35PM +0800, Li Jun wrote: > The general mux controller can be easily extended to support > various mux selection, this especially fits typec orientation > switch block with a dedicated driver. ... > @@ -42,10 +43,8 @@ static void *typec_switch_match(struct device_connection *con, int ep, > 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, > switch_fwnode_match); > - > return dev ? to_typec_switch(dev) : ERR_PTR(-EPROBE_DEFER); > } > Not related change. ... > + if (sw->mux_ctrl) > + return typec_switch_mux_ctrl(sw->mux_ctrl, orientation); > + else Redundant. > + return sw->set(sw, orientation);
Hi, > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: Monday, November 23, 2020 11:31 PM > To: Jun Li <jun.li@nxp.com> > Cc: heikki.krogerus@linux.intel.com; robh+dt@kernel.org; > rafael@kernel.org; gregkh@linuxfoundation.org; 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 v6 5/6] usb: typec: mux: add typec switch via general > mux control > > On Mon, Nov 23, 2020 at 10:01:35PM +0800, Li Jun wrote: > > The general mux controller can be easily extended to support various > > mux selection, this especially fits typec orientation switch block > > with a dedicated driver. > > ... > > > @@ -42,10 +43,8 @@ static void *typec_switch_match(struct > device_connection *con, int ep, > > 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, > > switch_fwnode_match); > > - > > return dev ? to_typec_switch(dev) : ERR_PTR(-EPROBE_DEFER); } > > > > Not related change. Oops, above 2 blank lines should be kept, I will remove the changes. > > ... > > > + if (sw->mux_ctrl) > > + return typec_switch_mux_ctrl(sw->mux_ctrl, orientation); > > > + else > > Redundant. will remove it. Thanks Li Jun > > > + return sw->set(sw, orientation); > > -- > With Best Regards, > Andy Shevchenko >
On Mon, Nov 23, 2020 at 10:01:34PM +0800, Li Jun wrote: > As the typec switch provider driver's dev may not the parent of > typec_switch dev(may be the switch's parent's parent), but we use > the typec switch provider driver dev's fwnode anyway, so change to > use fwnode's dev to get driver's user count. > > Signed-off-by: Li Jun <jun.li@nxp.com> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > New patch for v6. > > drivers/usb/typec/mux.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c > index 3da17d1..0219aa4 100644 > --- a/drivers/usb/typec/mux.c > +++ b/drivers/usb/typec/mux.c > @@ -65,7 +65,7 @@ struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode) > 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)); > + WARN_ON(!try_module_get(dev_fwnode(&sw->dev)->dev->driver->owner)); > > return sw; > } > @@ -80,7 +80,7 @@ EXPORT_SYMBOL_GPL(fwnode_typec_switch_get); > void typec_switch_put(struct typec_switch *sw) > { > if (!IS_ERR_OR_NULL(sw)) { > - module_put(sw->dev.parent->driver->owner); > + module_put(dev_fwnode(&sw->dev)->dev->driver->owner); > put_device(&sw->dev); > } > } > -- > 2.7.4 thanks,
On Mon, Nov 23, 2020 at 10:01:36PM +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 v6: > - Use general mux control for typec switch, currently only support > max 2 GPIOs to control one typec switch with 3 states: orientation > normal(cc1), reverse(cc2), none(deselected both channels). > > 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 | 126 ++++++++++++++++++++++++++++++++++ > 3 files changed, 137 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..4671169 > --- /dev/null > +++ b/drivers/usb/typec/mux/switch-simple.c > @@ -0,0 +1,126 @@ > +// 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/mux/driver.h> > +#include <linux/platform_device.h> > +#include <linux/usb/typec_mux.h> > + > +struct typec_switch_simple { > + struct typec_switch *sw; > + struct mux_control *mux; > + struct gpio_descs *switch_gpios; > +}; > + > +static int typec_switch_simple_mux_set(struct mux_control *mux, int state) > +{ > + struct typec_switch_simple *typec_ss = mux_chip_priv(mux->chip); > + DECLARE_BITMAP(values, BITS_PER_TYPE(state)); > + > + values[0] = state; > + gpiod_set_array_value_cansleep(typec_ss->switch_gpios->ndescs, > + typec_ss->switch_gpios->desc, > + typec_ss->switch_gpios->info, > + values); > + > + return 0; > +} > + > +static const struct mux_control_ops typec_switch_mux_ops = { > + .set = typec_switch_simple_mux_set, > +}; > + > +static int typec_switch_simple_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mux_chip *mux_chip; > + struct typec_switch_desc sw_desc; > + struct typec_switch_simple *typec_ss; > + int idle_state; > + int pins; > + int ret; > + > + pins = gpiod_count(dev, "switch"); > + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*typec_ss)); > + if (IS_ERR(mux_chip)) > + return PTR_ERR(mux_chip); > + > + typec_ss = mux_chip_priv(mux_chip); > + mux_chip->ops = &typec_switch_mux_ops; > + > + /* Get the super speed active channel selection GPIO */ > + typec_ss->switch_gpios = devm_gpiod_get_array_optional(dev, > + "switch", GPIOD_OUT_LOW); > + if (IS_ERR(typec_ss->switch_gpios)) > + return PTR_ERR(typec_ss->switch_gpios); > + > + if (pins) { > + WARN_ON(pins != typec_ss->switch_gpios->ndescs || pins > 2); > + mux_chip->mux->states = pins + 1; > + } > + > + ret = device_property_read_u32(dev, "idle-state", (u32 *)&idle_state); > + if (ret >= 0 && idle_state != MUX_IDLE_AS_IS) { > + if (idle_state < 0 || idle_state >= mux_chip->mux->states) { > + dev_err(dev, "invalid idle-state %u\n", idle_state); > + return -EINVAL; > + } > + mux_chip->mux->idle_state = idle_state; > + } > + > + ret = devm_mux_chip_register(dev, mux_chip); > + if (ret < 0) > + return ret; > + > + platform_set_drvdata(pdev, typec_ss); > + sw_desc.drvdata = typec_ss; > + sw_desc.fwnode = dev->fwnode; > + > + typec_ss->sw = typec_switch_register(&mux_chip->dev, &sw_desc); > + if (IS_ERR(typec_ss->sw)) { > + dev_err(dev, "Error registering typec switch: %ld\n", > + PTR_ERR(typec_ss->sw)); > + return PTR_ERR(typec_ss->sw); > + } > + > + return 0; > +} > + > +static int typec_switch_simple_remove(struct platform_device *pdev) > +{ > + struct typec_switch_simple *typec_ss = platform_get_drvdata(pdev); > + > + typec_switch_unregister(typec_ss->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,
On Mon, Nov 23, 2020 at 10:01:31PM +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> > --- > changes for v6: > - Use general mux bindings for typec switch, one typec switch > may have 1 or 2 GPIOs for channel selection, if 1 GPIO, only > can be used to select cc1 or cc2; if 2 GPIOs, the second GPIO > can be used to deselect both channels or keep normal operations. > - Add one more connection to usb controller. > > 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 | 122 +++++++++++++++++++++ > 1 file changed, 122 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..030ade5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > @@ -0,0 +1,122 @@ > +# 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 > + > + '#mux-control-cells': > + const: 0 Who is the mux-control consumer? > + > + switch-gpios: > + description: | > + GPIO specifiers to select the target channel of mux. > + The first GPIO is for cc1 and cc2 selection, the GPIO flag 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. > + The second gpio is to deselect any channles by places all channels > + in high-impedance state to reduce current consumption, the gpio flag > + use GPIO_ACTIVE_HIGH if GPIO physical state high is for > + high-impedance state (so low for normal operations); or Use > + GPIO_ACTIVE_LOW if GPIO physical state low is for high-impedance > + state(so high for normal operations). > + minItems: 1 > + maxItems: 2 > + > + idle-state: > + description: -| > + For TYPEC_ORIENTATION_NONE by deselect both channels if supported. > + const: 2 > + > + # Standard properties described in > + # Documentation/devicetree/bindings/mux/mux-controller.txt > + > + port: > + type: object > + additionalProperties: false > + description: -| > + Connection to the remote endpoint using OF graph bindings that model > + SS data bus to typec connector. > + > + properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + endpoint@0: > + type: object > + description: Endpoint connected to typec connector. > + additionalProperties: false > + > + properties: > + reg: > + const: 0 > + remote-endpoint: true > + > + required: > + - remote-endpoint > + > + endpoint@1: > + type: object > + description: Endpoint connected to usb controller. > + additionalProperties: false > + > + properties: > + reg: > + const: 1 > + remote-endpoint: true > + > + required: > + - remote-endpoint > + > + required: > + - endpoint@0 > + > +required: > + - compatible > + - port > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + typec_switch: mux-controller { > + compatible = "typec-orientation-switch"; > + #mux-control-cells = <0>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_ss_sel>; > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, > + <&gpio2 20 GPIO_ACTIVE_HIGH>; > + idle-state = <2>; > + > + port { > + #address-cells = <1>; > + #size-cells = <0>; > + > + usb3_orien_sel: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&typec_con_ss>; > + }; > + > + usb3_con_data: endpoint@1 { > + reg = <1>; > + remote-endpoint = <&dwc3_0>; > + }; > + }; This still seems unnecessarily complicated. What I had in mind is just something like this: mux: mux-controller { compatible = "gpio-mux"; #mux-control-cells = <0>; mux-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, <&gpio2 20 GPIO_ACTIVE_HIGH>; idle-state = <2>; }; connector { compatible = "usb-c-connector"; mux-controls = <&mux>; ... }; Then you aren't re-implementing the gpio-mux driver. This should work for *any* mux implementation as long as the mux states always have the same meanings from the perspective of the connector. Of course, I have little visibility into the possible h/w implementations for Type-C and maybe this isn't sufficient. So input from Type-C experts would be helpful. Rob
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Tuesday, December 1, 2020 5:47 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 v6 1/6] dt-bindings: usb: add documentation for typec > switch simple driver > > On Mon, Nov 23, 2020 at 10:01:31PM +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> > > --- > > changes for v6: > > - Use general mux bindings for typec switch, one typec switch > > may have 1 or 2 GPIOs for channel selection, if 1 GPIO, only > > can be used to select cc1 or cc2; if 2 GPIOs, the second GPIO > > can be used to deselect both channels or keep normal operations. > > - Add one more connection to usb controller. > > > > 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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c5c3016 > > > 35%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata= > > JCV9OtFkfLv4bH2LIxfnk0lVIO6j830%2F%2Bx2OMOi%2Fs6o%3D&reserved=0 > > > > .../bindings/usb/typec-switch-simple.yaml | 122 > +++++++++++++++++++++ > > 1 file changed, 122 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..030ade5 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > @@ -0,0 +1,122 @@ > > +# 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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3 > > > +bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWF > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > +Mn0%3D%7C3000&sdata=vSLcoGYWCgwpsi4LWbKo2VTyCVgN%2BA7zjHfE8QCS4rg > > +%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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c > > > +5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjo > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am > > > +p;sdata=0fd3wjAtKwLCQAY3NG2Be4qNA9FiFhtO0HIZmCK7AQs%3D&reserved=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 > > + > > + '#mux-control-cells': > > + const: 0 > > Who is the mux-control consumer? Typec(controller) ptn5110: tcpc@50 { compatible = "nxp,ptn5110"; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_typec>; reg = <0x50>; interrupt-parent = <&gpio4>; interrupts = <19 8>; mux-controls = <&typec_switch>; mux-control-names = "mux-typec-switch"; 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_orien_sel>; }; }; }; }; }; > > > + > > + switch-gpios: > > + description: | > > + GPIO specifiers to select the target channel of mux. > > + The first GPIO is for cc1 and cc2 selection, the GPIO flag 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. > > + The second gpio is to deselect any channles by places all channels > > + in high-impedance state to reduce current consumption, the gpio > flag > > + use GPIO_ACTIVE_HIGH if GPIO physical state high is for > > + high-impedance state (so low for normal operations); or Use > > + GPIO_ACTIVE_LOW if GPIO physical state low is for high-impedance > > + state(so high for normal operations). > > + minItems: 1 > > + maxItems: 2 > > + > > + idle-state: > > + description: -| > > + For TYPEC_ORIENTATION_NONE by deselect both channels if supported. > > + const: 2 > > + > > + # Standard properties described in > > + # Documentation/devicetree/bindings/mux/mux-controller.txt > > + > > + port: > > + type: object > > + additionalProperties: false > > + description: -| > > + Connection to the remote endpoint using OF graph bindings that model > > + SS data bus to typec connector. > > + > > + properties: > > + '#address-cells': > > + const: 1 > > + > > + '#size-cells': > > + const: 0 > > + > > + endpoint@0: > > + type: object > > + description: Endpoint connected to typec connector. > > + additionalProperties: false > > + > > + properties: > > + reg: > > + const: 0 > > + remote-endpoint: true > > + > > + required: > > + - remote-endpoint > > + > > + endpoint@1: > > + type: object > > + description: Endpoint connected to usb controller. > > + additionalProperties: false > > + > > + properties: > > + reg: > > + const: 1 > > + remote-endpoint: true > > + > > + required: > > + - remote-endpoint > > + > > + required: > > + - endpoint@0 > > + > > +required: > > + - compatible > > + - port > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + typec_switch: mux-controller { > > + compatible = "typec-orientation-switch"; > > + #mux-control-cells = <0>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pinctrl_ss_sel>; > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, > > + <&gpio2 20 GPIO_ACTIVE_HIGH>; > > + idle-state = <2>; > > + > > + port { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + usb3_orien_sel: endpoint@0 { > > + reg = <0>; > > + remote-endpoint = <&typec_con_ss>; > > + }; > > + > > + usb3_con_data: endpoint@1 { > > + reg = <1>; > > + remote-endpoint = <&dwc3_0>; > > + }; > > + }; > > This still seems unnecessarily complicated. What I had in mind is just > something like this: > > mux: mux-controller { > compatible = "gpio-mux"; > #mux-control-cells = <0>; > mux-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, > <&gpio2 20 GPIO_ACTIVE_HIGH>; > idle-state = <2>; > }; > > connector { > compatible = "usb-c-connector"; > mux-controls = <&mux>; > ... > }; > > Then you aren't re-implementing the gpio-mux driver. > > This should work for *any* mux implementation as long as the mux states always > have the same meanings from the perspective of the connector. > > Of course, I have little visibility into the possible h/w implementations > for Type-C and maybe this isn't sufficient. So input from Type-C experts > would be helpful. From h/w point view, this can work if only consider my(GPIO) case. But I still need register a typec switch in provider side to make it under the unified typec switch interface: 7 struct typec_switch { 6 struct device dev; 5 typec_switch_set_fn_t set; 4 struct mux_control *mux_ctrl; 3 }; typec_switch->dev.parent = mux_chip->dev; So either I put the typec switch creation in exiting mux-gpio driver; or I create a new typec switch driver with gpio mux code duplication. If I go directly with mux control w/o(out of) typec_switch, then I don't need any driver change as you expect, but I guess this is not preferred, @Heikki, any comments? Thanks Li Jun > > Rob
Hi Rob > -----Original Message----- > From: Jun Li > Sent: Tuesday, December 1, 2020 11:13 AM > To: Rob Herring <robh@kernel.org> > 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 v6 1/6] dt-bindings: usb: add documentation for typec > switch simple driver > > > > > -----Original Message----- > > From: Rob Herring <robh@kernel.org> > > Sent: Tuesday, December 1, 2020 5:47 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 v6 1/6] dt-bindings: usb: add documentation for > > typec switch simple driver > > > > On Mon, Nov 23, 2020 at 10:01:31PM +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> > > > --- > > > changes for v6: > > > - Use general mux bindings for typec switch, one typec switch > > > may have 1 or 2 GPIOs for channel selection, if 1 GPIO, only > > > can be used to select cc1 or cc2; if 2 GPIOs, the second GPIO > > > can be used to deselect both channels or keep normal operations. > > > - Add one more connection to usb controller. > > > > > > 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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c5c3016 > > > > > > 35%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj > > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata= > > > JCV9OtFkfLv4bH2LIxfnk0lVIO6j830%2F%2Bx2OMOi%2Fs6o%3D&reserved=0 > > > > > > .../bindings/usb/typec-switch-simple.yaml | 122 > > +++++++++++++++++++++ > > > 1 file changed, 122 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..030ade5 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml > > > @@ -0,0 +1,122 @@ > > > +# 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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3 > > > > > > +bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWF > > > > > > +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6 > > > > > > +Mn0%3D%7C3000&sdata=vSLcoGYWCgwpsi4LWbKo2VTyCVgN%2BA7zjHfE8QCS4rg > > > +%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%7C4436142512ac45fef86f08d8957983c2%7C686ea1d3bc2b4c6fa92cd99c > > > > > > +5c301635%7C0%7C1%7C637423696442152522%7CUnknown%7CTWFpbGZsb3d8eyJWIjo > > > > > > +iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&am > > > > > > +p;sdata=0fd3wjAtKwLCQAY3NG2Be4qNA9FiFhtO0HIZmCK7AQs%3D&reserved=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 > > > + > > > + '#mux-control-cells': > > > + const: 0 > > > > Who is the mux-control consumer? > > Typec(controller) > > ptn5110: tcpc@50 { > compatible = "nxp,ptn5110"; > pinctrl-names = "default"; > pinctrl-0 = <&pinctrl_typec>; > reg = <0x50>; > interrupt-parent = <&gpio4>; > interrupts = <19 8>; > mux-controls = <&typec_switch>; > mux-control-names = "mux-typec-switch"; > > 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_orien_sel>; > }; > }; > }; > }; > }; > > > > > > + > > > + switch-gpios: > > > + description: | > > > + GPIO specifiers to select the target channel of mux. > > > + The first GPIO is for cc1 and cc2 selection, the GPIO flag 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. > > > + The second gpio is to deselect any channles by places all channels > > > + in high-impedance state to reduce current consumption, the > > > + gpio > > flag > > > + use GPIO_ACTIVE_HIGH if GPIO physical state high is for > > > + high-impedance state (so low for normal operations); or Use > > > + GPIO_ACTIVE_LOW if GPIO physical state low is for high-impedance > > > + state(so high for normal operations). > > > + minItems: 1 > > > + maxItems: 2 > > > + > > > + idle-state: > > > + description: -| > > > + For TYPEC_ORIENTATION_NONE by deselect both channels if supported. > > > + const: 2 > > > + > > > + # Standard properties described in # > > > + Documentation/devicetree/bindings/mux/mux-controller.txt > > > + > > > + port: > > > + type: object > > > + additionalProperties: false > > > + description: -| > > > + Connection to the remote endpoint using OF graph bindings that > model > > > + SS data bus to typec connector. > > > + > > > + properties: > > > + '#address-cells': > > > + const: 1 > > > + > > > + '#size-cells': > > > + const: 0 > > > + > > > + endpoint@0: > > > + type: object > > > + description: Endpoint connected to typec connector. > > > + additionalProperties: false > > > + > > > + properties: > > > + reg: > > > + const: 0 > > > + remote-endpoint: true > > > + > > > + required: > > > + - remote-endpoint > > > + > > > + endpoint@1: > > > + type: object > > > + description: Endpoint connected to usb controller. > > > + additionalProperties: false > > > + > > > + properties: > > > + reg: > > > + const: 1 > > > + remote-endpoint: true > > > + > > > + required: > > > + - remote-endpoint > > > + > > > + required: > > > + - endpoint@0 > > > + > > > +required: > > > + - compatible > > > + - port > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/gpio/gpio.h> > > > + typec_switch: mux-controller { > > > + compatible = "typec-orientation-switch"; > > > + #mux-control-cells = <0>; > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&pinctrl_ss_sel>; > > > + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, > > > + <&gpio2 20 GPIO_ACTIVE_HIGH>; > > > + idle-state = <2>; > > > + > > > + port { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + usb3_orien_sel: endpoint@0 { > > > + reg = <0>; > > > + remote-endpoint = <&typec_con_ss>; > > > + }; > > > + > > > + usb3_con_data: endpoint@1 { > > > + reg = <1>; > > > + remote-endpoint = <&dwc3_0>; > > > + }; > > > + }; > > > > This still seems unnecessarily complicated. What I had in mind is just > > something like this: > > > > mux: mux-controller { > > compatible = "gpio-mux"; > > #mux-control-cells = <0>; > > mux-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, > > <&gpio2 20 GPIO_ACTIVE_HIGH>; > > idle-state = <2>; > > }; > > > > connector { > > compatible = "usb-c-connector"; > > mux-controls = <&mux>; > > ... > > }; > > > > Then you aren't re-implementing the gpio-mux driver. > > > > This should work for *any* mux implementation as long as the mux > > states always have the same meanings from the perspective of the connector. > > > > Of course, I have little visibility into the possible h/w > > implementations for Type-C and maybe this isn't sufficient. So input > > from Type-C experts would be helpful. More thinking of this, I have a proposal like this: 34 mux: mux-controller { 33 compatible = "gpio-mux"; 32 #mux-control-cells = <0>; 29 switch-gpios = <&gpio4 20 GPIO_ACTIVE_LOW>, 28 <&gpio2 20 GPIO_ACTIVE_HIGH>; 27 idle-state = <2>; 26 }; 25 Then I don't need re-implement the gpio-mux driver. 24 typec_switch { 23 compatible = "typec-orientation-switch"; 22 mux-controls = <&mux>; 21 mux-control-names = "mux-typec-switch"; 20 19 port { 18 usb3_data_ss: endpoint@0 { 17 remote-endpoint = <&typec_con_ss>; 16 }; 15 }; 14 }; The simple typec switch driver as the consumer of mux-control, and also as the provider of typec_switch to typec class, this can be extended to use other mux solutions. 31 usb_con: connector { 30 compatible = "usb-c-connector"; 29 label = "USB-C"; 20 ... ... 19 ports { 18 #address-cells = <1>; 17 #size-cells = <0>; 16 15 port@1 { 14 reg = <1>; 13 typec_con_ss: endpoint { 12 remote-endpoint = <&usb3_data_ss>; 11 }; 10 }; 9 }; 8 }; Then existing typec mux interface can be kept, is this making sense to you? Thanks Li Jun > > From h/w point view, this can work if only consider my(GPIO) case. > But I still need register a typec switch in provider side to make it under > the unified typec switch interface: > > 7 struct typec_switch { > 6 struct device dev; > 5 typec_switch_set_fn_t set; > 4 struct mux_control *mux_ctrl; > 3 }; > > typec_switch->dev.parent = mux_chip->dev; > > So either I put the typec switch creation in exiting mux-gpio driver; or > I create a new typec switch driver with gpio mux code duplication. > > If I go directly with mux control w/o(out of) typec_switch, then I don't > need any driver change as you expect, but I guess this is not preferred, > @Heikki, any comments? > > 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..030ade5 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/typec-switch-simple.yaml @@ -0,0 +1,122 @@ +# 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 + + '#mux-control-cells': + const: 0 + + switch-gpios: + description: | + GPIO specifiers to select the target channel of mux. + The first GPIO is for cc1 and cc2 selection, the GPIO flag 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. + The second gpio is to deselect any channles by places all channels + in high-impedance state to reduce current consumption, the gpio flag + use GPIO_ACTIVE_HIGH if GPIO physical state high is for + high-impedance state (so low for normal operations); or Use + GPIO_ACTIVE_LOW if GPIO physical state low is for high-impedance + state(so high for normal operations). + minItems: 1 + maxItems: 2 + + idle-state: + description: -| + For TYPEC_ORIENTATION_NONE by deselect both channels if supported. + const: 2 + + # Standard properties described in + # Documentation/devicetree/bindings/mux/mux-controller.txt + + port: + type: object + additionalProperties: false + description: -| + Connection to the remote endpoint using OF graph bindings that model + SS data bus to typec connector. + + properties: + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + endpoint@0: + type: object + description: Endpoint connected to typec connector. + additionalProperties: false + + properties: + reg: + const: 0 + remote-endpoint: true + + required: + - remote-endpoint + + endpoint@1: + type: object + description: Endpoint connected to usb controller. + additionalProperties: false + + properties: + reg: + const: 1 + remote-endpoint: true + + required: + - remote-endpoint + + required: + - endpoint@0 + +required: + - compatible + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + typec_switch: mux-controller { + compatible = "typec-orientation-switch"; + #mux-control-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_ss_sel>; + switch-gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>, + <&gpio2 20 GPIO_ACTIVE_HIGH>; + idle-state = <2>; + + port { + #address-cells = <1>; + #size-cells = <0>; + + usb3_orien_sel: endpoint@0 { + reg = <0>; + remote-endpoint = <&typec_con_ss>; + }; + + usb3_con_data: endpoint@1 { + reg = <1>; + remote-endpoint = <&dwc3_0>; + }; + }; + };
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> --- changes for v6: - Use general mux bindings for typec switch, one typec switch may have 1 or 2 GPIOs for channel selection, if 1 GPIO, only can be used to select cc1 or cc2; if 2 GPIOs, the second GPIO can be used to deselect both channels or keep normal operations. - Add one more connection to usb controller. 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 | 122 +++++++++++++++++++++ 1 file changed, 122 insertions(+)