[PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[NeilBrown]

The twl4030 charger has two current sources, 'USB' and 'AC'
(presumably "Accessory Charger" because it isn't Alternating Current).

If 'AC' is providing current, we should set the current limit
differently to when it isn't (and so USB is used).
So split 'cur' into 'usb_cur' and 'ac_cur' and use accordingly.

Now we must review the current setting on any interrupt or USB
event which might indicate that the charger-source has changed.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |   36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 3b7cc631bb8a..982675df21b7 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -22,6 +22,7 @@
 #include <linux/power_supply.h>
 #include <linux/notifier.h>
 #include <linux/usb/otg.h>
+#include <linux/i2c/twl4030-madc.h>
 
 #define TWL4030_BCIMSTATEC	0x02
 #define TWL4030_BCIICHG		0x08
@@ -101,10 +102,13 @@ struct twl4030_bci {
 	int			usb_enabled;
 
 	/*
-	 * ichg values in uA. If any are 'large', we set CGAIN to
-	 * '1' which doubles the range for half the precision.
+	 * ichg_* and *_cur values in uA. If any are 'large', we set
+	 * CGAIN to '1' which doubles the range for half the
+	 * precision.
 	 */
-	unsigned int		ichg_eoc, ichg_lo, ichg_hi, cur;
+	unsigned int		ichg_eoc, ichg_lo, ichg_hi;
+	unsigned int		usb_cur, ac_cur;
+	bool			ac_is_active;
 
 	unsigned long		event;
 };
@@ -225,11 +229,24 @@ static int ua2regval(int ua, bool cgain)
 static int twl4030_charger_update_current(struct twl4030_bci *bci)
 {
 	int status;
+	int cur;
 	unsigned reg, cur_reg;
 	u8 bcictl1, oldreg, fullreg;
 	bool cgain = false;
 	u8 boot_bci;
 
+	/*
+	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
+	 * and AC is enabled, set current for 'ac'
+	 */
+	if (twl4030_get_madc_conversion(11) > 4500) {
+		cur = bci->ac_cur;
+		bci->ac_is_active = true;
+	} else {
+		cur = bci->usb_cur;
+		bci->ac_is_active = false;
+	}
+
 	/* First, check thresholds and see if cgain is needed */
 	if (bci->ichg_eoc >= 200000)
 		cgain = true;
@@ -237,7 +254,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 		cgain = true;
 	if (bci->ichg_hi >= 820000)
 		cgain = true;
-	if (bci->cur > 852000)
+	if (cur > 852000)
 		cgain = true;
 
 	status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
@@ -318,7 +335,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	 * And finally, set the current.  This is stored in
 	 * two registers.
 	 */
-	reg = ua2regval(bci->cur, cgain);
+	reg = ua2regval(cur, cgain);
 	/* we have only 10 bits */
 	if (reg > 0x3ff)
 		reg = 0x3ff;
@@ -371,6 +388,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 
 	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
 
+		twl4030_charger_update_current(bci);
+
 		/* Need to keep phy powered */
 		if (!bci->usb_enabled) {
 			pm_runtime_get_sync(bci->transceiver->dev);
@@ -463,6 +482,7 @@ static irqreturn_t twl4030_charger_interrupt(int irq, void *arg)
 	struct twl4030_bci *bci = arg;
 
 	dev_dbg(bci->dev, "CHG_PRES irq\n");
+	twl4030_charger_update_current(bci);
 	power_supply_changed(bci->ac);
 	power_supply_changed(bci->usb);
 
@@ -495,6 +515,7 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
 		power_supply_changed(bci->ac);
 		power_supply_changed(bci->usb);
 	}
+	twl4030_charger_update_current(bci);
 
 	/* various monitoring events, for now we just log them here */
 	if (irqs1 & (TWL4030_TBATOR2 | TWL4030_TBATOR1))
@@ -724,10 +745,11 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	bci->ichg_eoc = 80100; /* Stop charging when current drops to here */
 	bci->ichg_lo = 241000; /* Low threshold */
 	bci->ichg_hi = 500000; /* High threshold */
+	bci->ac_cur = 500000; /* 500mA */
 	if (allow_usb)
-		bci->cur = 500000;  /* 500mA */
+		bci->usb_cur = 500000;  /* 500mA */
 	else
-		bci->cur = 100000;  /* 100mA */
+		bci->usb_cur = 100000;  /* 100mA */
 
 	bci->dev = &pdev->dev;
 	bci->irq_chg = platform_get_irq(pdev, 0);

[PATCH 01/13] twl4030_charger: use runtime_pm to keep usb phy active while charging.

[NeilBrown]

The twl4030 usb phy needs to be active while we are using
the USB VBUS as a current source for charging.
In particular, the usb3v1 regulator must be enabled and the
PHY_PWR_PHYPWD bit must be set to keep the phy powered.

commit ab37813f4093a5f59cb8e083cde277289dc72ed3
    twl4030_charger: Allow charger to control the regulator that feeds it

gave the charger control over the regulator, but didn't resolve
the PHY_PWR_PHYPWD issue.

Now that both of these are controlled by runtime_pm in
phy-twl4030-usb, we can simply take a runtime_pm reference to the USB
phy whenever the charger wants to use it as a current source.

So this patch reverts the above commit, and adds the necessary
runtime_pm calls.

Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/mfd/twl-core.c          |    9 ++++-----
 drivers/power/twl4030_charger.c |   18 +++++-------------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 489674a2497e..831696ee2472 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -788,9 +788,8 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 		static struct regulator_consumer_supply usb1v8 = {
 			.supply =	"usb1v8",
 		};
-		static struct regulator_consumer_supply usb3v1[] = {
-			{ .supply =	"usb3v1" },
-			{ .supply =	"bci3v1" },
+		static struct regulator_consumer_supply usb3v1 = {
+			.supply =	"usb3v1",
 		};
 
 	/* First add the regulators so that they can be used by transceiver */
@@ -818,7 +817,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 				return PTR_ERR(child);
 
 			child = add_regulator_linked(TWL4030_REG_VUSB3V1,
-						      &usb_fixed, usb3v1, 2,
+						      &usb_fixed, &usb3v1, 1,
 						      features);
 			if (IS_ERR(child))
 				return PTR_ERR(child);
@@ -838,7 +837,7 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 		if (IS_ENABLED(CONFIG_REGULATOR_TWL4030) && child) {
 			usb1v5.dev_name = dev_name(child);
 			usb1v8.dev_name = dev_name(child);
-			usb3v1[0].dev_name = dev_name(child);
+			usb3v1.dev_name = dev_name(child);
 		}
 	}
 
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 709d90dc75f1..fe71c61109f5 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -22,7 +22,6 @@
 #include <linux/power_supply.h>
 #include <linux/notifier.h>
 #include <linux/usb/otg.h>
-#include <linux/regulator/machine.h>
 
 #define TWL4030_BCIMSTATEC	0x02
 #define TWL4030_BCIICHG		0x08
@@ -94,7 +93,6 @@ struct twl4030_bci {
 	struct work_struct	work;
 	int			irq_chg;
 	int			irq_bci;
-	struct regulator	*usb_reg;
 	int			usb_enabled;
 
 	unsigned long		event;
@@ -208,7 +206,7 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 {
 	int ret;
 
-	if (enable) {
+	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
 		/* Check for USB charger connected */
 		if (!twl4030_bci_have_vbus(bci))
 			return -ENODEV;
@@ -222,14 +220,9 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 			return -EACCES;
 		}
 
-		/* Need to keep regulator on */
+		/* Need to keep phy powered */
 		if (!bci->usb_enabled) {
-			ret = regulator_enable(bci->usb_reg);
-			if (ret) {
-				dev_err(bci->dev,
-					"Failed to enable regulator\n");
-				return ret;
-			}
+			pm_runtime_get_sync(bci->transceiver->dev);
 			bci->usb_enabled = 1;
 		}
 
@@ -244,7 +237,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 	} else {
 		ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
 		if (bci->usb_enabled) {
-			regulator_disable(bci->usb_reg);
+			pm_runtime_mark_last_busy(bci->transceiver->dev);
+			pm_runtime_put_autosuspend(bci->transceiver->dev);
 			bci->usb_enabled = 0;
 		}
 	}
@@ -609,8 +603,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	bci->usb_reg = regulator_get(bci->dev, "bci3v1");
-
 	bci->usb = devm_power_supply_register(&pdev->dev, &twl4030_bci_usb_desc,
 					      NULL);
 	if (IS_ERR(bci->usb)) {

[PATCH 12/13] twl4030_charger: Increase current carefully while watching voltage.

[NeilBrown]

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.

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.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |   67 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 68117ad23564..2c537ee11bbe 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -119,6 +119,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 step back one
+	 * step.
+	 */
+	unsigned int		usb_cur_target;
+	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.7A */
+
 	unsigned long		event;
 };
 
@@ -257,6 +269,12 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	} else {
 		cur = bci->usb_cur;
 		bci->ac_is_active = false;
+		if (cur > bci->usb_cur_target) {
+			cur = bci->usb_cur_target;
+			bci->usb_cur = cur;
+		}
+		if (cur < bci->usb_cur_target)
+			schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
 	}
 
 	/* First, check thresholds and see if cgain is needed */
@@ -391,6 +409,41 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	return 0;
 }
 
+static int twl4030_charger_get_current(void);
+
+static void twl4030_current_worker(struct work_struct *data)
+{
+	int v, curr;
+	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;
+	curr = twl4030_charger_get_current();
+
+	dev_dbg(bci->dev, "v=%d cur=%d limit=%d target=%d\n", v, curr,
+		bci->usb_cur, bci->usb_cur_target);
+
+	if (v < USB_MIN_VOLT) {
+		/* Back up and stop adjusting. */
+		bci->usb_cur -= USB_CUR_STEP;
+		bci->usb_cur_target = bci->usb_cur;
+	} else if (bci->usb_cur >= bci->usb_cur_target ||
+		   bci->usb_cur + USB_CUR_STEP > USB_MAX_CURRENT) {
+		/* Reached target and voltage is OK - stop */
+		return;
+	} else {
+		bci->usb_cur += USB_CUR_STEP;
+		schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
+	}
+	twl4030_charger_update_current(bci);
+}
+
 /*
  * Enable/Disable USB Charge functionality.
  */
@@ -451,6 +504,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 = 0;
 	}
 
 	return ret;
@@ -599,7 +653,7 @@ twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
 	if (dev == &bci->ac->dev)
 		bci->ac_cur = cur;
 	else
-		bci->usb_cur = cur;
+		bci->usb_cur_target = cur;
 
 	twl4030_charger_update_current(bci);
 	return n;
@@ -621,7 +675,7 @@ static ssize_t twl4030_bci_max_current_show(struct device *dev,
 			cur = bci->ac_cur;
 	} else {
 		if (bci->ac_is_active)
-			cur = bci->usb_cur;
+			cur = bci->usb_cur_target;
 	}
 	if (cur < 0) {
 		cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
@@ -662,9 +716,9 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 
 	/* reset current on each 'plug' event */
 	if (allow_usb)
-		bci->usb_cur = 500000;
+		bci->usb_cur_target = 500000;
 	else
-		bci->usb_cur = 100000;
+		bci->usb_cur_target = 100000;
 
 	bci->event = val;
 	schedule_work(&bci->work);
@@ -927,9 +981,9 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	bci->ichg_hi = 500000; /* High threshold */
 	bci->ac_cur = 500000; /* 500mA */
 	if (allow_usb)
-		bci->usb_cur = 500000;  /* 500mA */
+		bci->usb_cur_target = 500000;  /* 500mA */
 	else
-		bci->usb_cur = 100000;  /* 100mA */
+		bci->usb_cur_target = 100000;  /* 100mA */
 	bci->usb_mode = CHARGE_AUTO;
 	bci->ac_mode = CHARGE_AUTO;
 
@@ -980,6 +1034,7 @@ static int 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) {

[PATCH 00/13] Enhance twl4030_charger functionality. - V3

[NeilBrown]

Following is most of my twl4030_charger patches, rebased against
 git://git.infradead.org/battery-2.6

Since the previous set I have added the conversion to
module_platform_driver so EPROBE_DEFER can be used, and fixed
a few minor typos.

This does not include the changes to add extcon support, in part
because extcon has seen some changes lately which leave me even more
confused about how best to use it than before.
I need to sort that out before I can resolve the rest of my usb phy
patches and then add a few more charger patches.

Thanks,
NeilBrown


---

NeilBrown (12):
      twl4030_charger: use runtime_pm to keep usb phy active while charging.
      twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node
      twl4030_charger: trust phy to determine when USB power is available.
      twl4030_charger: split uA calculation into a function.
      twl4030_charger: allow fine control of charger current.
      twl4030_charger: distinguish between USB current and 'AC' current
      twl4030_charger: allow max_current to be managed via sysfs.
      twl4030_charger: enable manual enable/disable of usb charging.
      twl4030_charger: add software controlled linear charging mode.
      twl4030_charger: add ac/mode to match usb/mode
      twl4030_charger: Increase current carefully while watching voltage.
      twl4030_charger: assume a 'charger' can supply maximum current.

Pavel Machek (1):
      twl4030_charger: convert to module_platform_driver instead of ..._probe.


 .../ABI/testing/sysfs-class-power-twl4030          |   45 ++
 drivers/mfd/twl-core.c                             |    9 
 drivers/power/twl4030_charger.c                    |  541 ++++++++++++++++++--
 3 files changed, 531 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-power-twl4030

--
Signature

[PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[NeilBrown]

Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
do so when devm_usb_get_phy_by_node returns that error.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 045238370d3f..ffc123fb7158 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 
 		phynode = of_find_compatible_node(bci->dev->of_node->parent,
 						  NULL, "ti,twl4030-usb");
-		if (phynode)
+		if (phynode) {
 			bci->transceiver = devm_usb_get_phy_by_node(
 				bci->dev, phynode, &bci->usb_nb);
+			if (IS_ERR(bci->transceiver) &&
+			    PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+		}
 	}
 
 	/* Enable interrupts now. */

[PATCH 06/13] twl4030_charger: allow fine control of charger current.

[NeilBrown]

The twl4030 allows control of the incoming current.
Part of this control is a 'CGAIN' setting which doubles
the range for half the precision.  This control affects
several different current setting, so all need to be updated
at once when CGAIN is changed.

With this patch, all of these current setting are managed
by the driver, but most are left at their default settings.

The current drawn is set to 500mA if the allow_usb module parameter is
set, and to 100mA otherwise.
More fine control will appear in later patches.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |  168 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 160 insertions(+), 8 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 29984b263a35..3b7cc631bb8a 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -31,6 +31,11 @@
 #define TWL4030_BCIMFSTS4	0x10
 #define TWL4030_BCICTL1		0x23
 #define TWL4030_BB_CFG		0x12
+#define TWL4030_BCIIREF1	0x27
+#define TWL4030_BCIIREF2	0x28
+#define TWL4030_BCIMFKEY	0x11
+#define TWL4030_BCIMFTH8	0x1d
+#define TWL4030_BCIMFTH9	0x1e
 
 #define TWL4030_BCIMFSTS1	0x01
 
@@ -95,6 +100,12 @@ struct twl4030_bci {
 	int			irq_bci;
 	int			usb_enabled;
 
+	/*
+	 * ichg values in uA. If any are 'large', we set CGAIN to
+	 * '1' which doubles the range for half the precision.
+	 */
+	unsigned int		ichg_eoc, ichg_lo, ichg_hi, cur;
+
 	unsigned long		event;
 };
 
@@ -211,6 +222,146 @@ static int ua2regval(int ua, bool cgain)
 	return ret;
 }
 
+static int twl4030_charger_update_current(struct twl4030_bci *bci)
+{
+	int status;
+	unsigned reg, cur_reg;
+	u8 bcictl1, oldreg, fullreg;
+	bool cgain = false;
+	u8 boot_bci;
+
+	/* First, check thresholds and see if cgain is needed */
+	if (bci->ichg_eoc >= 200000)
+		cgain = true;
+	if (bci->ichg_lo >= 400000)
+		cgain = true;
+	if (bci->ichg_hi >= 820000)
+		cgain = true;
+	if (bci->cur > 852000)
+		cgain = true;
+
+	status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
+	if (status < 0)
+		return status;
+	if (twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &boot_bci,
+			    TWL4030_PM_MASTER_BOOT_BCI) < 0)
+		boot_bci = 0;
+	boot_bci &= 7;
+
+	if ((!!cgain) != !!(bcictl1 & TWL4030_CGAIN))
+		/* Need to turn for charging while we change the
+		 * CGAIN bit.  Leave it off while everything is
+		 * updated.
+		 */
+		twl4030_clear_set_boot_bci(boot_bci, 0);
+
+	/*
+	 * For ichg_eoc, the hardware only supports reg values matching
+	 * 100XXXX000, and requires the XXXX be stored in the high nibble
+	 * of TWL4030_BCIMFTH8.
+	 */
+	reg = ua2regval(bci->ichg_eoc, cgain);
+	if (reg > 0x278)
+		reg = 0x278;
+	if (reg < 0x200)
+		reg = 0x200;
+	reg = (reg >> 3) & 0xf;
+	fullreg = reg << 4;
+
+	/*
+	 * For ichg_lo, reg value must match 10XXXX0000.
+	 * XXXX is stored in low nibble of TWL4030_BCIMFTH8.
+	 */
+	reg = ua2regval(bci->ichg_lo, cgain);
+	if (reg > 0x2F0)
+		reg = 0x2F0;
+	if (reg < 0x200)
+		reg = 0x200;
+	reg = (reg >> 4) & 0xf;
+	fullreg |= reg;
+
+	/* ichg_eoc and ichg_lo live in same register */
+	status = twl4030_bci_read(TWL4030_BCIMFTH8, &oldreg);
+	if (status < 0)
+		return status;
+	if (oldreg != fullreg) {
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xF4,
+					  TWL4030_BCIMFKEY);
+		if (status < 0)
+			return status;
+		twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+				 fullreg, TWL4030_BCIMFTH8);
+	}
+
+	/* ichg_hi threshold must be 1XXXX01100 (I think) */
+	reg = ua2regval(bci->ichg_hi, cgain);
+	if (reg > 0x3E0)
+		reg = 0x3E0;
+	if (reg < 0x200)
+		reg = 0x200;
+	fullreg = (reg >> 5) & 0xF;
+	fullreg <<= 4;
+	status = twl4030_bci_read(TWL4030_BCIMFTH9, &oldreg);
+	if (status < 0)
+		return status;
+	if ((oldreg & 0xF0) != fullreg) {
+		fullreg |= (oldreg & 0x0F);
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+					  TWL4030_BCIMFKEY);
+		if (status < 0)
+			return status;
+		twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+				 fullreg, TWL4030_BCIMFTH9);
+	}
+
+	/*
+	 * And finally, set the current.  This is stored in
+	 * two registers.
+	 */
+	reg = ua2regval(bci->cur, cgain);
+	/* we have only 10 bits */
+	if (reg > 0x3ff)
+		reg = 0x3ff;
+	status = twl4030_bci_read(TWL4030_BCIIREF1, &oldreg);
+	if (status < 0)
+		return status;
+	cur_reg = oldreg;
+	status = twl4030_bci_read(TWL4030_BCIIREF2, &oldreg);
+	if (status < 0)
+		return status;
+	cur_reg |= oldreg << 8;
+	if (reg != oldreg) {
+		/* disable write protection for one write access for
+		 * BCIIREF */
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+					  TWL4030_BCIMFKEY);
+		if (status < 0)
+			return status;
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+					  (reg & 0x100) ? 3 : 2,
+					  TWL4030_BCIIREF2);
+		if (status < 0)
+			return status;
+		/* disable write protection for one write access for
+		 * BCIIREF */
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xE7,
+					  TWL4030_BCIMFKEY);
+		if (status < 0)
+			return status;
+		status = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+					  reg & 0xff,
+					  TWL4030_BCIIREF1);
+	}
+	if ((!!cgain) != !!(bcictl1 & TWL4030_CGAIN)) {
+		/* Flip CGAIN and re-enable charging */
+		bcictl1 ^= TWL4030_CGAIN;
+		twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
+				 bcictl1, TWL4030_BCICTL1);
+		twl4030_clear_set_boot_bci(0, boot_bci);
+	}
+	return 0;
+}
+
 /*
  * Enable/Disable USB Charge functionality.
  */
