Virtual Battery Driver

RFC PATCH In review John Stultz 2 participants 5 messages Apr 25, 2011

Share & tools

John Stultz

author

2011-04-25 20:21 UTC #0 lore raw

cover Virtual Battery Driver

For awhile now, there has been an out of tree virtual battery driver that was created by Masashi YOKOTA <[email protected]> that folks have been using to test battery UI functionality on boards that don't actually have battery hardware.

More recently the power supply test driver (test_power.c) has been merged.

Both drivers provide somwhat similar functionality, although the virtual battery driver allows settings such as health and charge state to be manipulated at runtime via module params.

So while I'm not proposing to merge this patch set as is, I wanted to send it out for comment to see if there would be some interest or objections for merging some of the functionality from the virtual battery driver into the test_power driver?

Thoughts?

thanks
-john

CC Anton Vorontsov <[email protected]>

CC Akihiro MAEDA <[email protected]>

CC Masashi YOKOTA <[email protected]>

Akihiro MAEDA (1):
power: virtual battery driver tweaks

John Stultz (1):
power: Fixup stack usage in vritual battery driver

Masashi YOKOTA (1):
power: Add virtual battery driver

 drivers/power/Kconfig           |    5 +
 drivers/power/Makefile          |    1 +
 drivers/power/virtual_battery.c |  400 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 406 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/virtual_battery.c

Signature · 1 line

1.7.3.2.146.gca209

John Stultz

author

2011-04-25 20:20 UTC #1 lore raw

RFC PATCH 1/3 power: Add virtual battery driver

From: Masashi YOKOTA <[email protected]>

This patch adds virtual battery driver by Masashi YOKOTA. The original driver download can be found here:
http://downloads.pylone.jp/src/virtual_battery/virtual_battery-0.0.1.tar.bz2

CC Anton Vorontsov <[email protected]>

Signed-off-by Akihiro MAEDA <[email protected]>

[john.stultz: reworded commit log]

Signed-off-by John Stultz <[email protected]>

---
 drivers/power/Kconfig           |    5 +
 drivers/power/Makefile          |    1 +
 drivers/power/virtual_battery.c |  349 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 355 insertions(+), 0 deletions(-)
 create mode 100644 drivers/power/virtual_battery.c

drivers/power/Kconfig +5 -0

index 52a462f..c599f0c 100644
@@ -219,4 +219,9 @@ config CHARGER_GPIO
 	  This driver can be build as a module. If so, the module will be
 	  called gpio-charger.
 
+config BATTERY_VIRTUAL
+	tristate "Virtual Battery Driver"
+	help
+	  Say Y to include support for Virtual Battery.
+
 endif # POWER_SUPPLY

drivers/power/Makefile +1 -0

index 8385bfa..6ea09c9 100644
@@ -34,3 +34,4 @@ obj-$(CONFIG_BATTERY_INTEL_MID)	+= intel_mid_battery.o
 obj-$(CONFIG_CHARGER_ISP1704)	+= isp1704_charger.o
 obj-$(CONFIG_CHARGER_TWL4030)	+= twl4030_charger.o
 obj-$(CONFIG_CHARGER_GPIO)	+= gpio-charger.o
+obj-$(CONFIG_BATTERY_VIRTUAL)	+= virtual_battery.o

drivers/power/virtual_battery.c new file +349 -0

349 lines not on this page · raw message

Signature · 1 line

1.7.3.2.146.gca209

John Stultz

author

2011-04-25 20:21 UTC #2 lore raw

RFC PATCH 2/3 power: virtual battery driver tweaks

In reply to John Stultz

From: Akihiro MAEDA <[email protected]>

This patch tweaks the virtual battery driver, setting arguments as const and adding new kernel_param_ops.

CC Anton Vorontsov <[email protected]>

Signed-off-by Akihiro MAEDA <[email protected]>

[john.stultz: Reworded commit log]

Signed-off-by John Stultz <[email protected]>

---
 drivers/power/virtual_battery.c |   53 ++++++++++++++++++++++++++++++--------
 1 files changed, 42 insertions(+), 11 deletions(-)

drivers/power/virtual_battery.c +42 ~~-11~~

index 4ae79d4..ed686ef 100644
@@ -5,6 +5,7 @@
  *
  * Copyright (C) 2008 Pylone, Inc.
  * Author: Masashi YOKOTA <[email protected]>
+ * Modified by: Akihiro MAEDA <[email protected]>
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and

@@ -190,7 +191,7 @@ static const char * map_get_key(struct battery_property_map * map, int value, co
 	return def_key;
 }
 
-static int param_set_ac_status(const char *key, struct kernel_param *kp)
+static int param_set_ac_status(const char *key, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
 	ac_status = map_get_value( map_ac_online, key, ac_status);

@@ -198,14 +199,14 @@ static int param_set_ac_status(const char *key, struct kernel_param *kp)
 	return 0;
 }
 
