[PATCH 00/15] Enhance twl4030_charger functionality.

[PATCH 00/15] Enhance twl4030_charger functionality.

[NeilBrown]

These patches make a number of improvements to twl4030_charger.

So are just internal cleanups (e.g. use of devres).  Others allow
better control of charging through both manual and automatic means.

- the maximum current can be configured via sysfs.
- the charger will only draw that current if it can do so without
  the voltage dropping too much
- a 'continuous' mode is available which ignores voltage and just
  takes what it can (to be used with caution, but very useful in
  some circumstances).
- 'ac' and 'usb' power sources can be configured separately.

Some of this functionality requires patch to phy-twl4030-usb.c which
have been sent separately.

Thanks,
NeilBrown


---

NeilBrown (15):
      power_supply core: support use of devres to register/unregister a power supply.
      twl4030_charger: use devm_request_threaded_irq
      twl4030_charger: use devres for power_supply_register and kzalloc.
      twl4030_charger: use runtime_pm to keep usb phy active while charging.
      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: only draw USB current as negotiated with host.
      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.


 drivers/mfd/twl-core.c            |    9 -
 drivers/power/power_supply_core.c |   45 +++
 drivers/power/twl4030_charger.c   |  572 +++++++++++++++++++++++++++++++------
 include/linux/power_supply.h      |    4 
 4 files changed, 539 insertions(+), 91 deletions(-)

--
Signature

[PATCH 02/15] twl4030_charger: use devm_request_threaded_irq

[NeilBrown]

This simplifies the error paths.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 4cf5ffbc904a..300dd7a34e80 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -610,21 +610,21 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 		goto fail_register_usb;
 	}
 
-	ret = request_threaded_irq(bci->irq_chg, NULL,
+	ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL,
 			twl4030_charger_interrupt, IRQF_ONESHOT, pdev->name,
 			bci);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "could not request irq %d, status %d\n",
 			bci->irq_chg, ret);
-		goto fail_chg_irq;
+		goto fail;
 	}
 
-	ret = request_threaded_irq(bci->irq_bci, NULL,
+	ret = devm_request_threaded_irq(&pdev->dev, bci->irq_bci, NULL,
 			twl4030_bci_interrupt, IRQF_ONESHOT, pdev->name, bci);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "could not request irq %d, status %d\n",
 			bci->irq_bci, ret);
-		goto fail_bci_irq;
+		goto fail;
 	}
 
 	INIT_WORK(&bci->work, twl4030_bci_usb_work);
@@ -647,7 +647,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 			       TWL4030_INTERRUPTS_BCIIMR1A);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to unmask interrupts: %d\n", ret);
-		goto fail_unmask_interrupts;
+		goto fail;
 	}
 
 	reg = ~(u32)(TWL4030_VBATOV | TWL4030_VBUSOV | TWL4030_ACCHGOV);
@@ -666,11 +666,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 
 	return 0;
 
-fail_unmask_interrupts:
-	free_irq(bci->irq_bci, bci);
-fail_bci_irq:
-	free_irq(bci->irq_chg, bci);
-fail_chg_irq:
+fail:
 	power_supply_unregister(&bci->usb);
 fail_register_usb:
 	power_supply_unregister(&bci->ac);
@@ -695,8 +691,6 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR2A);
 
-	free_irq(bci->irq_bci, bci);
-	free_irq(bci->irq_chg, bci);
 	power_supply_unregister(&bci->usb);
 	power_supply_unregister(&bci->ac);
 	kfree(bci);

[PATCH 03/15] twl4030_charger: use devres for power_supply_register and kzalloc.

[NeilBrown]

Final allocations/registrations are now managed by devres.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 300dd7a34e80..51321f0c5548 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -565,7 +565,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	int ret;
 	u32 reg;
 
-	bci = kzalloc(sizeof(*bci), GFP_KERNEL);
+	bci = devm_kzalloc(&pdev->dev, sizeof(*bci), GFP_KERNEL);
 	if (bci == NULL)
 		return -ENOMEM;
 
@@ -580,7 +580,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	ret = twl4030_is_battery_present(bci);
 	if  (ret) {
 		dev_crit(&pdev->dev, "Battery was not detected:%d\n", ret);
-		goto fail_no_battery;
+		goto fail;
 	}
 
 	platform_set_drvdata(pdev, bci);
@@ -590,10 +590,10 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	bci->ac.num_properties = ARRAY_SIZE(twl4030_charger_props);
 	bci->ac.get_property = twl4030_bci_get_property;
 
-	ret = power_supply_register(&pdev->dev, &bci->ac);
+	ret = devm_power_supply_register(&pdev->dev, &bci->ac);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register ac: %d\n", ret);
-		goto fail_register_ac;
+		goto fail;
 	}
 
 	bci->usb.name = "twl4030_usb";
@@ -604,10 +604,10 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 
 	bci->usb_reg = regulator_get(bci->dev, "bci3v1");
 
-	ret = power_supply_register(&pdev->dev, &bci->usb);
+	ret = devm_power_supply_register(&pdev->dev, &bci->usb);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register usb: %d\n", ret);
-		goto fail_register_usb;
+		goto fail;
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, bci->irq_chg, NULL,
@@ -667,13 +667,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	return 0;
 
 fail:
-	power_supply_unregister(&bci->usb);
-fail_register_usb:
-	power_supply_unregister(&bci->ac);
-fail_register_ac:
-fail_no_battery:
-	kfree(bci);
-
 	return ret;
 }
 
@@ -691,10 +684,6 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR2A);
 
-	power_supply_unregister(&bci->usb);
-	power_supply_unregister(&bci->ac);
-	kfree(bci);
-
 	return 0;
 }
 

Re: [PATCH 03/15] twl4030_charger: use devres for power_supply_register and kzalloc.

[Sebastian Reichel]

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

Hi,

On Tue, Feb 24, 2015 at 03:33:50PM +1100, NeilBrown wrote:
> [...]
> @@ -667,13 +667,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
>  	return 0;
>  
>  fail:
> -	power_supply_unregister(&bci->usb);
> -fail_register_usb:
> -	power_supply_unregister(&bci->ac);
> -fail_register_ac:
> -fail_no_battery:
> -	kfree(bci);
> -
>  	return ret;
>  }

Please replace "goto fail" with "return ret", goto is no
longer needed.

-- Sebastian

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

[PATCH 01/15] power_supply core: support use of devres to register/unregister a power supply.

[NeilBrown]

Using devm_power_supply_register allows the unregister to happen
automatically on error or final put.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/power_supply_core.c |   45 +++++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h      |    4 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 694e8cddd5c1..44c810456212 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -617,6 +617,51 @@ int power_supply_register_no_ws(struct device *parent, struct power_supply *psy)
 }
 EXPORT_SYMBOL_GPL(power_supply_register_no_ws);
 