@@ -219,14 +370,6 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 	int ret;
 
 	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
-		/*
-		 * Until we can find out what current the device can provide,
-		 * require a module param to enable USB charging.
-		 */
-		if (!allow_usb) {
-			dev_warn(bci->dev, "USB charging is disabled.\n");
-			return -EACCES;
-		}
 
 		/* Need to keep phy powered */
 		if (!bci->usb_enabled) {
@@ -578,6 +721,14 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	if (!pdata)
 		pdata = twl4030_bci_parse_dt(&pdev->dev);
 
+	bci->ichg_eoc = 80100; /* Stop charging when current drops to here */
+	bci->ichg_lo = 241000; /* Low threshold */
+	bci->ichg_hi = 500000; /* High threshold */
+	if (allow_usb)
+		bci->cur = 500000;  /* 500mA */
+	else
+		bci->cur = 100000;  /* 100mA */
+
 	bci->dev = &pdev->dev;
 	bci->irq_chg = platform_get_irq(pdev, 0);
 	bci->irq_bci = platform_get_irq(pdev, 1);
@@ -657,6 +808,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	if (ret < 0)
 		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
 
+	twl4030_charger_update_current(bci);
 	twl4030_charger_enable_ac(true);
 	if (!IS_ERR_OR_NULL(bci->transceiver))
 		twl4030_bci_usb_ncb(&bci->usb_nb,

[PATCH 02/13] twl4030_charger: convert to module_platform_driver instead of ..._probe.

[NeilBrown]

From: Pavel Machek <pavel@ucw.cz>

Drivers using module_platform_driver_probe cannot return
EPROBE_DEFER from the probe function, which makes them rather useless
these days...

Convert to module_platform_driver() so EPROBE_DEFER can be used.

Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index fe71c61109f5..045238370d3f 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -568,7 +568,7 @@ static const struct power_supply_desc twl4030_bci_usb_desc = {
 	.get_property	= twl4030_bci_get_property,
 };
 
-static int __init twl4030_bci_probe(struct platform_device *pdev)
+static int twl4030_bci_probe(struct platform_device *pdev)
 {
 	struct twl4030_bci *bci;
 	const struct twl4030_bci_platform_data *pdata = pdev->dev.platform_data;
@@ -692,14 +692,14 @@ static const struct of_device_id twl_bci_of_match[] = {
 MODULE_DEVICE_TABLE(of, twl_bci_of_match);
 
 static struct platform_driver twl4030_bci_driver = {
+	.probe = twl4030_bci_probe,
 	.driver	= {
 		.name	= "twl4030_bci",
 		.of_match_table = of_match_ptr(twl_bci_of_match),
 	},
 	.remove	= __exit_p(twl4030_bci_remove),
 };
-
-module_platform_driver_probe(twl4030_bci_driver, twl4030_bci_probe);
+module_platform_driver(twl4030_bci_driver);
 
 MODULE_AUTHOR("Gražvydas Ignotas");
 MODULE_DESCRIPTION("TWL4030 Battery Charger Interface driver");

[PATCH 08/13] twl4030_charger: allow max_current to be managed via sysfs.

[NeilBrown]

'max_current' sysfs attributes are created which allow the
max to be set.
Whenever a current source changes, the default is restored.
This will be followed by a uevent, so user-space can decide to
update again.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 .../ABI/testing/sysfs-class-power-twl4030          |   15 ++++
 drivers/power/twl4030_charger.c                    |   72 ++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-power-twl4030

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
new file mode 100644
index 000000000000..0331bba4605d
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -0,0 +1,15 @@
+What: /sys/class/power_supply/twl4030_ac/max_current
+      /sys/class/power_supply/twl4030_usb/max_current
+Description:
+	Read/Write limit on current which may
+	be drawn from the ac (Accessory Charger) or
+	USB port.
+
+	Value is in micro-Amps.
+
+	Value is set automatically to an appropriate
+	value when a cable is plugged or unplugged.
+
+	Value can the set by writing to the attribute.
+	The change will only persist until the next
+	plug event.  These event are reported via udev.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 982675df21b7..b0a50adebfda 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -482,6 +482,8 @@ static irqreturn_t twl4030_charger_interrupt(int irq, void *arg)
 	struct twl4030_bci *bci = arg;
 
 	dev_dbg(bci->dev, "CHG_PRES irq\n");
+	/* reset current on each 'plug' event */
+	bci->ac_cur = 500000;
 	twl4030_charger_update_current(bci);
 	power_supply_changed(bci->ac);
 	power_supply_changed(bci->usb);
@@ -536,6 +538,63 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+/*
+ * Provide "max_current" attribute in sysfs.
+ */
+static ssize_t
+twl4030_bci_max_current_store(struct device *dev, struct device_attribute *attr,
+	const char *buf, size_t n)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+	int cur = 0;
+	int status = 0;
+	status = kstrtoint(buf, 10, &cur);
+	if (status)
+		return status;
+	if (cur < 0)
+		return -EINVAL;
+	if (dev == &bci->ac->dev)
+		bci->ac_cur = cur;
+	else
+		bci->usb_cur = cur;
+
+	twl4030_charger_update_current(bci);
+	return n;
+}
+
+/*
+ * sysfs max_current show
+ */
+static ssize_t twl4030_bci_max_current_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	int status = 0;
+	int cur = -1;
+	u8 bcictl1;
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+
+	if (dev == &bci->ac->dev) {
+		if (!bci->ac_is_active)
+			cur = bci->ac_cur;
+	} else {
+		if (bci->ac_is_active)
+			cur = bci->usb_cur;
+	}
+	if (cur < 0) {
+		cur = twl4030bci_read_adc_val(TWL4030_BCIIREF1);
+		if (cur < 0)
+			return cur;
+		status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
+		if (status < 0)
+			return status;
+		cur = regval2ua(cur, bcictl1 & TWL4030_CGAIN);
+	}
+	return scnprintf(buf, PAGE_SIZE, "%u\n", cur);
+}
+
+static DEVICE_ATTR(max_current, 0644, twl4030_bci_max_current_show,
+			twl4030_bci_max_current_store);
+
 static void twl4030_bci_usb_work(struct work_struct *data)
 {
 	struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
@@ -558,6 +617,12 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 
 	dev_dbg(bci->dev, "OTG notify %lu\n", val);
 
+	/* reset current on each 'plug' event */
+	if (allow_usb)
+		bci->usb_cur = 500000;
+	else
+		bci->usb_cur = 100000;
+
 	bci->event = val;
 	schedule_work(&bci->work);
 
@@ -831,6 +896,11 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
 
 	twl4030_charger_update_current(bci);
+	if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
+		dev_warn(&pdev->dev, "could not create sysfs file\n");
+	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
+		dev_warn(&pdev->dev, "could not create sysfs file\n");
+
 	twl4030_charger_enable_ac(true);
 	if (!IS_ERR_OR_NULL(bci->transceiver))
 		twl4030_bci_usb_ncb(&bci->usb_nb,
@@ -855,6 +925,8 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 	twl4030_charger_enable_usb(bci, false);
 	twl4030_charger_enable_backup(0, 0);
 
+	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
+	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR1A);

[PATCH 11/13] twl4030_charger: add ac/mode to match usb/mode

[NeilBrown]

This allows AC charging to be turned off, much like usb charging.
"continuous" mode is not available though.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 .../ABI/testing/sysfs-class-power-twl4030          |   10 ++++++
 drivers/power/twl4030_charger.c                    |   35 +++++++++++++++-----
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index e8baa36aaa2b..be26af0f1895 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -33,3 +33,13 @@ Description:
 			 This is useful for unstable power sources
 			 such as bicycle dynamo, but care should
 			 be taken that battery is not over-charged.
+
+What: /sys/class/power_supply/twl4030_ac/mode
+Description:
+	Changing mode for 'ac' port.
+	Writing to this can disable charging.
+
+	Possible values are:
+		"auto" - draw power as appropriate for detected
+			 power source and battery status.
+		"off"  - do not draw any power.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index de5430deaf23..68117ad23564 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -114,7 +114,7 @@ struct twl4030_bci {
 	unsigned int		ichg_eoc, ichg_lo, ichg_hi;
 	unsigned int		usb_cur, ac_cur;
 	bool			ac_is_active;
-	int			usb_mode; /* charging mode requested */
+	int			usb_mode, ac_mode; /* charging mode requested */
 #define	CHARGE_OFF	0
 #define	CHARGE_AUTO	1
 #define	CHARGE_LINEAR	2
@@ -459,10 +459,13 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 /*
  * Enable/Disable AC Charge funtionality.
  */
-static int twl4030_charger_enable_ac(bool enable)
+static int twl4030_charger_enable_ac(struct twl4030_bci *bci, bool enable)
 {
 	int ret;
 
+	if (bci->ac_mode == CHARGE_OFF)
+		enable = false;
+
 	if (enable)
 		ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
 	else
@@ -688,9 +691,17 @@ twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
 		mode = 2;
 	else
 		return -EINVAL;
-	twl4030_charger_enable_usb(bci, false);
-	bci->usb_mode = mode;
-	status = twl4030_charger_enable_usb(bci, true);
+	if (dev == &bci->ac->dev) {
+		if (mode == 2)
+			return -EINVAL;
+		twl4030_charger_enable_ac(bci, false);
+		bci->ac_mode = mode;
+		status = twl4030_charger_enable_ac(bci, true);
+	} else {
+		twl4030_charger_enable_usb(bci, false);
+		bci->usb_mode = mode;
+		status = twl4030_charger_enable_usb(bci, true);
+	}
 	return (status == 0) ? n : status;
 }
 
@@ -704,9 +715,13 @@ twl4030_bci_mode_show(struct device *dev,
 	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
 	int len = 0;
 	int i;
+	int mode = bci->usb_mode;
+
+	if (dev == &bci->ac->dev)
+		mode = bci->ac_mode;
 
 	for (i = 0; i < ARRAY_SIZE(modes); i++)
-		if (bci->usb_mode == i)
+		if (mode == i)
 			len += snprintf(buf+len, PAGE_SIZE-len,
 					"[%s] ", modes[i]);
 		else
@@ -916,6 +931,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	else
 		bci->usb_cur = 100000;  /* 100mA */
 	bci->usb_mode = CHARGE_AUTO;
+	bci->ac_mode = CHARGE_AUTO;
 
 	bci->dev = &pdev->dev;
 	bci->irq_chg = platform_get_irq(pdev, 0);
@@ -1001,10 +1017,12 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 	if (device_create_file(&bci->usb->dev, &dev_attr_mode))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
+	if (device_create_file(&bci->ac->dev, &dev_attr_mode))
+		dev_warn(&pdev->dev, "could not create sysfs file\n");
 	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
-	twl4030_charger_enable_ac(true);
+	twl4030_charger_enable_ac(bci, true);
 	if (!IS_ERR_OR_NULL(bci->transceiver))
 		twl4030_bci_usb_ncb(&bci->usb_nb,
 				    bci->transceiver->last_event,
@@ -1024,13 +1042,14 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 {
 	struct twl4030_bci *bci = platform_get_drvdata(pdev);
 
-	twl4030_charger_enable_ac(false);
+	twl4030_charger_enable_ac(bci, false);
 	twl4030_charger_enable_usb(bci, false);
 	twl4030_charger_enable_backup(0, 0);
 
 	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
 	device_remove_file(&bci->usb->dev, &dev_attr_mode);
 	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
+	device_remove_file(&bci->ac->dev, &dev_attr_mode);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR1A);

[PATCH 04/13] twl4030_charger: trust phy to determine when USB power is available.

[NeilBrown]

The usb phy driver already determines when VBUS is available,
so repeating the test in the charger driver is pointless duplication.

On probe, process the last event from the phy, and from then on,
do whatever the phy tells us without double-checking.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |   33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index ffc123fb7158..a075216d65ed 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -178,28 +178,6 @@ static int twl4030_is_battery_present(struct twl4030_bci *bci)
 }
 
 /*
- * Check if VBUS power is present
- */
-static int twl4030_bci_have_vbus(struct twl4030_bci *bci)
-{
-	int ret;
-	u8 hwsts;
-
-	ret = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &hwsts,
-			      TWL4030_PM_MASTER_STS_HW_CONDITIONS);
-	if (ret < 0)
-		return 0;
-
-	dev_dbg(bci->dev, "check_vbus: HW_CONDITIONS %02x\n", hwsts);
-
-	/* in case we also have STS_USB_ID, VBUS is driven by TWL itself */
-	if ((hwsts & TWL4030_STS_VBUS) && !(hwsts & TWL4030_STS_USB_ID))
-		return 1;
-
-	return 0;
-}
-
-/*
  * Enable/Disable USB Charge functionality.
  */
 static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
@@ -207,10 +185,6 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 	int ret;
 
 	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
-		/* Check for USB charger connected */
-		if (!twl4030_bci_have_vbus(bci))
-			return -ENODEV;
-
 		/*
 		 * Until we can find out what current the device can provide,
 		 * require a module param to enable USB charging.
@@ -662,7 +636,12 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
 
 	twl4030_charger_enable_ac(true);
-	twl4030_charger_enable_usb(bci, true);
+	if (!IS_ERR_OR_NULL(bci->transceiver))
+		twl4030_bci_usb_ncb(&bci->usb_nb,
+				    bci->transceiver->last_event,
+				    NULL);
+	else
+		twl4030_charger_enable_usb(bci, false);
 	if (pdata)
 		twl4030_charger_enable_backup(pdata->bb_uvolt,
 					      pdata->bb_uamp);

[PATCH 09/13] twl4030_charger: enable manual enable/disable of usb charging.

[NeilBrown]

'off' or 'auto' to

 /sys/class/power/twl4030_usb/mode

will now enable or disable charging from USB port.  Normally this is
enabled on 'plug' and disabled on 'unplug'.
Unplug will still disable charging.  'plug' will only enable it if
'auto' if selected.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 .../ABI/testing/sysfs-class-power-twl4030          |   11 ++++
 drivers/power/twl4030_charger.c                    |   59 ++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index 0331bba4605d..40e0f919cbde 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -13,3 +13,14 @@ Description:
 	Value can the set by writing to the attribute.
 	The change will only persist until the next
 	plug event.  These event are reported via udev.
+
+
+What: /sys/class/power_supply/twl4030_usb/mode
+Description:
+	Changing mode for USB port.
+	Writing to this can disable charging.
+
+	Possible values are:
+		"auto" - draw power as appropriate for detected
+			 power source and battery status.
+		"off"  - do not draw any power.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index b0a50adebfda..6fa928ed3128 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -109,10 +109,16 @@ struct twl4030_bci {
 	unsigned int		ichg_eoc, ichg_lo, ichg_hi;
 	unsigned int		usb_cur, ac_cur;
 	bool			ac_is_active;
+	int			usb_mode; /* charging mode requested */
+#define	CHARGE_OFF	0
+#define	CHARGE_AUTO	1
 
 	unsigned long		event;
 };
 
+/* strings for 'usb_mode' values */
+static char *modes[] = { "off", "auto" };
+
 /*
  * clear and set bits on an given register on a given module
  */
@@ -386,6 +392,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 {
 	int ret;
 
+	if (bci->usb_mode == CHARGE_OFF)
+		enable = false;
 	if (enable && !IS_ERR_OR_NULL(bci->transceiver)) {
 
 		twl4030_charger_update_current(bci);
@@ -629,6 +637,53 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 	return NOTIFY_OK;
 }
 
+/*
+ * sysfs charger enabled store
+ */
+static ssize_t
+twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t n)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+	int mode;
+	int status;
+
+	if (sysfs_streq(buf, modes[0]))
+		mode = 0;
+	else if (sysfs_streq(buf, modes[1]))
+		mode = 1;
+	else
+		return -EINVAL;
+	twl4030_charger_enable_usb(bci, false);
+	bci->usb_mode = mode;
+	status = twl4030_charger_enable_usb(bci, true);
+	return (status == 0) ? n : status;
+}
+
+/*
+ * sysfs charger enabled show
+ */
+static ssize_t
+twl4030_bci_mode_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct twl4030_bci *bci = dev_get_drvdata(dev->parent);
+	int len = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(modes); i++)
+		if (bci->usb_mode == i)
+			len += snprintf(buf+len, PAGE_SIZE-len,
+					"[%s] ", modes[i]);
+		else
+			len += snprintf(buf+len, PAGE_SIZE-len,
+					"%s ", modes[i]);
+	buf[len-1] = '\n';
+	return len;
+}
+static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
+		   twl4030_bci_mode_store);
+
 static int twl4030_charger_get_current(void)
 {
 	int curr;
@@ -815,6 +870,7 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 		bci->usb_cur = 500000;  /* 500mA */
 	else
 		bci->usb_cur = 100000;  /* 100mA */
+	bci->usb_mode = CHARGE_AUTO;
 
 	bci->dev = &pdev->dev;
 	bci->irq_chg = platform_get_irq(pdev, 0);
@@ -898,6 +954,8 @@ static int twl4030_bci_probe(struct platform_device *pdev)
 	twl4030_charger_update_current(bci);
 	if (device_create_file(&bci->usb->dev, &dev_attr_max_current))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
+	if (device_create_file(&bci->usb->dev, &dev_attr_mode))
+		dev_warn(&pdev->dev, "could not create sysfs file\n");
 	if (device_create_file(&bci->ac->dev, &dev_attr_max_current))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
@@ -926,6 +984,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 	twl4030_charger_enable_backup(0, 0);
 
 	device_remove_file(&bci->usb->dev, &dev_attr_max_current);
+	device_remove_file(&bci->usb->dev, &dev_attr_mode);
 	device_remove_file(&bci->ac->dev, &dev_attr_max_current);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,

[PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[NeilBrown]

Add a 'continuous' option for usb charging which enables
the "linear" charging mode of the twl4030.

Linear charging does a good job with not-so-reliable power sources.
Auto mode does not work well as it switches off when voltage drops
momentarily.  Care must be taken not to over-charge.

It was used with a bike hub dynamo since a year or so. In that case
there are automatically charging stops when the cyclist needs a break.

Original-by: Andreas Kemnade <andreas@kemnade.info>
Signed-off-by: NeilBrown <neil@brown.name>
---
 .../ABI/testing/sysfs-class-power-twl4030          |    9 +++
 drivers/power/twl4030_charger.c                    |   55 ++++++++++++++++++--
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power-twl4030 b/Documentation/ABI/testing/sysfs-class-power-twl4030
index 40e0f919cbde..e8baa36aaa2b 100644
--- a/Documentation/ABI/testing/sysfs-class-power-twl4030
+++ b/Documentation/ABI/testing/sysfs-class-power-twl4030
@@ -24,3 +24,12 @@ Description:
 		"auto" - draw power as appropriate for detected
 			 power source and battery status.
 		"off"  - do not draw any power.
+		"continuous"
+		       - activate mode described as "linear" in
+		         TWL data sheets.  This uses whatever
+			 current is available and doesn't switch off
+			 when voltage drops.
+
+			 This is useful for unstable power sources
+			 such as bicycle dynamo, but care should
+			 be taken that battery is not over-charged.
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 6fa928ed3128..de5430deaf23 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -24,6 +24,8 @@
 #include <linux/usb/otg.h>
 #include <linux/i2c/twl4030-madc.h>
 
+#define TWL4030_BCIMDEN		0x00
+#define TWL4030_BCIMDKEY	0x01
 #define TWL4030_BCIMSTATEC	0x02
 #define TWL4030_BCIICHG		0x08
 #define TWL4030_BCIVAC		0x0a
@@ -35,13 +37,16 @@
 #define TWL4030_BCIIREF1	0x27
 #define TWL4030_BCIIREF2	0x28
 #define TWL4030_BCIMFKEY	0x11
+#define TWL4030_BCIMFEN3	0x14
 #define TWL4030_BCIMFTH8	0x1d
 #define TWL4030_BCIMFTH9	0x1e
+#define TWL4030_BCIWDKEY	0x21
 
 #define TWL4030_BCIMFSTS1	0x01
 
 #define TWL4030_BCIAUTOWEN	BIT(5)
 #define TWL4030_CONFIG_DONE	BIT(4)
+#define TWL4030_CVENAC		BIT(2)
 #define TWL4030_BCIAUTOUSB	BIT(1)
 #define TWL4030_BCIAUTOAC	BIT(0)
 #define TWL4030_CGAIN		BIT(5)
@@ -112,12 +117,13 @@ struct twl4030_bci {
 	int			usb_mode; /* charging mode requested */
 #define	CHARGE_OFF	0
 #define	CHARGE_AUTO	1
+#define	CHARGE_LINEAR	2
 
 	unsigned long		event;
 };
 
 /* strings for 'usb_mode' values */
-static char *modes[] = { "off", "auto" };
+static char *modes[] = { "off", "auto", "continuous" };
 
 /*
  * clear and set bits on an given register on a given module
@@ -404,16 +410,42 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 			bci->usb_enabled = 1;
 		}
 
-		/* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
-		ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
-		if (ret < 0)
-			return ret;
+		if (bci->usb_mode == CHARGE_AUTO)
+			/* forcing the field BCIAUTOUSB (BOOT_BCI[1]) to 1 */
+			ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOUSB);
 
 		/* forcing USBFASTMCHG(BCIMFSTS4[2]) to 1 */
 		ret = twl4030_clear_set(TWL_MODULE_MAIN_CHARGE, 0,
 			TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
+		if (bci->usb_mode == CHARGE_LINEAR) {
+			twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC|TWL4030_CVENAC, 0);
+			/* Watch dog key: WOVF acknowledge */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x33,
+					       TWL4030_BCIWDKEY);
+			/* 0x24 + EKEY6: off mode */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
+					       TWL4030_BCIMDKEY);
+			/* EKEY2: Linear charge: USB path */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x26,
+					       TWL4030_BCIMDKEY);
+			/* WDKEY5: stop watchdog count */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf3,
+					       TWL4030_BCIWDKEY);
+			/* enable MFEN3 access */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x9c,
+					       TWL4030_BCIMFKEY);
+			 /* ICHGEOCEN - end-of-charge monitor (current < 80mA)
+			  *                      (charging continues)
+			  * ICHGLOWEN - current level monitor (charge continues)
+			  * don't monitor over-current or heat save
+			  */
+			ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0xf0,
+					       TWL4030_BCIMFEN3);
+		}
 	} else {
 		ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
+		ret |= twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE, 0x2a,
+					TWL4030_BCIMDKEY);
 		if (bci->usb_enabled) {
 			pm_runtime_mark_last_busy(bci->transceiver->dev);
 			pm_runtime_put_autosuspend(bci->transceiver->dev);
@@ -652,6 +684,8 @@ twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
 		mode = 0;
 	else if (sysfs_streq(buf, modes[1]))
 		mode = 1;
+	else if (sysfs_streq(buf, modes[2]))
+		mode = 2;
 	else
 		return -EINVAL;
 	twl4030_charger_enable_usb(bci, false);
@@ -750,6 +784,17 @@ static int twl4030_bci_get_property(struct power_supply *psy,
 		is_charging = state & TWL4030_MSTATEC_USB;
 	else
 		is_charging = state & TWL4030_MSTATEC_AC;
+	if (!is_charging) {
+		u8 s;
+		twl4030_bci_read(TWL4030_BCIMDEN, &s);
+		if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
+			is_charging = s & 1;
+		else
+			is_charging = s & 2;
+		if (is_charging)
+			/* A little white lie */
+			state = TWL4030_MSTATEC_QUICK1;
+	}
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_STATUS:

[PATCH 05/13] twl4030_charger: split uA calculation into a function.

[NeilBrown]

We will need this calculation in other places, so
create functions to map between register value and uA value.

Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |   48 ++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index a075216d65ed..29984b263a35 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -178,6 +178,40 @@ static int twl4030_is_battery_present(struct twl4030_bci *bci)
 }
 
 /*
+ * TI provided formulas:
+ * CGAIN == 0: ICHG = (BCIICHG * 1.7) / (2^10 - 1) - 0.85
+ * CGAIN == 1: ICHG = (BCIICHG * 3.4) / (2^10 - 1) - 1.7
+ * Here we use integer approximation of:
+ * CGAIN == 0: val * 1.6618 - 0.85 * 1000
+ * CGAIN == 1: (val * 1.6618 - 0.85 * 1000) * 2
+ */
+/*
+ * convert twl register value for currents into uA
+ */
+static int regval2ua(int regval, bool cgain)
+{
+	if (cgain)
+		return (regval * 16618 - 8500 * 1000) / 5;
+	else
+		return (regval * 16618 - 8500 * 1000) / 10;
+}
+
+/*
+ * convert uA currents into twl register value
+ */
+static int ua2regval(int ua, bool cgain)
+{
+	int ret;
+	if (cgain)
+		ua /= 2;
+	ret = (ua * 10 + 8500 * 1000) / 16618;
+	/* rounding problems */
+	if (ret < 512)
+		ret = 512;
+	return ret;
+}
+
+/*
  * Enable/Disable USB Charge functionality.
  */
 static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
@@ -366,14 +400,6 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 	return NOTIFY_OK;
 }
 
