2009-11-10 02:38:14

by Paul Fertser

[permalink] [raw]
Subject: [RFC PATCH 0/2] power: implement platform battery driver

Hi,

This is a simple generic implementation of a battery driver that
essentially gets all the information from platform-specific functions.

Tested on openmoko gta02 device but it looks like it can be useful for
other targets as well.

With such a simple driver it's quite easy for almost anybody to have an
opinion about an ideal implementation so come on, do not hesistate, i
really need your constructive feedback ;) One of the most obvious points is
the clumsy way to match entries in the properties arrays with the
callbacks. I did so to avoid an extra alloc + copy stuff but if you battery
guys find this really unacceptable, well, i'll do it the way you like :)

Pavel, i saw you posting spitz battery driver and thought you might find
this useful.

[RFC PATCH 1/2] power: implement platform battery driver
[RFC PATCH 2/2] gta02: add support for platform_battery


2009-11-10 02:38:27

by Paul Fertser

[permalink] [raw]
Subject: [RFC PATCH 1/2] power: implement platform battery driver

This driver can be used for dumb batteries when all knowledge about
their state belongs to the platform that does necessary ADC readings,
conversions, guessimations etc.

Signed-off-by: Paul Fertser <[email protected]>
---
drivers/power/Kconfig | 6 ++
drivers/power/Makefile | 1 +
drivers/power/platform_battery.c | 119 ++++++++++++++++++++++++++++++++++++++
include/linux/platform_battery.h | 12 ++++
4 files changed, 138 insertions(+), 0 deletions(-)
create mode 100644 drivers/power/platform_battery.c
create mode 100644 include/linux/platform_battery.h

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index bdbc4f7..5382c10 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -103,4 +103,10 @@ config CHARGER_PCF50633
help
Say Y to include support for NXP PCF50633 Main Battery Charger.