+static void devm_power_supply_release(struct device *dev, void *res)
+{
+	struct power_supply **psy = res;
+
+	power_supply_unregister(*psy);
+}
+
+int devm_power_supply_register(struct device *parent, struct power_supply *psy)
+{
+	struct power_supply **ptr = devres_alloc(devm_power_supply_release,
+						 sizeof(*ptr), GFP_KERNEL);
+	int ret;
+
+	if (!ptr)
+		return -ENOMEM;
+	ret = __power_supply_register(parent, psy, true);
+	if (ret < 0)
+		devres_free(ptr);
+	else {
+		*ptr = psy;
+		devres_add(parent, ptr);
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_power_supply_register);
+
+int devm_power_supply_register_no_ws(struct device *parent, struct power_supply *psy)
+{
+	struct power_supply **ptr = devres_alloc(devm_power_supply_release,
+						 sizeof(*ptr), GFP_KERNEL);
+	int ret;
+
+	if (!ptr)
+		return -ENOMEM;
+	ret = __power_supply_register(parent, psy, false);
+	if (ret < 0)
+		devres_free(ptr);
+	else {
+		*ptr = psy;
+		devres_add(parent, ptr);
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_power_supply_register_no_ws);
+
 void power_supply_unregister(struct power_supply *psy)
 {
 	cancel_work_sync(&psy->changed_work);
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 096dbced02ac..f606d6b4bd56 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -278,6 +278,10 @@ extern int power_supply_register(struct device *parent,
 				 struct power_supply *psy);
 extern int power_supply_register_no_ws(struct device *parent,
 				 struct power_supply *psy);
+extern int devm_power_supply_register(struct device *parent,
+				 struct power_supply *psy);
+extern int devm_power_supply_register_no_ws(struct device *parent,
+				 struct power_supply *psy);
 extern void power_supply_unregister(struct power_supply *psy);
 extern int power_supply_powers(struct power_supply *psy, struct device *dev);
 

Re: [PATCH 01/15] power_supply core: support use of devres to register/unregister a power supply.

[Sebastian Reichel]

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

Hi Heil,

On Tue, Feb 24, 2015 at 03:33:50PM +1100, NeilBrown wrote:
> Using devm_power_supply_register allows the unregister to happen
> automatically on error or final put.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

Thanks, applied to battery-2.6.git.

-- Sebastian

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

[PATCH 06/15] 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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 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 db8931a17541..0b6fb06a0c01 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);
 }
 
 /*

Re: [PATCH 06/15] twl4030_charger: split uA calculation into a function.

[Pavel Machek]

On Tue 2015-02-24 15:33:51, NeilBrown wrote:
> We will need this calculation in other places, so
> create functions to map between register value and uA value.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

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

> +static int regval2ua(int regval, bool cgain)
> +{
> +	if (cgain)
> +		return (regval * 16618 - 8500 * 1000) / 5;
> +	else
> +		return (regval * 16618 - 8500 * 1000) / 10;
> +}

   int res = (regval * 16618 - 8500 * 1000);
   if (cgain)
      return res / 5;
   return res / 10;

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

Re: [PATCH 06/15] twl4030_charger: split uA calculation into a function.

[NeilBrown]

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

On Mon, 2 Mar 2015 22:05:18 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:51, NeilBrown wrote:
> > We will need this calculation in other places, so
> > create functions to map between register value and uA value.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>

Thanks.


> 
> > +static int regval2ua(int regval, bool cgain)
> > +{
> > +	if (cgain)
> > +		return (regval * 16618 - 8500 * 1000) / 5;
> > +	else
> > +		return (regval * 16618 - 8500 * 1000) / 10;
> > +}
> 
>    int res = (regval * 16618 - 8500 * 1000);
>    if (cgain)
>       return res / 5;
>    return res / 10;
> 
> ?
> 									Pavel

Maybe ... not sure it is really more readable.  I think I'll leave it as is.

Thanks,
NeilBrown

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

[PATCH 07/15] 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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |  161 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 153 insertions(+), 8 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 0b6fb06a0c01..7c35cd9ba171 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,11 @@ 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.
+	 */
+	int			ichg_eoc, ichg_lo, ichg_hi, cur;
+
 	unsigned long		event;
 };
 
@@ -211,6 +221,140 @@ 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;
+	int cgain = 0;
+	u8 boot_bci;
+
+	/* First, check thresholds and see if cgain is needed */
+	if (bci->ichg_eoc >= 200000)
+		cgain = 1;
+	if (bci->ichg_lo >= 400000)
+		cgain = 1;
+	if (bci->ichg_hi >= 820000)
+		cgain = 1;
+	if (bci->cur > 852000)
+		cgain = 1;
+
+	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, reg value must be 100XXXX000, we only
+	 * set the XXXX in high nibble.
+	 */
+	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 be 10XXXX0000.
+	 * XXXX is stored in low nibble */
+	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 bit */
+	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 +363,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) {
@@ -562,6 +698,14 @@ static int __init 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);
@@ -644,6 +788,7 @@ static int __init 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,

Re: [PATCH 07/15] twl4030_charger: allow fine control of charger current.

[Pavel Machek]

Hi!

> +	/* ichg values in uA. If any are 'large', we set CGAIN to
> +	 * '1' which doubles the range for half the precision.
> +	 */
> +	int			ichg_eoc, ichg_lo, ichg_hi, cur;

We should really get uA_t.

[Plus, this is not kernel comment style, but...]

> +	/* For ichg_eoc, reg value must be 100XXXX000, we only
> +	 * set the XXXX in high nibble.
> +	 */

Confused. High nibble is 0xf0, right?

> +	/* And finally, set the current.  This is stored in
> +	 * two registers. */
> +	reg = ua2regval(bci->cur, cgain);
> +	/* we have only 10 bit */

bits

> @@ -562,6 +698,14 @@ static int __init 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 */

Low

> +	bci->ichg_hi = 500000; /* High threshold */

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 07/15] twl4030_charger: allow fine control of charger current.

[NeilBrown]

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

On Mon, 2 Mar 2015 22:05:01 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > +	/* ichg values in uA. If any are 'large', we set CGAIN to
> > +	 * '1' which doubles the range for half the precision.
> > +	 */
> > +	int			ichg_eoc, ichg_lo, ichg_hi, cur;
> 
> We should really get uA_t.

Maybe. 32bits allows for  4000A, which should be plenty...
I made it unsigned int.

> 
> [Plus, this is not kernel comment style, but...]

Fixed

> 
> > +	/* For ichg_eoc, reg value must be 100XXXX000, we only
> > +	 * set the XXXX in high nibble.
> > +	 */
> 
> Confused. High nibble is 0xf0, right?

does this help?

	/*
	 * For ichg_eoc, the hardware only supports reg values matching
	 * 100XXXX000, and requires the XXXX be stored in the high nibble
	 * of TWL4030_BCIMFTH8
	 */


> 
> > +	/* And finally, set the current.  This is stored in
> > +	 * two registers. */
> > +	reg = ua2regval(bci->cur, cgain);
> > +	/* we have only 10 bit */
> 
> bits
> 
> > @@ -562,6 +698,14 @@ static int __init 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 */
> 
> Low
> 
> > +	bci->ichg_hi = 500000; /* High threshold */
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 

Thanks again!

NeilBrown

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

Re: [PATCH 07/15] twl4030_charger: allow fine control of charger current.

[Pavel Machek]

Hi!

> does this help?
> 
> 	/*
> 	 * For ichg_eoc, the hardware only supports reg values matching
> 	 * 100XXXX000, and requires the XXXX be stored in the high nibble
> 	 * of TWL4030_BCIMFTH8
> 	 */

A bit, yes. Add "." at the end of sentence ;-).

(Now I'd need to study the datasheets... and figure out how to enable
this on N900).

Thanks,
									Pavel
									

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

Re: [PATCH 07/15] twl4030_charger: allow fine control of charger current.

[Sebastian Reichel]

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

Hi Pavel,

On Wed, Mar 04, 2015 at 11:24:15AM +0100, Pavel Machek wrote:
> (Now I'd need to study the datasheets... and figure out how to enable
> this on N900).

The N900 uses bq24150a for charging ;)

-- Sebastian

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

[PATCH 08/15] twl4030_charger: distinguish between USB current and 'AC' current

[NeilBrown]

The twl4030 charger has two current sources, 'USB' and 'AC' (which is
really DC of course...).

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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 7c35cd9ba171..bfc9b808301e 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
@@ -103,7 +104,9 @@ struct twl4030_bci {
 	/* ichg values in uA. If any are 'large', we set CGAIN to
 	 * '1' which doubles the range for half the precision.
 	 */
-	int			ichg_eoc, ichg_lo, ichg_hi, cur;
+	int			ichg_eoc, ichg_lo, ichg_hi;
+	int			usb_cur, ac_cur;
+	bool			ac_is_active;
 
 	unsigned long		event;
 };
@@ -224,11 +227,23 @@ 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;
 	int cgain = 0;
 	u8 boot_bci;
 
+	/* If VAC 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 = 1;
+	} else {
+		cur = bci->usb_cur;
+		bci->ac_is_active = 0;
+	}
+
 	/* First, check thresholds and see if cgain is needed */
 	if (bci->ichg_eoc >= 200000)
 		cgain = 1;
