[PATCH 1/4] ACPI: Battery: Add bit flags

[PATCH 1/4] ACPI: Battery: Add bit flags

[Alexey Starikovskiy]

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 3f4602b..7adc1fa 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -88,10 +88,13 @@ static const struct acpi_device_id battery_device_ids[] = {
 
 MODULE_DEVICE_TABLE(acpi, battery_device_ids);
 
-/* For buggy DSDTs that report negative 16-bit values for either charging
- * or discharging current and/or report 0 as 65536 due to bad math.
- */
-#define QUIRK_SIGNED16_CURRENT 0x0001
+enum {
+	ACPI_BATTERY_ALARM_PRESENT,
+	/* For buggy DSDTs that report negative 16-bit values for either charging
+	* or discharging current and/or report 0 as 65536 due to bad math.
+	*/
+	ACPI_BATTERY_QUIRK_SIGNED16_CURRENT,
+};
 
 struct acpi_battery {
 	struct mutex lock;
@@ -118,8 +121,7 @@ struct acpi_battery {
 	char oem_info[32];
 	int state;
 	int power_unit;
-	u8 alarm_present;
-	long quirks;
+	unsigned long flags;
 };
 
 #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
@@ -399,7 +401,7 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
 	battery->update_time = jiffies;
 	kfree(buffer.pointer);
 
-	if ((battery->quirks & QUIRK_SIGNED16_CURRENT) &&
+	if (test_bit(ACPI_BATTERY_QUIRK_SIGNED16_CURRENT, &battery->flags) &&
 	    battery->rate_now != -1)
 		battery->rate_now = abs((s16)battery->rate_now);
 
@@ -412,7 +414,8 @@ static int acpi_battery_set_alarm(struct acpi_battery *battery)
 	union acpi_object arg0 = { .type = ACPI_TYPE_INTEGER };
 	struct acpi_object_list arg_list = { 1, &arg0 };
 
-	if (!acpi_battery_present(battery)|| !battery->alarm_present)
+	if (!acpi_battery_present(battery)||
+	    !test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags))
 		return -ENODEV;
 
 	arg0.integer.value = battery->alarm;
@@ -437,10 +440,10 @@ static int acpi_battery_init_alarm(struct acpi_battery *battery)
 	/* See if alarms are supported, and if so, set default */
 	status = acpi_get_handle(battery->device->handle, "_BTP", &handle);
 	if (ACPI_FAILURE(status)) {
-		battery->alarm_present = 0;
+		clear_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags);
 		return 0;
 	}
-	battery->alarm_present = 1;
+	set_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags);
 	if (!battery->alarm)
 		battery->alarm = battery->design_capacity_warning;
 	return acpi_battery_set_alarm(battery);
@@ -510,9 +513,8 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
 
 static void acpi_battery_quirks(struct acpi_battery *battery)
 {
-	battery->quirks = 0;
 	if (dmi_name_in_vendors("Acer") && battery->power_unit) {
-		battery->quirks |= QUIRK_SIGNED16_CURRENT;
+		set_bit(ACPI_BATTERY_QUIRK_SIGNED16_CURRENT, &battery->flags);
 	}
 }
 

[PATCH 2/4] POWER: Add support for cycle_count

[Alexey Starikovskiy]

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/power/power_supply_sysfs.c |    1 +
 include/linux/power_supply.h       |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)


diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index 0814439..2a52345 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -96,6 +96,7 @@ static struct device_attribute power_supply_attrs[] = {
 	POWER_SUPPLY_ATTR(present),
 	POWER_SUPPLY_ATTR(online),
 	POWER_SUPPLY_ATTR(technology),
+	POWER_SUPPLY_ATTR(cycle_count),
 	POWER_SUPPLY_ATTR(voltage_max),
 	POWER_SUPPLY_ATTR(voltage_min),
 	POWER_SUPPLY_ATTR(voltage_max_design),
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index b5d096d..ebd2b8f 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -82,6 +82,7 @@ enum power_supply_property {
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_ONLINE,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN,
 	POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,

Re: [PATCH 2/4] POWER: Add support for cycle_count

[Len Brown]

applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 3/4] ACPI: SBS: Export cycle_count

[Alexey Starikovskiy]

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/sbs.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 52b9db8..38412ec 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -217,6 +217,9 @@ static int acpi_sbs_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
 		val->intval = acpi_battery_technology(battery);
 		break;
+	case POWER_SUPPLY_PROP_CYCLE_COUNT:
+		val->intval = battery->cycle_count;
+		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		val->intval = battery->design_voltage *
 			acpi_battery_vscale(battery) * 1000;
@@ -276,6 +279,7 @@ static enum power_supply_property sbs_charge_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
@@ -560,6 +564,7 @@ static int acpi_battery_read_info(struct seq_file *seq, void *offset)
 		   battery->design_voltage * acpi_battery_vscale(battery));
 	seq_printf(seq, "design capacity warning: unknown\n");
 	seq_printf(seq, "design capacity low:     unknown\n");
+	seq_printf(seq, "cycle count:		  %i\n", battery->cycle_count);
 	seq_printf(seq, "capacity granularity 1:  unknown\n");
 	seq_printf(seq, "capacity granularity 2:  unknown\n");
 	seq_printf(seq, "model number:            %s\n", battery->device_name);

Re: [PATCH 3/4] ACPI: SBS: Export cycle_count

[Len Brown]

applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Alexey Starikovskiy]

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |   59 +++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 53 insertions(+), 6 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 7adc1fa..4ac8f2d 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -54,6 +54,7 @@
 #define ACPI_BATTERY_DEVICE_NAME	"Battery"
 #define ACPI_BATTERY_NOTIFY_STATUS	0x80
 #define ACPI_BATTERY_NOTIFY_INFO	0x81
+#define ACPI_BATTERY_NOTIFY_THRESHOLD   0x82
 
 #define _COMPONENT		ACPI_BATTERY_COMPONENT
 
@@ -90,6 +91,7 @@ MODULE_DEVICE_TABLE(acpi, battery_device_ids);
 
 enum {
 	ACPI_BATTERY_ALARM_PRESENT,
+	ACPI_BATTERY_XINFO_PRESENT,
 	/* For buggy DSDTs that report negative 16-bit values for either charging
 	* or discharging current and/or report 0 as 65536 due to bad math.
 	*/
@@ -112,6 +114,12 @@ struct acpi_battery {
 	int design_voltage;
 	int design_capacity_warning;
 	int design_capacity_low;
+	int cycle_count;
+	int measurement_accuracy;
+	int max_sampling_time;
+	int min_sampling_time;
+	int max_averaging_interval;
+	int min_averaging_interval;
 	int capacity_granularity_1;
 	int capacity_granularity_2;
 	int alarm;
@@ -200,6 +208,9 @@ static int acpi_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
 		val->intval = acpi_battery_technology(battery);
 		break;
+	case POWER_SUPPLY_PROP_CYCLE_COUNT:
+		val->intval = battery->cycle_count;
+		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		val->intval = battery->design_voltage * 1000;
 		break;
@@ -241,6 +252,7 @@ static enum power_supply_property charge_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
@@ -256,6 +268,7 @@ static enum power_supply_property energy_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
@@ -307,6 +320,28 @@ static struct acpi_offsets info_offsets[] = {
 	{offsetof(struct acpi_battery, oem_info), 1},
 };
 
+static struct acpi_offsets extended_info_offsets[] = {
+	{offsetof(struct acpi_battery, power_unit), 0},
+	{offsetof(struct acpi_battery, design_capacity), 0},
+	{offsetof(struct acpi_battery, full_charge_capacity), 0},
+	{offsetof(struct acpi_battery, technology), 0},
+	{offsetof(struct acpi_battery, design_voltage), 0},
+	{offsetof(struct acpi_battery, design_capacity_warning), 0},
+	{offsetof(struct acpi_battery, design_capacity_low), 0},
+	{offsetof(struct acpi_battery, cycle_count), 0},
+	{offsetof(struct acpi_battery, measurement_accuracy), 0},
+	{offsetof(struct acpi_battery, max_sampling_time), 0},
+	{offsetof(struct acpi_battery, min_sampling_time), 0},
+	{offsetof(struct acpi_battery, max_averaging_interval), 0},
+	{offsetof(struct acpi_battery, min_averaging_interval), 0},
+	{offsetof(struct acpi_battery, capacity_granularity_1), 0},
+	{offsetof(struct acpi_battery, capacity_granularity_2), 0},
+	{offsetof(struct acpi_battery, model_number), 1},
+	{offsetof(struct acpi_battery, serial_number), 1},
+	{offsetof(struct acpi_battery, type), 1},
+	{offsetof(struct acpi_battery, oem_info), 1},
+};
+
 static int extract_package(struct acpi_battery *battery,
 			   union acpi_object *package,
 			   struct acpi_offsets *offsets, int num)
@@ -352,22 +387,29 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
 {
 	int result = -EFAULT;
 	acpi_status status = 0;
+	char *name = test_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags)?
+			"_BIX" : "_BIF";
+
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 
 	if (!acpi_battery_present(battery))
 		return 0;
 	mutex_lock(&battery->lock);
-	status = acpi_evaluate_object(battery->device->handle, "_BIF",
-				      NULL, &buffer);
+	status = acpi_evaluate_object(battery->device->handle, name,
+						NULL, &buffer);
 	mutex_unlock(&battery->lock);
 
 	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BIF"));
+		ACPI_EXCEPTION((AE_INFO, status, "Evaluating %s", name));
 		return -ENODEV;
 	}
-
-	result = extract_package(battery, buffer.pointer,
-				 info_offsets, ARRAY_SIZE(info_offsets));
+	if (test_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags))
+		result = extract_package(battery, buffer.pointer,
+				extended_info_offsets,
+				ARRAY_SIZE(extended_info_offsets));
+	else
+		result = extract_package(battery, buffer.pointer,
+				info_offsets, ARRAY_SIZE(info_offsets));
 	kfree(buffer.pointer);
 	return result;
 }