-/*
- * TI provided formulas:
- * CGAIN == 0: ICHG = (BCIICHG * 1.7) / (2^10 - 1) - 0.85
- * CGAIN == 1: ICHG = (BCIICHG * 3.4) / (2^10 - 1) - 1.7
- * Here we use integer approximation of:
- * CGAIN == 0: val * 1.6618 - 0.85
- * CGAIN == 1: (val * 1.6618 - 0.85) * 2
- */
 static int twl4030_charger_get_current(void)
 {
 	int curr;
@@ -388,11 +414,7 @@ static int twl4030_charger_get_current(void)
 	if (ret)
 		return ret;
 
-	ret = (curr * 16618 - 850 * 10000) / 10;
-	if (bcictl1 & TWL4030_CGAIN)
-		ret *= 2;
-
-	return ret;
+	return regval2ua(curr, bcictl1 & TWL4030_CGAIN);
 }
 
 /*

[PATCH 13/13] twl4030_charger: assume a 'charger' can supply maximum current.

[NeilBrown]

If it cannot, we will stop pulling more current when voltage drops.

Signed-off-by: NeilBrown <neil@brown.name>
---
 drivers/power/twl4030_charger.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 2c537ee11bbe..c7432f532a83 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -697,8 +697,10 @@ static void twl4030_bci_usb_work(struct work_struct *data)
 	struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
 
 	switch (bci->event) {
-	case USB_EVENT_VBUS:
 	case USB_EVENT_CHARGER:
+		bci->usb_cur = USB_MAX_CURRENT;
+		/* FALL THROUGH */
+	case USB_EVENT_VBUS:
 		twl4030_charger_enable_usb(bci, true);
 		break;
 	case USB_EVENT_NONE:

