diff mbox

[14/15] twl4030_charger: Increase current carefully while watching voltage.

Message ID 20150224043353.4243.32335.stgit@notabene.brown (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

NeilBrown Feb. 24, 2015, 4:33 a.m. UTC
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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   54 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Pavel Machek March 2, 2015, 9:29 p.m. UTC | #1
On Tue 2015-02-24 15:33:53, 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

"configured"?

> quality I have reports of charging failure due to the voltage dropping
> too low.
> 
> To address both these concern, this patch introduce incremental

concerns.

> 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

"too low"?

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

1.7A?


> @@ -249,8 +261,14 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
>  		cur = bci->ac_cur;
>  		bci->ac_is_active = 1;
>  	} else {
> -		cur = bci->usb_cur;
> +		cur = bci->usb_cur_actual;

usb_cur_actual is not a really great variable name...

>  		bci->ac_is_active = 0;
> +		if (cur > bci->usb_cur) {
> +			cur = bci->usb_cur;
> +			bci->usb_cur_actual = cur;
> +		}
> +		if (cur < bci->usb_cur)
> +			schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
>  	}
>  
>  	/* First, check thresholds and see if cgain is needed */
> @@ -379,6 +397,38 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
>  	return 0;
>  }
>  
> +static void twl4030_current_worker(struct work_struct *data)
> +{
> +	int v;
> +	int res;
> +	struct twl4030_bci *bci = container_of(data, struct twl4030_bci,
> +					       current_worker.work);
> +
> +	res = twl4030bci_read_adc_val(TWL4030_BCIVBUS);
> +	if (res < 0)
> +		v = 0;
> +	else
> +		/* BCIVBUS uses ADCIN8, 7/1023 V/step */
> +		v = res * 6843;
> +
> +	printk("v=%d cur=%d target=%d\n", v, bci->usb_cur_actual,
> +	       bci->usb_cur);
> +
> +	if (v < USB_MIN_VOLT) {
> +		/* Back up and stop adjusting. */
> +		bci->usb_cur_actual -= USB_CUR_STEP;
> +		bci->usb_cur = bci->usb_cur_actual;
> +	} else if (bci->usb_cur_actual >= bci->usb_cur ||
> +		   bci->usb_cur_actual + USB_CUR_STEP > USB_MAX_CURRENT) {
> +		/* Reach target and volts are OK - stop */

Reached ... and the voltage is OK - stop.
NeilBrown March 5, 2015, 6:51 a.m. UTC | #2
On Mon, 2 Mar 2015 22:29:45 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:53, 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
> 
> "configured"?
> 
> > quality I have reports of charging failure due to the voltage dropping
> > too low.
> > 
> > To address both these concern, this patch introduce incremental
> 
> concerns.
> 
> > 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
> 
> "too low"?
> 
> > 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.
> 
> 1.7A?
> 
> 
> > @@ -249,8 +261,14 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
> >  		cur = bci->ac_cur;
> >  		bci->ac_is_active = 1;
> >  	} else {
> > -		cur = bci->usb_cur;
> > +		cur = bci->usb_cur_actual;
> 
> usb_cur_actual is not a really great variable name...

I changed "usb_cur" to "usb_cur_target" and
"usb_cur_actual" to "usb_cur".

> 
> >  		bci->ac_is_active = 0;
> > +		if (cur > bci->usb_cur) {
> > +			cur = bci->usb_cur;
> > +			bci->usb_cur_actual = cur;
> > +		}
> > +		if (cur < bci->usb_cur)
> > +			schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
> >  	}
> >  
> >  	/* First, check thresholds and see if cgain is needed */
> > @@ -379,6 +397,38 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
> >  	return 0;
> >  }
> >  
> > +static void twl4030_current_worker(struct work_struct *data)
> > +{
> > +	int v;
> > +	int res;
> > +	struct twl4030_bci *bci = container_of(data, struct twl4030_bci,
> > +					       current_worker.work);
> > +
> > +	res = twl4030bci_read_adc_val(TWL4030_BCIVBUS);
> > +	if (res < 0)
> > +		v = 0;
> > +	else
> > +		/* BCIVBUS uses ADCIN8, 7/1023 V/step */
> > +		v = res * 6843;
> > +
> > +	printk("v=%d cur=%d target=%d\n", v, bci->usb_cur_actual,
> > +	       bci->usb_cur);
> > +
> > +	if (v < USB_MIN_VOLT) {
> > +		/* Back up and stop adjusting. */
> > +		bci->usb_cur_actual -= USB_CUR_STEP;
> > +		bci->usb_cur = bci->usb_cur_actual;
> > +	} else if (bci->usb_cur_actual >= bci->usb_cur ||
> > +		   bci->usb_cur_actual + USB_CUR_STEP > USB_MAX_CURRENT) {
> > +		/* Reach target and volts are OK - stop */
> 
> Reached ... and the voltage is OK - stop.
> 
> 

Thanks for all your proof-reading :-)

NeilBrown
diff mbox

Patch

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 */
+
 	unsigned long		event;
 };
 
@@ -249,8 +261,14 @@  static int twl4030_charger_update_current(struct twl4030_bci *bci)
 		cur = bci->ac_cur;
 		bci->ac_is_active = 1;
 	} else {
-		cur = bci->usb_cur;
+		cur = bci->usb_cur_actual;
 		bci->ac_is_active = 0;
+		if (cur > bci->usb_cur) {
+			cur = bci->usb_cur;
+			bci->usb_cur_actual = cur;
+		}
+		if (cur < bci->usb_cur)
+			schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
 	}
 
 	/* First, check thresholds and see if cgain is needed */
@@ -379,6 +397,38 @@  static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	return 0;
 }
 
+static void twl4030_current_worker(struct work_struct *data)
+{
+	int v;
+	int res;
+	struct twl4030_bci *bci = container_of(data, struct twl4030_bci,
+					       current_worker.work);
+
+	res = twl4030bci_read_adc_val(TWL4030_BCIVBUS);
+	if (res < 0)
+		v = 0;
+	else
+		/* BCIVBUS uses ADCIN8, 7/1023 V/step */
+		v = res * 6843;
+
+	printk("v=%d cur=%d target=%d\n", v, bci->usb_cur_actual,
+	       bci->usb_cur);
+
+	if (v < USB_MIN_VOLT) {
+		/* Back up and stop adjusting. */
+		bci->usb_cur_actual -= USB_CUR_STEP;
+		bci->usb_cur = bci->usb_cur_actual;
+	} else if (bci->usb_cur_actual >= bci->usb_cur ||
+		   bci->usb_cur_actual + USB_CUR_STEP > USB_MAX_CURRENT) {
+		/* Reach target and volts are OK - stop */
+		return;
+	} else {
+		bci->usb_cur_actual += USB_CUR_STEP;
+		schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
+	}
+	twl4030_charger_update_current(bci);
+}
+
 /*
  * Enable/Disable USB Charge functionality.
  */
@@ -441,6 +491,7 @@  static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 			pm_runtime_put_autosuspend(bci->transceiver->dev);
 			bci->usb_enabled = 0;
 		}
+		bci->usb_cur_actual = 0;
 	}
 
 	return ret;
@@ -972,6 +1023,7 @@  static int __init twl4030_bci_probe(struct platform_device *pdev)
 	}
 
 	INIT_WORK(&bci->work, twl4030_bci_usb_work);
+	INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);
 
 	bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
 	if (bci->dev->of_node) {