Message-ID: <1552286616.10179.29.camel@mhfsdcap03>
Date:   Mon, 11 Mar 2019 14:43:36 +0800
From:   Chunfeng Yun <chunfeng.yun@...iatek.com>
To:     Jun Li <jun.li@....com>
CC:     Hans de Goede <hdegoede@...hat.com>,
        Rob Herring <robh+dt@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Mark Rutland <mark.rutland@....com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Min Guo <min.guo@...iatek.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>
Subject: RE: [PATCH 1/5] dt-bindings: connector: add optional properties for
 Type-B

Hi Jun,
On Mon, 2019-03-11 at 06:06 +0000, Jun Li wrote:
> 
> > -----Original Message-----
> > From: Chunfeng Yun <chunfeng.yun@...iatek.com>
> > Sent: 2019年3月11日 13:33
> > To: Hans de Goede <hdegoede@...hat.com>
> > Cc: Rob Herring <robh+dt@...nel.org>; Greg Kroah-Hartman
> > <gregkh@...uxfoundation.org>; Heikki Krogerus
> > <heikki.krogerus@...ux.intel.com>; Mark Rutland <mark.rutland@....com>;
> > Matthias Brugger <matthias.bgg@...il.com>; Adam Thomson
> > <Adam.Thomson.Opensource@...semi.com>; Jun Li <jun.li@....com>; Badhri
> > Jagan Sridharan <badhri@...gle.com>; Andy Shevchenko
> > <andy.shevchenko@...il.com>; Min Guo <min.guo@...iatek.com>;
> > devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> > linux-usb@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> > linux-mediatek@...ts.infradead.org
> > 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@...iatek.com>
> > > > ---
> > > >   .../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&amp;data=02%7C01%7Cjun.li%40nxp.co
> > m%7C963df62e15ed4bedb14d08d6a5e30de2%7C686ea1d3bc2b4c6fa92cd99c5
> > c301635%7C0%7C0%7C636878791953122760&amp;sdata=grPIs2MbdaarTa17dr
> > pASVkGpyW7TAexB24igOJopGU%3D&amp;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&amp;data=02%7C01%7Cjun.li%4
> > 0nx
> > >
> > p.com%7C963df62e15ed4bedb14d08d6a5e30de2%7C686ea1d3bc2b4c6fa92cd9
> > 9c5c3
> > >
> > 01635%7C0%7C0%7C636878791953122760&amp;sdata=Judz7gdwQTOC7Jh84
> > 57N4x21a
> > > fWci%2FEH79ARqWZzbX8%3D&amp;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
> > 
> 

Powered by blists - more mailing lists