Mailing List Archive: Linux: Kernel

[PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change

Index | Next | Previous | View Threaded

fercerpav at gmail

Nov 4, 2009, 1:24 PM

Post #1 of 6 (173 views)
Permalink

[PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change

If chgmod == BATFULL, setting chgena has no effect. Datasheet says we
need to set resume instead but when autoresume is used resume doesn't
work. Clear and set chgena instead.

This enables a user to force charging by re-plugging USB even when the
charger entered Battery Full mode, might be handy before a long trip.

Signed-off-by: Paul Fertser <fercerpav [at] gmail>
---
drivers/power/pcf50633-charger.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/power/pcf50633-charger.c b/drivers/power/pcf50633-charger.c
index 3383119..4718733 100644
--- a/drivers/power/pcf50633-charger.c
+++ b/drivers/power/pcf50633-charger.c
@@ -92,14 +92,18 @@ int pcf50633_mbc_usb_curlim_set(struct pcf50633 *pcf, int ma)
chgmod = (mbcs2 & PCF50633_MBCS2_MBC_MASK);

/* If chgmod == BATFULL, setting chgena has no effect.
- * We need to set resume instead.
+ * Datasheet says we need to set resume instead but when autoresume is
+ * used resume doesn't work. Clear and set chgena instead.
*/
if (chgmod != PCF50633_MBCS2_MBC_BAT_FULL)
pcf50633_reg_set_bit_mask(pcf, PCF50633_REG_MBCC1,
PCF50633_MBCC1_CHGENA, PCF50633_MBCC1_CHGENA);
- else
+ else {
+ pcf50633_reg_clear_bits(pcf, PCF50633_REG_MBCC1,
+ PCF50633_MBCC1_CHGENA);
pcf50633_reg_set_bit_mask(pcf, PCF50633_REG_MBCC1,
- PCF50633_MBCC1_RESUME, PCF50633_MBCC1_RESUME);
+ PCF50633_MBCC1_CHGENA, PCF50633_MBCC1_CHGENA);
+ }

mbc->usb_active = charging_start;

--
1.6.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

pavel at ucw

Nov 4, 2009, 1:46 PM

Post #2 of 6 (157 views)
Permalink

Re: [PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change [In reply to]

On Thu 2009-11-05 00:24:58, Paul Fertser wrote:
> If chgmod == BATFULL, setting chgena has no effect. Datasheet says we
> need to set resume instead but when autoresume is used resume doesn't
> work. Clear and set chgena instead.
>
> This enables a user to force charging by re-plugging USB even when the
> charger entered Battery Full mode, might be handy before a long
> trip.

And is against manufacturer recommendation and will unneccessary
shorted life of battery with frequent plugs/unplugs. Is it good idea?

If you want to "top-up" the battery, perhaps that should be on
explicit request, like something in /sys?
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

fercerpav at gmail

Nov 4, 2009, 2:24 PM

Post #3 of 6 (152 views)
Permalink

Re: [PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change [In reply to]

Hi,

On Wed, Nov 04, 2009 at 10:46:22PM +0100, Pavel Machek wrote:
> On Thu 2009-11-05 00:24:58, Paul Fertser wrote:
> > If chgmod == BATFULL, setting chgena has no effect. Datasheet says we
> > need to set resume instead but when autoresume is used resume doesn't
> > work. Clear and set chgena instead.
> >
> > This enables a user to force charging by re-plugging USB even when the
> > charger entered Battery Full mode, might be handy before a long
> > trip.
>
> And is against manufacturer recommendation and will unneccessary
> shorted life of battery with frequent plugs/unplugs. Is it good idea?

Well, to me it seems that frequent plugs/unplugs is not exactly common
usecase so i consider that to be acceptable.

Especially comparing to millions of laptops and netbooks that always
top their battery while being externally powered and heat it at the
same time, probably the worst conditions lifetime-wise one can
imagine.

> If you want to "top-up" the battery, perhaps that should be on
> explicit request, like something in /sys?

Well, probably but to me it seems like an unnecessary complexity. Also
since Li* batteries are dominating the market nowadays this
functionality would need to be introduced in the power_supply core to
provide userspace with a consistent interface. Anton, can you please
comment on this?

--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav [at] gmail
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

pavel at ucw

Nov 4, 2009, 3:16 PM

Post #4 of 6 (152 views)
Permalink

Re: [PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change [In reply to]

On Thu 2009-11-05 01:24:31, Paul Fertser wrote:
> Hi,
>
> On Wed, Nov 04, 2009 at 10:46:22PM +0100, Pavel Machek wrote:
> > On Thu 2009-11-05 00:24:58, Paul Fertser wrote:
> > > If chgmod == BATFULL, setting chgena has no effect. Datasheet says we
> > > need to set resume instead but when autoresume is used resume doesn't
> > > work. Clear and set chgena instead.
> > >
> > > This enables a user to force charging by re-plugging USB even when the
> > > charger entered Battery Full mode, might be handy before a long
> > > trip.
> >
> > And is against manufacturer recommendation and will unneccessary
> > shorted life of battery with frequent plugs/unplugs. Is it good idea?
>
> Well, to me it seems that frequent plugs/unplugs is not exactly common
> usecase so i consider that to be acceptable.

Well, they are quite common here with my HTC Dream at least (I'm not
sure what your device is). Often, I unplug it from 1A charger to
connect it to PC for file transfer, and back.

> Especially comparing to millions of laptops and netbooks that always
> top their battery while being externally powered and heat it at the
> same time, probably the worst conditions lifetime-wise one can
> imagine.

Well, at least thinkpads will not start charging the battery when over
95%. (And the limit is configurable).

> > If you want to "top-up" the battery, perhaps that should be on
> > explicit request, like something in /sys?
>
> Well, probably but to me it seems like an unnecessary complexity.

And leaving behaviour before patch, that is "do not charge if battery
is over 80%" -- AFAICT from the changelog.

>Also
> since Li* batteries are dominating the market nowadays this
> functionality would need to be introduced in the power_supply core to
> provide userspace with a consistent interface. Anton, can you please
> comment on this?

I agree that it is not important part of functionality, but at least
Dream, Spitz and Collie could use that support....
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

avorontsov at ru

Nov 4, 2009, 3:45 PM

Post #5 of 6 (155 views)
Permalink

Re: [PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change [In reply to]

On Thu, Nov 05, 2009 at 01:24:31AM +0300, Paul Fertser wrote:
[...]
> > If you want to "top-up" the battery, perhaps that should be on
> > explicit request, like something in /sys?
>
> Well, probably but to me it seems like an unnecessary complexity. Also
> since Li* batteries are dominating the market nowadays this
> functionality would need to be introduced in the power_supply core to
> provide userspace with a consistent interface. Anton, can you please
> comment on this?

Power supply class doesn't have writeable properties (initially
it was a 'battery monitor class'). Though. I'd happily merge
the support for writeable properties. ACPI batteries would
benefit too, i.e. they could export 'alarm' property (see
drivers/acpi/battery.c).

Though, I'd want to apply this patch set as is (i.e. I prefer
the new behaviour, as it's really more 'user friendly'), and
we could implement configurable option later.

Pavel do you have a strong objection against this patch, or
you can live with it? :-)

Thanks!

--
Anton Vorontsov
email: cbouatmailru [at] gmail
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

pavel at ucw

Nov 4, 2009, 3:56 PM

Post #6 of 6 (152 views)
Permalink

Re: [PATCH 5/6] power: pcf50633: properly reenable charging when the supply conditions change [In reply to]

On Thu 2009-11-05 02:45:19, Anton Vorontsov wrote:
> On Thu, Nov 05, 2009 at 01:24:31AM +0300, Paul Fertser wrote:
> [...]
> > > If you want to "top-up" the battery, perhaps that should be on
> > > explicit request, like something in /sys?
> >
> > Well, probably but to me it seems like an unnecessary complexity. Also
> > since Li* batteries are dominating the market nowadays this
> > functionality would need to be introduced in the power_supply core to
> > provide userspace with a consistent interface. Anton, can you please
> > comment on this?
>
> Power supply class doesn't have writeable properties (initially
> it was a 'battery monitor class'). Though. I'd happily merge
> the support for writeable properties. ACPI batteries would
> benefit too, i.e. they could export 'alarm' property (see
> drivers/acpi/battery.c).

Yep, writable properties would be nice for at least Spitz and
Openmoko. Both of them have slow and fast charging modes, and
sometimes user will need to override the default.

> Though, I'd want to apply this patch set as is (i.e. I prefer
> the new behaviour, as it's really more 'user friendly'), and
> we could implement configurable option later.
>
> Pavel do you have a strong objection against this patch, or
> you can live with it? :-)

I can live with it... but thanks for asking :-).

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads