This adds a function to let the power supply core know that the battery
is fully charged now. This can be done by watching a GPIO line, for
example.
Some battery drivers might need that information to calibrate
themselves.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Ian Molton <[email protected]>
Cc: Anton Vorontsov <[email protected]>
Cc: Matt Reimer <[email protected]>
---
drivers/power/power_supply_core.c | 16 ++++++++++++++++
include/linux/power_supply.h | 3 +++
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 5520040..7303c45 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -111,6 +111,22 @@ int power_supply_is_system_supplied(void)
return error;
}
+static int __power_supply_set_battery_charged(struct device *dev, void *data)
+{
+ struct power_supply *psy = dev_get_drvdata(dev);
+
+ if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged)
+ psy->set_charged(psy);
+
+ return 0;
+}
+
+void power_supply_set_battery_charged(void)
+{
+ class_for_each_device(power_supply_class, NULL, NULL,
+ __power_supply_set_battery_charged);
+}
+
int power_supply_register(struct device *parent, struct power_supply *psy)
{
int rc = 0;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 594c494..b2b39e2 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -126,6 +126,7 @@ struct power_supply {
enum power_supply_property psp,
union power_supply_propval *val);
void (*external_power_changed)(struct power_supply *psy);
+ void (*set_charged)(struct power_supply *psy);
/* For APM emulation, think legacy userspace. */
int use_for_apm;
@@ -170,8 +171,10 @@ extern int power_supply_am_i_supplied(struct power_supply *psy);
#if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
extern int power_supply_is_system_supplied(void);
+extern void power_supply_set_battery_charged(void);
#else
static inline int power_supply_is_system_supplied(void) { return -ENOSYS; }
+static inline void power_supply_set_battery_charged(void) {}
#endif
extern int power_supply_register(struct device *parent,
--
1.6.3.1
While I'm at it, cleanup the power supply code so that EXPORT_SYMBOL_GPL
appears directly after the symbole declaration. checkpatch.pl wants it
that way.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Ian Molton <[email protected]>
Cc: Anton Vorontsov <[email protected]>
Cc: Matt Reimer <[email protected]>
---
drivers/power/power_supply_core.c | 17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 7303c45..0a10799 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -20,6 +20,9 @@
struct class *power_supply_class;
+/* exported for the APM Power driver, APM emulation */
+EXPORT_SYMBOL_GPL(power_supply_class);
+
static int __power_supply_changed_work(struct device *dev, void *data)
{
struct power_supply *psy = (struct power_supply *)data;
@@ -55,6 +58,7 @@ void power_supply_changed(struct power_supply *psy)
schedule_work(&psy->changed_work);
}
+EXPORT_SYMBOL_GPL(power_supply_changed);
static int __power_supply_am_i_supplied(struct device *dev, void *data)
{
@@ -86,6 +90,7 @@ int power_supply_am_i_supplied(struct power_supply *psy)
return error;
}
+EXPORT_SYMBOL_GPL(power_supply_am_i_supplied);
static int __power_supply_is_system_supplied(struct device *dev, void *data)
{
@@ -110,6 +115,7 @@ int power_supply_is_system_supplied(void)
return error;
}
+EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
static int __power_supply_set_battery_charged(struct device *dev, void *data)
{
@@ -160,6 +166,7 @@ dev_create_failed:
success:
return rc;
}
+EXPORT_SYMBOL_GPL(power_supply_register);
void power_supply_unregister(struct power_supply *psy)
{
@@ -168,6 +175,7 @@ void power_supply_unregister(struct power_supply *psy)
power_supply_remove_attrs(psy);
device_unregister(psy->dev);
}
+EXPORT_SYMBOL_GPL(power_supply_unregister);
static int __init power_supply_class_init(void)
{
@@ -186,15 +194,6 @@ static void __exit power_supply_class_exit(void)
class_destroy(power_supply_class);
}
-EXPORT_SYMBOL_GPL(power_supply_changed);
-EXPORT_SYMBOL_GPL(power_supply_am_i_supplied);
-EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
-EXPORT_SYMBOL_GPL(power_supply_register);
-EXPORT_SYMBOL_GPL(power_supply_unregister);
-
-/* exported for the APM Power driver, APM emulation */
-EXPORT_SYMBOL_GPL(power_supply_class);
-
subsys_initcall(power_supply_class_init);
module_exit(power_supply_class_exit);
--
1.6.3.1
The ds2760's internal current meter is not reliable enough as it has an
inacurracy of around ~15%. Without any correction for that error, the
current accumulator is couting up all the time, even though the battery
is already fully charged and hence destroys the static information. The
longer it is connected, the worse is the aberration.
Fortunately, this can be corrected by the DS2760_CURRENT_OFFSET_BIAS
register. Using the external power_supply_set_battery_charged()
function, this register is now gauging the measurement.
A delayed work is used to debounce flaky GPIO signals and to let the
current value settle. Also see Maxim's application note AN4188.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Szabolcs Gyurko <[email protected]>
Cc: Matt Reimer <[email protected]>
Cc: Anton Vorontsov <[email protected]>
---
drivers/power/ds2760_battery.c | 47 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c
index f4a9258..30ebd82 100644
--- a/drivers/power/ds2760_battery.c
+++ b/drivers/power/ds2760_battery.c
@@ -56,6 +56,7 @@ struct ds2760_device_info {
struct device *w1_dev;
struct workqueue_struct *monitor_wqueue;
struct delayed_work monitor_work;
+ struct delayed_work set_charged_work;
};
static unsigned int cache_time = 1000;
@@ -327,6 +328,48 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)
queue_delayed_work(di->monitor_wqueue, &di->monitor_work, HZ/10);
}
+
+static void ds2760_battery_set_charged_work(struct work_struct *work)
+{
+ char bias;
+ struct ds2760_device_info *di = container_of(work,
+ struct ds2760_device_info, set_charged_work.work);
+
+ dev_dbg(di->dev, "%s\n", __func__);
+
+ ds2760_battery_read_status(di);
+
+ /* When we get notified by external circuitry that the battery is
+ * considered fully charged now, we know that there is no current
+ * flow any more. However, the ds2760's internal current meter is
+ * too inaccurate to rely on - spec say something ~15% failure.
+ * Hence, we use the current offset bias register to compensate
+ * that error.
+ */
+
+ if (!power_supply_am_i_supplied(&di->bat))
+ return;
+
+ bias = di->current_raw +
+ (signed char) di->raw[DS2760_CURRENT_OFFSET_BIAS];
+
+ dev_dbg(di->dev, "%s: bias = %d\n", __func__, bias);
+
+ w1_ds2760_write(di->w1_dev, &bias, DS2760_CURRENT_OFFSET_BIAS, 1);
+ w1_ds2760_store_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+ w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+}
+
+static void ds2760_battery_set_charged(struct power_supply *psy)
+{
+ struct ds2760_device_info *di = to_ds2760_device_info(psy);
+
+ /* postpone the actual work by 20 secs. This is for debouncing GPIO
+ * signals and to let the current value settle. See AN4188. */
+ cancel_delayed_work(&di->set_charged_work);
+ queue_delayed_work(di->monitor_wqueue, &di->set_charged_work, HZ * 20);
+}
+
static int ds2760_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -412,6 +455,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
di->bat.properties = ds2760_battery_props;
di->bat.num_properties = ARRAY_SIZE(ds2760_battery_props);
di->bat.get_property = ds2760_battery_get_property;
+ di->bat.set_charged = ds2760_battery_set_charged;
di->bat.external_power_changed =
ds2760_battery_external_power_changed;
@@ -443,6 +487,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
}
INIT_DELAYED_WORK(&di->monitor_work, ds2760_battery_work);
+ INIT_DELAYED_WORK(&di->set_charged_work, ds2760_battery_set_charged_work);
di->monitor_wqueue = create_singlethread_workqueue(dev_name(&pdev->dev));
if (!di->monitor_wqueue) {
retval = -ESRCH;
@@ -467,6 +512,8 @@ static int ds2760_battery_remove(struct platform_device *pdev)
cancel_rearming_delayed_workqueue(di->monitor_wqueue,
&di->monitor_work);
+ cancel_rearming_delayed_workqueue(di->monitor_wqueue,
+ &di->set_charged_work);
destroy_workqueue(di->monitor_wqueue);
power_supply_unregister(&di->bat);
--
1.6.3.1
On Thu, Jul 16, 2009 at 05:44:04PM +0200, Daniel Mack wrote:
> The ds2760's internal current meter is not reliable enough as it has an
> inacurracy of around ~15%. Without any correction for that error, the
> current accumulator is couting up all the time, even though the battery
> is already fully charged and hence destroys the static information. The
> longer it is connected, the worse is the aberration.
>
> Fortunately, this can be corrected by the DS2760_CURRENT_OFFSET_BIAS
> register. Using the external power_supply_set_battery_charged()
> function, this register is now gauging the measurement.
>
> A delayed work is used to debounce flaky GPIO signals and to let the
> current value settle. Also see Maxim's application note AN4188.
One more small correction - we need to manually write back the BIAS
level to the di->raw buffer as ds2760_battery_read_status() won't do it
for us after the first time.
Daniel
commit ebb1173a2c6e1334b1afc7cd119b39eca2350351
Author: Daniel Mack <[email protected]>
Date: Thu Jul 16 17:28:56 2009 +0200
ds2760: implement set_charged() feature
The ds2760's internal current meter is not reliable enough as it has an
inacurracy of around ~15%. Without any correction for that error, the
current accumulator is couting up all the time, even though the battery
is already fully charged and hence destroys the static information. The
longer it is connected, the worse is the aberration.
Fortunately, this can be corrected by the DS2760_CURRENT_OFFSET_BIAS
register. Using the external power_supply_set_battery_charged()
function, this register is now gauging the measurement.
A delayed work is used to debounce flaky GPIO signals and to let the
current value settle. Also see Maxim's application note AN4188.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Szabolcs Gyurko <[email protected]>
Cc: Matt Reimer <[email protected]>
Cc: Anton Vorontsov <[email protected]>
diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c
index f4a9258..9a95bc1 100644
--- a/drivers/power/ds2760_battery.c
+++ b/drivers/power/ds2760_battery.c
@@ -56,6 +56,7 @@ struct ds2760_device_info {
struct device *w1_dev;
struct workqueue_struct *monitor_wqueue;
struct delayed_work monitor_work;
+ struct delayed_work set_charged_work;
};
static unsigned int cache_time = 1000;
@@ -327,6 +328,52 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)
queue_delayed_work(di->monitor_wqueue, &di->monitor_work, HZ/10);
}
+
+static void ds2760_battery_set_charged_work(struct work_struct *work)
+{
+ char bias;
+ struct ds2760_device_info *di = container_of(work,
+ struct ds2760_device_info, set_charged_work.work);
+
+ dev_dbg(di->dev, "%s\n", __func__);
+
+ ds2760_battery_read_status(di);
+
+ /* When we get notified by external circuitry that the battery is
+ * considered fully charged now, we know that there is no current
+ * flow any more. However, the ds2760's internal current meter is
+ * too inaccurate to rely on - spec say something ~15% failure.
+ * Hence, we use the current offset bias register to compensate
+ * that error.
+ */
+
+ if (!power_supply_am_i_supplied(&di->bat))
+ return;
+
+ bias = di->current_raw +
+ (signed char) di->raw[DS2760_CURRENT_OFFSET_BIAS];
+
+ dev_dbg(di->dev, "%s: bias = %d\n", __func__, bias);
+
+ w1_ds2760_write(di->w1_dev, &bias, DS2760_CURRENT_OFFSET_BIAS, 1);
+ w1_ds2760_store_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+ w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+
+ /* Write to the di->raw[] buffer directly - the CURRENT_OFFSET_BIAS
+ * value won't be read back by ds2760_battery_read_status() */
+ di->raw[DS2760_CURRENT_OFFSET_BIAS] = bias;
+}
+
+static void ds2760_battery_set_charged(struct power_supply *psy)
+{
+ struct ds2760_device_info *di = to_ds2760_device_info(psy);
+
+ /* postpone the actual work by 20 secs. This is for debouncing GPIO
+ * signals and to let the current value settle. See AN4188. */
+ cancel_delayed_work(&di->set_charged_work);
+ queue_delayed_work(di->monitor_wqueue, &di->set_charged_work, HZ * 20);
+}
+
static int ds2760_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -412,6 +459,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
di->bat.properties = ds2760_battery_props;
di->bat.num_properties = ARRAY_SIZE(ds2760_battery_props);
di->bat.get_property = ds2760_battery_get_property;
+ di->bat.set_charged = ds2760_battery_set_charged;
di->bat.external_power_changed =
ds2760_battery_external_power_changed;
@@ -443,6 +491,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
}
INIT_DELAYED_WORK(&di->monitor_work, ds2760_battery_work);
+ INIT_DELAYED_WORK(&di->set_charged_work, ds2760_battery_set_charged_work);
di->monitor_wqueue = create_singlethread_workqueue(dev_name(&pdev->dev));
if (!di->monitor_wqueue) {
retval = -ESRCH;
@@ -467,6 +516,8 @@ static int ds2760_battery_remove(struct platform_device *pdev)
cancel_rearming_delayed_workqueue(di->monitor_wqueue,
&di->monitor_work);
+ cancel_rearming_delayed_workqueue(di->monitor_wqueue,
+ &di->set_charged_work);
destroy_workqueue(di->monitor_wqueue);
power_supply_unregister(&di->bat);
On Thu, Jul 16, 2009 at 05:44:02PM +0200, Daniel Mack wrote:
> This adds a function to let the power supply core know that the battery
> is fully charged now. This can be done by watching a GPIO line, for
> example.
>
> Some battery drivers might need that information to calibrate
> themselves.
Any comments about the last patch series? I've sent one inlined in a
reply and three more apart from it, which might have been confusing. Do
you want me to resend it as one block?
Thanks,
Daniel
On Jul 16, 2009, at 8:44 AM, Daniel Mack wrote:
> This adds a function to let the power supply core know that the
> battery
> is fully charged now. This can be done by watching a GPIO line, for
> example.
>
> Some battery drivers might need that information to calibrate
> themselves.
Looks fine to me.
Acked-by: Matt Reimer <[email protected]>
Matt
On Thu, Jul 16, 2009 at 05:44:02PM +0200, Daniel Mack wrote:
> This adds a function to let the power supply core know that the battery
> is fully charged now. This can be done by watching a GPIO line, for
> example.
>
> Some battery drivers might need that information to calibrate
> themselves.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Ian Molton <[email protected]>
> Cc: Anton Vorontsov <[email protected]>
> Cc: Matt Reimer <[email protected]>
> ---
> drivers/power/power_supply_core.c | 16 ++++++++++++++++
> include/linux/power_supply.h | 3 +++
The patch is missing pda_power.c part?
> 2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 5520040..7303c45 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -111,6 +111,22 @@ int power_supply_is_system_supplied(void)
> return error;
> }
>
> +static int __power_supply_set_battery_charged(struct device *dev, void *data)
> +{
> + struct power_supply *psy = dev_get_drvdata(dev);
> +
> + if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged)
> + psy->set_charged(psy);
> +
> + return 0;
> +}
> +
> +void power_supply_set_battery_charged(void)
> +{
> + class_for_each_device(power_supply_class, NULL, NULL,
> + __power_supply_set_battery_charged);
> +}
> +
> int power_supply_register(struct device *parent, struct power_supply *psy)
> {
> int rc = 0;
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 594c494..b2b39e2 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -126,6 +126,7 @@ struct power_supply {
> enum power_supply_property psp,
> union power_supply_propval *val);
> void (*external_power_changed)(struct power_supply *psy);
> + void (*set_charged)(struct power_supply *psy);
>
> /* For APM emulation, think legacy userspace. */
> int use_for_apm;
> @@ -170,8 +171,10 @@ extern int power_supply_am_i_supplied(struct power_supply *psy);
>
> #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
> extern int power_supply_is_system_supplied(void);
> +extern void power_supply_set_battery_charged(void);
I don't quite like this interface. Note that system might want
to know if it's supplied (hence global is_system_supplied(void) call),
but having set_battery_charged(void) interface makes little sense
(well, it makes sense for small embedded systems that have just
one battery and one charger)...
There is a supplied_to psy field that might help you there...
The cleanup patch looks OK though, I'll apply it.
Thanks,
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Mon, Jul 20, 2009 at 10:27:52PM +0400, Anton Vorontsov wrote:
> On Thu, Jul 16, 2009 at 05:44:02PM +0200, Daniel Mack wrote:
> > This adds a function to let the power supply core know that the battery
> > is fully charged now. This can be done by watching a GPIO line, for
> > example.
> >
> > Some battery drivers might need that information to calibrate
> > themselves.
> >
> > Signed-off-by: Daniel Mack <[email protected]>
> > Cc: Ian Molton <[email protected]>
> > Cc: Anton Vorontsov <[email protected]>
> > Cc: Matt Reimer <[email protected]>
> > ---
> > drivers/power/power_supply_core.c | 16 ++++++++++++++++
> > include/linux/power_supply.h | 3 +++
>
> The patch is missing pda_power.c part?
Erm, does it? What would you expect it to do in there?
> > #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
> > extern int power_supply_is_system_supplied(void);
> > +extern void power_supply_set_battery_charged(void);
>
> I don't quite like this interface. Note that system might want
> to know if it's supplied (hence global is_system_supplied(void) call),
> but having set_battery_charged(void) interface makes little sense
> (well, it makes sense for small embedded systems that have just
> one battery and one charger)...
>
> There is a supplied_to psy field that might help you there...
Hmm. I thought about that too, but didn't find a way to access the
single members of the pda-power device and find the right one to
access. They're registered on a string base and matched magically once
the battery monitor is probed when found on the one-wire bus. Any
proposal how an interface could look like that does that?
All I need is a way to pass this information from my platform code down
to the battery driver.
> The cleanup patch looks OK though, I'll apply it.
Thanks. What about the one adding support for the bootstrapping of the
current_accum value? I've sent that inline as a reply to the other patch
series (4/4) last week (which was probably not a good idea).
Daniel
On Mon, Jul 20, 2009 at 08:37:11PM +0200, Daniel Mack wrote:
> On Mon, Jul 20, 2009 at 10:27:52PM +0400, Anton Vorontsov wrote:
> > On Thu, Jul 16, 2009 at 05:44:02PM +0200, Daniel Mack wrote:
> > > This adds a function to let the power supply core know that the battery
> > > is fully charged now. This can be done by watching a GPIO line, for
> > > example.
> > >
> > > Some battery drivers might need that information to calibrate
> > > themselves.
> > >
> > > Signed-off-by: Daniel Mack <[email protected]>
> > > Cc: Ian Molton <[email protected]>
> > > Cc: Anton Vorontsov <[email protected]>
> > > Cc: Matt Reimer <[email protected]>
> > > ---
> > > drivers/power/power_supply_core.c | 16 ++++++++++++++++
> > > include/linux/power_supply.h | 3 +++
> >
> > The patch is missing pda_power.c part?
>
> Erm, does it? What would you expect it to do in there?
Um... then the patch subject is misleading. It says "pda-power".
Is there any user of this new call, btw? I mean a mainline user,
or soon to be?
> > > #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
> > > extern int power_supply_is_system_supplied(void);
> > > +extern void power_supply_set_battery_charged(void);
> >
> > I don't quite like this interface. Note that system might want
> > to know if it's supplied (hence global is_system_supplied(void) call),
> > but having set_battery_charged(void) interface makes little sense
> > (well, it makes sense for small embedded systems that have just
> > one battery and one charger)...
> >
> > There is a supplied_to psy field that might help you there...
>
> Hmm. I thought about that too, but didn't find a way to access the
> single members of the pda-power device and find the right one to
> access. They're registered on a string base and matched magically once
> the battery monitor is probed when found on the one-wire bus. Any
> proposal how an interface could look like that does that?
>
> All I need is a way to pass this information from my platform code down
> to the battery driver.
Yes, I see the problem, but I don't have a solution off-hand. :-/
Let me think about it for awhile.
> > The cleanup patch looks OK though, I'll apply it.
>
> Thanks. What about the one adding support for the bootstrapping of the
> current_accum value? I've sent that inline as a reply to the other patch
> series (4/4) last week (which was probably not a good idea).
"ds2760: add current_accum module parameter"? Looks perfectly OK.
Thanks!
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Mon, Jul 20, 2009 at 11:00:34PM +0400, Anton Vorontsov wrote:
> > Erm, does it? What would you expect it to do in there?
>
> Um... then the patch subject is misleading. It says "pda-power".
Well, that's the subsystem it addresses :)
> Is there any user of this new call, btw? I mean a mainline user,
> or soon to be?
No, not yet. We might be merging our platform code mainline some day,
but at the moment it doesn't make sense to anyone to have have it. The
only prototypes tha exist are on our desks :)
Anyway - the surrounding code is perfectly simple. It just registers an
IRQ and calls set_battery_charged() in it. The battery driver cares for
debouncing and the like.
> > Hmm. I thought about that too, but didn't find a way to access the
> > single members of the pda-power device and find the right one to
> > access. They're registered on a string base and matched magically once
> > the battery monitor is probed when found on the one-wire bus. Any
> > proposal how an interface could look like that does that?
> >
> > All I need is a way to pass this information from my platform code down
> > to the battery driver.
>
> Yes, I see the problem, but I don't have a solution off-hand. :-/
>
> Let me think about it for awhile.
So will I :)
> > Thanks. What about the one adding support for the bootstrapping of the
> > current_accum value? I've sent that inline as a reply to the other patch
> > series (4/4) last week (which was probably not a good idea).
>
> "ds2760: add current_accum module parameter"? Looks perfectly OK.
That one, yes. Thanks for applying.
Daniel
On Mon, Jul 20, 2009 at 11:00:34PM +0400, Anton Vorontsov wrote:
> > Hmm. I thought about that too, but didn't find a way to access the
> > single members of the pda-power device and find the right one to
> > access. They're registered on a string base and matched magically once
> > the battery monitor is probed when found on the one-wire bus. Any
> > proposal how an interface could look like that does that?
> >
> > All I need is a way to pass this information from my platform code down
> > to the battery driver.
>
> Yes, I see the problem, but I don't have a solution off-hand. :-/
>
> Let me think about it for awhile.
What about something like the patch below?
The platform code interface would look like
struct power_supply *psy =
power_support_get_by_name("ds2760.0");
if (psy)
power_supply_set_battery_charged(psy);
Couldn't test it yet as my hardware is missing, but I would still like
to hear your comments about it :)
Daniel
commit 1a865b0023619c93de095fdb35823e57a496fc0b
Author: Daniel Mack <[email protected]>
Date: Thu Jul 16 15:54:46 2009 +0200
pwoer_supply: get_by_name and set_charged functionaltity
This adds a function to get a power_supply device from the class of
registered devices by name reference. It can be used to find a specific
battery to call power_supply_set_battery_charged() on.
Some battery drivers might need that information to calibrate
themselves.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Ian Molton <[email protected]>
Cc: Anton Vorontsov <[email protected]>
Cc: Matt Reimer <[email protected]>
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 58f10ac..ddb4800 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
}
EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
+int power_supply_set_battery_charged(struct power_supply *psy)
+{
+ if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
+ psy->set_charged(psy);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
+
+static int power_supply_match_device_by_name(struct device *dev, void *data)
+{
+ const char *name = data;
+ struct power_supply *psy = dev_get_drvdata(dev);
+
+ return strcmp(psy->name, name) == 0;
+}
+
+struct power_supply *power_supply_get_by_name(char *name)
+{
+ struct device *dev = class_find_device(power_supply_class, NULL, name,
+ power_supply_match_device_by_name);
+
+ return dev ? dev_get_drvdata(dev) : NULL;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_by_name);
+
int power_supply_register(struct device *parent, struct power_supply *psy)
{
int rc = 0;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 594c494..49f3296 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -126,6 +126,7 @@ struct power_supply {
enum power_supply_property psp,
union power_supply_propval *val);
void (*external_power_changed)(struct power_supply *psy);
+ void (*set_charged)(struct power_supply *psy);
/* For APM emulation, think legacy userspace. */
int use_for_apm;
@@ -165,8 +166,10 @@ struct power_supply_info {
int use_for_apm;
};
+extern struct power_supply *power_supply_get_by_name(char *name);
extern void power_supply_changed(struct power_supply *psy);
extern int power_supply_am_i_supplied(struct power_supply *psy);
+extern int power_supply_set_battery_charged(struct power_supply *psy);
#if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
extern int power_supply_is_system_supplied(void);
On Wed, Jul 22, 2009 at 07:20:45PM +0200, Daniel Mack wrote:
> On Mon, Jul 20, 2009 at 11:00:34PM +0400, Anton Vorontsov wrote:
> > > Hmm. I thought about that too, but didn't find a way to access the
> > > single members of the pda-power device and find the right one to
> > > access. They're registered on a string base and matched magically once
> > > the battery monitor is probed when found on the one-wire bus. Any
> > > proposal how an interface could look like that does that?
> > >
> > > All I need is a way to pass this information from my platform code down
> > > to the battery driver.
> >
> > Yes, I see the problem, but I don't have a solution off-hand. :-/
> >
> > Let me think about it for awhile.
>
> What about something like the patch below?
>
> The platform code interface would look like
>
> struct power_supply *psy =
> power_support_get_by_name("ds2760.0");
> if (psy)
> power_supply_set_battery_charged(psy);
>
> Couldn't test it yet as my hardware is missing, but I would still like
> to hear your comments about it :)
Wow, looks perfect. Let me know when you test it, and I'll
apply that patch.
Thanks!
> Daniel
>
>
>
> commit 1a865b0023619c93de095fdb35823e57a496fc0b
> Author: Daniel Mack <[email protected]>
> Date: Thu Jul 16 15:54:46 2009 +0200
>
> pwoer_supply: get_by_name and set_charged functionaltity
>
> This adds a function to get a power_supply device from the class of
> registered devices by name reference. It can be used to find a specific
> battery to call power_supply_set_battery_charged() on.
>
> Some battery drivers might need that information to calibrate
> themselves.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Ian Molton <[email protected]>
> Cc: Anton Vorontsov <[email protected]>
> Cc: Matt Reimer <[email protected]>
>
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 58f10ac..ddb4800 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
> }
> EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
>
> +int power_supply_set_battery_charged(struct power_supply *psy)
> +{
> + if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
> + psy->set_charged(psy);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
> +
> +static int power_supply_match_device_by_name(struct device *dev, void *data)
> +{
> + const char *name = data;
> + struct power_supply *psy = dev_get_drvdata(dev);
> +
> + return strcmp(psy->name, name) == 0;
> +}
> +
> +struct power_supply *power_supply_get_by_name(char *name)
> +{
> + struct device *dev = class_find_device(power_supply_class, NULL, name,
> + power_supply_match_device_by_name);
> +
> + return dev ? dev_get_drvdata(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_get_by_name);
> +
> int power_supply_register(struct device *parent, struct power_supply *psy)
> {
> int rc = 0;
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 594c494..49f3296 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -126,6 +126,7 @@ struct power_supply {
> enum power_supply_property psp,
> union power_supply_propval *val);
> void (*external_power_changed)(struct power_supply *psy);
> + void (*set_charged)(struct power_supply *psy);
>
> /* For APM emulation, think legacy userspace. */
> int use_for_apm;
> @@ -165,8 +166,10 @@ struct power_supply_info {
> int use_for_apm;
> };
>
> +extern struct power_supply *power_supply_get_by_name(char *name);
> extern void power_supply_changed(struct power_supply *psy);
> extern int power_supply_am_i_supplied(struct power_supply *psy);
> +extern int power_supply_set_battery_charged(struct power_supply *psy);
>
> #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
> extern int power_supply_is_system_supplied(void);
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Wed, Jul 22, 2009 at 09:41:11PM +0400, Anton Vorontsov wrote:
> On Wed, Jul 22, 2009 at 07:20:45PM +0200, Daniel Mack wrote:
> > What about something like the patch below?
> >
> > The platform code interface would look like
> >
> > struct power_supply *psy =
> > power_support_get_by_name("ds2760.0");
> > if (psy)
> > power_supply_set_battery_charged(psy);
> >
> > Couldn't test it yet as my hardware is missing, but I would still like
> > to hear your comments about it :)
>
> Wow, looks perfect. Let me know when you test it, and I'll
> apply that patch.
That seems to work well. I'm still resending the last two remaining
patches as a follow-up to this one - the changelog had a typo.
Thanks,
Daniel
This adds a function to get a power_supply device from the class of
registered devices by name reference. It can be used to find a specific
battery to call power_supply_set_battery_charged() on.
Some battery drivers might need that information to calibrate
themselves.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Ian Molton <[email protected]>
Cc: Anton Vorontsov <[email protected]>
Cc: Matt Reimer <[email protected]>
---
drivers/power/power_supply_core.c | 28 ++++++++++++++++++++++++++++
include/linux/power_supply.h | 3 +++
2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 58f10ac..ddb4800 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
}
EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
+int power_supply_set_battery_charged(struct power_supply *psy)
+{
+ if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
+ psy->set_charged(psy);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
+
+static int power_supply_match_device_by_name(struct device *dev, void *data)
+{
+ const char *name = data;
+ struct power_supply *psy = dev_get_drvdata(dev);
+
+ return strcmp(psy->name, name) == 0;
+}
+
+struct power_supply *power_supply_get_by_name(char *name)
+{
+ struct device *dev = class_find_device(power_supply_class, NULL, name,
+ power_supply_match_device_by_name);
+
+ return dev ? dev_get_drvdata(dev) : NULL;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_by_name);
+
int power_supply_register(struct device *parent, struct power_supply *psy)
{
int rc = 0;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 594c494..49f3296 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -126,6 +126,7 @@ struct power_supply {
enum power_supply_property psp,
union power_supply_propval *val);
void (*external_power_changed)(struct power_supply *psy);
+ void (*set_charged)(struct power_supply *psy);
/* For APM emulation, think legacy userspace. */
int use_for_apm;
@@ -165,8 +166,10 @@ struct power_supply_info {
int use_for_apm;
};
+extern struct power_supply *power_supply_get_by_name(char *name);
extern void power_supply_changed(struct power_supply *psy);
extern int power_supply_am_i_supplied(struct power_supply *psy);
+extern int power_supply_set_battery_charged(struct power_supply *psy);
#if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
extern int power_supply_is_system_supplied(void);
--
1.6.3.1
The ds2760's internal current meter is not reliable enough as it has an
inacurracy of around ~15%. Without any correction for that error, the
current accumulator is couting up all the time, even though the battery
is already fully charged and hence destroys the static information. The
longer it is connected, the worse is the aberration.
Fortunately, this can be corrected by the DS2760_CURRENT_OFFSET_BIAS
register. Using the external power_supply_set_battery_charged()
function, this register is now gauging the measurement.
A delayed work is used to debounce flaky GPIO signals and to let the
current value settle. Also see Maxim's application note AN4188.
Signed-off-by: Daniel Mack <[email protected]>
Cc: Szabolcs Gyurko <[email protected]>
Cc: Matt Reimer <[email protected]>
Cc: Anton Vorontsov <[email protected]>
---
drivers/power/ds2760_battery.c | 51 ++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c
index f4a9258..d545fee 100644
--- a/drivers/power/ds2760_battery.c
+++ b/drivers/power/ds2760_battery.c
@@ -56,6 +56,7 @@ struct ds2760_device_info {
struct device *w1_dev;
struct workqueue_struct *monitor_wqueue;
struct delayed_work monitor_work;
+ struct delayed_work set_charged_work;
};
static unsigned int cache_time = 1000;
@@ -327,6 +328,52 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)
queue_delayed_work(di->monitor_wqueue, &di->monitor_work, HZ/10);
}
+
+static void ds2760_battery_set_charged_work(struct work_struct *work)
+{
+ char bias;
+ struct ds2760_device_info *di = container_of(work,
+ struct ds2760_device_info, set_charged_work.work);
+
+ dev_dbg(di->dev, "%s\n", __func__);
+
+ ds2760_battery_read_status(di);
+
+ /* When we get notified by external circuitry that the battery is
+ * considered fully charged now, we know that there is no current
+ * flow any more. However, the ds2760's internal current meter is
+ * too inaccurate to rely on - spec say something ~15% failure.
+ * Hence, we use the current offset bias register to compensate
+ * that error.
+ */
+
+ if (!power_supply_am_i_supplied(&di->bat))
+ return;
+
+ bias = (signed char) di->current_raw +
+ (signed char) di->raw[DS2760_CURRENT_OFFSET_BIAS];
+
+ dev_dbg(di->dev, "%s: bias = %d\n", __func__, bias);
+
+ w1_ds2760_write(di->w1_dev, &bias, DS2760_CURRENT_OFFSET_BIAS, 1);
+ w1_ds2760_store_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+ w1_ds2760_recall_eeprom(di->w1_dev, DS2760_EEPROM_BLOCK1);
+
+ /* Write to the di->raw[] buffer directly - the CURRENT_OFFSET_BIAS
+ * value won't be read back by ds2760_battery_read_status() */
+ di->raw[DS2760_CURRENT_OFFSET_BIAS] = bias;
+}
+
+static void ds2760_battery_set_charged(struct power_supply *psy)
+{
+ struct ds2760_device_info *di = to_ds2760_device_info(psy);
+
+ /* postpone the actual work by 20 secs. This is for debouncing GPIO
+ * signals and to let the current value settle. See AN4188. */
+ cancel_delayed_work(&di->set_charged_work);
+ queue_delayed_work(di->monitor_wqueue, &di->set_charged_work, HZ * 20);
+}
+
static int ds2760_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -412,6 +459,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
di->bat.properties = ds2760_battery_props;
di->bat.num_properties = ARRAY_SIZE(ds2760_battery_props);
di->bat.get_property = ds2760_battery_get_property;
+ di->bat.set_charged = ds2760_battery_set_charged;
di->bat.external_power_changed =
ds2760_battery_external_power_changed;
@@ -443,6 +491,7 @@ static int ds2760_battery_probe(struct platform_device *pdev)
}
INIT_DELAYED_WORK(&di->monitor_work, ds2760_battery_work);
+ INIT_DELAYED_WORK(&di->set_charged_work, ds2760_battery_set_charged_work);
di->monitor_wqueue = create_singlethread_workqueue(dev_name(&pdev->dev));
if (!di->monitor_wqueue) {
retval = -ESRCH;
@@ -467,6 +516,8 @@ static int ds2760_battery_remove(struct platform_device *pdev)
cancel_rearming_delayed_workqueue(di->monitor_wqueue,
&di->monitor_work);
+ cancel_rearming_delayed_workqueue(di->monitor_wqueue,
+ &di->set_charged_work);
destroy_workqueue(di->monitor_wqueue);
power_supply_unregister(&di->bat);
--
1.6.3.1
ping?
On Thu, Jul 23, 2009 at 08:35:53PM +0200, Daniel Mack wrote:
> This adds a function to get a power_supply device from the class of
> registered devices by name reference. It can be used to find a specific
> battery to call power_supply_set_battery_charged() on.
>
> Some battery drivers might need that information to calibrate
> themselves.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Cc: Ian Molton <[email protected]>
> Cc: Anton Vorontsov <[email protected]>
> Cc: Matt Reimer <[email protected]>
> ---
> drivers/power/power_supply_core.c | 28 ++++++++++++++++++++++++++++
> include/linux/power_supply.h | 3 +++
> 2 files changed, 31 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 58f10ac..ddb4800 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
> }
> EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
>
> +int power_supply_set_battery_charged(struct power_supply *psy)
> +{
> + if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
> + psy->set_charged(psy);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
> +
> +static int power_supply_match_device_by_name(struct device *dev, void *data)
> +{
> + const char *name = data;
> + struct power_supply *psy = dev_get_drvdata(dev);
> +
> + return strcmp(psy->name, name) == 0;
> +}
> +
> +struct power_supply *power_supply_get_by_name(char *name)
> +{
> + struct device *dev = class_find_device(power_supply_class, NULL, name,
> + power_supply_match_device_by_name);
> +
> + return dev ? dev_get_drvdata(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(power_supply_get_by_name);
> +
> int power_supply_register(struct device *parent, struct power_supply *psy)
> {
> int rc = 0;
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 594c494..49f3296 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -126,6 +126,7 @@ struct power_supply {
> enum power_supply_property psp,
> union power_supply_propval *val);
> void (*external_power_changed)(struct power_supply *psy);
> + void (*set_charged)(struct power_supply *psy);
>
> /* For APM emulation, think legacy userspace. */
> int use_for_apm;
> @@ -165,8 +166,10 @@ struct power_supply_info {
> int use_for_apm;
> };
>
> +extern struct power_supply *power_supply_get_by_name(char *name);
> extern void power_supply_changed(struct power_supply *psy);
> extern int power_supply_am_i_supplied(struct power_supply *psy);
> +extern int power_supply_set_battery_charged(struct power_supply *psy);
>
> #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
> extern int power_supply_is_system_supplied(void);
> --
> 1.6.3.1
>
Daniel Mack wrote:
> ping?
Looks ok. the only comment I have right now is that the comment doesnt
quite match the patch - it suggests that the patch doesnt include the
power_supply_set_battery_charged() function, when it does in fact.
> On Thu, Jul 23, 2009 at 08:35:53PM +0200, Daniel Mack wrote:
>> This adds a function to get a power_supply device from the class of
>> registered devices by name reference. It can be used to find a specific
>> battery to call power_supply_set_battery_charged() on.
>>
>> Some battery drivers might need that information to calibrate
>> themselves.
>>
>> Signed-off-by: Daniel Mack <[email protected]>
>> Cc: Ian Molton <[email protected]>
>> Cc: Anton Vorontsov <[email protected]>
>> Cc: Matt Reimer <[email protected]>
>> ---
>> drivers/power/power_supply_core.c | 28 ++++++++++++++++++++++++++++
>> include/linux/power_supply.h | 3 +++
>> 2 files changed, 31 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
>> index 58f10ac..ddb4800 100644
>> --- a/drivers/power/power_supply_core.c
>> +++ b/drivers/power/power_supply_core.c
>> @@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
>> }
>> EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
>>
>> +int power_supply_set_battery_charged(struct power_supply *psy)
>> +{
>> + if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
>> + psy->set_charged(psy);
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
>> +
>> +static int power_supply_match_device_by_name(struct device *dev, void *data)
>> +{
>> + const char *name = data;
>> + struct power_supply *psy = dev_get_drvdata(dev);
>> +
>> + return strcmp(psy->name, name) == 0;
>> +}
>> +
>> +struct power_supply *power_supply_get_by_name(char *name)
>> +{
>> + struct device *dev = class_find_device(power_supply_class, NULL, name,
>> + power_supply_match_device_by_name);
>> +
>> + return dev ? dev_get_drvdata(dev) : NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(power_supply_get_by_name);
>> +
>> int power_supply_register(struct device *parent, struct power_supply *psy)
>> {
>> int rc = 0;
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index 594c494..49f3296 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -126,6 +126,7 @@ struct power_supply {
>> enum power_supply_property psp,
>> union power_supply_propval *val);
>> void (*external_power_changed)(struct power_supply *psy);
>> + void (*set_charged)(struct power_supply *psy);
>>
>> /* For APM emulation, think legacy userspace. */
>> int use_for_apm;
>> @@ -165,8 +166,10 @@ struct power_supply_info {
>> int use_for_apm;
>> };
>>
>> +extern struct power_supply *power_supply_get_by_name(char *name);
>> extern void power_supply_changed(struct power_supply *psy);
>> extern int power_supply_am_i_supplied(struct power_supply *psy);
>> +extern int power_supply_set_battery_charged(struct power_supply *psy);
>>
>> #if defined(CONFIG_POWER_SUPPLY) || defined(CONFIG_POWER_SUPPLY_MODULE)
>> extern int power_supply_is_system_supplied(void);
>> --
>> 1.6.3.1
>>
>
>
On Wed, Jul 29, 2009 at 11:29:48AM +0100, Ian molton wrote:
> Looks ok. the only comment I have right now is that the comment doesnt
> quite match the patch - it suggests that the patch doesnt include the
> power_supply_set_battery_charged() function, when it does in fact.
The subject mentions it, and the body references it again. Hmm. Anyway.
Want me to resend? Or could the commiter add another sentence to point
that out? I think that would be Anton, as he agreed to pick all the
other patches I recently sent in.
Thanks,
Daniel
>> On Thu, Jul 23, 2009 at 08:35:53PM +0200, Daniel Mack wrote:
>>> This adds a function to get a power_supply device from the class of
>>> registered devices by name reference. It can be used to find a specific
>>> battery to call power_supply_set_battery_charged() on.
>>>
>>> Some battery drivers might need that information to calibrate
>>> themselves.
>>>
>>> Signed-off-by: Daniel Mack <[email protected]>
>>> Cc: Ian Molton <[email protected]>
>>> Cc: Anton Vorontsov <[email protected]>
>>> Cc: Matt Reimer <[email protected]>
>>> ---
>>> drivers/power/power_supply_core.c | 28 ++++++++++++++++++++++++++++
>>> include/linux/power_supply.h | 3 +++
>>> 2 files changed, 31 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
>>> index 58f10ac..ddb4800 100644
>>> --- a/drivers/power/power_supply_core.c
>>> +++ b/drivers/power/power_supply_core.c
>>> @@ -117,6 +117,34 @@ int power_supply_is_system_supplied(void)
>>> }
>>> EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
>>> +int power_supply_set_battery_charged(struct power_supply *psy)
>>> +{
>>> + if (psy->type == POWER_SUPPLY_TYPE_BATTERY && psy->set_charged) {
>>> + psy->set_charged(psy);
>>> + return 0;
>>> + }
>>> +
>>> + return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(power_supply_set_battery_charged);
>>> +
>>> +static int power_supply_match_device_by_name(struct device *dev, void *data)
>>> +{
>>> + const char *name = data;
>>> + struct power_supply *psy = dev_get_drvdata(dev);
>>> +
>>> + return strcmp(psy->name, name) == 0;
>>> +}
>>> +
>>> +struct power_supply *power_supply_get_by_name(char *name)
>>> +{
>>> + struct device *dev = class_find_device(power_supply_class, NULL, name,
>>> + power_supply_match_device_by_name);
>>> +
>>> + return dev ? dev_get_drvdata(dev) : NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(power_supply_get_by_name);
>>> +
>>> int power_supply_register(struct device *parent, struct power_supply *psy)
>>> {
>>> int rc = 0;
>>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>>> index 594c494..49f3296 100644
>>> --- a/include/linux/power_supply.h
>>> +++ b/include/linux/power_supply.h
>>> @@ -126,6 +126,7 @@ struct power_supply {
>>> enum power_supply_property psp,
>>> union power_supply_propval *val);
>>> void (*external_power_changed)(struct power_supply *psy);
>>> + void (*set_charged)(struct power_supply *psy);
>>> /* For APM emulation, think legacy userspace. */
>>> int use_for_apm;
>>> @@ -165,8 +166,10 @@ struct power_supply_info {
>>> int use_for_apm;
>>> };
>>> +extern struct power_supply *power_supply_get_by_name(char *name);
>>> extern void power_supply_changed(struct power_supply *psy);
>>> extern int power_supply_am_i_supplied(struct power_supply *psy);
>>> +extern int power_supply_set_battery_charged(struct power_supply *psy);
>>> #if defined(CONFIG_POWER_SUPPLY) ||
>>> defined(CONFIG_POWER_SUPPLY_MODULE)
>>> extern int power_supply_is_system_supplied(void);
>>> --
>
Daniel Mack wrote:
> On Wed, Jul 29, 2009 at 11:29:48AM +0100, Ian molton wrote:
>> Looks ok. the only comment I have right now is that the comment doesnt
>> quite match the patch - it suggests that the patch doesnt include the
>> power_supply_set_battery_charged() function, when it does in fact.
>
> The subject mentions it, and the body references it again. Hmm. Anyway.
> Want me to resend? Or could the commiter add another sentence to point
> that out? I think that would be Anton, as he agreed to pick all the
> other patches I recently sent in.
I'm not that fussed - you just seemed to want feedback, so I had a read ;-)
If you change it, I'd write something like:
This adds a function that indicates that a battery is fully charged. It
also includes functions to get a power_supply device from the class of
registered devices by name reference. These can be used to find a
specific battery to call power_supply_set_battery_charged() on.
Some battery drivers might need this information to calibrate
themselves.
Or something.
On Wed, Jul 29, 2009 at 01:45:05PM +0100, Ian Molton wrote:
> Daniel Mack wrote:
> >On Wed, Jul 29, 2009 at 11:29:48AM +0100, Ian molton wrote:
> >>Looks ok. the only comment I have right now is that the comment
> >>doesnt quite match the patch - it suggests that the patch
> >>doesnt include the power_supply_set_battery_charged() function,
> >>when it does in fact.
> >
> >The subject mentions it, and the body references it again. Hmm. Anyway.
> >Want me to resend? Or could the commiter add another sentence to point
> >that out? I think that would be Anton, as he agreed to pick all the
> >other patches I recently sent in.
>
> I'm not that fussed - you just seemed to want feedback, so I had a read ;-)
>
> If you change it, I'd write something like:
>
> This adds a function that indicates that a battery is fully charged.
> It also includes functions to get a power_supply device from the
> class of registered devices by name reference. These can be used to
> find a specific battery to call power_supply_set_battery_charged()
> on.
>
> Some battery drivers might need this information to calibrate
> themselves.
Ian, Matt, much thanks for reviews and acks!
Daniel, thanks for the patches, and for the patience. :-)
Following patches applied to battery-2.6.git (for -next):
Daniel Mack (8):
ds2760_battery: delay power supply registration
ds2760_battery: export more features
ds2760_battery: add rated_capacity module parameter
ds2760_battery: handle full_active_uAh == 0 case correctly
ds2760_battery: add current_accum module parameter
power_supply: EXPORT_SYMBOL cleanups
power_supply: get_by_name and set_charged functionality
ds2760_battery: implement set_charged() feature
Let me know if I missed anything.
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
On Thu, Jul 30, 2009 at 06:11:34PM +0400, Anton Vorontsov wrote:
> Ian, Matt, much thanks for reviews and acks!
> Daniel, thanks for the patches, and for the patience. :-)
>
>
> Following patches applied to battery-2.6.git (for -next):
>
> Daniel Mack (8):
> ds2760_battery: delay power supply registration
> ds2760_battery: export more features
> ds2760_battery: add rated_capacity module parameter
> ds2760_battery: handle full_active_uAh == 0 case correctly
> ds2760_battery: add current_accum module parameter
> power_supply: EXPORT_SYMBOL cleanups
> power_supply: get_by_name and set_charged functionality
> ds2760_battery: implement set_charged() feature
>
> Let me know if I missed anything.
Looks good. Thanks a lot :) There might be some more coming up the next
days.
Daniel