@@ -236,7 +251,7 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 		cgain = 1;
 	if (bci->ichg_hi >= 820000)
 		cgain = 1;
-	if (bci->cur > 852000)
+	if (cur > 852000)
 		cgain = 1;
 
 	status = twl4030_bci_read(TWL4030_BCICTL1, &bcictl1);
@@ -311,7 +326,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 bit */
 	if (reg > 0x3ff)
 		reg = 0x3ff;
@@ -364,6 +379,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);
@@ -456,6 +473,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);
 
@@ -488,6 +506,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))
@@ -701,10 +720,11 @@ static int __init 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);

Re: [PATCH 08/15] twl4030_charger: distinguish between USB current and 'AC' current

[Pavel Machek]

Hi!

> The twl4030 charger has two current sources, 'USB' and 'AC' (which is
> really DC of course...).
> 
> 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.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

> @@ -103,7 +104,9 @@ struct twl4030_bci {
>  	/* ichg values in uA. If any are 'large', we set CGAIN to

ichg/_cur values

>  	 * '1' which doubles the range for half the precision.
>  	 */
> -	int			ichg_eoc, ichg_lo, ichg_hi, cur;
> +	int			ichg_eoc, ichg_lo, ichg_hi;
> +	int			usb_cur, ac_cur;

> +	/* If VAC exceeds 4.5V (MADC 11) and ac is enabled, set current
> +	 * for 'ac'

	/*
	 * If AC voltage exceeds 4.5V (MADC 11) and AC is enabled, set current
	 * for 'ac'.


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 08/15] twl4030_charger: distinguish between USB current and 'AC' current

[NeilBrown]

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

On Mon, 2 Mar 2015 22:05:10 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > The twl4030 charger has two current sources, 'USB' and 'AC' (which is
> > really DC of course...).
> > 
> > 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.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> 
> > @@ -103,7 +104,9 @@ struct twl4030_bci {
> >  	/* ichg values in uA. If any are 'large', we set CGAIN to
> 
> ichg/_cur values
> 
> >  	 * '1' which doubles the range for half the precision.
> >  	 */
> > -	int			ichg_eoc, ichg_lo, ichg_hi, cur;
> > +	int			ichg_eoc, ichg_lo, ichg_hi;
> > +	int			usb_cur, ac_cur;
> 
> > +	/* If VAC exceeds 4.5V (MADC 11) and ac is enabled, set current
> > +	 * for 'ac'
> 
> 	/*
> 	 * If AC voltage exceeds 4.5V (MADC 11) and AC is enabled, set current
> 	 * for 'ac'.
> 
> 
> Acked-by: Pavel Machek <pavel@ucw.cz>
> 

Thanks - I made those changes, or something much like them.

NeilBrown

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

[PATCH 05/15] 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 <neilb@suse.de>
---
 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 11f352a5ef55..db8931a17541 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.
@@ -649,7 +623,12 @@ static int __init 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 04/15] 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.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 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 51321f0c5548..11f352a5ef55 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;
 		}
 	}
@@ -602,8 +596,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
 	bci->usb.get_property = twl4030_bci_get_property;
 
-	bci->usb_reg = regulator_get(bci->dev, "bci3v1");
-
 	ret = devm_power_supply_register(&pdev->dev, &bci->usb);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register usb: %d\n", ret);

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

[Lee Jones]

On Tue, 24 Feb 2015, 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.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/mfd/twl-core.c          |    9 ++++-----

Acked-by: Lee Jones <lee.jones@linaro.org>

>  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 51321f0c5548..11f352a5ef55 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;
>  		}
>  	}
> @@ -602,8 +596,6 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
>  	bci->usb.num_properties = ARRAY_SIZE(twl4030_charger_props);
>  	bci->usb.get_property = twl4030_bci_get_property;
>  
> -	bci->usb_reg = regulator_get(bci->dev, "bci3v1");
> -
>  	ret = devm_power_supply_register(&pdev->dev, &bci->usb);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to register usb: %d\n", ret);
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[NeilBrown]

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

On Wed, 25 Feb 2015 07:24:43 +0000 Lee Jones <lee.jones@linaro.org> wrote:

> On Tue, 24 Feb 2015, 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.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/mfd/twl-core.c          |    9 ++++-----
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks.
I guess that clear it to go in though the 'power supply' tree.

NeilBrown

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

[PATCH 11/15] 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.

Signed-off-by: NeilBrown <neilb@suse.de>

Conflicts:
	drivers/power/twl4030_charger.c
---
 drivers/power/twl4030_charger.c |   57 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 01090a440583..19e8dbb1303e 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -107,6 +107,9 @@ struct twl4030_bci {
 	int			ichg_eoc, ichg_lo, ichg_hi;
 	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;
 };
@@ -377,6 +380,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 +634,54 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 	return NOTIFY_OK;
 }
 
+/*
+ * sysfs charger enabled store
+ */
+static char *modes[] = { "off", "auto" };
+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;
@@ -799,6 +852,7 @@ static int __init 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);
@@ -885,6 +939,8 @@ static int __init 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");
 
@@ -917,6 +973,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
 
 	device_remove_file(bci->usb.dev, &dev_attr_max_current);
 	device_remove_file(bci->ac.dev, &dev_attr_max_current);
+	device_remove_file(bci->usb.dev, &dev_attr_mode);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR1A);

Re: [PATCH 11/15] twl4030_charger: enable manual enable/disable of usb charging.

[Pavel Machek]

On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> '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.

This should go to Documentation/

> Signed-off-by: NeilBrown <neilb@suse.de>
> 
> Conflicts:
> 	drivers/power/twl4030_charger.c

Is it supposed to be here?

> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index 01090a440583..19e8dbb1303e 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -107,6 +107,9 @@ struct twl4030_bci {
>  	int			ichg_eoc, ichg_lo, ichg_hi;
>  	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;
>  };
> @@ -377,6 +380,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
>  {
>  	int ret;
>  
n> +	if (bci->usb_mode == CHARGE_OFF)
> +		enable = false;

Sometimes, you use = false and sometimes = 0 when assigning to bools...

> @@ -629,6 +634,54 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
>  	return NOTIFY_OK;
>  }
>  
> +/*
> + * sysfs charger enabled store
> + */
> +static char *modes[] = { "off", "auto" };

I'd move this before the comment. Or better near struct twl4030_bci.

> +static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
> +		   twl4030_bci_mode_store);
> +
>  static int twl4030_charger_get_current(void)
>  {
>  	int curr;
> @@ -799,6 +852,7 @@ static int __init 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);
> @@ -885,6 +939,8 @@ static int __init 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");
>  
> @@ -917,6 +973,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
>  
>  	device_remove_file(bci->usb.dev, &dev_attr_max_current);
>  	device_remove_file(bci->ac.dev, &dev_attr_max_current);
> +	device_remove_file(bci->usb.dev, &dev_attr_mode);

move the line above for consistency with create?

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 11/15] twl4030_charger: enable manual enable/disable of usb charging.

[NeilBrown]

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

On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > '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.
> 
> This should go to Documentation/

You mean in Documentation/ABI/stable I guess??

That strikes me as a failed experiment - there is hardly anything there.

I'd be very happy to put the documentation with the code if there was some
mechanism to automatically extract it.  But I really see little value in
Documentation/ABI.

Or did you mean somewhere else?


> 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > 
> > Conflicts:
> > 	drivers/power/twl4030_charger.c
> 
> Is it supposed to be here?

ah, no.  Gone now.  Thanks.


