[PATCH 1/3] pcf50633_charger: Fix typo

[PATCH 1/3] pcf50633_charger: Fix typo

[Balaji Rao]

Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
Cc: Andy Green <andy@openmoko.com>
Cc: Anton Vorontsov <cbou@mail.ru>
---
 drivers/power/pcf50633-charger.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
index e988ec1..41aec2a 100644
--- a/drivers/power/pcf50633-charger.c
+++ b/drivers/power/pcf50633-charger.c
@@ -199,7 +199,8 @@ static int adapter_get_property(struct power_supply *psy,
 			enum power_supply_property psp,
 			union power_supply_propval *val)
 {
-	struct pcf50633_mbc *mbc = container_of(psy, struct pcf50633_mbc, usb);
+	struct pcf50633_mbc *mbc = container_of(psy,
+				struct pcf50633_mbc, adapter);
 	int ret = 0;
 
 	switch (psp) {

[PATCH 2/3] pcf50633_charger: Remove mbc_set_status

[Balaji Rao]

Remove unused 'pcf50633_mbc_set_status' function.

Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
Cc: Andy Green <andy@openmoko.com>
Cc: Anton Vorontsov <cbou@mail.ru>
---
 drivers/power/pcf50633-charger.c |   15 ---------------
 include/linux/mfd/pcf50633/mbc.h |    1 -
 2 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
index 41aec2a..aceffdc 100644
--- a/drivers/power/pcf50633-charger.c
+++ b/drivers/power/pcf50633-charger.c
@@ -84,21 +84,6 @@ int pcf50633_mbc_get_status(struct pcf50633 *pcf)
 }
 EXPORT_SYMBOL_GPL(pcf50633_mbc_get_status);
 
-void pcf50633_mbc_set_status(struct pcf50633 *pcf, int what, int status)
-{
-	struct pcf50633_mbc *mbc = platform_get_drvdata(pcf->mbc_pdev);
-
-	if (what & PCF50633_MBC_USB_ONLINE)
-		mbc->usb_online = !!status;
-	if (what & PCF50633_MBC_USB_ACTIVE)
-		mbc->usb_active = !!status;
-	if (what & PCF50633_MBC_ADAPTER_ONLINE)
-		mbc->adapter_online = !!status;
-	if (what & PCF50633_MBC_ADAPTER_ACTIVE)
-		mbc->adapter_active = !!status;
-}
-EXPORT_SYMBOL_GPL(pcf50633_mbc_set_status);
-
 static ssize_t
 show_chgmode(struct device *dev, struct device_attribute *attr, char *buf)
 {
diff --git a/include/linux/mfd/pcf50633/mbc.h b/include/linux/mfd/pcf50633/mbc.h
index 6e17619..4119579 100644
--- a/include/linux/mfd/pcf50633/mbc.h
+++ b/include/linux/mfd/pcf50633/mbc.h
@@ -128,7 +128,6 @@ enum pcf50633_reg_mbcs3 {
 int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma);
 
 int pcf50633_mbc_get_status(struct pcf50633 *);
-void pcf50633_mbc_set_status(struct pcf50633 *, int what, int status);
 
 #endif
 

[PATCH 3/3] pcf50633_charger: Enable periodic charging restart

[Balaji Rao]

The battery charger state machine switches into charging mode when
the battery voltage falls below 96% of a battery float voltage. But the
voltage drop in Li-ion batteries is marginal(1~2 %) till about 80% of its
capacity - which means, after a BATFULL, charging won't be restarted until 80%.

This work_struct function restarts charging at regular intervals to make sure
the battery doesn't discharge too much.

Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
Cc: Andy Green <andy@openmoko.com>
Cc: Anton Vorontsov <cbou@mail.ru>
---
 drivers/power/pcf50633-charger.c  |   73 ++++++++++++++++++++++++++++++++++++-
 include/linux/mfd/pcf50633/core.h |    2 +
 2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
index aceffdc..e8b278f 100644
--- a/drivers/power/pcf50633-charger.c
+++ b/drivers/power/pcf50633-charger.c
@@ -36,6 +36,8 @@ struct pcf50633_mbc {
 
 	struct power_supply usb;
 	struct power_supply adapter;
+
+	struct delayed_work charging_restart_work;
 };
 
 int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
@@ -43,6 +45,8 @@ int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
 	struct pcf50633_mbc *mbc = platform_get_drvdata(pcf->mbc_pdev);
 	int ret = 0;
 	u8 bits;
+	int charging_start = 1;
+	u8 mbcs2, chgmod;
 
 	if (ma >= 1000)
 		bits = PCF50633_MBCC7_USB_1000mA;
@@ -50,8 +54,10 @@ int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
 		bits = PCF50633_MBCC7_USB_500mA;
 	else if (ma >= 100)
 		bits = PCF50633_MBCC7_USB_100mA;
-	else
+	else {
 		bits = PCF50633_MBCC7_USB_SUSPEND;
+		charging_start = 0;
+	}
 
 	ret = pcf50633_reg_set_bit_mask(pcf, PCF50633_REG_MBCC7,
 					PCF50633_MBCC7_USB_MASK, bits);
@@ -60,6 +66,22 @@ int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
 	else
 		dev_info(pcf->dev, "usb curlim to %d mA\n", ma);
 
+	/* Manual charging start */
+	mbcs2 = pcf50633_reg_read(pcf, PCF50633_REG_MBCS2);
+	chgmod = (mbcs2 & PCF50633_MBCS2_MBC_MASK);
+
+	/* If chgmod == BATFULL, setting chgena has no effect.
+	 * We need to set resume instead.
+	 */
+	if (chgmod != PCF50633_MBCS2_MBC_BAT_FULL)
+		pcf50633_reg_set_bit_mask(pcf, PCF50633_REG_MBCC1,
+				PCF50633_MBCC1_CHGENA, PCF50633_MBCC1_CHGENA);
+	else
+		pcf50633_reg_set_bit_mask(pcf, PCF50633_REG_MBCC1,
+				PCF50633_MBCC1_RESUME, PCF50633_MBCC1_RESUME);
+
+	mbc->usb_active = charging_start;
+
 	power_supply_changed(&mbc->usb);
 
 	return ret;
@@ -145,10 +167,44 @@ static struct attribute_group mbc_attr_group = {
 	.attrs	= pcf50633_mbc_sysfs_entries,
 };
 
+/* MBC state machine switches into charging mode when the battery voltage
+ * falls below 96% of a battery float voltage. But the voltage drop in Li-ion
+ * batteries is marginal(1~2 %) till about 80% of its capacity - which means,
+ * after a BATFULL, charging won't be restarted until 80%.
+ *
+ * This work_struct function restarts charging at regular intervals to make
+ * sure we don't discharge too much
+ */
+
+static void pcf50633_mbc_charging_restart(struct work_struct *work)
+{
+	struct pcf50633_mbc *mbc;
+	u8 mbcs2, chgmod;
+
+	mbc = container_of(work, struct pcf50633_mbc,
+				charging_restart_work.work);
+
+	mbcs2 = pcf50633_reg_read(mbc->pcf, PCF50633_REG_MBCS2);
+	chgmod = (mbcs2 & PCF50633_MBCS2_MBC_MASK);
+
+	if (chgmod != PCF50633_MBCS2_MBC_BAT_FULL)
+		return;
+
+	/* Restart charging */
+	pcf50633_reg_set_bit_mask(mbc->pcf, PCF50633_REG_MBCC1,
+				PCF50633_MBCC1_RESUME, PCF50633_MBCC1_RESUME);
+	mbc->usb_active = 1;
+	power_supply_changed(&mbc->usb);
+
+	dev_info(mbc->pcf->dev, "Charging restarted\n");
+}
+
 static void
 pcf50633_mbc_irq_handler(int irq, void *data)
 {
 	struct pcf50633_mbc *mbc = data;
+	int chg_restart_interval =
+			mbc->pcf->pdata->charging_restart_interval;
 
 	/* USB */
 	if (irq == PCF50633_IRQ_USBINS) {
@@ -157,6 +213,7 @@ pcf50633_mbc_irq_handler(int irq, void *data)
 		mbc->usb_online = 0;
 		mbc->usb_active = 0;
 		pcf50633_mbc_usb_curlim_set(mbc->pcf, 0);
+		cancel_delayed_work_sync(&mbc->charging_restart_work);
 	}
 
 	/* Adapter */
@@ -171,7 +228,14 @@ pcf50633_mbc_irq_handler(int irq, void *data)
 	if (irq == PCF50633_IRQ_BATFULL) {
 		mbc->usb_active = 0;
 		mbc->adapter_active = 0;
-	}
+
+		if (chg_restart_interval > 0)
+			schedule_delayed_work(&mbc->charging_restart_work,
+							chg_restart_interval);
+	} else if (irq == PCF50633_IRQ_USBLIMON)
+		mbc->usb_active = 0;
+	else if (irq == PCF50633_IRQ_USBLIMOFF)
+		mbc->usb_active = 1;
 
 	power_supply_changed(&mbc->usb);
 	power_supply_changed(&mbc->adapter);
@@ -288,6 +352,9 @@ static int __devinit pcf50633_mbc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	INIT_DELAYED_WORK(&mbc->charging_restart_work,
+				pcf50633_mbc_charging_restart);
+
 	ret = sysfs_create_group(&pdev->dev.kobj, &mbc_attr_group);
 	if (ret)
 		dev_err(mbc->pcf->dev, "failed to create sysfs entries\n");
@@ -313,6 +380,8 @@ static int __devexit pcf50633_mbc_remove(struct platform_device *pdev)
 	power_supply_unregister(&mbc->usb);
 	power_supply_unregister(&mbc->adapter);
 
+	cancel_delayed_work_sync(&mbc->charging_restart_work);
+
 	kfree(mbc);
 
 	return 0;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 4455b21..c8f51c3 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -29,6 +29,8 @@ struct pcf50633_platform_data {
 	char **batteries;
 	int num_batteries;
 
+	int charging_restart_interval;
+
 	/* Callbacks */
 	void (*probe_done)(struct pcf50633 *);
 	void (*mbc_event_callback)(struct pcf50633 *, int);

Re: [PATCH 3/3] pcf50633_charger: Enable periodic charging restart

[Andrew Morton]

On Tue, 27 Jan 2009 19:23:12 +0530
Balaji Rao <balajirrao@openmoko.org> wrote:

> @@ -157,6 +213,7 @@ pcf50633_mbc_irq_handler(int irq, void *data)
>  		mbc->usb_online = 0;
>  		mbc->usb_active = 0;
>  		pcf50633_mbc_usb_curlim_set(mbc->pcf, 0);
> +		cancel_delayed_work_sync(&mbc->charging_restart_work);
>  	}

eek, can't call cancel_delayed_work_sync() from an interrupt handler! 
If the work was actually running on this CPU or on a different one,
cancel_delayed_work_sync() will need to wait for it to complete.  The
kernel will explode in many tinkly little pieces.

Re: [PATCH 1/3] pcf50633_charger: Fix typo

[Andrew Morton]

On Tue, 27 Jan 2009 19:22:38 +0530
Balaji Rao <balajirrao@openmoko.org> wrote:

> Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
> Cc: Andy Green <andy@openmoko.com>
> Cc: Anton Vorontsov <cbou@mail.ru>
> ---
>  drivers/power/pcf50633-charger.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
> index e988ec1..41aec2a 100644
> --- a/drivers/power/pcf50633-charger.c
> +++ b/drivers/power/pcf50633-charger.c
> @@ -199,7 +199,8 @@ static int adapter_get_property(struct power_supply *psy,
>  			enum power_supply_property psp,
>  			union power_supply_propval *val)
>  {
> -	struct pcf50633_mbc *mbc = container_of(psy, struct pcf50633_mbc, usb);
> +	struct pcf50633_mbc *mbc = container_of(psy,
> +				struct pcf50633_mbc, adapter);
>  	int ret = 0;
>  
>  	switch (psp) {

This looks like it fixes a rather fatal bug?

Re: [PATCH 1/3] pcf50633_charger: Fix typo

[Anton Vorontsov]

On Mon, Feb 02, 2009 at 02:41:55PM -0800, Andrew Morton wrote:
> On Tue, 27 Jan 2009 19:22:38 +0530
> Balaji Rao <balajirrao@openmoko.org> wrote:
> 
> > Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
> > Cc: Andy Green <andy@openmoko.com>
> > Cc: Anton Vorontsov <cbou@mail.ru>
> > ---
> >  drivers/power/pcf50633-charger.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
> > index e988ec1..41aec2a 100644
> > --- a/drivers/power/pcf50633-charger.c
> > +++ b/drivers/power/pcf50633-charger.c
> > @@ -199,7 +199,8 @@ static int adapter_get_property(struct power_supply *psy,
> >  			enum power_supply_property psp,
> >  			union power_supply_propval *val)
> >  {
> > -	struct pcf50633_mbc *mbc = container_of(psy, struct pcf50633_mbc, usb);
> > +	struct pcf50633_mbc *mbc = container_of(psy,
> > +				struct pcf50633_mbc, adapter);
> >  	int ret = 0;
> >  
> >  	switch (psp) {
> 
> This looks like it fixes a rather fatal bug?

Yeah, the fix is already queued for 2.6.29 (battery-2.6.29 tree).

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 3/3] pcf50633_charger: Enable periodic charging restart

[Anton Vorontsov]

On Mon, Feb 02, 2009 at 02:41:31PM -0800, Andrew Morton wrote:
> On Tue, 27 Jan 2009 19:23:12 +0530
> Balaji Rao <balajirrao@openmoko.org> wrote:
> 
> > @@ -157,6 +213,7 @@ pcf50633_mbc_irq_handler(int irq, void *data)
> >  		mbc->usb_online = 0;
> >  		mbc->usb_active = 0;
> >  		pcf50633_mbc_usb_curlim_set(mbc->pcf, 0);
> > +		cancel_delayed_work_sync(&mbc->charging_restart_work);
> >  	}
> 
> eek, can't call cancel_delayed_work_sync() from an interrupt handler! 

It's rather confusing, but it's not an irq handler, in the sense that
it does not run in atomic context -- the PCF IRQ handlers run in
kernel threads.

It's done that way because PCF50633 is an I2C device, and normally
we can't access I2C devices in the atomic context (plus it's rather
slow, thus would cause huge latencies if we'd try).

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

Re: [PATCH 2/3] pcf50633_charger: Remove mbc_set_status

[Anton Vorontsov]

On Tue, Jan 27, 2009 at 07:22:55PM +0530, Balaji Rao wrote:
> Remove unused 'pcf50633_mbc_set_status' function.
> 
> Signed-off-by: Balaji Rao <balajirrao@openmoko.org>
> Cc: Andy Green <andy@openmoko.com>
> Cc: Anton Vorontsov <cbou@mail.ru>
> ---

This patch applied to battery-2.6.git (i.e. for 2.6.30).

Thanks!

>  drivers/power/pcf50633-charger.c |   15 ---------------
>  include/linux/mfd/pcf50633/mbc.h |    1 -
>  2 files changed, 0 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
> index 41aec2a..aceffdc 100644
> --- a/drivers/power/pcf50633-charger.c
> +++ b/drivers/power/pcf50633-charger.c
> @@ -84,21 +84,6 @@ int pcf50633_mbc_get_status(struct pcf50633 *pcf)
>  }
>  EXPORT_SYMBOL_GPL(pcf50633_mbc_get_status);
>  
> -void pcf50633_mbc_set_status(struct pcf50633 *pcf, int what, int status)
> -{
> -	struct pcf50633_mbc *mbc = platform_get_drvdata(pcf->mbc_pdev);
> -
> -	if (what & PCF50633_MBC_USB_ONLINE)
> -		mbc->usb_online = !!status;
> -	if (what & PCF50633_MBC_USB_ACTIVE)
> -		mbc->usb_active = !!status;
> -	if (what & PCF50633_MBC_ADAPTER_ONLINE)
> -		mbc->adapter_online = !!status;
> -	if (what & PCF50633_MBC_ADAPTER_ACTIVE)
> -		mbc->adapter_active = !!status;
> -}
> -EXPORT_SYMBOL_GPL(pcf50633_mbc_set_status);
> -
>  static ssize_t
>  show_chgmode(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> diff --git a/include/linux/mfd/pcf50633/mbc.h b/include/linux/mfd/pcf50633/mbc.h
> index 6e17619..4119579 100644
> --- a/include/linux/mfd/pcf50633/mbc.h
> +++ b/include/linux/mfd/pcf50633/mbc.h
> @@ -128,7 +128,6 @@ enum pcf50633_reg_mbcs3 {
>  int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma);
>  
>  int pcf50633_mbc_get_status(struct pcf50633 *);
> -void pcf50633_mbc_set_status(struct pcf50633 *, int what, int status);
>  
>  #endif

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2