Re: [PATCH 00/13] Enhance twl4030_charger functionality. - V3

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Hi,

On Thu, Jul 30, 2015 at 10:11:24AM +1000, NeilBrown wrote:
> Following is most of my twl4030_charger patches, rebased against
>  git://git.infradead.org/battery-2.6
> 
> Since the previous set I have added the conversion to
> module_platform_driver so EPROBE_DEFER can be used, and fixed
> a few minor typos.
> 
> This does not include the changes to add extcon support, in part
> because extcon has seen some changes lately which leave me even more
> confused about how best to use it than before.
> I need to sort that out before I can resolve the rest of my usb phy
> patches and then add a few more charger patches.

Thanks, queued.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[Tony Lindgren]

* NeilBrown <neil@brown.name> [150729 17:28]:
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
>  static int twl4030_charger_update_current(struct twl4030_bci *bci)
>  {
>  	int status;
> +	int cur;
>  	unsigned reg, cur_reg;
>  	u8 bcictl1, oldreg, fullreg;
>  	bool cgain = false;
>  	u8 boot_bci;
>  
> +	/*
> +	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> +	 * and AC is enabled, set current for 'ac'
> +	 */
> +	if (twl4030_get_madc_conversion(11) > 4500) {
> +		cur = bci->ac_cur;
> +		bci->ac_is_active = true;
> +	} else {
> +		cur = bci->usb_cur;
> +		bci->ac_is_active = false;
> +	}
> +
>  	/* First, check thresholds and see if cgain is needed */
>  	if (bci->ichg_eoc >= 200000)
>  		cgain = true;

Neil, you need a stub or something for twl4030_get_madc_conversion
if madc is not selected. Now at least omap2plus_defconfig and
ARM allmodconfig fails in Linux next.

Regards,

Tony

Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[NeilBrown]

On Thu, 6 Aug 2015 20:11:16 -0700 Tony Lindgren <tony@atomide.com>
wrote:

> * NeilBrown <neil@brown.name> [150729 17:28]:
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> >  static int twl4030_charger_update_current(struct twl4030_bci *bci)
> >  {
> >  	int status;
> > +	int cur;
> >  	unsigned reg, cur_reg;
> >  	u8 bcictl1, oldreg, fullreg;
> >  	bool cgain = false;
> >  	u8 boot_bci;
> >  
> > +	/*
> > +	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> > +	 * and AC is enabled, set current for 'ac'
> > +	 */
> > +	if (twl4030_get_madc_conversion(11) > 4500) {
> > +		cur = bci->ac_cur;
> > +		bci->ac_is_active = true;
> > +	} else {
> > +		cur = bci->usb_cur;
> > +		bci->ac_is_active = false;
> > +	}
> > +
> >  	/* First, check thresholds and see if cgain is needed */
> >  	if (bci->ichg_eoc >= 200000)
> >  		cgain = true;
> 
> Neil, you need a stub or something for twl4030_get_madc_conversion
> if madc is not selected. Now at least omap2plus_defconfig and
> ARM allmodconfig fails in Linux next.
> 
> Regards,
> 
> Tony

Thanks, I did get notified about that by Fengguang's test robot, but
it's still on my list....

I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
acceptable?  That would pull in IIO (it didn't use to...).

If this OK?

Thanks,
NeilBrown


From: NeilBrown <neil@brown.name>
Date: Fri, 7 Aug 2015 13:44:37 +1000
Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
 available.

We can only use the madc to check for 'ac' availability
if the madc has been compiled in.
If not: assume always using USB.

Reported-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: NeilBrown <neil@brown.name>

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index c7432f532a83..265fd236f4c0 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -91,6 +91,21 @@
 #define TWL4030_MSTATEC_COMPLETE1	0x0b
 #define TWL4030_MSTATEC_COMPLETE4	0x0e
 
+#if IS_ENABLED(CONFIG_TWL4030_MADC)
+/*
+ * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
+ * then AC is available.
+ */
+static inline int ac_available(void)
+{
+	return twl4030_get_madc_conversion(11) > 4500;
+}
+#else
+static inline int ac_available(void)
+{
+	return 0;
+}
+#endif
 static bool allow_usb;
 module_param(allow_usb, bool, 0644);
 MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
@@ -263,7 +278,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
 	 * and AC is enabled, set current for 'ac'
 	 */
-	if (twl4030_get_madc_conversion(11) > 4500) {
+	if (ac_available()) {
 		cur = bci->ac_cur;
 		bci->ac_is_active = true;
 	} else {

Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[Tony Lindgren]

* NeilBrown <neil@brown.name> [150806 20:48]:
> 
> Thanks, I did get notified about that by Fengguang's test robot, but
> it's still on my list....
> 
> I guess making CHARGER_TWL4030 auto-select TWL4030_MADC would not be
> acceptable?  That would pull in IIO (it didn't use to...).
> 
> If this OK?

Looks OK to me thanks:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

Hi,

This actually slipped through my review. IMHO madc should be
accessed through IIO, as already done for twl4030-madc-battery
and rx51-battery. That way the custom API can be removed at
some point.

Anyway, I queued the below patch with Tony's ACK to fix the build
issue in next.

On Fri, Aug 07, 2015 at 01:45:25PM +1000, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> Date: Fri, 7 Aug 2015 13:44:37 +1000
> Subject: [PATCH] twl4030_charger: fix compile error when TWL4030_MADC not
>  available.
> 
> We can only use the madc to check for 'ac' availability
> if the madc has been compiled in.
> If not: assume always using USB.
> 
> Reported-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: NeilBrown <neil@brown.name>
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index c7432f532a83..265fd236f4c0 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -91,6 +91,21 @@
>  #define TWL4030_MSTATEC_COMPLETE1	0x0b
>  #define TWL4030_MSTATEC_COMPLETE4	0x0e
>  
> +#if IS_ENABLED(CONFIG_TWL4030_MADC)
> +/*
> + * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
> + * then AC is available.
> + */
> +static inline int ac_available(void)
> +{
> +	return twl4030_get_madc_conversion(11) > 4500;
> +}
> +#else
> +static inline int ac_available(void)
> +{
> +	return 0;
> +}
> +#endif
>  static bool allow_usb;
>  module_param(allow_usb, bool, 0644);
>  MODULE_PARM_DESC(allow_usb, "Allow USB charge drawing default current");
> @@ -263,7 +278,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
>  	 * If AC (Accessory Charger) voltage exceeds 4.5V (MADC 11)
>  	 * and AC is enabled, set current for 'ac'
>  	 */
> -	if (twl4030_get_madc_conversion(11) > 4500) {
> +	if (ac_available()) {
>  		cur = bci->ac_cur;
>  		bci->ac_is_active = true;
>  	} else {

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 07/13] twl4030_charger: distinguish between USB current and 'AC' current

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Fri, 7 Aug 2015 07:13:09 +0200 Sebastian Reichel <sre@kernel.org>
wrote:

> Hi,
> 
> This actually slipped through my review. IMHO madc should be
> accessed through IIO, as already done for twl4030-madc-battery
> and rx51-battery. That way the custom API can be removed at
> some point.
> 
> Anyway, I queued the below patch with Tony's ACK to fix the build
> issue in next.
> 

OK, thanks.

I'll try to figure out are more proper approach ... might be a week or
so though.

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Tony Lindgren]

* NeilBrown <neil@brown.name> [150729 17:29]:
> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> do so when devm_usb_get_phy_by_node returns that error.
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  drivers/power/twl4030_charger.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index 045238370d3f..ffc123fb7158 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
>  
>  		phynode = of_find_compatible_node(bci->dev->of_node->parent,
>  						  NULL, "ti,twl4030-usb");
> -		if (phynode)
> +		if (phynode) {
>  			bci->transceiver = devm_usb_get_phy_by_node(
>  				bci->dev, phynode, &bci->usb_nb);
> +			if (IS_ERR(bci->transceiver) &&
> +			    PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +		}
>  	}

Neil, the return with -EPROBE_DEFER here causes flakeyness booting
for me somehow at least on my logicpd-torpedo-37xx-devkit using
omap2plus_defconfig.

It seems that the twl4030_bci_probe keeps looping or something about
1/3 of the boots and that probably prevents the other twl modules
from loading? Reverting this patch alone seems to fix the issue.

I don't think I have a battery wired on this board the USB is wired
the same way as on beagle-xm. So I'd assume also flakeyness on beagle
xm with this patch.

Regards,

Tony

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[NeilBrown]

On Tue, 18 Aug 2015 01:07:58 -0700 Tony Lindgren <tony@atomide.com>
wrote:

> * NeilBrown <neil@brown.name> [150729 17:29]:
> > Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> > do so when devm_usb_get_phy_by_node returns that error.
> > 
> > Signed-off-by: NeilBrown <neil@brown.name>
> > ---
> >  drivers/power/twl4030_charger.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 045238370d3f..ffc123fb7158 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
> >  
> >  		phynode = of_find_compatible_node(bci->dev->of_node->parent,
> >  						  NULL, "ti,twl4030-usb");
> > -		if (phynode)
> > +		if (phynode) {
> >  			bci->transceiver = devm_usb_get_phy_by_node(
> >  				bci->dev, phynode, &bci->usb_nb);
> > +			if (IS_ERR(bci->transceiver) &&
> > +			    PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> > +				return -EPROBE_DEFER;
> > +		}
> >  	}
> 
> Neil, the return with -EPROBE_DEFER here causes flakeyness booting
> for me somehow at least on my logicpd-torpedo-37xx-devkit using
> omap2plus_defconfig.
> 
> It seems that the twl4030_bci_probe keeps looping or something about
> 1/3 of the boots and that probably prevents the other twl modules
> from loading? Reverting this patch alone seems to fix the issue.
> 
> I don't think I have a battery wired on this board the USB is wired
> the same way as on beagle-xm. So I'd assume also flakeyness on beagle
> xm with this patch.
> 
> Regards,
> 
> Tony

What dts file are you using?
I'm guessing that it doesn't have something like:

&usb_otg_hs {
        interface-type = <0>;
        usb-phy = <&usb2_phy>;
        phys = <&usb2_phy>;
        phy-names = "usb2-phy";
        mode = <3>;
        power = <50>;
};

? i.e. with a usb-phy=<&usb2_phy> ?

What if you add

&usb2_phy {
 status = "disabled";
}

to your dts file?

Should the 'status' be disabled in twl4030.dtsi, and then marked OK in
any dts that uses it?  I'm not at all clear on how 'status' is meant to
be used.

Thanks,
NeilBrown

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Tony Lindgren]

* NeilBrown <neil@brown.name> [150818 17:32]:
> On Tue, 18 Aug 2015 01:07:58 -0700 Tony Lindgren <tony@atomide.com>
> wrote:
> 
> > * NeilBrown <neil@brown.name> [150729 17:29]:
> > > Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> > > do so when devm_usb_get_phy_by_node returns that error.
> > > 
> > > Signed-off-by: NeilBrown <neil@brown.name>
> > > ---
> > >  drivers/power/twl4030_charger.c |    6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > > index 045238370d3f..ffc123fb7158 100644
> > > --- a/drivers/power/twl4030_charger.c
> > > +++ b/drivers/power/twl4030_charger.c
> > > @@ -636,9 +636,13 @@ static int twl4030_bci_probe(struct platform_device *pdev)
> > >  
> > >  		phynode = of_find_compatible_node(bci->dev->of_node->parent,
> > >  						  NULL, "ti,twl4030-usb");
> > > -		if (phynode)
> > > +		if (phynode) {
> > >  			bci->transceiver = devm_usb_get_phy_by_node(
> > >  				bci->dev, phynode, &bci->usb_nb);
> > > +			if (IS_ERR(bci->transceiver) &&
> > > +			    PTR_ERR(bci->transceiver) == -EPROBE_DEFER)
> > > +				return -EPROBE_DEFER;
> > > +		}
> > >  	}
> > 
> > Neil, the return with -EPROBE_DEFER here causes flakeyness booting
> > for me somehow at least on my logicpd-torpedo-37xx-devkit using
> > omap2plus_defconfig.
> > 
> > It seems that the twl4030_bci_probe keeps looping or something about
> > 1/3 of the boots and that probably prevents the other twl modules
> > from loading? Reverting this patch alone seems to fix the issue.
> > 
> > I don't think I have a battery wired on this board the USB is wired
> > the same way as on beagle-xm. So I'd assume also flakeyness on beagle
> > xm with this patch.
> 
> What dts file are you using?
> I'm guessing that it doesn't have something like:
> 
> &usb_otg_hs {
>         interface-type = <0>;
>         usb-phy = <&usb2_phy>;
>         phys = <&usb2_phy>;
>         phy-names = "usb2-phy";
>         mode = <3>;
>         power = <50>;
> };

It's the logicpd-torpedo-37xx-devkit.dts like I mentioned above,
and yes it has the same entry above like beagle xm.
 
> ? i.e. with a usb-phy=<&usb2_phy> ?
> 
> What if you add
> 
> &usb2_phy {
>  status = "disabled";
> }
> 
> to your dts file?

Yes then the PHY driver won't get probed and modprobe of the
charger won't hang with -EPROBE_DEFER.. The USB is working on this
one like on beagle xm so that's not a solution.
 
> Should the 'status' be disabled in twl4030.dtsi, and then marked OK in
> any dts that uses it?  I'm not at all clear on how 'status' is meant to
> be used.

Well status = "disabled" makes kernel completely ignore the
hardware, so the struct device is never created. I'd stay away from
using that in general for proper runtime idling of devices.. The
hardware is there for sure :)

Can you check if something like the the following allows you to
reproduce the hang of modprobe twl4030_charger:

# rmmod phy-twl4030-usb
# modprobe twl4030_charger

So just don't probe phy-twl4030-usb before twl4030_charger and
then modprobe of twl4030_charger hangs?

That is with proper usb2_phy configured in .dts file and not
set to status = "disabled".

Regards,

Tony

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Kevin Hilman]

On Wed, Jul 29, 2015 at 5:11 PM, NeilBrown <neil@brown.name> wrote:
> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
> do so when devm_usb_get_phy_by_node returns that error.
>
> Signed-off-by: NeilBrown <neil@brown.name>

This patch has hit linux-next in the form of coommit 3fc3895e4fe1
(twl4030_charger: correctly handle -EPROBE_DEFER from
devm_usb_get_phy_by_node) and kernelci.org found a regression on
omap3-beagle-xm[1].  Bisecting[2] this boot failure pointed at this
commit, and I verified that reverting it on top of next-20150827 gets
the board booting again.  I haven't debugged any further.

Kevin

[1] http://storage.kernelci.org/next/next-20150827/arm-omap2plus_defconfig/lab-khilman/boot-omap3-beagle-xm.html
[2] https://ci.linaro.org/view/people/job/tbaker-boot-bisect-bot/88/console

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Kevin Hilman]

ping... this boot failure has now landed in mainline

On Thu, Aug 27, 2015 at 1:51 PM, Kevin Hilman <khilman@kernel.org> wrote:
> On Wed, Jul 29, 2015 at 5:11 PM, NeilBrown <neil@brown.name> wrote:
>> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
>> do so when devm_usb_get_phy_by_node returns that error.
>>
>> Signed-off-by: NeilBrown <neil@brown.name>
>
> This patch has hit linux-next in the form of coommit 3fc3895e4fe1
> (twl4030_charger: correctly handle -EPROBE_DEFER from
> devm_usb_get_phy_by_node) and kernelci.org found a regression on
> omap3-beagle-xm[1].  Bisecting[2] this boot failure pointed at this
> commit, and I verified that reverting it on top of next-20150827 gets
> the board booting again.  I haven't debugged any further.
>
> Kevin
>
> [1] http://storage.kernelci.org/next/next-20150827/arm-omap2plus_defconfig/lab-khilman/boot-omap3-beagle-xm.html
> [2] https://ci.linaro.org/view/people/job/tbaker-boot-bisect-bot/88/console

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Neil Brown]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

