[Gta04-owner] [PATCH 14/15] twl4030_charger: Increase current carefully while watching voltage.

[NeilBrown]

On Wed, 25 Feb 2015 04:11:10 +0100 EdorFaus <edorfaus at xepher.net> wrote:

> On 02/24/2015 05:33 AM, NeilBrown wrote:
> > The USB Battery Charging spec (BC1.2) suggests a dedicated
> > charging port can deliver from 0.5 to 5.0A at between 4.75 and 5.25
> > volts.
> > 
> > To choose the "correct" current voltage setting requires a trial
> > and error approach: try to draw current and see if the voltage drops
> > too low.
> > 
> > Even with a configure Standard Downstream Port, it may not be possible
> > to reliably pull 500mA - depending on cable quality and source
> > quality I have reports of charging failure due to the voltage dropping
> > too low.
> > 
> > To address both these concern, this patch introduce incremental
> > current setting.
> > The current pull from VBUS is increased in steps of 20mA every 100ms
> > until the target is reached or until the measure voltage drops below
> > 4.75V.  If the voltage does go too long, the target current is reduced
> > by 20mA and kept there.
> > 
> > This applies to currents selected automatically, or to values
> > set via sysfs.  So setting a large value will cause the maximum
> > available to be used - up to the limit of 1.7mA imposed by the
> > hardware.
> 
> As 1.7mA makes little sense here, I assume this should say 1.7A?
> 
> The relative values of USB_CUR_STEP and USB_MAX_CURRENT seem to agree.
> 
> > 
> > Signed-off-by: NeilBrown <neilb at suse.de>
> > ---
> >  drivers/power/twl4030_charger.c |   54 ++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 53 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index e5a0225ea87e..7ad6b4b531d7 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -117,6 +117,18 @@ struct twl4030_bci {
> >  #define	CHARGE_AUTO	1
> >  #define	CHARGE_LINEAR	2
> >  
> > +	/* When setting the USB current we slowly increase the
> > +	 * requested current until target is reached or the voltage
> > +	 * drops below 4.75V.  In the latter case we set back one
> > +	 * step.
> > +	 */
> > +	int			usb_cur_actual;
> > +	struct delayed_work	current_worker;
> > +#define	USB_CUR_STEP	20000	/* 20mA at a time */
> > +#define	USB_MIN_VOLT	4750000	/* 4.75V */
> > +#define	USB_CUR_DELAY	msecs_to_jiffies(100)
> > +#define	USB_MAX_CURRENT	1700000 /* TWL4030 caps at 1.7mA */
> 
> As above, this comment should probably say 1.7A, or 1700mA, not 1.7mA.

Yes, 1.7A.  Thanks for the review.  I've updated my patch so it'll be correct
if/when I resend.

> 
> 
> By the way, I agree with Boudewijn: even though I don't really know what
> I'm doing when it comes to kernel code, it's still interesting, so
> thanks for posting the patches here. :)

Thanks for the positive feedback.  As you've just show, you don't need to
understand everything to be able to contribute!

Thanks,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.goldelico.com/pipermail/gta04-owner/attachments/20150225/56b8bf83/attachment.asc>