@@ -592,6 +634,7 @@ static int acpi_battery_print_info(struct seq_file *seq, int result)
 	seq_printf(seq, "design capacity low:     %d %sh\n",
 		   battery->design_capacity_low,
 		   acpi_battery_units(battery));
+	seq_printf(seq, "cycle count:		  %i\n", battery->cycle_count);
 	seq_printf(seq, "capacity granularity 1:  %d %sh\n",
 		   battery->capacity_granularity_1,
 		   acpi_battery_units(battery));
@@ -843,6 +886,7 @@ static int acpi_battery_add(struct acpi_device *device)
 {
 	int result = 0;
 	struct acpi_battery *battery = NULL;
+	acpi_handle handle;
 	if (!device)
 		return -EINVAL;
 	battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
@@ -853,6 +897,9 @@ static int acpi_battery_add(struct acpi_device *device)
 	strcpy(acpi_device_class(device), ACPI_BATTERY_CLASS);
 	device->driver_data = battery;
 	mutex_init(&battery->lock);
+	if (ACPI_SUCCESS(acpi_get_handle(battery->device->handle,
+			"_BIX", &handle)))
+		set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
 	acpi_battery_update(battery);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	result = acpi_battery_add_fs(device);

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Alan Jenkins]

On 10/15/09, Alexey Starikovskiy <astarikovskiy@suse.de> wrote:
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
>
>  drivers/acpi/battery.c |   59
> +++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 53 insertions(+), 6 deletions(-)
>
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 7adc1fa..4ac8f2d 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -54,6 +54,7 @@
>  #define ACPI_BATTERY_DEVICE_NAME	"Battery"
>  #define ACPI_BATTERY_NOTIFY_STATUS	0x80
>  #define ACPI_BATTERY_NOTIFY_INFO	0x81
> +#define ACPI_BATTERY_NOTIFY_THRESHOLD   0x82
>
>  #define _COMPONENT		ACPI_BATTERY_COMPONENT
>
> @@ -90,6 +91,7 @@ MODULE_DEVICE_TABLE(acpi, battery_device_ids);
>
>  enum {
>  	ACPI_BATTERY_ALARM_PRESENT,
> +	ACPI_BATTERY_XINFO_PRESENT,
>  	/* For buggy DSDTs that report negative 16-bit values for either charging
>  	* or discharging current and/or report 0 as 65536 due to bad math.
>  	*/
> @@ -112,6 +114,12 @@ struct acpi_battery {
>  	int design_voltage;
>  	int design_capacity_warning;
>  	int design_capacity_low;
> +	int cycle_count;
> +	int measurement_accuracy;
> +	int max_sampling_time;
> +	int min_sampling_time;
> +	int max_averaging_interval;
> +	int min_averaging_interval;
>  	int capacity_granularity_1;
>  	int capacity_granularity_2;
>  	int alarm;
> @@ -200,6 +208,9 @@ static int acpi_battery_get_property(struct power_supply
> *psy,
>  	case POWER_SUPPLY_PROP_TECHNOLOGY:
>  		val->intval = acpi_battery_technology(battery);
>  		break;
> +	case POWER_SUPPLY_PROP_CYCLE_COUNT:
> +		val->intval = battery->cycle_count;
> +		break;

So userspace is supposed to realise that "0" means "this attribute is
not supported", as opposed to "I am a brand new battery and don't
remember being factory-tested".

How about returning -1 for batteries without _BIX?

<reads spec>.  Particularly since Cycle Count in _BIX is spec'ed to
return 0xFFFFFFFF for unknown, which we will return as -1, no?

Regards
Alan

[PATCH] ACPI: Battery: Add support for _BIX extended info method

[Alexey Starikovskiy]

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |   61 +++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 55 insertions(+), 6 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 7adc1fa..4d272db 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -54,6 +54,7 @@
 #define ACPI_BATTERY_DEVICE_NAME	"Battery"
 #define ACPI_BATTERY_NOTIFY_STATUS	0x80
 #define ACPI_BATTERY_NOTIFY_INFO	0x81
+#define ACPI_BATTERY_NOTIFY_THRESHOLD   0x82
 
 #define _COMPONENT		ACPI_BATTERY_COMPONENT
 
@@ -90,6 +91,7 @@ MODULE_DEVICE_TABLE(acpi, battery_device_ids);
 
 enum {
 	ACPI_BATTERY_ALARM_PRESENT,
+	ACPI_BATTERY_XINFO_PRESENT,
 	/* For buggy DSDTs that report negative 16-bit values for either charging
 	* or discharging current and/or report 0 as 65536 due to bad math.
 	*/
@@ -112,6 +114,12 @@ struct acpi_battery {
 	int design_voltage;
 	int design_capacity_warning;
 	int design_capacity_low;
+	int cycle_count;
+	int measurement_accuracy;
+	int max_sampling_time;
+	int min_sampling_time;
+	int max_averaging_interval;
+	int min_averaging_interval;
 	int capacity_granularity_1;
 	int capacity_granularity_2;
 	int alarm;
@@ -200,6 +208,9 @@ static int acpi_battery_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_TECHNOLOGY:
 		val->intval = acpi_battery_technology(battery);
 		break;
+	case POWER_SUPPLY_PROP_CYCLE_COUNT:
+		val->intval = battery->cycle_count;
+		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
 		val->intval = battery->design_voltage * 1000;
 		break;
@@ -241,6 +252,7 @@ static enum power_supply_property charge_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
@@ -256,6 +268,7 @@ static enum power_supply_property energy_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_CYCLE_COUNT,
 	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CURRENT_NOW,
@@ -307,6 +320,28 @@ static struct acpi_offsets info_offsets[] = {
 	{offsetof(struct acpi_battery, oem_info), 1},
 };
 
+static struct acpi_offsets extended_info_offsets[] = {
+	{offsetof(struct acpi_battery, power_unit), 0},
+	{offsetof(struct acpi_battery, design_capacity), 0},
+	{offsetof(struct acpi_battery, full_charge_capacity), 0},
+	{offsetof(struct acpi_battery, technology), 0},
+	{offsetof(struct acpi_battery, design_voltage), 0},
+	{offsetof(struct acpi_battery, design_capacity_warning), 0},
+	{offsetof(struct acpi_battery, design_capacity_low), 0},
+	{offsetof(struct acpi_battery, cycle_count), 0},
+	{offsetof(struct acpi_battery, measurement_accuracy), 0},
+	{offsetof(struct acpi_battery, max_sampling_time), 0},
+	{offsetof(struct acpi_battery, min_sampling_time), 0},
+	{offsetof(struct acpi_battery, max_averaging_interval), 0},
+	{offsetof(struct acpi_battery, min_averaging_interval), 0},
+	{offsetof(struct acpi_battery, capacity_granularity_1), 0},
+	{offsetof(struct acpi_battery, capacity_granularity_2), 0},
+	{offsetof(struct acpi_battery, model_number), 1},
+	{offsetof(struct acpi_battery, serial_number), 1},
+	{offsetof(struct acpi_battery, type), 1},
+	{offsetof(struct acpi_battery, oem_info), 1},
+};
+
 static int extract_package(struct acpi_battery *battery,
 			   union acpi_object *package,
 			   struct acpi_offsets *offsets, int num)
@@ -352,22 +387,31 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
 {
 	int result = -EFAULT;
 	acpi_status status = 0;
+	char *name = test_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags)?
+			"_BIX" : "_BIF";
+
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 
 	if (!acpi_battery_present(battery))
 		return 0;
 	mutex_lock(&battery->lock);
-	status = acpi_evaluate_object(battery->device->handle, "_BIF",
-				      NULL, &buffer);
+	status = acpi_evaluate_object(battery->device->handle, name,
+						NULL, &buffer);
 	mutex_unlock(&battery->lock);
 
 	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BIF"));
+		ACPI_EXCEPTION((AE_INFO, status, "Evaluating %s", name));
 		return -ENODEV;
 	}
-
-	result = extract_package(battery, buffer.pointer,
-				 info_offsets, ARRAY_SIZE(info_offsets));
+	if (test_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags))
+		result = extract_package(battery, buffer.pointer,
+				extended_info_offsets,
+				ARRAY_SIZE(extended_info_offsets));
+	else {
+		battery->cycle_count = -1;
+		result = extract_package(battery, buffer.pointer,
+				info_offsets, ARRAY_SIZE(info_offsets));
+	}
 	kfree(buffer.pointer);
 	return result;
 }
@@ -592,6 +636,7 @@ static int acpi_battery_print_info(struct seq_file *seq, int result)
 	seq_printf(seq, "design capacity low:     %d %sh\n",
 		   battery->design_capacity_low,
 		   acpi_battery_units(battery));
+	seq_printf(seq, "cycle count:		  %i\n", battery->cycle_count);
 	seq_printf(seq, "capacity granularity 1:  %d %sh\n",
 		   battery->capacity_granularity_1,
 		   acpi_battery_units(battery));
@@ -843,6 +888,7 @@ static int acpi_battery_add(struct acpi_device *device)
 {
 	int result = 0;
 	struct acpi_battery *battery = NULL;
+	acpi_handle handle;
 	if (!device)
 		return -EINVAL;
 	battery = kzalloc(sizeof(struct acpi_battery), GFP_KERNEL);
@@ -853,6 +899,9 @@ static int acpi_battery_add(struct acpi_device *device)
 	strcpy(acpi_device_class(device), ACPI_BATTERY_CLASS);
 	device->driver_data = battery;
 	mutex_init(&battery->lock);
+	if (ACPI_SUCCESS(acpi_get_handle(battery->device->handle,
+			"_BIX", &handle)))
+		set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
 	acpi_battery_update(battery);
 #ifdef CONFIG_ACPI_PROCFS_POWER
 	result = acpi_battery_add_fs(device);

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Henrique de Moraes Holschuh]

On Fri, 16 Oct 2009, Alan Jenkins wrote:
> So userspace is supposed to realise that "0" means "this attribute is
> not supported", as opposed to "I am a brand new battery and don't
> remember being factory-tested".
> 
> How about returning -1 for batteries without _BIX?

How about NOT registering attributes that don't exist?  That's the proper
way of doing things in sysfs.

When that's not possible, I'd suggest doing the right thing and returning an
error of some sort (ENXIO, ENOTSUP, etc)...

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Alexey Starikovskiy]

Henrique de Moraes Holschuh пишет:
> On Fri, 16 Oct 2009, Alan Jenkins wrote:
>   
>> So userspace is supposed to realise that "0" means "this attribute is
>> not supported", as opposed to "I am a brand new battery and don't
>> remember being factory-tested".
>>
>> How about returning -1 for batteries without _BIX?
>>     
>
> How about NOT registering attributes that don't exist?  That's the proper
> way of doing things in sysfs.
>
>   
That is going to be too complex (2 duplicated arrays)
> When that's not possible, I'd suggest doing the right thing and returning an
> error of some sort (ENXIO, ENOTSUP, etc)...
>
>   
None of power class members does it, and all ACPI classes are returning
-1 in not-supporting case.

Regards,
Alex.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Henrique de Moraes Holschuh]