> 
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 01090a440583..19e8dbb1303e 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -107,6 +107,9 @@ struct twl4030_bci {
> >  	int			ichg_eoc, ichg_lo, ichg_hi;
> >  	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;
> >  };
> > @@ -377,6 +380,8 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> >  {
> >  	int ret;
> >  
> n> +	if (bci->usb_mode == CHARGE_OFF)
> > +		enable = false;
> 
> Sometimes, you use = false and sometimes = 0 when assigning to bools...

I found a fixed a few - thanks.


> 
> > @@ -629,6 +634,54 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
> >  	return NOTIFY_OK;
> >  }
> >  
> > +/*
> > + * sysfs charger enabled store
> > + */
> > +static char *modes[] = { "off", "auto" };
> 
> I'd move this before the comment. Or better near struct twl4030_bci.

Makes sense.  Done.  Thanks.


> 
> > +static DEVICE_ATTR(mode, 0644, twl4030_bci_mode_show,
> > +		   twl4030_bci_mode_store);
> > +
> >  static int twl4030_charger_get_current(void)
> >  {
> >  	int curr;
> > @@ -799,6 +852,7 @@ static int __init 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);
> > @@ -885,6 +939,8 @@ static int __init 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");
> >  
> > @@ -917,6 +973,7 @@ static int __exit twl4030_bci_remove(struct platform_device *pdev)
> >  
> >  	device_remove_file(bci->usb.dev, &dev_attr_max_current);
> >  	device_remove_file(bci->ac.dev, &dev_attr_max_current);
> > +	device_remove_file(bci->usb.dev, &dev_attr_mode);
> 
> move the line above for consistency with create?

Yep.

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

Thanks a lot!

NeilBrown

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

Re: [PATCH 11/15] twl4030_charger: enable manual enable/disable of usb charging.

[Pavel Machek]

On Wed 2015-03-04 17:15:09, NeilBrown wrote:
> On Mon, 2 Mar 2015 22:03:42 +0100 Pavel Machek <pavel@ucw.cz> wrote:
> 
> > On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > > '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.
> > 
> > This should go to Documentation/
> 
> You mean in Documentation/ABI/stable I guess??

Yes.

> That strikes me as a failed experiment - there is hardly anything there.
> 
> I'd be very happy to put the documentation with the code if there was some
> mechanism to automatically extract it.  But I really see little value in
> Documentation/ABI.

It was useful in past, and rules say it is required. Feel free to talk
to greg about better place, but this is the place we have now and
documentation is useful. And since you already document it in the
changelogs... it should not be that much work.

> > Acked-by: Pavel Machek <pavel@ucw.cz>
> 
> Thanks a lot!

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

[PATCH 12/15] twl4030_charger: add software controlled linear charging mode.

[NeilBrown]

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

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.

Orignal-by: Andreas Kemnade <andreas@kemnade.info>
Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   57 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 19e8dbb1303e..6c53f0b601a4 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)
@@ -110,6 +115,7 @@ struct twl4030_bci {
 	int			usb_mode; /* charging mode requested */
 #define	CHARGE_OFF	0
 #define	CHARGE_AUTO	1
+#define	CHARGE_LINEAR	2
 
 	unsigned long		event;
 };
@@ -392,16 +398,44 @@ 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);
+			twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC|TWL4030_CVENAC);
+		}
 
 		/* 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);
@@ -637,7 +671,7 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 /*
  * sysfs charger enabled store
  */
-static char *modes[] = { "off", "auto" };
+static char *modes[] = { "off", "auto", "continuous" };
 static ssize_t
 twl4030_bci_mode_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t n)
@@ -650,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);
@@ -748,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->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:

Re: [PATCH 12/15] twl4030_charger: add software controlled linear charging mode.

[Pavel Machek]

On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> Add a 'continuous' option for usb charging which enabled
> the "linear" charging mode of the twl4030.

Documentation/ :-).

> Linear charging does a good job with not so reliable power sources, since
> several voltage controlling is then often too intelligent.

Parse error.

> 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.
> 
> Orignal-by: Andreas Kemnade <andreas@kemnade.info>
> Signed-off-by: NeilBrown <neilb@suse.de>


> +			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 */

USB

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

Heat save? Is ignoring over-current good idea?

> @@ -650,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;

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

Re: [PATCH 12/15] twl4030_charger: add software controlled linear charging mode.

[NeilBrown]

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

On Mon, 2 Mar 2015 22:09:26 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > Add a 'continuous' option for usb charging which enabled
> > the "linear" charging mode of the twl4030.
> 
> Documentation/ :-).

!

> 
> > Linear charging does a good job with not so reliable power sources, since
> > several voltage controlling is then often too intelligent.
> 
> Parse error.

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

> 
> > 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.
> > 
> > Orignal-by: Andreas Kemnade <andreas@kemnade.info>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> 
> 
> > +			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 */
> 
> USB
> 
> > +			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
> 
> Heat save? Is ignoring over-current good idea?

The data sheet refers to a flag "HSEN" which is "Heat Save function enable".

your guess is as good as mine.

We don't currently have any code for responding to warnings.  Maybe we should.

For now this feature is "use at your own risk" - and one person has found it
very useful.

> 
> > @@ -650,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;
> 
> Time for loop?

When we get one more mode it will be :-)

Thanks,
NeilBrown


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

twl4030_charger: need changes to get probed?

[Pavel Machek]

Hi!

According to n900 dts, twl4030-bci (aka charger) should be included.

(But it does not seem to do anything useful on n900. I was hoping for
measurement of input voltage, but .. no.)

Any ideas why the patch below is needed?

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

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index d35b83e..96bbbe9 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -714,6 +722,7 @@ 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),
@@ -721,7 +730,7 @@ static struct platform_driver twl4030_bci_driver = {
 	.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");


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

Re: twl4030_charger: need changes to get probed?

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 224 bytes --]

On Friday 06 March 2015 22:24:17 Pavel Machek wrote:
> Hi!
> 
> According to n900 dts, twl4030-bci (aka charger) should be
> included.
> 

AFAIK it is not present on n900...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: twl4030_charger: need changes to get probed?

[Grazvydas Ignotas]

On Fri, Mar 6, 2015 at 11:57 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Friday 06 March 2015 22:24:17 Pavel Machek wrote:
>> Hi!
>>
>> According to n900 dts, twl4030-bci (aka charger) should be
>> included.
>>
>
> AFAIK it is not present on n900...

Right, it uses twl5030 variant without the charger, charging on n900
is provided by separate chip and for a good reason as twl's charger is
not that good. Forcing the driver to load just ends up with it
accessing non-existent registers over i2c.


Gražvydas

Re: twl4030_charger: need changes to get probed?

[Pavel Machek]

On Sat 2015-03-07 00:12:07, Grazvydas Ignotas wrote:
> On Fri, Mar 6, 2015 at 11:57 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> > On Friday 06 March 2015 22:24:17 Pavel Machek wrote:
> >> Hi!
> >>
> >> According to n900 dts, twl4030-bci (aka charger) should be
> >> included.
> >>
> >
> > AFAIK it is not present on n900...
> 
> Right, it uses twl5030 variant without the charger, charging on n900
> is provided by separate chip and for a good reason as twl's charger is
> not that good. Forcing the driver to load just ends up with it
> accessing non-existent registers over i2c.

Ok, but:

1) Why is the twl4030-bci enabled in n900's dts, then

and

2) When it is enabled, why it does not load?

(I guess there's no way to get to input voltage on n900...?)

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

Re: twl4030_charger: need changes to get probed?

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1357 bytes --]

On Friday 06 March 2015 23:40:34 Pavel Machek wrote:
> On Sat 2015-03-07 00:12:07, Grazvydas Ignotas wrote:
> > On Fri, Mar 6, 2015 at 11:57 PM, Pali Rohár
> > <pali.rohar@gmail.com> wrote:
> > > On Friday 06 March 2015 22:24:17 Pavel Machek wrote:
> > >> Hi!
> > >> 
> > >> According to n900 dts, twl4030-bci (aka charger) should
> > >> be included.
> > > 
> > > AFAIK it is not present on n900...
> > 
> > Right, it uses twl5030 variant without the charger, charging
> > on n900 is provided by separate chip and for a good reason
> > as twl's charger is not that good. Forcing the driver to
> > load just ends up with it accessing non-existent registers
> > over i2c.
> 
> Ok, but:
> 
> 1) Why is the twl4030-bci enabled in n900's dts, then
> 