+config BATTERY_PLATFORM
+ tristate "Platform battery driver"
+ help
+ Say Y here to include support for battery driver that gets all
+ information from platform functions.
+
endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 380d17c..b962d1a 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_BATTERY_BQ27x00) += bq27x00_battery.o
obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
obj-$(CONFIG_BATTERY_MAX17040) += max17040_battery.o
obj-$(CONFIG_CHARGER_PCF50633) += pcf50633-charger.o
+obj-$(CONFIG_BATTERY_PLATFORM) += platform_battery.o
diff --git a/drivers/power/platform_battery.c b/drivers/power/platform_battery.c
new file mode 100644
index 0000000..99e155a
--- /dev/null
+++ b/drivers/power/platform_battery.c
@@ -0,0 +1,119 @@
+/*
+ * Driver for platform battery
+ *
+ * Copyright (c) Paul Fertser <[email protected]>
+ * Inspired by Balaji Rao <[email protected]>
+ *
+ * This driver can be used for dumb batteries when all knowledge about
+ * their state belongs to the platform that does necessary ADC readings,
+ * conversions, guessimations etc.
+ *
+ * Use consistent with the GNU GPL is permitted, provided that this
+ * copyright notice is preserved in its entirety in all copies and derived
+ * works.
+ */
+
+#include <linux/module.h>
+#include <linux/param.h>
+#include <linux/delay.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/platform_battery.h>
+
+struct platform_battery {
+ struct power_supply psy;
+ struct platform_bat_platform_data *pdata;
+};
+
+static int platform_bat_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct platform_battery *bat =
+ container_of(psy, struct platform_battery, psy);
+ size_t i;
+ int present = 1;
+
+ if (bat->pdata->is_present)
+ present = bat->pdata->is_present();
+
+ if (psp != POWER_SUPPLY_PROP_PRESENT && !present)
+ return -ENODEV;
+
+ for (i = 0; i < psy->num_properties; i++)
+ if (psy->properties[i] == psp) {
+ val->intval = bat->pdata->get_property[i]();
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static void platform_bat_ext_changed(struct power_supply *psy)
+{
+ struct platform_battery *bat =
+ container_of(psy, struct platform_battery, psy);
+ power_supply_changed(&bat->psy);
+}
+
+static int platform_battery_probe(struct platform_device *pdev)
+{
+ struct platform_battery *platform_bat;
+ struct platform_bat_platform_data *pdata =
+ (struct platform_bat_platform_data *)pdev->dev.platform_data;
+
+ platform_bat = kzalloc(sizeof(*platform_bat), GFP_KERNEL);
+ if (!platform_bat)
+ return -ENOMEM;
+
+ if (pdata->name)
+ platform_bat->psy.name = pdata->name;
+ else
+ platform_bat->psy.name = dev_name(&pdev->dev);
+ platform_bat->psy.type = POWER_SUPPLY_TYPE_BATTERY;
+ platform_bat->psy.properties = pdata->properties;
+ platform_bat->psy.num_properties = pdata->num_properties;
+ platform_bat->psy.get_property = platform_bat_get_property;
+ platform_bat->psy.external_power_changed = platform_bat_ext_changed;
+
+ platform_bat->pdata = pdata;
+ platform_set_drvdata(pdev, platform_bat);
+ power_supply_register(&pdev->dev, &platform_bat->psy);
+
+ return 0;
+}
+
+static int platform_battery_remove(struct platform_device *pdev)
+{
+ struct platform_battery *bat = platform_get_drvdata(pdev);
+
+ power_supply_unregister(&bat->psy);
+ kfree(bat);
+
+ return 0;
+}
+
+static struct platform_driver platform_battery_driver = {
+ .driver = {
+ .name = "platform_battery",
+ },
+ .probe = platform_battery_probe,
+ .remove = platform_battery_remove,
+};
+
+static int __init platform_battery_init(void)
+{
+ return platform_driver_register(&platform_battery_driver);
+}
+module_init(platform_battery_init);
+
+static void __exit platform_battery_exit(void)
+{
+ platform_driver_unregister(&platform_battery_driver);
+}
+module_exit(platform_battery_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Paul Fertser <[email protected]>");
+MODULE_DESCRIPTION("platform battery driver");
diff --git a/include/linux/platform_battery.h b/include/linux/platform_battery.h
new file mode 100644
index 0000000..00f7651
--- /dev/null
+++ b/include/linux/platform_battery.h
@@ -0,0 +1,12 @@
+#ifndef __PLATFORM_BATTERY_H__
+#define __PLATFORM_BATTERY_H__
+
+struct platform_bat_platform_data {
+ const char *name;
+ int (**get_property)(void);
+ int (*is_present)(void);
+ enum power_supply_property *properties;
+ size_t num_properties;
+};
+
+#endif
--
1.6.4.4

2009-11-10 02:38:29

by Paul Fertser

[permalink] [raw]
Subject: [RFC PATCH 2/2] gta02: add support for platform_battery

This adds support for platform_battery driver which allows to specify a set
of power supply properties and callbacks to acquire them. It is needed to
support dumb batteries where all the information about their status can
only be obtained by platform-specific actions such as specific ADC
measurements, some guessimation etc.

Signed-off-by: Paul Fertser <[email protected]>
---
arch/arm/mach-s3c2442/mach-gta02.c | 113 ++++++++++++++++++++++++++++++++++++
1 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-s3c2442/mach-gta02.c b/arch/arm/mach-s3c2442/mach-gta02.c
index 8c61026..2159066 100644
--- a/arch/arm/mach-s3c2442/mach-gta02.c
+++ b/arch/arm/mach-s3c2442/mach-gta02.c
@@ -106,6 +106,8 @@
#include <linux/hdq.h>
#include <linux/bq27000_battery.h>

+#include <linux/platform_battery.h>
+
#include <linux/gta02-vibrator.h>

#include <mach/ts.h>
@@ -866,6 +868,117 @@ struct platform_device bq27000_battery_device = {
},
};

+/* Platform battery */
+
+/* Capacity of a typical BL-5C dumb battery */
+#define GTA02_BAT_CHARGE_FULL 850000
+
+static int gta02_bat_voltscale(int volt)
+{
+ /* This table is suggested by SpeedEvil based on analysis of
+ * experimental data */
+ static const int lut[][2] = {
+ { 4120, 100 },
+ { 3900, 60 },
+ { 3740, 25 },
+ { 3600, 5 },
+ { 3000, 0 } };
+ int i, res = 0;
+
+ if (volt > lut[0][0])
+ res = lut[0][1];
+ else
+ for (i = 0; lut[i][1]; i++) {
+ if (volt <= lut[i][0] && volt >= lut[i+1][0]) {
+ res = lut[i][1] - (lut[i][0]-volt)*
+ (lut[i][1]-lut[i+1][1])/
+ (lut[i][0]-lut[i+1][0]);
+ break;
+ }
+ }
+ return res;
+}
+
+static int gta02_bat_get_voltage(void)
+{
+ struct pcf50633 *pcf = gta02_pcf;
+ u16 adc, mv = 0;
+ adc = pcf50633_adc_sync_read(pcf,
+ PCF50633_ADCC1_MUX_BATSNS_RES,
+ PCF50633_ADCC1_AVERAGE_16);
+ /* The formula from DS is for divide-by-two mode, current driver uses
+ divide-by-three */
+ mv = (adc * 6000) / 1023;
+ return mv * 1000;
+}
+
+static int gta02_bat_get_present(void)
+{
+ /* There is no reliable way to tell if it is present or not */
+ return 1;
+}
+
+static int gta02_bat_get_status(void)
+{
+#ifdef CONFIG_CHARGER_PCF50633
+ if (gta02_get_charger_active_status())
+ return POWER_SUPPLY_STATUS_CHARGING;
+ else
+ return POWER_SUPPLY_STATUS_DISCHARGING;
+#else
+ return POWER_SUPPLY_STATUS_UNKNOWN;
+#endif
+}
+
+static int gta02_bat_get_capacity(void)
+{
+ return gta02_bat_voltscale(gta02_bat_get_voltage()/1000);
+}
+
+static int gta02_bat_get_charge_full(void)
+{
+ return GTA02_BAT_CHARGE_FULL;
+}
+
+static int gta02_bat_get_charge_now(void)
+{
+ return gta02_bat_get_capacity() * gta02_bat_get_charge_full() / 100;
+}
+
+static enum power_supply_property gta02_platform_bat_properties[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_CHARGE_FULL,
+ POWER_SUPPLY_PROP_CHARGE_NOW,
+};
+
+int (*gta02_platform_bat_get_property[])(void) = {
+ gta02_bat_get_present,
+ gta02_bat_get_status,
+ gta02_bat_get_voltage,
+ gta02_bat_get_capacity,
+ gta02_bat_get_charge_full,
+ gta02_bat_get_charge_now,
+};
+
+static struct platform_bat_platform_data gta02_platform_bat_pdata = {
+ .name = "battery",
+ .properties = gta02_platform_bat_properties,
+ .num_properties = ARRAY_SIZE(gta02_platform_bat_properties),
+ .get_property = gta02_platform_bat_get_property,
+ .is_present = gta02_bat_get_present,
+};
+
+struct platform_device gta02_platform_bat = {
+ .name = "platform_battery",
+ .id = -1,
+ .dev = {
+ .platform_data = &gta02_platform_bat_pdata,
+ }
+};
+
/* HDQ */

static void gta02_hdq_attach_child_devices(struct device *parent_device)
--
1.6.4.4

2009-11-10 13:21:25

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] power: implement platform battery driver