On Mon, 15 Mar 2010, Alexey Starikovskiy wrote:
> Henrique de Moraes Holschuh ??????????:
> > On Fri, 16 Oct 2009, Alan Jenkins wrote:
> >> So userspace is supposed to realise that "0" means "this attribute is
> >> not supported", as opposed to "I am a brand new battery and don't
> >> remember being factory-tested".
> >>
> >> How about returning -1 for batteries without _BIX?
> >
> > How about NOT registering attributes that don't exist?  That's the proper
> > way of doing things in sysfs.
> >   
> That is going to be too complex (2 duplicated arrays)

It would be the Right Thing to do.  Still, you could return a proper error
as I suggested.

> > When that's not possible, I'd suggest doing the right thing and returning an
> > error of some sort (ENXIO, ENOTSUP, etc)...
> >   
> None of power class members does it, and all ACPI classes are returning
> -1 in not-supporting case.

That just means the ACPI sysfs conversion on that area was not as good as it
should have been.  It is hardly the only place where the sysfs ABI is not
well implemented, but that doesn't mean the breakage should remain, or that
it should be made worse.

What is the technical case to returning invalid values for something that is
not supported when you could return a proper error on open() instead (since
you're not going to do the Right Thing and not register that attribute it in
the first place)?  Not only the "-1" way wastes more system resources (all
clients have to do open+read+close), it also needs userspace to special case
something, and that is always a Bad Idea for *many* reasons.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Alexey Starikovskiy]

