linux-kernel - Re: [PATCH 13/14] twl4030_charger: Increase current carefully while watching voltage.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150330082637.4bf4832d@notabene.brown>
Date:	Mon, 30 Mar 2015 08:26:37 +1100
From:	NeilBrown <neilb@...e.de>
To:	Pavel Machek <pavel@....cz>
Cc:	NeilBrown <neil@...wn.name>, Sebastian Reichel <sre@...nel.org>,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
	GTA04 owners <gta04-owner@...delico.com>,
	inux-pm@...r.kernel.org, linux-omap@...r.kernel.org,
	Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH 13/14] twl4030_charger: Increase current carefully while
 watching voltage.

On Mon, 23 Mar 2015 22:25:41 +0100 Pavel Machek <pavel@....cz> wrote:

> Hi!
> 
> > 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 configured 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 concerns, 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 low, the target current is reduced
> > by 20mA and kept there.
> 
> Still nervous. If it is possible to overheat the charger, without
> tripping internal fuse, then you'll do it.

If it is possible to overheat the charger without tripping an internal fuse,
then sure the charger is mis-designed - is it not?

Can you suggest an algorithm for determining how much current can safely be
pulled from a charger that would *not* make you nervous?



> 
> > 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.7A imposed by the
> > hardware.
> >
> 
> > +	printk("v=%d cur=%d target=%d\n", v, bci->usb_cur,
> > +	       bci->usb_cur_target);
> 
> dev_info() and a bit better message, or drop it for production?

Changed to dev_dbg() - thanks.


> 
> > +	if (v < USB_MIN_VOLT) {
> > +		/* Back up and stop adjusting. */
> > +		bci->usb_cur -= USB_CUR_STEP;
> > +		bci->usb_cur_target = bci->usb_cur;
> 
> More importantly.... how does it work with device drawing power for
> operation, too?
> 
> Imagine device need 500mA with wifi hotspot, nearly nothing while idle.
> 
> Idle device. Code will find that it can charge using 1A, backs up to
> 0.9A. User starts hotspot. Now device will draw 1.4A, overloading the
> charger and not charging at all...?

The current being measured and controlled is the current flowing in from the
USB VBUS, not flowing out to the battery.
So I the code choose 0.9A, that is all that will be drawn.

This is a possible issue similar to this though.
If the device is idle and the battery is fully charged, then it won't draw
much current from USB even if we allow it too.
So the algorithm might decide it is OK to draw 1.7A because at that time the
device cannot use more than 200mA, and that doesn't cause the voltage to drop.

Then later when user enabled wifi-hotspot, the current needed might go up
above what the charger can provide.

Maybe I should only increase the limit while the actual current is also
increasing.  Maybe also revisit the setting when the battery starts charging.

NeilBrown


> 
> Best regards,
> 									Pavel


Content of type "application/pgp-signature" skipped