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

[Jun Li]

Hi,

> -----Original Message-----
> From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx]
> Sent: Friday, March 10, 2017 6:52 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 10 March 2017 at 16:27, Jun Li <jun.li@xxxxxxx> wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx]
> >> Sent: Friday, March 10, 2017 3:15 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 10 March 2017 at 14:30, Jun Li <jun.li@xxxxxxx> 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).

Li Jun

> 
> --
> Baolin.wang
> Best Regards