Kevin Hilman <khilman@kernel.org> writes:

> ping... this boot failure has now landed in mainline

sorry, I'm on leave at the moment and travelling so I'm unlikely to be
able to look at this properly.  I should be able to examine this issue
before the end of the month but cannot promise sooner than that (though
it is not impossible).

Maybe it would be best to just revert it until a proper analysis can be
done.

NeilBrown


>
> On Thu, Aug 27, 2015 at 1:51 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> On Wed, Jul 29, 2015 at 5:11 PM, NeilBrown <neil@brown.name> wrote:
>>> Now that twl4030_bci_probe can safely return -EPROBE_DEFER,
>>> do so when devm_usb_get_phy_by_node returns that error.
>>>
>>> Signed-off-by: NeilBrown <neil@brown.name>
>>
>> This patch has hit linux-next in the form of coommit 3fc3895e4fe1
>> (twl4030_charger: correctly handle -EPROBE_DEFER from
>> devm_usb_get_phy_by_node) and kernelci.org found a regression on
>> omap3-beagle-xm[1].  Bisecting[2] this boot failure pointed at this
>> commit, and I verified that reverting it on top of next-20150827 gets
>> the board booting again.  I haven't debugged any further.
>>
>> Kevin
>>
>> [1] http://storage.kernelci.org/next/next-20150827/arm-omap2plus_defconfig/lab-khilman/boot-omap3-beagle-xm.html
>> [2] https://ci.linaro.org/view/people/job/tbaker-boot-bisect-bot/88/console

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Tony Lindgren]