maybe it is bug in n900 dts...

Grazvydas, is there some runtime check if twl4030/twl5030 chip 
has charger or not? or do we need to explicitly disable device 
twl4030-bci in DT?

> and
> 
> 2) When it is enabled, why it does not load?
> 
> (I guess there's no way to get to input voltage on n900...?)
> 
> 									Pavel

you can read voltage only from rx51_battery.ko (TWL ADC) or 
bq27x00_battery.ko

look for Nokia_N900_RX-51_Schematics.pdf file where you can find 
information what is connected to ADC and bq27200 chip.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: twl4030_charger: need changes to get probed?

[Grazvydas Ignotas]

On Sat, Mar 7, 2015 at 12:56 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Friday 06 March 2015 23:40:34 Pavel Machek wrote:
>> On Sat 2015-03-07 00:12:07, Grazvydas Ignotas wrote:
>> > On Fri, Mar 6, 2015 at 11:57 PM, Pali Rohár
>> > <pali.rohar@gmail.com> wrote:
>> > > On Friday 06 March 2015 22:24:17 Pavel Machek wrote:
>> > >> Hi!
>> > >>
>> > >> According to n900 dts, twl4030-bci (aka charger) should
>> > >> be included.
>> > >
>> > > AFAIK it is not present on n900...
>> >
>> > Right, it uses twl5030 variant without the charger, charging
>> > on n900 is provided by separate chip and for a good reason
>> > as twl's charger is not that good. Forcing the driver to
>> > load just ends up with it accessing non-existent registers
>> > over i2c.
>>
>> Ok, but:
>>
>> 1) Why is the twl4030-bci enabled in n900's dts, then
>>
>
> maybe it is bug in n900 dts...
>
> Grazvydas, is there some runtime check if twl4030/twl5030 chip
> has charger or not? or do we need to explicitly disable device
> twl4030-bci in DT?

Actually from looking at the schematics, it looks like the charger
pins are still there but all connected to ground. So it probably has
the charger after all, it's just not connected or used.

I'm not aware or any registers for direct detection, and indirect
detection is difficult because BCI mostly disables itself when no
charger is connected and most registers read as 0 or have old values
from last charging session (which will never happen on n900).

There is IDCODE register on twl4030 itself, but it's documented as not
meaningful when accessed over i2c (when is it meaningful then??).

drivers/mfd/twl-core.c has a i2c_device_id table of various twl4030
variants, some of which have no charger. N900 has GAIA/twl5030, which
differs from twl4030 only by vaux2 regulator according to that file.
N900's old board files specify 5030, but .dts does not.


Gražvydas

Re: twl4030_charger: need changes to get probed?

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 223 bytes --]

On Saturday 07 March 2015 16:56:01 Grazvydas Ignotas wrote:
> N900's old board files specify 5030, but .dts does not.

I would guess this is bug and DTS file needs to be fixed.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: twl4030_charger: need changes to get probed?

[Pavel Machek]

Hi!

> >> Ok, but:
> >>
> >> 1) Why is the twl4030-bci enabled in n900's dts, then
> >>
> >
> > maybe it is bug in n900 dts...
> >
> > Grazvydas, is there some runtime check if twl4030/twl5030 chip
> > has charger or not? or do we need to explicitly disable device
> > twl4030-bci in DT?
> 
> Actually from looking at the schematics, it looks like the charger
> pins are still there but all connected to ground. So it probably has
> the charger after all, it's just not connected or used.
> 
> I'm not aware or any registers for direct detection, and indirect
> detection is difficult because BCI mostly disables itself when no
> charger is connected and most registers read as 0 or have old values
> from last charging session (which will never happen on n900).
> 
> There is IDCODE register on twl4030 itself, but it's documented as not
> meaningful when accessed over i2c (when is it meaningful then??).
> 
> drivers/mfd/twl-core.c has a i2c_device_id table of various twl4030
> variants, some of which have no charger. N900 has GAIA/twl5030, which
> differs from twl4030 only by vaux2 regulator according to that file.
> N900's old board files specify 5030, but .dts does not.

I have enabled the (not-too-useful) twl4030 charger on my n900, and
it seems to break boot with kernels
34c9a0ffc75ad25b6a60f61e27c4a4b1189b8085 and newer.

I'll probably not have time to investigate it further (charger not
connected to anything is not too useful), but maybe Neil wants to test
on his phone...?
									
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: twl4030_charger: need changes to get probed?

[Sebastian Reichel]

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

Hi,

On Fri, Mar 06, 2015 at 10:24:17PM +0100, Pavel Machek wrote:
> According to n900 dts, twl4030-bci (aka charger) should be
> included.

its part of twl, but not used on N900 afaik.

> (But it does not seem to do anything useful on n900. I was hoping for
> measurement of input voltage, but .. no.)

check for rx51-battery.

> Any ideas why the patch below is needed?

platform_driver_probe() does not support deferred probing.

Neil, can you take this patch into your series for the next round?

-- Sebastian

> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index d35b83e..96bbbe9 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -714,6 +722,7 @@ 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),
> @@ -721,7 +730,7 @@ static struct platform_driver twl4030_bci_driver = {
>  	.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");

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

Re: twl4030_charger: need changes to get probed?

[NeilBrown]

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

On Sat, 7 Mar 2015 22:01:02 +0100 Sebastian Reichel <sre@debian.org> wrote:

> Hi,
> 
> On Fri, Mar 06, 2015 at 10:24:17PM +0100, Pavel Machek wrote:
> > According to n900 dts, twl4030-bci (aka charger) should be
> > included.
> 
> its part of twl, but not used on N900 afaik.
> 
> > (But it does not seem to do anything useful on n900. I was hoping for
> > measurement of input voltage, but .. no.)
> 
> check for rx51-battery.
> 
> > Any ideas why the patch below is needed?
> 
> platform_driver_probe() does not support deferred probing.
> 
> Neil, can you take this patch into your series for the next round?

I could, but I do wonder if it is the right thing to do.

Shouldn't we fix platform_driver_probe() to support deferred probing.

As I understand it, it refused to retry a probe if there is an error, and the
comments suggest that such retrying is avoided because it would be a waste
of time:

	/*
	 * Prevent driver from requesting probe deferral to avoid further
	 * futile probe attempts.
	 */

In this case, it isn't futile.

Earlier there is a comment saying:

 * Use this instead of platform_driver_register() when you know the device
 * is not hotpluggable and has already been registered, and you want to
 * remove its run-once probe() infrastructure from memory after the driver
 * has bound to the device.

I presume all this applies.  I assume that the only problem is a probe-order
thing.  So maybe we should fix platform_driver_probe() to do the right thing
with -EPROBEDEFER??

Trouble is, I really don't understand the  point or mechanism for
platform_driver_probe(), so I cannot suggest anything.
But I have been annoyed before that platform_driver_probe doesn't cope with
EPROBEDEFER, so I would like it fixed.

NeilBrown

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

Re: twl4030_charger: need changes to get probed?

[Sebastian Reichel]

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

Hi,

On Mon, Mar 09, 2015 at 11:06:53AM +1100, NeilBrown wrote:
> On Sat, 7 Mar 2015 22:01:02 +0100 Sebastian Reichel <sre@debian.org> wrote:
> > platform_driver_probe() does not support deferred probing.
> > 
> > Neil, can you take this patch into your series for the next round?
> 
> I could, but I do wonder if it is the right thing to do.
> 
> Shouldn't we fix platform_driver_probe() to support deferred probing.

well most drivers use platform_driver_register anyways. Other
subsystems, like e.g. i2c have converted all drivers already.
In drivers/power/ there are only three drivers using
platform_driver_probe:

drivers/power/avs/smartreflex.c - ok here
drivers/power/reset/brcmstb-reboot.c - looks ok, too
drivers/power/twl4030_charger.c - should probably be converted

> As I understand it, it refused to retry a probe if there is an error, and the
> comments suggest that such retrying is avoided because it would be a waste
> of time:
> 
> 	/*
> 	 * Prevent driver from requesting probe deferral to avoid further
> 	 * futile probe attempts.
> 	 */
> 
> In this case, it isn't futile.

All drivers would benefit of being probed again if they returned
EPROBEDEFER, but their probe function can't be called again if
they use driver_platform_probe, since the probe function will be
unloaded when it should be called again. Apart from that the
.probe function pointer is not set. Thus trying to probe the
driver again at a later point is "a waste of time" and "futile",
since it will definitely fail.

> Earlier there is a comment saying:
> 
>  * Use this instead of platform_driver_register() when you know the device
>  * is not hotpluggable and has already been registered, and you want to
>  * remove its run-once probe() infrastructure from memory after the driver
>  * has bound to the device.
> 
> I presume all this applies.  I assume that the only problem is a probe-order
> thing.  So maybe we should fix platform_driver_probe() to do the right thing
> with -EPROBEDEFER??
> 
> Trouble is, I really don't understand the  point or mechanism for
> platform_driver_probe(), so I cannot suggest anything.
> But I have been annoyed before that platform_driver_probe doesn't cope with
> EPROBEDEFER, so I would like it fixed.

platform_driver_probe is not about probe-order, but about not having
the probe function in memory once the driver is loaded. So the probe
function cannot be called again. If you don't want this use
platform_driver_register, as most drivers actually do.

I guess we should add some coccinelle scripts for detection of
potentially broken drivers (e.g. everything requesting gpios/pinctrl
together with platform_driver_register).

-- Sebastian

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

[PATCH 09/15] 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.

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

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index bfc9b808301e..b0242786d047 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -473,6 +473,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);
@@ -527,6 +529,67 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+/*
+ * sysfs max_current store
+ */
+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) {
+		if (bci->ac_cur == cur)
+			return n;
+		bci->ac_cur = cur;
+	} else {
+		if (bci->usb_cur == cur)
+			return n;
+		bci->usb_cur = cur;
+	}
+	twl4030_charger_update_current(bci);
+	return (status == 0) ? n : status;
+}
+
+/*
+ * 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);
@@ -549,6 +612,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);
 
@@ -809,6 +878,11 @@ static int __init 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,
@@ -836,6 +910,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);

Re: [PATCH 09/15] twl4030_charger: allow max_current to be managed via sysfs.

[Pavel Machek]

On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> '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.

Does this need Documentation update?

> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/power/twl4030_charger.c |   76 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index bfc9b808301e..b0242786d047 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -527,6 +529,67 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +/*
> + * sysfs max_current store
> + */