Henrique de Moraes Holschuh пишет:
> That just means the ACPI sysfs conversion on that area was not as good as it
> should have been.  It is hardly the only place where the sysfs ABI is not
> well implemented, but that doesn't mean the breakage should remain, or that
> it should be made worse.
> What is the technical case to returning invalid values for something that is
> not supported when you could return a proper error on open() instead (since
Power Class interface does not give me control over open() as far as I know.
> you're not going to do the Right Thing and not register that attribute it in
> the first place)?  Not only the "-1" way wastes more system resources (all
> clients have to do open+read+close), it also needs userspace to special case
> something, and that is always a Bad Idea for *many* reasons.
If you know how this could be done, please show the patch...

Thanks,
Alex.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Henrique de Moraes Holschuh]

On Mon, 15 Mar 2010, Alexey Starikovskiy wrote:
> Henrique de Moraes Holschuh ??????????:
> > That just means the ACPI sysfs conversion on that area was not as good as it
> > should have been.  It is hardly the only place where the sysfs ABI is not
> > well implemented, but that doesn't mean the breakage should remain, or that
> > it should be made worse.
> > What is the technical case to returning invalid values for something that is
> > not supported when you could return a proper error on open() instead (since

> Power Class interface does not give me control over open() as far as I know.

Hmm, you're correct.  We can return errors, but they'll be on
read()/write(), not open().

> > you're not going to do the Right Thing and not register that attribute it in
> > the first place)?  Not only the "-1" way wastes more system resources (all
> > clients have to do open+read+close), it also needs userspace to special case
> > something, and that is always a Bad Idea for *many* reasons.
> If you know how this could be done, please show the patch...

