RE: [PATCH v19 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -----Original Message-----
> From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx]
> Sent: Thursday, March 09, 2017 7:23 PM
> To: Jun Li <jun.li@xxxxxxx>
> Cc: NeilBrown <neilb@xxxxxxxx>; Felipe Balbi <balbi@xxxxxxxxxx>; Greg KH
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Sebastian Reichel <sre@xxxxxxxxxx>; Dmitry
> Eremin-Solenikov <dbaryshkov@xxxxxxxxx>; David Woodhouse
> <dwmw2@xxxxxxxxxxxxx>; robh@xxxxxxxxxx; Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx>; Ruslan Bilovol <ruslan.bilovol@xxxxxxxxx>;
> Peter Chen <peter.chen@xxxxxxxxxxxxx>; Alan Stern
> <stern@xxxxxxxxxxxxxxxxxxx>; grygorii.strashko@xxxxxx; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Lee Jones <lee.jones@xxxxxxxxxx>;
> Mark Brown <broonie@xxxxxxxxxx>; John Stultz <john.stultz@xxxxxxxxxx>;
> Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>;
> patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx; Linux PM list <linux-
> pm@xxxxxxxxxxxxxxx>; USB <linux-usb@xxxxxxxxxxxxxxx>; device-
> mainlining@xxxxxxxxxxxxxxxxxxxxxxxxx; LKML <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 9 March 2017 at 18:34, Jun Li <jun.li@xxxxxxx> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx]
> >> Sent: Thursday, March 09, 2017 2:11 PM
> >> To: Jun Li <jun.li@xxxxxxx>
> >> Cc: NeilBrown <neilb@xxxxxxxx>; Felipe Balbi <balbi@xxxxxxxxxx>; Greg
> >> KH <gregkh@xxxxxxxxxxxxxxxxxxx>; Sebastian Reichel <sre@xxxxxxxxxx>;
> >> Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>; David Woodhouse
> >> <dwmw2@xxxxxxxxxxxxx>; robh@xxxxxxxxxx; Marek Szyprowski
> >> <m.szyprowski@xxxxxxxxxxx>; Ruslan Bilovol
> >> <ruslan.bilovol@xxxxxxxxx>; Peter Chen <peter.chen@xxxxxxxxxxxxx>;
> >> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>; grygorii.strashko@xxxxxx;
> >> Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Lee Jones
> >> <lee.jones@xxxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; John Stultz
> >> <john.stultz@xxxxxxxxxx>; Charles Keepax
> >> <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>;
> >> patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx; Linux PM list <linux-
> >> pm@xxxxxxxxxxxxxxx>; USB <linux-usb@xxxxxxxxxxxxxxx>; device-
> >> mainlining@xxxxxxxxxxxxxxxxxxxxxxxxx; LKML
> >> <linux-kernel@xxxxxxxxxxxxxxx>
> >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal
> >> with the usb gadget power negotation
> >>
> >> Hi,
> >>
> >> On 9 March 2017 at 09:50, Jun Li <jun.li@xxxxxxx> wrote:
> >> > Hi,
> >> >
> >> >> -----Original Message-----
> >> >> From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx]
> >> >> Sent: Tuesday, March 07, 2017 5:39 PM
> >> >> To: NeilBrown <neilb@xxxxxxxx>
> >> >> Cc: Felipe Balbi <balbi@xxxxxxxxxx>; Greg KH
> >> >> <gregkh@xxxxxxxxxxxxxxxxxxx>; Sebastian Reichel <sre@xxxxxxxxxx>;
> >> >> Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>; David
> Woodhouse
> >> >> <dwmw2@xxxxxxxxxxxxx>; robh@xxxxxxxxxx; Jun Li <jun.li@xxxxxxx>;
> >> >> Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>; Ruslan Bilovol
> >> >> <ruslan.bilovol@xxxxxxxxx>; Peter Chen
> <peter.chen@xxxxxxxxxxxxx>;
> >> >> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>; grygorii.strashko@xxxxxx;
> >> >> Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; Lee Jones
> >> >> <lee.jones@xxxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; John
> >> >> Stultz <john.stultz@xxxxxxxxxx>; Charles Keepax
> >> >> <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>;
> >> >> patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx; Linux PM list <linux-
> >> >> pm@xxxxxxxxxxxxxxx>; USB <linux-usb@xxxxxxxxxxxxxxx>; device-
> >> >> mainlining@xxxxxxxxxxxxxxxxxxxxxxxxx; LKML
> >> >> <linux-kernel@xxxxxxxxxxxxxxx>
> >> >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to
> >> >> deal with the usb gadget power negotation
> >> >>
> >> >> On 3 March 2017 at 10:23, NeilBrown <neilb@xxxxxxxx> wrote:
> >> >> > On Mon, Feb 20 2017, Baolin Wang wrote:
> >> >> >
> >> >> >> Currently the Linux kernel does not provide any standard
> >> >> >> integration of this feature that integrates the USB subsystem
> >> >> >> with the system power regulation provided by PMICs meaning that
> >> >> >> either vendors must add this in their kernels or USB gadget
> >> >> >> devices based on Linux (such as mobile phones) may not behave as
> they should.
> >> >> >> Thus provide a
> >> >> standard framework for doing this in kernel.
> >> >> >>
> >> >> >> Now introduce one user with wm831x_power to support and test
> >> >> >> the usb
> >> >> charger.
> >> >> >> Another user introduced to support charger detection by Jun Li:
> >> >> >> https://www.spinics.net/lists/linux-usb/msg139425.html
> >> >> >> Moreover there may be other potential users will use it in future.
> >> >> >>
> >> >> >> 1. Before v19 patchset we've fixed below issues in extcon
> >> >> >> subsystem and usb phy driver, now all were merged. (Thanks for
> >> >> >> Neil's
> >> >> >> suggestion)
> >> >> >> (1) Have fixed the inconsistencies with USB connector types in
> >> >> >> extcon subsystem by following links:
> >> >> >> https://lkml.org/lkml/2016/12/21/13
> >> >> >> https://lkml.org/lkml/2016/12/21/15
> >> >> >> https://lkml.org/lkml/2016/12/21/79
> >> >> >> https://lkml.org/lkml/2017/1/3/13
> >> >> >>
> >> >> >> (2) Instead of using 'set_power' callback in phy drivers, we
> >> >> >> will introduce USB charger to set PMIC current drawn from USB
> >> >> >> configuration, moreover some 'set_power' callbacks did not
> >> >> >> implement anything to set PMIC current, thus remove them by
> >> following links:
> >> >> >> https://lkml.org/lkml/2017/1/18/436
> >> >> >> https://lkml.org/lkml/2017/1/18/439
> >> >> >> https://lkml.org/lkml/2017/1/18/438
> >> >> >> Now only two phy drivers (phy-isp1301-omap.c and
> >> >> >> phy-gpio-vbus-usb.c) still used 'set_power' callback to set
> >> >> >> current, we can remove them in future. (I have no platform with
> >> >> >> enabling these two phy drivers, so I can not test them if I
> >> >> >> converted 'set_power' callback to USB
> >> >> >> charger.)
> >> >> >>
> >> >> >> 2. Some issues pointed by Neil Brown were sill kept in this v19
> >> >> >> patchset, and I expalined each issue and may be need discuss again:
> >> >> >> (1) Change all usb phys to register an extcon and to send
> >> >> >> appropriate
> >> >> notifications.
> >> >> >> Firstly, now only 3 USB phy drivers (phy-qcom-8x16-usb.c,
> >> >> >> phy-omap-otg.c and
> >> >> >> phy-msm-usb.c) had registered an extcon, mostly did not. I can
> >> >> >> not change all usb phys to register an extcon, since there are
> >> >> >> no extcon device to register for these different phy drivers.
> >> >> >
> >> >> > You don't have to change every driver.  You just need to make it
> >> >> > easy and obvious how to change drivers in a consistent coherent way.
> >> >> > For a start you would add a 'struct extcon_dev' to 'struct
> >> >> > usb_phy', and possibly add or extend some 'static inline's in
> >> >> > linux/usb/phy.h to send notification on that extcon (if it is non-NULL).
> >> >> > e.g. usb_phy_vbus_on() could send an extcon notification.
> >> >> >
> >> >> > Then any phy driver which adds support for setting
> >> >> > phy->extcon_dev appropriately, immediately gets the relevant
> notifications sent.
> >> >>
> >> >> OK. We can make these extcon related code into phy common part.
> >> >>
> >> >
> >> > 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).
- If the extcon_dev is directly added in usb_phy/phy, PMIC needs some API to findup it.

> 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?

Li Jun