diff mbox series

test_power: Add CURRENT and CHARGE_COUNTER properties

Message ID 1557828298-16591-1-git-send-email-lecopzer.chen@mediatek.com (mailing list archive)
State Not Applicable, archived
Headers show
Series test_power: Add CURRENT and CHARGE_COUNTER properties | expand

Commit Message

Lecopzer Chen May 14, 2019, 10:04 a.m. UTC 
From: "Lecopzer Chen" <lecopzer.chen@mediatek.com>

Emulate battery current (variable) and battery CHARGE_COUNTER
(same as battery_capacity) properties.

Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
Cc: YJ Chiang <yj.chiang@mediatek.com>

---
 drivers/power/supply/test_power.c | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Sebastian Reichel June 25, 2019, 10:35 p.m. UTC | #1 
Hi,

On Tue, May 14, 2019 at 06:04:58PM +0800, Lecopzer Chen wrote:
> From: "Lecopzer Chen" <lecopzer.chen@mediatek.com>
> 
> Emulate battery current (variable) and battery CHARGE_COUNTER
> (same as battery_capacity) properties.
> 
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> Cc: YJ Chiang <yj.chiang@mediatek.com>

Thanks for the patch and thanks for taking care of testing infrastructure.
I don't think it's a good idea to use CHARGE_CAPACITY/CHARGE_NOW also for
CHARGE_COUNTER. CHARGE_COUNTER may be negative, but capacity should not be
negative. Also the patch should be splitted (one for the current feature and
one for the charge counter feature). Last but not least I don't see any
conversion from mA to µA. The userspace sysfs ABI should always
return µA.

-- Sebastian