Let me look at that code, and I will get back to you.  Might have to use the
recently introduced sysfs helpers or some somewhat ugly crap I had to add to
thinkpad-acpi to do just that, though.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 4/4] ACPI: Battery: Add support for _BIX extended info method

[Len Brown]

applied to acpi-test with checkpatch error fixed:

ERROR: spaces required around that '?' (ctx:VxE)
#106: FILE: drivers/acpi/battery.c:390:
+	char *name = test_bit(ACPI_BATTERY_XINFO_PRESENT, 
&battery->flags)?
 	                                                                  
^

total: 1 errors, 0 warnings, 135 lines checked


thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH 1/4] ACPI: Battery: Add bit flags

[Len Brown]

applied to acpi-test as below -- with the checkpatch issues fixed...

thanks,
Len Brown, Intel Open Source Technology Center

commit 7b3bcc4a1a7cd2d53b403ca29d06ceb5fa617eb7
Author: Alexey Starikovskiy <astarikovskiy@suse.de>
Date:   Thu Oct 15 14:31:24 2009 +0400

    ACPI: Battery: Add bit flags
    
    Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
    Signed-off-by: Len Brown <len.brown@intel.com>

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index cada73f..b2b48f8 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -88,10 +88,13 @@ static const struct acpi_device_id battery_device_ids[] = {
 
 MODULE_DEVICE_TABLE(acpi, battery_device_ids);
 
-/* For buggy DSDTs that report negative 16-bit values for either charging
- * or discharging current and/or report 0 as 65536 due to bad math.
- */
-#define QUIRK_SIGNED16_CURRENT 0x0001
+enum {
+	ACPI_BATTERY_ALARM_PRESENT,
+	/* For buggy DSDTs that report negative 16-bit values for either charging
+	* or discharging current and/or report 0 as 65536 due to bad math.
+	*/
+	ACPI_BATTERY_QUIRK_SIGNED16_CURRENT,
+};
 
 struct acpi_battery {
 	struct mutex lock;
@@ -118,8 +121,7 @@ struct acpi_battery {
 	char oem_info[32];
 	int state;
 	int power_unit;
-	u8 alarm_present;
-	long quirks;
+	unsigned long flags;
 };
 
 #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
@@ -399,7 +401,7 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
 	battery->update_time = jiffies;
 	kfree(buffer.pointer);
 
-	if ((battery->quirks & QUIRK_SIGNED16_CURRENT) &&
+	if (test_bit(ACPI_BATTERY_QUIRK_SIGNED16_CURRENT, &battery->flags) &&
 	    battery->rate_now != -1)
 		battery->rate_now = abs((s16)battery->rate_now);
 
@@ -412,7 +414,8 @@ static int acpi_battery_set_alarm(struct acpi_battery *battery)
 	union acpi_object arg0 = { .type = ACPI_TYPE_INTEGER };
 	struct acpi_object_list arg_list = { 1, &arg0 };
 
-	if (!acpi_battery_present(battery)|| !battery->alarm_present)
+	if (!acpi_battery_present(battery)||
+	    !test_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags))
 		return -ENODEV;
 
 	arg0.integer.value = battery->alarm;
@@ -437,10 +440,10 @@ static int acpi_battery_init_alarm(struct acpi_battery *battery)
 	/* See if alarms are supported, and if so, set default */
 	status = acpi_get_handle(battery->device->handle, "_BTP", &handle);
 	if (ACPI_FAILURE(status)) {
-		battery->alarm_present = 0;
+		clear_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags);
 		return 0;
 	}
-	battery->alarm_present = 1;
+	set_bit(ACPI_BATTERY_ALARM_PRESENT, &battery->flags);
 	if (!battery->alarm)
 		battery->alarm = battery->design_capacity_warning;
 	return acpi_battery_set_alarm(battery);
@@ -510,9 +513,8 @@ static void sysfs_remove_battery(struct acpi_battery *battery)
 
 static void acpi_battery_quirks(struct acpi_battery *battery)
 {
-	battery->quirks = 0;
 	if (dmi_name_in_vendors("Acer") && battery->power_unit) {
-		battery->quirks |= QUIRK_SIGNED16_CURRENT;
+		set_bit(ACPI_BATTERY_QUIRK_SIGNED16_CURRENT, &battery->flags);
 	}
 }