That's not exactly useful comment.

> +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) {
> +		if (bci->ac_cur == cur)
> +			return n;
> +		bci->ac_cur = cur;
> +	} else {
> +		if (bci->usb_cur == cur)
> +			return n;
> +		bci->usb_cur = cur;
> +	}
> +	twl4030_charger_update_current(bci);
> +	return (status == 0) ? n : status;
> +}

Uff. but we know that status == 0 at this point, no? Also... is
optimalization of not calling update_current() when nothing changed
worth it?

> +/*
> + * 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);
> +}

Is this in uA or mA? uA. Ok.

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

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

Re: [PATCH 09/15] twl4030_charger: allow max_current to be managed via sysfs.

[NeilBrown]

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

On Mon, 2 Mar 2015 22:05:26 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> > '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.
> 
> Does this need Documentation update?

Oh all right... I've created the relevant documentation in Documentation/ABI.

It seems persistence pays off :-)


> 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/power/twl4030_charger.c |   76 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 76 insertions(+)
> > 
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index bfc9b808301e..b0242786d047 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -527,6 +529,67 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void *arg)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +/*
> > + * sysfs max_current store
> > + */
> 
> That's not exactly useful comment.

Now:

 * 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) {
> > +		if (bci->ac_cur == cur)
> > +			return n;
> > +		bci->ac_cur = cur;
> > +	} else {
> > +		if (bci->usb_cur == cur)
> > +			return n;
> > +		bci->usb_cur = cur;
> > +	}
> > +	twl4030_charger_update_current(bci);
> > +	return (status == 0) ? n : status;
> > +}
> 
> Uff. but we know that status == 0 at this point, no? 

Yes.  Fixed.

>                                                       Also... is
> optimalization of not calling update_current() when nothing changed
> worth it?

Probably not... and  code looks a lot nicer if I remove that.
So I have.


> 
> > +/*
> > + * 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);
> > +}
> 
> Is this in uA or mA? uA. Ok.

uA, now described in Documentation/ABI/testing/sysfs-class-power-twl4030

+What: /sys/class/power_supply/twl4030_ac/max_current
+      /sys/class/power_supply/twl4030_usb/max_current
+Description:
+       Read/Write limit on current which 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 on 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.


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

Thanks,
NeilBrown

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

Re: [PATCH 09/15] twl4030_charger: allow max_current to be managed via sysfs.

[Pavel Machek]

> > Is this in uA or mA? uA. Ok.
> 
> uA, now described in Documentation/ABI/testing/sysfs-class-power-twl4030
> 
> +What: /sys/class/power_supply/twl4030_ac/max_current
> +      /sys/class/power_supply/twl4030_usb/max_current
> +Description:
> +       Read/Write limit on current which 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 on 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.

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

Re: [PATCH 09/15] twl4030_charger: allow max_current to be managed via sysfs.

[Sebastian Reichel]

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

Hi,

On Thu, Mar 05, 2015 at 05:26:00PM +1100, NeilBrown wrote:
> [...]
> +What: /sys/class/power_supply/twl4030_ac/max_current
> +      /sys/class/power_supply/twl4030_usb/max_current
> +Description:
> +       Read/Write limit on current which 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 on unplugged.
                                        ^^
s/on/or

-- Sebastian

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

[PATCH 10/15] twl4030_charger: only draw USB current as negotiated with host.

[NeilBrown]

If the phy has been told what current it can draw, it tells us
and now we use that number.

Note that 'vbus_draw' is in mA, while usb_cur is in uA.

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

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index b0242786d047..01090a440583 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -597,6 +597,7 @@ static void twl4030_bci_usb_work(struct work_struct *data)
 	switch (bci->event) {
 	case USB_EVENT_VBUS:
 	case USB_EVENT_CHARGER:
+	case USB_EVENT_ENUMERATED:
 		twl4030_charger_enable_usb(bci, true);
 		break;
 	case USB_EVENT_NONE:
@@ -609,6 +610,7 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 			       void *priv)
 {
 	struct twl4030_bci *bci = container_of(nb, struct twl4030_bci, usb_nb);
+	unsigned *vbus_draw = priv;
 
 	dev_dbg(bci->dev, "OTG notify %lu\n", val);
 
@@ -619,6 +621,9 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
 		bci->usb_cur = 100000;
 
 	bci->event = val;
+	if (val == USB_EVENT_ENUMERATED && vbus_draw &&
+	    *vbus_draw * 1000 > bci->usb_cur)
+		bci->usb_cur = *vbus_draw * 1000;
 	schedule_work(&bci->work);
 
 	return NOTIFY_OK;

Re: [PATCH 10/15] twl4030_charger: only draw USB current as negotiated with host.

[Pavel Machek]

On Tue 2015-02-24 15:33:52, NeilBrown wrote:
> If the phy has been told what current it can draw, it tells us
> and now we use that number.
> 
> Note that 'vbus_draw' is in mA, while usb_cur is in uA.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

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

> @@ -609,6 +610,7 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
>  			       void *priv)
>  {
>  	struct twl4030_bci *bci = container_of(nb, struct twl4030_bci, usb_nb);
> +	unsigned *vbus_draw = priv;

unsigned int? [Or better mA_t *? :-)]

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

[PATCH 14/15] 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 configure Standard Downstream Port, it may not be possible
to reliably pull 500mA - depending on cable quality and source
quality I have reports of charging failure due to the voltage dropping
too low.

To address both these concern, this patch introduce incremental
current setting.
The current pull from VBUS is increased in steps of 20mA every 100ms
until the target is reached or until the measure voltage drops below
4.75V.  If the voltage does go too long, the target current is reduced
by 20mA and kept there.

This applies to currents selected automatically, or to values
set via sysfs.  So setting a large value will cause the maximum
available to be used - up to the limit of 1.7mA imposed by the
hardware.

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

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index e5a0225ea87e..7ad6b4b531d7 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -117,6 +117,18 @@ struct twl4030_bci {
 #define	CHARGE_AUTO	1
 #define	CHARGE_LINEAR	2
 
+	/* When setting the USB current we slowly increase the
+	 * requested current until target is reached or the voltage
+	 * drops below 4.75V.  In the latter case we set back one
+	 * step.
+	 */
+	int			usb_cur_actual;
+	struct delayed_work	current_worker;
+#define	USB_CUR_STEP	20000	/* 20mA at a time */
+#define	USB_MIN_VOLT	4750000	/* 4.75V */
+#define	USB_CUR_DELAY	msecs_to_jiffies(100)
+#define	USB_MAX_CURRENT	1700000 /* TWL4030 caps at 1.7mA */
+
 	unsigned long		event;
 };
 