> ---
>  drivers/power/supply/test_power.c | 33 +++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
> index 57246cdbd042..9f85060c84de 100644
> --- a/drivers/power/supply/test_power.c
> +++ b/drivers/power/supply/test_power.c
> @@ -36,6 +36,7 @@ static int battery_present		= 1; /* true */
>  static int battery_technology		= POWER_SUPPLY_TECHNOLOGY_LION;
>  static int battery_capacity		= 50;
>  static int battery_voltage		= 3300;
> +static int battery_current		= 1600;
>  
>  static bool module_initialized;
>  
> @@ -101,6 +102,7 @@ static int test_power_get_battery_property(struct power_supply *psy,
>  		break;
>  	case POWER_SUPPLY_PROP_CAPACITY:
>  	case POWER_SUPPLY_PROP_CHARGE_NOW:
> +	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
>  		val->intval = battery_capacity;
>  		break;
>  	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> @@ -117,6 +119,10 @@ static int test_power_get_battery_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>  		val->intval = battery_voltage;
>  		break;
> +	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		val->intval = battery_current;
> +		break;
>  	default:
>  		pr_info("%s: some properties deliberately report errors.\n",
>  			__func__);
> @@ -138,6 +144,7 @@ static enum power_supply_property test_power_battery_props[] = {
>  	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
>  	POWER_SUPPLY_PROP_CHARGE_FULL,
>  	POWER_SUPPLY_PROP_CHARGE_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_COUNTER,
>  	POWER_SUPPLY_PROP_CAPACITY,
>  	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
>  	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
> @@ -147,6 +154,8 @@ static enum power_supply_property test_power_battery_props[] = {
>  	POWER_SUPPLY_PROP_SERIAL_NUMBER,
>  	POWER_SUPPLY_PROP_TEMP,
>  	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_AVG,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
>  };
>  
>  static char *test_power_ac_supplied_to[] = {
> @@ -450,6 +459,21 @@ static int param_set_battery_voltage(const char *key,
>  
>  #define param_get_battery_voltage param_get_int
>  
> +static int param_set_battery_current(const char *key,
> +					const struct kernel_param *kp)
> +{
> +	int tmp;
> +
> +	if (1 != sscanf(key, "%d", &tmp))
> +		return -EINVAL;
> +
> +	battery_current = tmp;
> +	signal_power_supply_changed(test_power_supplies[TEST_BATTERY]);
> +	return 0;
> +}
> +
> +#define param_get_battery_current param_get_int
> +
>  static const struct kernel_param_ops param_ops_ac_online = {
>  	.set = param_set_ac_online,
>  	.get = param_get_ac_online,
> @@ -490,6 +514,11 @@ static const struct kernel_param_ops param_ops_battery_voltage = {
>  	.get = param_get_battery_voltage,
>  };
>  
> +static const struct kernel_param_ops param_ops_battery_current = {
> +	.set = param_set_battery_current,
> +	.get = param_get_battery_current,
> +};
> +
>  #define param_check_ac_online(name, p) __param_check(name, p, void);
>  #define param_check_usb_online(name, p) __param_check(name, p, void);
>  #define param_check_battery_status(name, p) __param_check(name, p, void);
> @@ -498,6 +527,7 @@ static const struct kernel_param_ops param_ops_battery_voltage = {
>  #define param_check_battery_health(name, p) __param_check(name, p, void);
>  #define param_check_battery_capacity(name, p) __param_check(name, p, void);
>  #define param_check_battery_voltage(name, p) __param_check(name, p, void);
> +#define param_check_battery_current(name, p) __param_check(name, p, void);
>  
>  
>  module_param(ac_online, ac_online, 0644);
> @@ -528,6 +558,9 @@ MODULE_PARM_DESC(battery_capacity, "battery capacity (percentage)");
>  module_param(battery_voltage, battery_voltage, 0644);
>  MODULE_PARM_DESC(battery_voltage, "battery voltage (millivolts)");
>  
> +module_param(battery_current, battery_current, 0644);
> +MODULE_PARM_DESC(battery_current, "battery current (milliampere)");
> +
>  MODULE_DESCRIPTION("Power supply driver for testing");
>  MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
>  MODULE_LICENSE("GPL");
> -- 
> 2.18.0
>
Lecopzer Chen June 26, 2019, 2:36 a.m. UTC | #2 
Dear Sebastian,

Thanks for your review and helpful suggestion,
Since our platform is relative simple, I just miss the part you refer to.

I will fix them in patch v2.


Thanks,
	Lecopzer
diff mbox series

Patch

diff --git a/drivers/power/supply/test_power.c b/drivers/power/supply/test_power.c
index 57246cdbd042..9f85060c84de 100644
--- a/drivers/power/supply/test_power.c
+++ b/drivers/power/supply/test_power.c
@@ -36,6 +36,7 @@  static int battery_present		= 1; /* true */
 static int battery_technology		= POWER_SUPPLY_TECHNOLOGY_LION;
 static int battery_capacity		= 50;
 static int battery_voltage		= 3300;
+static int battery_current		= 1600;
 
 static bool module_initialized;
 
@@ -101,6 +102,7 @@  static int test_power_get_battery_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_CAPACITY:
 	case POWER_SUPPLY_PROP_CHARGE_NOW:
+	case POWER_SUPPLY_PROP_CHARGE_COUNTER:
 		val->intval = battery_capacity;
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
@@ -117,6 +119,10 @@  static int test_power_get_battery_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
 		val->intval = battery_voltage;
 		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		val->intval = battery_current;
+		break;
 	default:
 		pr_info("%s: some properties deliberately report errors.\n",
 			__func__);
@@ -138,6 +144,7 @@  static enum power_supply_property test_power_battery_props[] = {
 	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
 	POWER_SUPPLY_PROP_CHARGE_FULL,
 	POWER_SUPPLY_PROP_CHARGE_NOW,
+	POWER_SUPPLY_PROP_CHARGE_COUNTER,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
 	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
@@ -147,6 +154,8 @@  static enum power_supply_property test_power_battery_props[] = {
 	POWER_SUPPLY_PROP_SERIAL_NUMBER,
 	POWER_SUPPLY_PROP_TEMP,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
 };
 
 static char *test_power_ac_supplied_to[] = {
@@ -450,6 +459,21 @@  static int param_set_battery_voltage(const char *key,
 
 #define param_get_battery_voltage param_get_int
 
+static int param_set_battery_current(const char *key,
+					const struct kernel_param *kp)
+{
+	int tmp;
+
+	if (1 != sscanf(key, "%d", &tmp))
+		return -EINVAL;
+
+	battery_current = tmp;
+	signal_power_supply_changed(test_power_supplies[TEST_BATTERY]);
+	return 0;
+}
+
+#define param_get_battery_current param_get_int
+
 static const struct kernel_param_ops param_ops_ac_online = {
 	.set = param_set_ac_online,
 	.get = param_get_ac_online,
@@ -490,6 +514,11 @@  static const struct kernel_param_ops param_ops_battery_voltage = {
 	.get = param_get_battery_voltage,
 };
 
+static const struct kernel_param_ops param_ops_battery_current = {
+	.set = param_set_battery_current,
+	.get = param_get_battery_current,
+};
+
 #define param_check_ac_online(name, p) __param_check(name, p, void);
 #define param_check_usb_online(name, p) __param_check(name, p, void);
 #define param_check_battery_status(name, p) __param_check(name, p, void);
@@ -498,6 +527,7 @@  static const struct kernel_param_ops param_ops_battery_voltage = {
 #define param_check_battery_health(name, p) __param_check(name, p, void);
 #define param_check_battery_capacity(name, p) __param_check(name, p, void);
 #define param_check_battery_voltage(name, p) __param_check(name, p, void);
+#define param_check_battery_current(name, p) __param_check(name, p, void);
 
 
 module_param(ac_online, ac_online, 0644);
@@ -528,6 +558,9 @@  MODULE_PARM_DESC(battery_capacity, "battery capacity (percentage)");
 module_param(battery_voltage, battery_voltage, 0644);
 MODULE_PARM_DESC(battery_voltage, "battery voltage (millivolts)");
 
+module_param(battery_current, battery_current, 0644);
+MODULE_PARM_DESC(battery_current, "battery current (milliampere)");
+
 MODULE_DESCRIPTION("Power supply driver for testing");
 MODULE_AUTHOR("Anton Vorontsov <cbouatmailru@gmail.com>");
 MODULE_LICENSE("GPL");