* Neil Brown <neil@brown.name> [150901 23:23]:
> Kevin Hilman <khilman@kernel.org> writes:
> 
> > ping... this boot failure has now landed in mainline
> 
> sorry, I'm on leave at the moment and travelling so I'm unlikely to be
> able to look at this properly.  I should be able to examine this issue
> before the end of the month but cannot promise sooner than that (though
> it is not impossible).
> 
> Maybe it would be best to just revert it until a proper analysis can be
> done.

OK yeah let's revert this one for now until we know what goes wrong.

Regards,

Tony

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Kevin Hilman]

On Wed, Sep 2, 2015 at 6:07 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Neil Brown <neil@brown.name> [150901 23:23]:
>> Kevin Hilman <khilman@kernel.org> writes:
>>
>> > ping... this boot failure has now landed in mainline
>>
>> sorry, I'm on leave at the moment and travelling so I'm unlikely to be
>> able to look at this properly.  I should be able to examine this issue
>> before the end of the month but cannot promise sooner than that (though
>> it is not impossible).
>>
>> Maybe it would be best to just revert it until a proper analysis can be
>> done.
>
> OK yeah let's revert this one for now until we know what goes wrong.

Looks like this is still in mainline.

Tony, can you revert?

Kevin

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Tony Lindgren]