> This driver can be used for dumb batteries when all knowledge about
> their state belongs to the platform that does necessary ADC readings,
> conversions, guessimations etc.
>
> Signed-off-by: Paul Fertser <[email protected]>


> +++ b/drivers/power/platform_battery.c
> @@ -0,0 +1,119 @@
> +/*
> + * Driver for platform battery
> + *
> + * Copyright (c) Paul Fertser <[email protected]>
> + * Inspired by Balaji Rao <[email protected]>
> + *
> + * This driver can be used for dumb batteries when all knowledge about
> + * their state belongs to the platform that does necessary ADC readings,
> + * conversions, guessimations etc.
> + *
> + * Use consistent with the GNU GPL is permitted, provided that this
> + * copyright notice is preserved in its entirety in all copies and derived
> + * works.

gpl with additional restrictions? can we get standard gplv2 header?


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

2009-11-10 16:22:54

by Paul Fertser

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] power: implement platform battery driver

Hi,

On Tue, Nov 10, 2009 at 02:21:29PM +0100, Pavel Machek wrote:
> > + * Use consistent with the GNU GPL is permitted, provided that this
> > + * copyright notice is preserved in its entirety in all copies and derived
> > + * works.
>
> gpl with additional restrictions? can we get standard gplv2 header?

Yep, sure. Sorry, that's a dumb copy/paste from the old Balaji driver.

--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:[email protected]

2009-11-10 20:51:19

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] power: implement platform battery driver

Hi!

> > > + * Use consistent with the GNU GPL is permitted, provided that this
> > > + * copyright notice is preserved in its entirety in all copies and derived
> > > + * works.
> >
> > gpl with additional restrictions? can we get standard gplv2 header?
>
> Yep, sure. Sorry, that's a dumb copy/paste from the old Balaji driver.

Thanks.

I do not think I want to use it for zauruses. Battery API is easy
enough to implement, and I need some advanced features. Also zauruses
are similar to each other, so driver<->platform interface needs to be
elsewhere.

Anyway, code looks good from quick look, so good luck :-).
Pavel

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

2009-11-16 01:08:22

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] power: implement platform battery driver

Hi Paul,

On Tue, Nov 10, 2009 at 05:37:52AM +0300, Paul Fertser wrote:
[...]
> +static int platform_bat_get_property(struct power_supply *psy,
> + enum power_supply_property psp,
> + union power_supply_propval *val)
> +{
> + struct platform_battery *bat =
> + container_of(psy, struct platform_battery, psy);
> + size_t i;
> + int present = 1;
> +
> + if (bat->pdata->is_present)
> + present = bat->pdata->is_present();
> +
> + if (psp != POWER_SUPPLY_PROP_PRESENT && !present)
> + return -ENODEV;
> +
> + for (i = 0; i < psy->num_properties; i++)
> + if (psy->properties[i] == psp) {
> + val->intval = bat->pdata->get_property[i]();
> + return 0;
> + }

I'm not sure I like this. Why don't you just pass the enum
to pdata hook? So platform devices would just use a single
get_property function and a 'switch', like the rest of the
power supply drivers.

Thanks,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

2009-11-16 14:55:35

by Paul Fertser

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] power: implement platform battery driver

Hi,

On Mon, Nov 16, 2009 at 04:08:14AM +0300, Anton Vorontsov wrote:
> On Tue, Nov 10, 2009 at 05:37:52AM +0300, Paul Fertser wrote:
> [...]
> > +static int platform_bat_get_property(struct power_supply *psy,
> > + enum power_supply_property psp,
> > + union power_supply_propval *val)
> > +{
> > + struct platform_battery *bat =
> > + container_of(psy, struct platform_battery, psy);
> > + size_t i;
> > + int present = 1;
> > +
> > + if (bat->pdata->is_present)
> > + present = bat->pdata->is_present();
> > +
> > + if (psp != POWER_SUPPLY_PROP_PRESENT && !present)
> > + return -ENODEV;
> > +
> > + for (i = 0; i < psy->num_properties; i++)
> > + if (psy->properties[i] == psp) {
> > + val->intval = bat->pdata->get_property[i]();
> > + return 0;
> > + }
>
> I'm not sure I like this. Why don't you just pass the enum
> to pdata hook? So platform devices would just use a single
> get_property function and a 'switch', like the rest of the
> power supply drivers.

Of course, i thought about that. It seemed to me that it would tie platform
code and power_supply API a bit too much, so if you ever change the API,
the platform code would need to be changed as well. But if you think that
it is a non-issue, sure, i'll do it the way you suggest.

--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:[email protected]