RE: [PATCH 1/5] dt-bindings: connector: add optional properties for Type-B
From: Chunfeng Yun
Date: Mon Mar 11 2019 - 02:43:48 EST
Hi Jun,
On Mon, 2019-03-11 at 06:06 +0000, Jun Li wrote:
>
> > -----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).
Got it, it's just an analogy:)
>
> 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
> >
>