* Kevin Hilman <khilman@kernel.org> [150908 11:36]:
> On Wed, Sep 2, 2015 at 6:07 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Neil Brown <neil@brown.name> [150901 23:23]:
> >> Kevin Hilman <khilman@kernel.org> writes:
> >>
> >> > ping... this boot failure has now landed in mainline
> >>
> >> sorry, I'm on leave at the moment and travelling so I'm unlikely to be
> >> able to look at this properly.  I should be able to examine this issue
> >> before the end of the month but cannot promise sooner than that (though
> >> it is not impossible).
> >>
> >> Maybe it would be best to just revert it until a proper analysis can be
> >> done.
> >
> > OK yeah let's revert this one for now until we know what goes wrong.
> 
> Looks like this is still in mainline.
> 
> Tony, can you revert?

Probably best that Sebastian does it as there's another fix needed
too. It seems the following are needed:

- Revert $subject patch 3fc3895e4fe1 ("twl4030_charger: correctly
  handle -EPROBE_DEFER from devm_usb_get_phy_by_node")

- Apply compile fix "[PATCH] twl4030_charger: fix another compile
  error"

Just in case Sebastian is travelling or something.. Sebastian,
can you please confirm?

Regards,

Tony

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1501 bytes --]

Hi,

On Tue, Sep 08, 2015 at 01:14:17PM -0700, Tony Lindgren wrote:
> * Kevin Hilman <khilman@kernel.org> [150908 11:36]:
> > On Wed, Sep 2, 2015 at 6:07 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > * Neil Brown <neil@brown.name> [150901 23:23]:
> > >> Kevin Hilman <khilman@kernel.org> writes:
> > >>
> > >> > ping... this boot failure has now landed in mainline
> > >>
> > >> sorry, I'm on leave at the moment and travelling so I'm unlikely to be
> > >> able to look at this properly.  I should be able to examine this issue
> > >> before the end of the month but cannot promise sooner than that (though
> > >> it is not impossible).
> > >>
> > >> Maybe it would be best to just revert it until a proper analysis can be
> > >> done.
> > >
> > > OK yeah let's revert this one for now until we know what goes wrong.
> > 
> > Looks like this is still in mainline.
> > 
> > Tony, can you revert?
> 
> Probably best that Sebastian does it as there's another fix needed
> too. It seems the following are needed:
> 
> - Revert $subject patch 3fc3895e4fe1 ("twl4030_charger: correctly
>   handle -EPROBE_DEFER from devm_usb_get_phy_by_node")
> 
> - Apply compile fix "[PATCH] twl4030_charger: fix another compile
>   error"
> 
> Just in case Sebastian is travelling or something..

Right, I was on vacation without internet access until now.

> Sebastian, can you please confirm?

Sounds fine to me. I will take care of it once I've slept a bit.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

Hi,

On Thu, Sep 10, 2015 at 10:08:50AM +0200, Sebastian Reichel wrote:
> > Probably best that Sebastian does it as there's another fix needed
> > too. It seems the following are needed:
> > 
> > - Revert $subject patch 3fc3895e4fe1 ("twl4030_charger: correctly
> >   handle -EPROBE_DEFER from devm_usb_get_phy_by_node")
> > 
> > - Apply compile fix "[PATCH] twl4030_charger: fix another compile
> >   error"

I just added these to my next branch:

https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/commit/?h=next&id=aefc574bbbbe74bb891ba392d98f2d59a417c774
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/commit/?h=next&id=e11fc21e756e662e465cac3da6547d438d0b1446

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 03/13] twl4030_charger: correctly handle -EPROBE_DEFER from devm_usb_get_phy_by_node

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150910 13:31]:
> Hi,
> 
> On Thu, Sep 10, 2015 at 10:08:50AM +0200, Sebastian Reichel wrote:
> > > Probably best that Sebastian does it as there's another fix needed
> > > too. It seems the following are needed:
> > > 
> > > - Revert $subject patch 3fc3895e4fe1 ("twl4030_charger: correctly
> > >   handle -EPROBE_DEFER from devm_usb_get_phy_by_node")
> > > 
> > > - Apply compile fix "[PATCH] twl4030_charger: fix another compile
> > >   error"
> 
> I just added these to my next branch:
> 
> https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/commit/?h=next&id=aefc574bbbbe74bb891ba392d98f2d59a417c774
> https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/commit/?h=next&id=e11fc21e756e662e465cac3da6547d438d0b1446

Thanks!

Tony

Re: [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Pavel Machek]

On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> 
> Add a 'continuous' option for usb charging which enables
> the "linear" charging mode of the twl4030.
> 
> Linear charging does a good job with not-so-reliable power sources.
> Auto mode does not work well as it switches off when voltage drops
> momentarily.  Care must be taken not to over-charge.

Can you explain how the user can "care not to over-charge"?

> @@ -750,6 +784,17 @@ static int twl4030_bci_get_property(struct power_supply *psy,
>  		is_charging = state & TWL4030_MSTATEC_USB;
>  	else
>  		is_charging = state & TWL4030_MSTATEC_AC;
> +	if (!is_charging) {
> +		u8 s;
> +		twl4030_bci_read(TWL4030_BCIMDEN, &s);
> +		if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
> +			is_charging = s & 1;
> +		else
> +			is_charging = s & 2;
> +		if (is_charging)
> +			/* A little white lie */
> +			state = TWL4030_MSTATEC_QUICK1;

I'm not sure... can't this white lie turn into black smoke?

Like.. normally, when battery is below something (like 3.5V) it must
not be quick-charged (because something is very wrong with it). Are
you just forcing the quick charge here?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 01/13] twl4030_charger: use runtime_pm to keep usb phy active while charging.

[Pavel Machek]

On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> The twl4030 usb phy needs to be active while we are using
> the USB VBUS as a current source for charging.
> In particular, the usb3v1 regulator must be enabled and the
> PHY_PWR_PHYPWD bit must be set to keep the phy powered.
> 
> commit ab37813f4093a5f59cb8e083cde277289dc72ed3
>     twl4030_charger: Allow charger to control the regulator that feeds it
> 
> gave the charger control over the regulator, but didn't resolve
> the PHY_PWR_PHYPWD issue.
> 
> Now that both of these are controlled by runtime_pm in
> phy-twl4030-usb, we can simply take a runtime_pm reference to the USB
> phy whenever the charger wants to use it as a current source.
> 
> So this patch reverts the above commit, and adds the necessary
> runtime_pm calls.
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: NeilBrown <neil@brown.name>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 04/13] twl4030_charger: trust phy to determine when USB power is available.

[Pavel Machek]

On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> The usb phy driver already determines when VBUS is available,
> so repeating the test in the charger driver is pointless duplication.
> 
> On probe, process the last event from the phy, and from then on,
> do whatever the phy tells us without double-checking.
> 
> Signed-off-by: NeilBrown <neil@brown.name>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Gta04-owner] [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Christ van Willegen]

Hi all,

On Tue, Sep 15, 2015 at 1:28 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Thu 2015-07-30 10:11:24, NeilBrown wrote:
>>
>> Add a 'continuous' option for usb charging which enables
>> the "linear" charging mode of the twl4030.
>>
>> Linear charging does a good job with not-so-reliable power sources.
>> Auto mode does not work well as it switches off when voltage drops
>> momentarily.  Care must be taken not to over-charge.
>
> Can you explain how the user can "care not to over-charge"?

Just my $0.02, I assumed (whoops!) that Neil mean that the _code_
should take care not to over-charge... but not sure how he intended to
to that.

Anyway, that was my interpretation of the (indeed, it could be
explained in different ways...) comment.

Christ van Willegen

Re: [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Neil Brown]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

Pavel Machek <pavel@ucw.cz> writes:

> On Thu 2015-07-30 10:11:24, NeilBrown wrote:
>> 
>> Add a 'continuous' option for usb charging which enables
>> the "linear" charging mode of the twl4030.
>> 
>> Linear charging does a good job with not-so-reliable power sources.
>> Auto mode does not work well as it switches off when voltage drops
>> momentarily.  Care must be taken not to over-charge.
>
> Can you explain how the user can "care not to over-charge"?

The following text reads:

    It was used with a bike hub dynamo since a year or so. In that case
    there are automatically charging stops when the cyclist needs a break.

so: take a break from cycling occasionally.

