Date:   Mon, 13 Mar 2017 16:43:35 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Jun Li <jun.li@....com>
Cc:     NeilBrown <neilb@...e.com>, Felipe Balbi <balbi@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Sebastian Reichel <sre@...nel.org>,
        Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        "robh@...nel.org" <robh@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Ruslan Bilovol <ruslan.bilovol@...il.com>,
        Peter Chen <peter.chen@...escale.com>,
        Alan Stern <stern@...land.harvard.edu>,
        "grygorii.strashko@...com" <grygorii.strashko@...com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Lee Jones <lee.jones@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        John Stultz <john.stultz@...aro.org>,
        Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>,
        "patches@...nsource.wolfsonmicro.com" 
        <patches@...nsource.wolfsonmicro.com>,
        Linux PM list <linux-pm@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>,
        "device-mainlining@...ts.linuxfoundation.org" 
        <device-mainlining@...ts.linuxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with the
 usb gadget power negotation

On 13 March 2017 at 09:09, Jun Li <jun.li@....com> wrote:
> Hi,
>
>> -----Original Message-----
>> From: Baolin Wang [mailto:baolin.wang@...aro.org]
>> Sent: Friday, March 10, 2017 6:52 PM
>> To: Jun Li <jun.li@....com>
>> Cc: NeilBrown <neilb@...e.com>; Felipe Balbi <balbi@...nel.org>; Greg KH
>> <gregkh@...uxfoundation.org>; Sebastian Reichel <sre@...nel.org>; Dmitry
>> Eremin-Solenikov <dbaryshkov@...il.com>; David Woodhouse
>> <dwmw2@...radead.org>; robh@...nel.org; Marek Szyprowski
>> <m.szyprowski@...sung.com>; Ruslan Bilovol <ruslan.bilovol@...il.com>;
>> Peter Chen <peter.chen@...escale.com>; Alan Stern
>> <stern@...land.harvard.edu>; grygorii.strashko@...com; Yoshihiro Shimoda
>> <yoshihiro.shimoda.uh@...esas.com>; Lee Jones <lee.jones@...aro.org>;
>> Mark Brown <broonie@...nel.org>; John Stultz <john.stultz@...aro.org>;
>> Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>;
>> patches@...nsource.wolfsonmicro.com; Linux PM list <linux-
>> pm@...r.kernel.org>; USB <linux-usb@...r.kernel.org>; device-
>> mainlining@...ts.linuxfoundation.org; LKML <linux-kernel@...r.kernel.org>
>> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 10 March 2017 at 16:27, Jun Li <jun.li@....com> wrote:
>> > Hi
>> >
>> >> -----Original Message-----
>> >> From: Baolin Wang [mailto:baolin.wang@...aro.org]
>> >> Sent: Friday, March 10, 2017 3:15 PM
>> >> To: Jun Li <jun.li@....com>
>> >> Cc: NeilBrown <neilb@...e.com>; Felipe Balbi <balbi@...nel.org>; Greg
>> >> KH <gregkh@...uxfoundation.org>; Sebastian Reichel <sre@...nel.org>;
>> >> Dmitry Eremin-Solenikov <dbaryshkov@...il.com>; David Woodhouse
>> >> <dwmw2@...radead.org>; robh@...nel.org; Marek Szyprowski
>> >> <m.szyprowski@...sung.com>; Ruslan Bilovol
>> >> <ruslan.bilovol@...il.com>; Peter Chen <peter.chen@...escale.com>;
>> >> Alan Stern <stern@...land.harvard.edu>; grygorii.strashko@...com;
>> >> Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>; Lee Jones
>> >> <lee.jones@...aro.org>; Mark Brown <broonie@...nel.org>; John Stultz
>> >> <john.stultz@...aro.org>; Charles Keepax
>> >> <ckeepax@...nsource.wolfsonmicro.com>;
>> >> patches@...nsource.wolfsonmicro.com; Linux PM list <linux-
>> >> pm@...r.kernel.org>; USB <linux-usb@...r.kernel.org>; device-
>> >> mainlining@...ts.linuxfoundation.org; LKML
>> >> <linux-kernel@...r.kernel.org>
>> >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal
>> >> with the usb gadget power negotation
>> >>
>> >> On 10 March 2017 at 14:30, Jun Li <jun.li@....com> wrote:
>> >> >> >> >
>> >> >> >> > Will generic phy need add extcon as well?
>> >> >> >>
>> >> >> >> Yes, will add a 'struct extcon_dev*' in 'struct usb_phy', which
>> >> >> >> will be common code.
>> >> >> >>
>> >> >> >
>> >> >> > I mean the common code need add 'struct extcon_dev' into both
>> >> >> > 'struct phy' and 'struct usb_phy', right? as some/new usb phy
>> >> >> > use that generic phy
>> >> >> driver.
>> >> >>
>> >> >> Ah, you remind me. Seems we need also add one 'struct extcon_dev'
>> >> >> into 'struct phy'.
>> >> >>
>> >> >> >> >
>> >> >> >> >> >
>> >> >> >> >> >> Secondly, I also agreed with Peter's comments: Not only
>> >> >> >> >> >> USB PHY to register an extcon, but also for the drivers
>> >> >> >> >> >> which can detect USB charger type, it may be USB
>> >> >> >> >> >> controller driver, USB type-c driver, pmic driver, and
>> >> >> >> >> >> these drivers may not have an extcon device since the
>> >> >> >> >> >> internal part can finish
>> >> the vbus detect.
>> >> >> >> >> >
>> >> >> >> >> > Whichever part can detect vbus, the driver for that part
>> >> >> >> >> > must be able to find the extcon and trigger a notification.
>> >> >> >> >> > Maybe one part can detect VBUS, another can measure the
>> >> >> >> >> > resistance on ID and a third can work through the state
>> >> >> >> >> > machine to determine if D+ and D- are shorted together.
>> >> >> >> >> > Somehow these three need to work together to determine
>> >> >> >> >> > what is
>> >> >> >> >> plugged
>> >> >> >> >> > in to the external connection port.  Somewhere there much
>> >> >> >> >> > an
>> >> >> 'extcon'
>> >> >> >> >> > device which represents that port and which the three
>> >> >> >> >> > devices can find and can interact with.
>> >> >> >> >> > I think it makes sense for the usb_phy to be the connection
>> point.
>> >> >> >> >> > Each of the devices can get to the phy, and the phy can
>> >> >> >> >> > get to the
>> >> >> >> extcon.
>> >> >> >> >> > It doesn't matter very much if the usb phy driver creates
>> >> >> >> >> > the extcon, or if something else creates the extcon and
>> >> >> >> >> > the phy driver performs a lookup to find it (e.g. based on
>> devicetree info).
>> >> >> >> >> >
>> >> >> >> >> > The point is that there is obviously an external physical
>> >> >> >> >> > connection, and so there should be an 'extcon' device that
>> >> >> represents it.
>> >> >> >> >>
>> >> >> >> >> Peter & Jun, is it OK for you every phy has one extcon
>> >> >> >> >> device to receive VBUS notification, especially for
>> >> >> >> >> detecting the charger type by
>> >> >> >> software?
>> >> >> >> >>
>> >> >> >> >
>> >> >> >> > My understanding is phy/usb_phy as the connection point, will
>> >> >> >> > send the notification to PMIC driver which actually control
>> >> >> >> > the charge current, also this will be done in your common
>> >> >> >> > framework,
>> >> right?
>> >> >> >>
>> >> >> >> Not in USB charger framework. If we are all agree every usb_phy
>> >> >> >> can register one extcon device, can get correct charger type
>> >> >> >> and send out correct vbus_draw information, then we don't need
>> >> >> >> USB charger framework as Neil suggested. So this will be okay
>> >> >> >> for your case (especially for detecting the charger type by
>> software) ?
>> >> >> >
>> >> >> > In my case, charger detection is done by controller driver and I
>> >> >> > need do charger type detection internally, and only pass the
>> >> >> > current draw info via phy which will send out, this seems ok for
>> >> >> > me, but I think it will be good if you or someone can show us an
>> >> >> > example user based on the
>> >> >> design Neil suggested.
>> >> >> > Will you work out that common code if this USB charger framework
>> >> >> > is not
>> >> >> needed?
>> >> >>
>> >> >> I will add  a 'struct extcon_dev*' in 'struct usb_phy' and struct
>> >> >> phy“. Others are already ready if everyone has no complain about
>> >> >> current design, except
>> >> >
>> >> > Only adding extcon_dev into usb_phy/phy  and all others are ready?
>> >> > My understanding you will also do:
>> >> > - We need find a central place to send the notification(phy common
>> part).
>> >>
>> >> That will include these implementation when adding extcon_dev.
>> >>
>> >
>> > OK, thanks.
>> >
>> >> > - If the extcon_dev is directly added in usb_phy/phy, PMIC needs
>> >> > some
>> >> API to findup it.
>> >>
>> >> PMIC can find extcon device by phandle.
>> >
>> > extcon_dev(not a reference pointer) is directly added in usb_phy/phy,
>> > not via devicetree, how PMIC find it by phandle?
>>
>> From my understanding, here we should add one pointer (extcon_dev *),
>> since usb phy is not one external connect device.
>
> Agreed.
>
>>
>> >
>> >>
>> >> >
>> >> >> my one concern. (I am afraid if it is enough to send out vbus draw
>> >> >> information from USB phy driver, for example you will miss super
>> >> >> speed (900mA), which need get the speed information from gadget
>> >> >> driver.)
>> >> >>
>> >> >
>> >> > Can we handle this in USB(so has super speed information) and just
>> >> > send out 900mA directly?
>> >>
>> >> From Neil's suggestion, we only have one place to send out current
>> >> information from usb phy, so I have this concern and doubt if we
>> >> still need the USB charger framework.
>> >
>> > So if put it in phy/usb_phy, this is a problem, that only one place
>> > should have the infor of both speed and usb state, how about put it at
>> > usb_gadget, then, e.g. send the notification in
>> usb_gadget_vbus_connect()?
>>
>> That is same what USB charger did, from this point, we need USB charger to
>> send out vbus draw information according to speed and usb state. But I
>> should listen to other guys suggestion. Peter and Felipe, what do you think?
>
> So now the only to do work is to find a common place to send the notification out
> (based on gadget speed and sate).

Yes.

-- 
Baolin.wang
Best Regards

Powered by blists - more mailing lists