-static int param_get_ac_status(char *buffer, struct kernel_param *kp)
+static int param_get_ac_status(char *buffer, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_ac_online, ac_status, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_status(const char *key, struct kernel_param *kp)
+static int param_set_battery_status(const char *key, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s.\n", __func__, kp->name, key);
 	battery_status = map_get_value( map_status, key, battery_status);

@@ -213,14 +214,14 @@ static int param_set_battery_status(const char *key, struct kernel_param *kp)
 	return 0;
 }
 
-static int param_get_battery_status(char *buffer, struct kernel_param *kp)
+static int param_get_battery_status(char *buffer, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_status, battery_status, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_health(const char *key, struct kernel_param *kp)
+static int param_set_battery_health(const char *key, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
 	battery_health = map_get_value( map_health, key, battery_health);

@@ -228,14 +229,14 @@ static int param_set_battery_health(const char *key, struct kernel_param *kp)
 	return 0;
 }
 
-static int param_get_battery_health(char *buffer, struct kernel_param *kp)
+static int param_get_battery_health(char *buffer, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_health, battery_health, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_present(const char *key, struct kernel_param *kp)
+static int param_set_battery_present(const char *key, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
 	battery_present = map_get_value( map_present, key, battery_present);

@@ -243,14 +244,14 @@ static int param_set_battery_present(const char *key, struct kernel_param *kp)
 	return 0;
 }
 
-static int param_get_battery_present(char *buffer, struct kernel_param *kp)
+static int param_get_battery_present(char *buffer, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_present, battery_present, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_technology(const char *key, struct kernel_param *kp)
+static int param_set_battery_technology(const char *key, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
 	battery_technology = map_get_value( map_technology, key, battery_technology);

@@ -258,14 +259,14 @@ static int param_set_battery_technology(const char *key, struct kernel_param *kp
 	return 0;
 }
 
-static int param_get_battery_technology(char *buffer, struct kernel_param *kp)
+static int param_get_battery_technology(char *buffer, const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_technology, battery_technology, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_capacity(const char *key, struct kernel_param *kp)
+static int param_set_battery_capacity(const char *key, const struct kernel_param *kp)
 {
 	int tmp;

@@ -314,6 +315,36 @@ static void __exit virtual_battery_exit(void)
 	printk(KERN_INFO KBUILD_BASENAME": unregistered \n");
 }
 
+static struct kernel_param_ops param_ops_ac_status = {
+	.set = param_set_ac_status,
+	.get = param_get_ac_status,
+};
+
+static struct kernel_param_ops param_ops_battery_status = {
+	.set = param_set_battery_status,
+	.get = param_get_battery_status,
+};
+
+static struct kernel_param_ops param_ops_battery_present = {
+	.set = param_set_battery_present,
+	.get = param_get_battery_present,
+};
+
+static struct kernel_param_ops param_ops_battery_technology = {
+	.set = param_set_battery_technology,
+	.get = param_get_battery_technology,
+};
+
+static struct kernel_param_ops param_ops_battery_health = {
+	.set = param_set_battery_health,
+	.get = param_get_battery_health,
+};
+
+static struct kernel_param_ops param_ops_battery_capacity = {
+	.set = param_set_battery_capacity,
+	.get = param_get_battery_capacity,
+};
+
 module_init(virtual_battery_init);
 module_exit(virtual_battery_exit);

Signature · 1 line

1.7.3.2.146.gca209

John Stultz

author

2011-04-25 20:20 UTC #3 lore raw

RFC PATCH 3/3 power: Fixup stack usage in vritual battery driver

In reply to John Stultz

For some reason the virtual battery driver code allocates 4k on the stack. This is clearly broken, so keep the length smaller (256) and cleanup the string management code to use the bounds checking versions.

Also cleans up some 80+ char line formatting issues.

CC Anton Vorontsov <[email protected]>

CC Akihiro MAEDA <[email protected]>

CC Masashi YOKOTA <[email protected]>

Signed-off-by John Stultz <[email protected]>

---
 drivers/power/virtual_battery.c |   62 +++++++++++++++++++++++++-------------
 1 files changed, 41 insertions(+), 21 deletions(-)

drivers/power/virtual_battery.c +41 ~~-21~~

index ed686ef..e865230 100644
@@ -115,6 +115,7 @@ static struct power_supply power_supply_bat = {
 };
 
 
+#define MAX_KEYLENGTH 256
 struct battery_property_map {
 	int value;
 	char const * key;

@@ -160,18 +161,21 @@ static struct battery_property_map map_technology[] = {
 };
 
 
-static int map_get_value(struct battery_property_map * map, const char * key, int def_val)
+static int map_get_value(struct battery_property_map * map, const char * key,
+				int def_val)
 {
-	char buf[4096];
+	char buf[MAX_KEYLENGTH];
 	int cr;
 
-	strcpy(buf, key);
-	cr = strlen(buf) - 1;
+	strncpy(buf, key, MAX_KEYLENGTH);
+	buf[MAX_KEYLENGTH-1] = '\0';
+
+	cr = strnlen(buf, MAX_KEYLENGTH) - 1;
 	if (buf[cr] == '\n')
 		buf[cr] = '\0';
 
 	while (map->key) {
-		if (strcasecmp(map->key, buf) == 0)
+		if (strncasecmp(map->key, buf, MAX_KEYLENGTH) == 0)
 			return map->value;
 		map++;
 	}

@@ -180,7 +184,8 @@ static int map_get_value(struct battery_property_map * map, const char * key, in
 }
 
 
-static const char * map_get_key(struct battery_property_map * map, int value, const char * def_key)
+static const char * map_get_key(struct battery_property_map * map, int value,
+				const char * def_key)
 {
 	while (map->key) {
 		if (map->value == value)

@@ -193,7 +198,8 @@ static const char * map_get_key(struct battery_property_map * map, int value, co
 
 static int param_set_ac_status(const char *key, const struct kernel_param *kp)
 {
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n",
+			__func__, kp->name, key);
 	ac_status = map_get_value( map_ac_online, key, ac_status);
 	power_supply_changed(&power_supply_ac);
 	return 0;

@@ -206,9 +212,11 @@ static int param_get_ac_status(char *buffer, const struct kernel_param *kp)
 	return strlen(buffer);
 }
 
-static int param_set_battery_status(const char *key, const struct kernel_param *kp)
+static int param_set_battery_status(const char *key,
+					const struct kernel_param *kp)
 {
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s.\n", __func__, kp->name, key);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s.\n",
+			__func__, kp->name, key);
 	battery_status = map_get_value( map_status, key, battery_status);
 	power_supply_changed(&power_supply_bat);
 	return 0;

@@ -221,9 +229,11 @@ static int param_get_battery_status(char *buffer, const struct kernel_param *kp)
 	return strlen(buffer);
 }
 
-static int param_set_battery_health(const char *key, const struct kernel_param *kp)
+static int param_set_battery_health(const char *key,
+					const struct kernel_param *kp)
 {
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n",
+			__func__, kp->name, key);
 	battery_health = map_get_value( map_health, key, battery_health);
 	power_supply_changed(&power_supply_bat);
 	return 0;

@@ -236,41 +246,51 @@ static int param_get_battery_health(char *buffer, const struct kernel_param *kp)
 	return strlen(buffer);
 }
 
-static int param_set_battery_present(const char *key, const struct kernel_param *kp)
+static int param_set_battery_present(const char *key,
+					const struct kernel_param *kp)
 {
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n",
+			__func__, kp->name, key);
 	battery_present = map_get_value( map_present, key, battery_present);
 	power_supply_changed(&power_supply_ac);
 	return 0;
 }
 
-static int param_get_battery_present(char *buffer, const struct kernel_param *kp)
+static int param_get_battery_present(char *buffer,
+					const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
 	strcpy(buffer, map_get_key( map_present, battery_present, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_technology(const char *key, const struct kernel_param *kp)
+static int param_set_battery_technology(const char *key,
+					const struct kernel_param *kp)
 {
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
-	battery_technology = map_get_value( map_technology, key, battery_technology);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n",
+			__func__, kp->name, key);
+	battery_technology = map_get_value(map_technology, key,
+						battery_technology);
 	power_supply_changed(&power_supply_bat);
 	return 0;
 }
 
-static int param_get_battery_technology(char *buffer, const struct kernel_param *kp)
+static int param_get_battery_technology(char *buffer,
+					const struct kernel_param *kp)
 {
 	dev_dbg(&bat_pdev->dev, "%s: name=%s\n", __func__, kp->name);
-	strcpy(buffer, map_get_key( map_technology, battery_technology, "unknown"));
+	strcpy(buffer,
+		map_get_key( map_technology, battery_technology, "unknown"));
 	return strlen(buffer);
 }
 
-static int param_set_battery_capacity(const char *key, const struct kernel_param *kp)
+static int param_set_battery_capacity(const char *key,
+					const struct kernel_param *kp)
 {
 	int tmp;
 
-	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n", __func__, kp->name, key);
+	dev_dbg(&bat_pdev->dev, "%s: name=%s, key=%s\n",
+			__func__, kp->name, key);
 
 	if (1 != sscanf(key, "%d", &tmp))
 		return -EINVAL;

Signature · 1 line

1.7.3.2.146.gca209

Anton Vorontsov

2011-04-25 21:11 UTC #4 lore raw

In reply to John Stultz

Quoted 5 lines · John Stultz

So while I'm not proposing to merge this patch set as is, I wanted to send it out for comment to see if there would be some interest or objections for merging some of the functionality from the virtual battery driver into the test_power driver?

I have no problem with test_power being more 'dynamic', so feel free to extend it as needed.

Thanks!

Signature · 2 lines

Anton Vorontsov
Email: [email protected]