>
>> @@ -750,6 +784,17 @@ static int twl4030_bci_get_property(struct power_supply *psy,
>>  		is_charging = state & TWL4030_MSTATEC_USB;
>>  	else
>>  		is_charging = state & TWL4030_MSTATEC_AC;
>> +	if (!is_charging) {
>> +		u8 s;
>> +		twl4030_bci_read(TWL4030_BCIMDEN, &s);
>> +		if (psy->desc->type == POWER_SUPPLY_TYPE_USB)
>> +			is_charging = s & 1;
>> +		else
>> +			is_charging = s & 2;
>> +		if (is_charging)
>> +			/* A little white lie */
>> +			state = TWL4030_MSTATEC_QUICK1;
>
> I'm not sure... can't this white lie turn into black smoke?
>
> Like.. normally, when battery is below something (like 3.5V) it must
> not be quick-charged (because something is very wrong with it). Are
> you just forcing the quick charge here?

No.  That value is only used for reporting a status via sysfs.
That 'lie' gets translated to "POWER_SUPPLY_STATUS_CHARGING" which is
not a lie.

Thanks,
NeilBrown


>
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Pavel Machek]

Hi!

> Pavel Machek <pavel@ucw.cz> writes:
> > On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> >> 
> >> Add a 'continuous' option for usb charging which enables
> >> the "linear" charging mode of the twl4030.
> >> 
> >> Linear charging does a good job with not-so-reliable power sources.
> >> Auto mode does not work well as it switches off when voltage drops
> >> momentarily.  Care must be taken not to over-charge.
> >
> > Can you explain how the user can "care not to over-charge"?
> 
> The following text reads:
> 
>     It was used with a bike hub dynamo since a year or so. In that case
>     there are automatically charging stops when the cyclist needs a break.
> 
> so: take a break from cycling occasionally.

If the charger does not exceed 4.2V, I'd not call it overcharge. (Yes, some clever
chargers actually let the battery drop below 4.2V when charge is done, but...)

If the charger _does_ exceed 4.2V, then the battery will explode. Don't do that. Don't
offer that to the user.

On a related note... I've just killed USB charger by overloading it. They are not protected.

I believe your automatically-pull-max-power really should stick to the well-known charging
currents (.5A, 1A, 1.7A), at the very minimum.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Gta04-owner] [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 2283 bytes --]

On Tue, 6 Oct 2015 16:34:07 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > Pavel Machek <pavel@ucw.cz> writes:
> > > On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> > >> 
> > >> Add a 'continuous' option for usb charging which enables
> > >> the "linear" charging mode of the twl4030.
> > >> 
> > >> Linear charging does a good job with not-so-reliable power sources.
> > >> Auto mode does not work well as it switches off when voltage drops
> > >> momentarily.  Care must be taken not to over-charge.
> > >
> > > Can you explain how the user can "care not to over-charge"?
> > 
> > The following text reads:
> > 
> >     It was used with a bike hub dynamo since a year or so. In that case
> >     there are automatically charging stops when the cyclist needs a break.
> > 
> > so: take a break from cycling occasionally.
> 
> If the charger does not exceed 4.2V, I'd not call it overcharge. (Yes, some clever
> chargers actually let the battery drop below 4.2V when charge is done, but...)
> 
Yes, that is the case. Perhaps it is not to be called overcharge but
it is said that lithium battery charging has to stop if in CV mode the
current drops too low.  In automatic mode the charger does exactly
that.
I would not let a battery for days at 4.2V CV.mode although a lot
of cheap chargers 

> If the charger _does_ exceed 4.2V, then the battery will explode. Don't do that. Don't
> offer that to the user.
> 
> On a related note... I've just killed USB charger by overloading it. They are not protected.
> 
> I believe your automatically-pull-max-power really should stick to the well-known charging
> currents (.5A, 1A, 1.7A), at the very minimum.
> 
The main reason for the patch was to prevent switching off charging
when Vbus drops low. The reason was not to get out extremely much
current out of the charger.
The electrical characteristics of a  bicycle as a power source are.
- the amount of current available changes
   - 500mA at around 17km/h
- you cannot destroy it by electrically overloading

If the current is set to e.g. 500mA and that linear charging mode is
enabled, the battery gets the maximum current available (upto
500mA) regardless of the speed which is often changing.

Regards,
Andreas Kemnade

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [Gta04-owner] [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Pavel Machek]

Hi!

> > > Pavel Machek <pavel@ucw.cz> writes:
> > > > On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> > > >> 
> > > >> Add a 'continuous' option for usb charging which enables
> > > >> the "linear" charging mode of the twl4030.
> > > >> 
> > > >> Linear charging does a good job with not-so-reliable power sources.
> > > >> Auto mode does not work well as it switches off when voltage drops
> > > >> momentarily.  Care must be taken not to over-charge.
> > > >
> > > > Can you explain how the user can "care not to over-charge"?
> > > 
> > > The following text reads:
> > > 
> > >     It was used with a bike hub dynamo since a year or so. In that case
> > >     there are automatically charging stops when the cyclist needs a break.
> > > 
> > > so: take a break from cycling occasionally.
> > 
> > If the charger does not exceed 4.2V, I'd not call it overcharge. (Yes, some clever
> > chargers actually let the battery drop below 4.2V when charge is done, but...)
> > 
> Yes, that is the case. Perhaps it is not to be called overcharge but
> it is said that lithium battery charging has to stop if in CV mode the
> current drops too low.  In automatic mode the charger does exactly
> that.
> I would not let a battery for days at 4.2V CV.mode although a lot
> of cheap chargers

Well, I agree that keeping battery at 4.2V constant voltage mode is
bad, but I'd not call it overcharge. If someone can fix the comment,
that would be nice.

> > If the charger _does_ exceed 4.2V, then the battery will explode. Don't do that. Don't
> > offer that to the user.
> > 
> > On a related note... I've just killed USB charger by overloading it. They are not protected.
> > 
> > I believe your automatically-pull-max-power really should stick to the well-known charging
> > currents (.5A, 1A, 1.7A), at the very minimum.
> > 
> The main reason for the patch was to prevent switching off charging
> when Vbus drops low. The reason was not to get out extremely much
> current out of the charger.
> The electrical characteristics of a  bicycle as a power source are.
> - the amount of current available changes
>    - 500mA at around 17km/h
> - you cannot destroy it by electrically overloading
> 
> If the current is set to e.g. 500mA and that linear charging mode is
> enabled, the battery gets the maximum current available (upto
> 500mA) regardless of the speed which is often changing.

Yes... I guess that makes sense for you, but I wonder if we should be
doing this by default. It seems a lot of cheap chargers can be easily
destroyed if you overload them.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [Gta04-owner] [PATCH 10/13] twl4030_charger: add software controlled linear charging mode.

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 4637 bytes --]

On Sat, 14 Nov 2015 19:12:16 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > > > Pavel Machek <pavel@ucw.cz> writes:
> > > > > On Thu 2015-07-30 10:11:24, NeilBrown wrote:
> > > > >> 
> > > > >> Add a 'continuous' option for usb charging which enables
> > > > >> the "linear" charging mode of the twl4030.
> > > > >> 
> > > > >> Linear charging does a good job with not-so-reliable power sources.
> > > > >> Auto mode does not work well as it switches off when voltage drops
> > > > >> momentarily.  Care must be taken not to over-charge.
> > > > >
> > > > > Can you explain how the user can "care not to over-charge"?
> > > > 
> > > > The following text reads:
> > > > 
> > > >     It was used with a bike hub dynamo since a year or so. In that case
> > > >     there are automatically charging stops when the cyclist needs a break.
> > > > 
> > > > so: take a break from cycling occasionally.
> > > 
> > > If the charger does not exceed 4.2V, I'd not call it overcharge. (Yes, some clever
> > > chargers actually let the battery drop below 4.2V when charge is done, but...)
> > > 
> > Yes, that is the case. Perhaps it is not to be called overcharge but
> > it is said that lithium battery charging has to stop if in CV mode the
> > current drops too low.  In automatic mode the charger does exactly
> > that.
> > I would not let a battery for days at 4.2V CV.mode although a lot
> > of cheap chargers
> 
> Well, I agree that keeping battery at 4.2V constant voltage mode is
> bad, but I'd not call it overcharge. If someone can fix the comment,
> that would be nice.
>
here is my original comment ("on" was replaced by continuous "now"):

    twl4030_charger: add software controlled linear charging mode.
    
       adds a sysfs control node to achieve that.
       It can be set to
       auto: normal automatic charging is enabled (default)
       off: charging is off
       on: charing is on (software controlled)
           CC/CV mode is still automatically done,
           but end of charge due to low current not.
    
    Note: If linear charging mode is used there should be some method of
    stopping charging automatically. It is not a so time-critical, but
    it is the wrong setting for leaving a charger connected for several
days since Lithium batteries should not be kept at 100% for longer
periods. Linear charging does a good job with not so reliable power
sources, since several voltage controlling is then often too
intelligent. It was used with a bike hub dynamo since a year or so. In
that case there are automatically charging stops when the cyclist needs
a break. Signed-off-by: Andreas Kemnade <andreas@kemnade.info>


> > > If the charger _does_ exceed 4.2V, then the battery will explode. Don't do that. Don't
> > > offer that to the user.
> > > 
> > > On a related note... I've just killed USB charger by overloading it. They are not protected.
> > > 
> > > I believe your automatically-pull-max-power really should stick to the well-known charging
> > > currents (.5A, 1A, 1.7A), at the very minimum.
> > > 
> > The main reason for the patch was to prevent switching off charging
> > when Vbus drops low. The reason was not to get out extremely much
> > current out of the charger.
> > The electrical characteristics of a  bicycle as a power source are.
> > - the amount of current available changes
> >    - 500mA at around 17km/h
> > - you cannot destroy it by electrically overloading
> > 
> > If the current is set to e.g. 500mA and that linear charging mode is
> > enabled, the battery gets the maximum current available (upto
> > 500mA) regardless of the speed which is often changing.
> 
> Yes... I guess that makes sense for you, but I wonder if we should be
> doing this by default. It seems a lot of cheap chargers can be easily
> destroyed if you overload them.
> 
Hmm, I guess the twl4030_charger would not be the only one destroying
such chargers. I have seen such hub dynamo-friendly behaviour on every
device I had connected to it before (an ipaq h2200, openmoko gta02).
I have checked all usb wall plug chargers I have seen and I found none
which has a lower current then 500mA. Only one has 500mA. The rest has
1A or even 2A.

But I think the non-ending cv stuff is a reason enough so that it is not
the default charge method. I use it only at bootup when battery is low
to have some time to fix charging issues manually and when cycling.
Cycling is detected by acceleration values and I get some feedback if
that charge mode is enabled or disabled.

Regards.
Andreas Kemnade

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]