RE: [PATCH 1/5] dt-bindings: connector: add optional properties for Type-B
From: Jun Li
Date: Mon Mar 11 2019 - 02:07:01 EST
> -----Original Message-----
> From: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx>
> Sent: 2019å3æ11æ 13:33
> To: Hans de Goede <hdegoede@xxxxxxxxxx>
> Cc: Rob Herring <robh+dt@xxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Heikki Krogerus
> <heikki.krogerus@xxxxxxxxxxxxxxx>; Mark Rutland <mark.rutland@xxxxxxx>;
> Matthias Brugger <matthias.bgg@xxxxxxxxx>; Adam Thomson
> <Adam.Thomson.Opensource@xxxxxxxxxxx>; Jun Li <jun.li@xxxxxxx>; Badhri
> Jagan Sridharan <badhri@xxxxxxxxxx>; Andy Shevchenko
> <andy.shevchenko@xxxxxxxxx>; Min Guo <min.guo@xxxxxxxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-usb@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-mediatek@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/5] dt-bindings: connector: add optional properties for
> Type-B
>
> Hi,
>
> On Fri, 2019-03-08 at 13:07 +0100, Hans de Goede wrote:
> > Hi,
> >
> > On 08-03-19 07:13, Chunfeng Yun wrote:
> > > Add id-gpios, vbus-gpios, vbus-supply and pinctrl properties for
> > > usb-b-connector
> > >
> > > Signed-off-by: Chunfeng Yun <chunfeng.yun@xxxxxxxxxxxx>
> > > ---
> > > .../devicetree/bindings/connector/usb-connector.txt | 10 ++++++++++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > index a9a2f2fc44f2..7a07b0f4f973 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> > > @@ -17,6 +17,16 @@ Optional properties:
> > > - self-powered: Set this property if the usb device that has its own power
> > > source.
> > >
> > > +Optional properties for usb-b-connector:
> > > +- id-gpios: gpio for USB ID pin.
> >
> > What about boards where the ID pin is *not* connected to a GPIO, but
> > e.g. to a special pin on the PMIC which can also detect an ACA adapter
> > ? Currently this case is handled by extcon drivers, but we have no way
> > to set e.g. vbus-supply for the connector. Maybe in this case the
> > usb-connector node should be a child of the PMIC node ?
> Yes, it would be, PMIC is in charger of detecting the status of ID pin
> >
> > And in many cases there also is a mux to switch the datalines between
> > the host and device(gadget) controllers, how should that be described
> > in this model? See the new usb-role-switch code under
> > drivers/usb/roles
> >
> > In some cases the mux is controlled through a gpio, so we may want to
> > add a "mux-gpios" here in which case we also need to define what 0/1
> > means.
> I'm not sure, the mux seems not belong to this connector, and may need another
> driver to register usb-role-switch, similar to:
>
> [v2,2/2] usb: typec: add typec switch via GPIO control
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork
> .kernel.org%2Fpatch%2F10834327%2F&data=02%7C01%7Cjun.li%40nxp.co
> m%7C963df62e15ed4bedb14d08d6a5e30de2%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C636878791953122760&sdata=grPIs2MbdaarTa17dr
> pASVkGpyW7TAexB24igOJopGU%3D&reserved=0
>
No, this is not for usb role switch, this is a typec switch driver to select the super speed
active channel by orientation(CC1/CC2).
Li Jun
>
> >
> > > +- vbus-gpios: gpio for USB VBUS pin.
> > > + see gpio/gpio.txt.
> > > +- vbus-supply: reference to the VBUS regulator, needed when
> > > +supports
> > > + dual-role mode.
> >
> > I think this needs some text that there can be either a vbus-gpio or a
> > vbus-supply. Oh wait reading:
> >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >
> chwork.kernel.org%2Fpatch%2F10819377%2F&data=02%7C01%7Cjun.li%4
> 0nx
> >
> p.com%7C963df62e15ed4bedb14d08d6a5e30de2%7C686ea1d3bc2b4c6fa92cd9
> 9c5c3
> >
> 01635%7C0%7C0%7C636878791953122760&sdata=Judz7gdwQTOC7Jh84
> 57N4x21a
> > fWci%2FEH79ARqWZzbX8%3D&reserved=0
> >
> > I see that this GPIO is for detecting vbus presence, not for
> > driving/enabling 5v to Vbus from the board, that needs to be described more
> clearly.
> Ok
>
> Thanks a lot
> >
> > > +- pinctrl-names : a pinctrl state named "default" is optional
> > > +- pinctrl-0 : pin control group
> > > + see pinctrl/pinctrl-bindings.txt
> > > +
> > > Optional properties for usb-c-connector:
> > > - power-role: should be one of "source", "sink" or "dual"(DRP) if typec
> > > connector has power support.
> > >
> >
> >
> > Regards,
> >
> > Hans
>