@@ -249,8 +261,14 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 		cur = bci->ac_cur;
 		bci->ac_is_active = 1;
 	} else {
-		cur = bci->usb_cur;
+		cur = bci->usb_cur_actual;
 		bci->ac_is_active = 0;
+		if (cur > bci->usb_cur) {
+			cur = bci->usb_cur;
+			bci->usb_cur_actual = cur;
+		}
+		if (cur < bci->usb_cur)
+			schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
 	}
 
 	/* First, check thresholds and see if cgain is needed */
@@ -379,6 +397,38 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
 	return 0;
 }
 
+static void twl4030_current_worker(struct work_struct *data)
+{
+	int v;
+	int res;
+	struct twl4030_bci *bci = container_of(data, struct twl4030_bci,
+					       current_worker.work);
+
+	res = twl4030bci_read_adc_val(TWL4030_BCIVBUS);
+	if (res < 0)
+		v = 0;
+	else
+		/* BCIVBUS uses ADCIN8, 7/1023 V/step */
+		v = res * 6843;
+
+	printk("v=%d cur=%d target=%d\n", v, bci->usb_cur_actual,
+	       bci->usb_cur);
+
+	if (v < USB_MIN_VOLT) {
+		/* Back up and stop adjusting. */
+		bci->usb_cur_actual -= USB_CUR_STEP;
+		bci->usb_cur = bci->usb_cur_actual;
+	} else if (bci->usb_cur_actual >= bci->usb_cur ||
+		   bci->usb_cur_actual + USB_CUR_STEP > USB_MAX_CURRENT) {
+		/* Reach target and volts are OK - stop */
+		return;
+	} else {
+		bci->usb_cur_actual += USB_CUR_STEP;
+		schedule_delayed_work(&bci->current_worker, USB_CUR_DELAY);
+	}
+	twl4030_charger_update_current(bci);
+}
+
 /*
  * Enable/Disable USB Charge functionality.
  */
@@ -441,6 +491,7 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
 			pm_runtime_put_autosuspend(bci->transceiver->dev);
 			bci->usb_enabled = 0;
 		}
+		bci->usb_cur_actual = 0;
 	}
 
 	return ret;
@@ -972,6 +1023,7 @@ static int __init twl4030_bci_probe(struct platform_device *pdev)
 	}
 
 	INIT_WORK(&bci->work, twl4030_bci_usb_work);
+	INIT_DELAYED_WORK(&bci->current_worker, twl4030_current_worker);
 
 	bci->usb_nb.notifier_call = twl4030_bci_usb_ncb;
 	if (bci->dev->of_node) {

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

[Pavel Machek]

On Tue 2015-02-24 15:33:53, NeilBrown wrote:
> The USB Battery Charging spec (BC1.2) suggests a dedicated
> charging port can deliver from 0.5 to 5.0A at between 4.75 and 5.25
> volts.
> 
> To choose the "correct" current voltage setting requires a trial
> and error approach: try to draw current and see if the voltage drops
> too low.
> 
> Even with a configure Standard Downstream Port, it may not be possible
> to reliably pull 500mA - depending on cable quality and source

"configured"?

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

concerns.

> current setting.
> The current pull from VBUS is increased in steps of 20mA every 100ms
> until the target is reached or until the measure voltage drops below
> 4.75V.  If the voltage does go too long, the target current is reduced

"too low"?

> by 20mA and kept there.
> 
> This applies to currents selected automatically, or to values
> set via sysfs.  So setting a large value will cause the maximum
> available to be used - up to the limit of 1.7mA imposed by the
> hardware.

1.7A?


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

usb_cur_actual is not a really great variable name...

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

Reached ... and the voltage is OK - stop.


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

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

[NeilBrown]

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

On Mon, 2 Mar 2015 22:29:45 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:53, NeilBrown wrote:
> > The USB Battery Charging spec (BC1.2) suggests a dedicated
> > charging port can deliver from 0.5 to 5.0A at between 4.75 and 5.25
> > volts.
> > 
> > To choose the "correct" current voltage setting requires a trial
> > and error approach: try to draw current and see if the voltage drops
> > too low.
> > 
> > Even with a configure Standard Downstream Port, it may not be possible
> > to reliably pull 500mA - depending on cable quality and source
> 
> "configured"?
> 
> > quality I have reports of charging failure due to the voltage dropping
> > too low.
> > 
> > To address both these concern, this patch introduce incremental
> 
> concerns.
> 
> > current setting.
> > The current pull from VBUS is increased in steps of 20mA every 100ms
> > until the target is reached or until the measure voltage drops below
> > 4.75V.  If the voltage does go too long, the target current is reduced
> 
> "too low"?
> 
> > by 20mA and kept there.
> > 
> > This applies to currents selected automatically, or to values
> > set via sysfs.  So setting a large value will cause the maximum
> > available to be used - up to the limit of 1.7mA imposed by the
> > hardware.
> 
> 1.7A?
> 
> 
> > @@ -249,8 +261,14 @@ static int twl4030_charger_update_current(struct twl4030_bci *bci)
> >  		cur = bci->ac_cur;
> >  		bci->ac_is_active = 1;
> >  	} else {
> > -		cur = bci->usb_cur;
> > +		cur = bci->usb_cur_actual;
> 
> usb_cur_actual is not a really great variable name...

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

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

Thanks for all your proof-reading :-)

NeilBrown

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

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

[NeilBrown]

This allows AC charging to be turned off, much like usb charging.

"continuous" (aka "linear") mode maps to the CVENAC (constant voltage)
feature of the twl4030.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 drivers/power/twl4030_charger.c |   40 +++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index 6c53f0b601a4..e5a0225ea87e 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -112,7 +112,7 @@ struct twl4030_bci {
 	int			ichg_eoc, ichg_lo, ichg_hi;
 	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
@@ -449,12 +449,18 @@ 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 (enable)
-		ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
+	if (bci->ac_mode == CHARGE_OFF)
+		enable = false;
+
+	if (enable && bci->ac_mode == CHARGE_LINEAR)
+		ret = twl4030_clear_set_boot_bci(0, (TWL4030_CVENAC |
+						     TWL4030_BCIAUTOAC));
+	else if (enable)
+		ret = twl4030_clear_set_boot_bci(TWL4030_CVENAC, TWL4030_BCIAUTOAC);
 	else
 		ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC, 0);
 
@@ -688,9 +694,15 @@ 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) {
+		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 +716,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
@@ -900,6 +916,7 @@ static int __init 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);
@@ -988,10 +1005,12 @@ static int __init 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,
@@ -1014,13 +1033,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->ac.dev, &dev_attr_max_current);
 	device_remove_file(bci->usb.dev, &dev_attr_mode);
