Message-ID: <CAMz4kuJAq0RGsqz5XoKOft_DKGTDXscwenPp97Drbm79ON49=w@mail.gmail.com>
Date:	Tue, 5 Apr 2016 17:34:02 +0800
From:	Baolin Wang <baolin.wang@...aro.org>
To:	Peter Chen <hzpeterchen@...il.com>
Cc:	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>,
	Peter Chen <peter.chen@...escale.com>,
	Alan Stern <stern@...land.harvard.edu>, r.baldyga@...sung.com,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	Lee Jones <lee.jones@...aro.org>,
	Mark Brown <broonie@...nel.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 v9 0/4] Introduce usb charger framework to deal with the
 usb gadget power negotation

On 5 April 2016 at 16:12, Peter Chen <hzpeterchen@...il.com> wrote:
> On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
>> Hi Peter,
>>
>> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@...il.com> wrote:
>> >
>> > We are thinking USB charger framework for Freescale i.mx SoC series,
>> > since our internal framework is not good enough.
>> > So I have more questions for your framework since there are many
>> > different USB charger designs, and I hope it is universal.
>>
>> Great, thanks for your attention and suggestions.
>>
>> >
>> > - I would like to see your all code to let the charger work, eg
>> > you have said the charger detection is done by PMIC automatically,
>> > but I did not find your PMIC code to read charger type.
>>
>> Yeah, this patchset did not give an example to read charger type from
>> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
>> But I think user can get it easily by implementing below callbacks.
>> (1) gadget->ops->get_charger_type();
>> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
>> (3) uchger->get_charger_type();
>>
>
> I just would like if you can have this, then, we (you) can test it, eg,
> you can test if the wm831x can charge more than 1500mA for DCP.

Mark, could you please address Peter's comments about if the the
wm831x can charge more than 1500mA for DCP? (I have no environment to
test wm831x) Thanks.

>
>> In some solutions, when one cable is plugin, the PMIC will detect the
>> charger type automatically and you can read it from PMIC registers
>> then. Then maybe one interrupt or other ways to report to gadget to do
>> enumeration. So user can read the PMIC registers to get charger type
>> by implementing any above methods.
>>
>
> Wait. When the USB cable is plugging in, which module will get
> interrupt? Only PMIC or Both PMIC and USB (or extcon)? If PMIC
> can trigger USB "connect" interrupt, then it is ok.

Sorry for confusing you. What I mean is this is one hardware design,
when USB cable is plugging in, the PMIC will detect the charger type
automatically and the gadget does not know any plugin event at the
same time. Then after the hardware detection, system will create one
VBUS interrupt to gadget to do enumeration. Anyway, no matter how you
design your hardware detection method, it should guarantee the charger
detection action safely.

>
>> >
>> > Besides, how you can make sure the charger detection has finished
>> > before the framework handles USB_CHARGER_PRESENT event?
>>
>> I think we don't need to care about this situation. If the charger
>> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
>> the charger framework will not set current (current is 0) for power
>> driver.
>
> Then, when we notify the charger IC for larger current, eg, for DCP.

I suppose If the usb charger framework gets the charger type, then
notify the charger IC for larger current.

>
>>
>> >
>> > - I commented the current limit at different situations for USB
>> > charger last time, but I have not seen your further comments.
>> > I would like give it again. For DCP, you can notify charger IC
>> > once you get the charger type. But for CDP/SDP, you need to
>> > notify charger IC after set configuration has finished, since
>> > the host may still not be ready to give high current.
>>
>> As my understanding, if the usb charger framework get the charger
>> type, it means we can notify the power driver to set the current. If
>> you don't ready for setting current, please don't give the charger
>> type to usb charger framework.
>>
>> The framework does not want to focus on charger detection too much,
>> and just supplies one callback '->charger_detect()' for user to be
>> implemented if they ensure they need to do the SW charger detection
>> manually (Note: must at the right time to do the SW detection.). So
>> the usb charger just focus on dealing with the usb gadget power
>> negotiation, and it does not need to care much how to do charger
>> detection on your platform.
>
> No, this comment is common one, but only for SW detection. Eg, when
> the PMIC tells you it is a SDP, you can't notify to charger IC about
> 500mA at once, you need to do it after host allows you to do it.

Well. Sounds reasonable. I just give an ugly example to implement the
'gadget->ops->get_charger_type()' method to get the charger type.

enum usb_charger_type get_charger_type(struct usb_gadget *gadget)
{
        if (host is allowed)
                read charger type from PMIC;
        else
                return UNKNOWN_TYPE;
}

So that will makes usb charger do not need to care about too much to
make things more complicated. Or do you have any other good
suggestions? Thanks.

>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Powered by blists - more mailing lists