On Fri, Nov 6, 2020 at 5:07 AM Jun Li <jun.li@xxxxxxx> wrote: > > > > > -----Original Message----- > > From: Rob Herring <robh@xxxxxxxxxx> > > Sent: Friday, November 6, 2020 6:26 AM > > To: Jun Li <jun.li@xxxxxxx> > > Cc: heikki.krogerus@xxxxxxxxxxxxxxx; rafael@xxxxxxxxxx; > > gregkh@xxxxxxxxxxxxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; > > hdegoede@xxxxxxxxxx; lee.jones@xxxxxxxxxx; > > mika.westerberg@xxxxxxxxxxxxxxx; dmitry.torokhov@xxxxxxxxx; > > prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx; > > laurent.pinchart+renesas@xxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > > devicetree@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Peter Chen > > <peter.chen@xxxxxxx> > > 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@xxxxxxx> > > > --- > > > 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@xxxxxxx> > > > + > > > +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