+	device_remove_file(bci->ac.dev, &dev_attr_mode);
 	/* mask interrupts */
 	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
 			 TWL4030_INTERRUPTS_BCIIMR1A);

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

[Pavel Machek]

On Tue 2015-02-24 15:33:53, NeilBrown wrote:
> This allows AC charging to be turned off, much like usb charging.
> 
> "continuous" (aka "linear") mode maps to the CVENAC (constant voltage)
> feature of the twl4030.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>

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

> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -112,7 +112,7 @@ struct twl4030_bci {
>  	int			ichg_eoc, ichg_lo, ichg_hi;
>  	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
> @@ -449,12 +449,18 @@ 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 (enable)
> -		ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
> +	if (bci->ac_mode == CHARGE_OFF)
> +		enable = false;
> +
> +	if (enable && bci->ac_mode == CHARGE_LINEAR)
> +		ret = twl4030_clear_set_boot_bci(0, (TWL4030_CVENAC |
> +						     TWL4030_BCIAUTOAC));
> +	else if (enable)
> +		ret = twl4030_clear_set_boot_bci(TWL4030_CVENAC, TWL4030_BCIAUTOAC);
>  	else
>  		ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC, 0);
>  
> @@ -688,9 +694,15 @@ 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) {
> +		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 +716,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
> @@ -900,6 +916,7 @@ static int __init 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);
> @@ -988,10 +1005,12 @@ static int __init 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,
> @@ -1014,13 +1033,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->ac.dev, &dev_attr_max_current);
>  	device_remove_file(bci->usb.dev, &dev_attr_mode);
> +	device_remove_file(bci->ac.dev, &dev_attr_mode);
>  	/* mask interrupts */
>  	twl_i2c_write_u8(TWL4030_MODULE_INTERRUPTS, 0xff,
>  			 TWL4030_INTERRUPTS_BCIIMR1A);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

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

[Grazvydas Ignotas]

On Tue, Feb 24, 2015 at 6:33 AM, NeilBrown <neilb@suse.de> wrote:
> This allows AC charging to be turned off, much like usb charging.
>
> "continuous" (aka "linear") mode maps to the CVENAC (constant voltage)
> feature of the twl4030.

Are you sure? Before your patches CVENAC was set at all times and and
charger still worked in automatic mode.

>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/power/twl4030_charger.c |   40 +++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> index 6c53f0b601a4..e5a0225ea87e 100644
> --- a/drivers/power/twl4030_charger.c
> +++ b/drivers/power/twl4030_charger.c
> @@ -112,7 +112,7 @@ struct twl4030_bci {
>         int                     ichg_eoc, ichg_lo, ichg_hi;
>         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
> @@ -449,12 +449,18 @@ 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 (enable)
> -               ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
> +       if (bci->ac_mode == CHARGE_OFF)
> +               enable = false;
> +
> +       if (enable && bci->ac_mode == CHARGE_LINEAR)
> +               ret = twl4030_clear_set_boot_bci(0, (TWL4030_CVENAC |
> +                                                    TWL4030_BCIAUTOAC));
> +       else if (enable)
> +               ret = twl4030_clear_set_boot_bci(TWL4030_CVENAC, TWL4030_BCIAUTOAC);
>         else
>                 ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC, 0);

CVENAC is required to be set for operation on AC without battery
(which works fine on most pandora boards). After this patch, when
booted without battery,  the board will reset before there is a chance
to set the linear mode by userspace, because this is called on
probe...


Gražvydas

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

[NeilBrown]

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

On Fri, 6 Mar 2015 23:59:04 +0200 Grazvydas Ignotas <notasas@gmail.com> wrote:

> On Tue, Feb 24, 2015 at 6:33 AM, NeilBrown <neilb@suse.de> wrote:
> > This allows AC charging to be turned off, much like usb charging.
> >
> > "continuous" (aka "linear") mode maps to the CVENAC (constant voltage)
> > feature of the twl4030.
> 
> Are you sure? Before your patches CVENAC was set at all times and and
> charger still worked in automatic mode.
> 
> >
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/power/twl4030_charger.c |   40 +++++++++++++++++++++++++++++----------
> >  1 file changed, 30 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> > index 6c53f0b601a4..e5a0225ea87e 100644
> > --- a/drivers/power/twl4030_charger.c
> > +++ b/drivers/power/twl4030_charger.c
> > @@ -112,7 +112,7 @@ struct twl4030_bci {
> >         int                     ichg_eoc, ichg_lo, ichg_hi;
> >         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
> > @@ -449,12 +449,18 @@ 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 (enable)
> > -               ret = twl4030_clear_set_boot_bci(0, TWL4030_BCIAUTOAC);
> > +       if (bci->ac_mode == CHARGE_OFF)
> > +               enable = false;
> > +
> > +       if (enable && bci->ac_mode == CHARGE_LINEAR)
> > +               ret = twl4030_clear_set_boot_bci(0, (TWL4030_CVENAC |
> > +                                                    TWL4030_BCIAUTOAC));
> > +       else if (enable)
> > +               ret = twl4030_clear_set_boot_bci(TWL4030_CVENAC, TWL4030_BCIAUTOAC);
> >         else
> >                 ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOAC, 0);
> 
> CVENAC is required to be set for operation on AC without battery
> (which works fine on most pandora boards). After this patch, when
> booted without battery,  the board will reset before there is a chance
> to set the linear mode by userspace, because this is called on
> probe...
> 

Yes, it looks like I misunderstood CVENAC a bit - thanks.

I've removed 'continuous' mode for AC and no longer clear or set that bit.

Thanks,
NeilBrown

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

[PATCH 15/15] 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 <neilb@suse.de>
---
 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 7ad6b4b531d7..89e2c121dd22 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -691,8 +691,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:
 	case USB_EVENT_ENUMERATED:
 		twl4030_charger_enable_usb(bci, true);
 		break;

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

[Pavel Machek]

On Tue 2015-02-24 15:33:53, NeilBrown wrote:
> If it cannot, we will stop pulling more current when voltage drops.

Can you justify it a bit more?

I mean... maybe there's a fuse in the charger? Or maybe it will supply
the current but overheat in the process? (USB_MAX_CURRENT is 500mA or
1.7A?)

Thanks,
									Pavel	

> Signed-off-by: NeilBrown <neilb@suse.de>

> @@ -691,8 +691,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:
>  	case USB_EVENT_ENUMERATED:
>  		twl4030_charger_enable_usb(bci, true);
>  		break;

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

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

[NeilBrown]

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

On Mon, 2 Mar 2015 22:29:39 +0100 Pavel Machek <pavel@ucw.cz> wrote:

> On Tue 2015-02-24 15:33:53, NeilBrown wrote:
> > If it cannot, we will stop pulling more current when voltage drops.
> 
> Can you justify it a bit more?
> 
> I mean... maybe there's a fuse in the charger? Or maybe it will supply
> the current but overheat in the process? (USB_MAX_CURRENT is 500mA or
> 1.7A?)

USB_MAX_CURRENT is 1.7A - the most the twl4030 will manage.

The relevant specs say that a charger can deliver from 0.5A to 5.0A at between
4.75 and 5.25 volts.
They don't, as far as I can tell, describe how a gadget can determine where
in those ranges the charger is actually happy.

My understanding of electronics suggests that if you start to pull too much
current, the voltage will start to drop.  It is that voltage drop across
internal resistance which causes over-heating.

So if voltage is above 4.75 volts, it seems reasonable to assume that the
charger is happy.

The code currently (see previous patch) ramps up the current until the
voltage drops below 4.75, or until the maximum is reached.
If the voltage drops, it backs off.
The current should  only be too high for 100ms.  Hopefully not too long.

I'm very open if anyone can suggest a more safe way to make full use of a
charger without risking excess current draw, but I cannot find one.

Maybe I should measure the unloaded voltage, and not let it drop more than
0.25V below that?

I'm also very open to someone finding out how to get the twl4030 to detect a
"D+ and D- shorted" charger.  I think it is supposed to be able to do this,
but I haven't managed to make it work yet.

Thanks,
NeilBrown


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