ACPI: Battery: don't use acpi_extract_package()
authorAlexey Starikovskiy <astarikovskiy@suse.de>
Wed, 26 Sep 2007 15:42:46 +0000 (19:42 +0400)
committerLen Brown <len.brown@intel.com>
Thu, 27 Sep 2007 19:50:21 +0000 (15:50 -0400)
acpi_extract_package() creates more problems with memory management than
it solves as helper for package handling.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
drivers/acpi/battery.c

index 9b2c0f74f869a6c4ce74e64462432c24ebf770b0..de506f39d3bd799d72e7a5c87934342c32dccdb1 100644 (file)
@@ -36,9 +36,6 @@
 
 #define ACPI_BATTERY_VALUE_UNKNOWN 0xFFFFFFFF
 
-#define ACPI_BATTERY_FORMAT_BIF        "NNNNNNNNNSSSS"
-#define ACPI_BATTERY_FORMAT_BST        "NNNN"
-
 #define ACPI_BATTERY_COMPONENT         0x00040000
 #define ACPI_BATTERY_CLASS             "battery"
 #define ACPI_BATTERY_DEVICE_NAME       "Battery"
@@ -90,61 +87,49 @@ static struct acpi_driver acpi_battery_driver = {
                },
 };
 
-struct acpi_battery_state {
-       acpi_integer state;
-       acpi_integer present_rate;
-       acpi_integer remaining_capacity;
-       acpi_integer present_voltage;
-};
-
-struct acpi_battery_info {
-       acpi_integer power_unit;
-       acpi_integer design_capacity;
-       acpi_integer last_full_capacity;
-       acpi_integer battery_technology;
-       acpi_integer design_voltage;
-       acpi_integer design_capacity_warning;
-       acpi_integer design_capacity_low;
-       acpi_integer battery_capacity_granularity_1;
-       acpi_integer battery_capacity_granularity_2;
-       acpi_string model_number;
-       acpi_string serial_number;
-       acpi_string battery_type;
-       acpi_string oem_info;
-};
-
-enum acpi_battery_files{
+enum acpi_battery_files {
        ACPI_BATTERY_INFO = 0,
        ACPI_BATTERY_STATE,
        ACPI_BATTERY_ALARM,
        ACPI_BATTERY_NUMFILES,
 };
 
-struct acpi_battery_flags {
-       u8 battery_present_prev;
-       u8 alarm_present;
-       u8 init_update;
-       u8 update[ACPI_BATTERY_NUMFILES];
-       u8 power_unit;
-};
-
 struct acpi_battery {
-       struct mutex mutex;
        struct acpi_device *device;
-       struct acpi_battery_flags flags;
-       struct acpi_buffer bif_data;
-       struct acpi_buffer bst_data;
+       struct mutex lock;
        unsigned long alarm;
        unsigned long update_time[ACPI_BATTERY_NUMFILES];
+       int state;
+       int present_rate;
+       int remaining_capacity;
+       int present_voltage;
+       int power_unit;
+       int design_capacity;
+       int last_full_capacity;
+       int technology;
+       int design_voltage;
+       int design_capacity_warning;
+       int design_capacity_low;
+       int capacity_granularity_1;
+       int capacity_granularity_2;
+       char model_number[32];
+       char serial_number[32];
+       char type[32];
+       char oem_info[32];
+       u8 present_prev;
+       u8 alarm_present;
+       u8 init_update;
+       u8 update[ACPI_BATTERY_NUMFILES];
 };
 
 inline int acpi_battery_present(struct acpi_battery *battery)
 {
        return battery->device->status.battery_present;
 }
+
 inline char *acpi_battery_power_units(struct acpi_battery *battery)
 {
-       if (battery->flags.power_unit)
+       if (battery->power_unit)
                return ACPI_BATTERY_UNITS_AMPS;
        else
                return ACPI_BATTERY_UNITS_WATTS;
@@ -165,43 +150,63 @@ static void acpi_battery_check_result(struct acpi_battery *battery, int result)
                return;
 
        if (result) {
-               battery->flags.init_update = 1;
+               battery->init_update = 1;
        }
 }
 
-static int acpi_battery_extract_package(struct acpi_battery *battery,
-                                       union acpi_object *package,
-                                       struct acpi_buffer *format,
-                                       struct acpi_buffer *data,
-                                       char *package_name)
-{
-       acpi_status status = AE_OK;
-       struct acpi_buffer data_null = { 0, NULL };
+struct acpi_offsets {
+       size_t offset;          /* offset inside struct acpi_sbs_battery */
+       u8 mode;                /* int or string? */
+};
 
-       status = acpi_extract_package(package, format, &data_null);
-       if (status != AE_BUFFER_OVERFLOW) {
-               ACPI_EXCEPTION((AE_INFO, status, "Extracting size %s",
-                               package_name));
-               return -ENODEV;
-       }
+static struct acpi_offsets state_offsets[] = {
+       {offsetof(struct acpi_battery, state), 0},
+       {offsetof(struct acpi_battery, present_rate), 0},
+       {offsetof(struct acpi_battery, remaining_capacity), 0},
+       {offsetof(struct acpi_battery, present_voltage), 0},
+};
 
-       if (data_null.length != data->length) {
-               kfree(data->pointer);
-               data->pointer = kzalloc(data_null.length, GFP_KERNEL);
-               if (!data->pointer) {
-                       ACPI_EXCEPTION((AE_INFO, AE_NO_MEMORY, "kzalloc()"));
-                       return -ENOMEM;
-               }
-               data->length = data_null.length;
-       }
+static struct acpi_offsets info_offsets[] = {
+       {offsetof(struct acpi_battery, power_unit), 0},
+       {offsetof(struct acpi_battery, design_capacity), 0},
+       {offsetof(struct acpi_battery, last_full_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, 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},
+};
 
-       status = acpi_extract_package(package, format, data);
-       if (ACPI_FAILURE(status)) {
-               ACPI_EXCEPTION((AE_INFO, status, "Extracting %s",
-                               package_name));
-               return -ENODEV;
+static int extract_package(struct acpi_battery *battery,
+                          union acpi_object *package,
+                          struct acpi_offsets *offsets, int num)
+{
+       int i, *x;
+       union acpi_object *element;
+       if (package->type != ACPI_TYPE_PACKAGE)
+               return -EFAULT;
+       for (i = 0; i < num; ++i) {
+               if (package->package.count <= i)
+                       return -EFAULT;
+               element = &package->package.elements[i];
+               if (offsets[i].mode) {
+                       if (element->type != ACPI_TYPE_STRING &&
+                           element->type != ACPI_TYPE_BUFFER)
+                               return -EFAULT;
+                       strncpy((u8 *)battery + offsets[i].offset,
+                               element->string.pointer, 32);
+               } else {
+                       if (element->type != ACPI_TYPE_INTEGER)
+                               return -EFAULT;
+                       x = (int *)((u8 *)battery + offsets[i].offset);
+                       *x = element->integer.value;
+               }
        }
-
        return 0;
 }
 
@@ -222,49 +227,21 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
        int result = 0;
        acpi_status status = 0;
        struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-       struct acpi_buffer format = { sizeof(ACPI_BATTERY_FORMAT_BIF),
-               ACPI_BATTERY_FORMAT_BIF
-       };
-       union acpi_object *package = NULL;
-       struct acpi_buffer *data = NULL;
-       struct acpi_battery_info *bif = NULL;
 
        battery->update_time[ACPI_BATTERY_INFO] = get_seconds();
-
        if (!acpi_battery_present(battery))
                return 0;
-
-       /* Evaluate _BIF */
-
-       status =
-           acpi_evaluate_object(acpi_battery_handle(battery), "_BIF", NULL,
-                                &buffer);
+       mutex_lock(&battery->lock);
+       status = acpi_evaluate_object(acpi_battery_handle(battery), "_BIF",
+                                     NULL, &buffer);
+       mutex_unlock(&battery->lock);
        if (ACPI_FAILURE(status)) {
                ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BIF"));
                return -ENODEV;
        }
-
-       package = buffer.pointer;
-
-       data = &battery->bif_data;
-
-       /* Extract Package Data */
-
-       result =
-           acpi_battery_extract_package(battery, package, &format, data,
-                                        "_BIF");
-       if (result)
-               goto end;
-
-      end:
-
+       result = extract_package(battery, buffer.pointer,
+                                info_offsets, ARRAY_SIZE(info_offsets));
        kfree(buffer.pointer);
-
-       if (!result) {
-               bif = data->pointer;
-               battery->flags.power_unit = bif->power_unit;
-       }
-
        return result;
 }
 
@@ -273,42 +250,24 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
        int result = 0;
        acpi_status status = 0;
        struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-       struct acpi_buffer format = { sizeof(ACPI_BATTERY_FORMAT_BST),
-               ACPI_BATTERY_FORMAT_BST
-       };
-       union acpi_object *package = NULL;
-       struct acpi_buffer *data = NULL;
 
        battery->update_time[ACPI_BATTERY_STATE] = get_seconds();
 
        if (!acpi_battery_present(battery))
                return 0;
 
-       /* Evaluate _BST */
+       mutex_lock(&battery->lock);
+       status = acpi_evaluate_object(acpi_battery_handle(battery), "_BST",
+                                     NULL, &buffer);
+       mutex_unlock(&battery->lock);
 
-       status =
-           acpi_evaluate_object(acpi_battery_handle(battery), "_BST", NULL,
-                                &buffer);
        if (ACPI_FAILURE(status)) {
                ACPI_EXCEPTION((AE_INFO, status, "Evaluating _BST"));
                return -ENODEV;
        }
-
-       package = buffer.pointer;
-
-       data = &battery->bst_data;
-
-       /* Extract Package Data */
-
-       result =
-           acpi_battery_extract_package(battery, package, &format, data,
-                                        "_BST");
-       if (result)
-               goto end;
-
-      end:
+       result = extract_package(battery, buffer.pointer,
+                                state_offsets, ARRAY_SIZE(state_offsets));
        kfree(buffer.pointer);
-
        return result;
 }
 
@@ -331,14 +290,15 @@ static int acpi_battery_set_alarm(struct acpi_battery *battery,
        if (!acpi_battery_present(battery))
                return -ENODEV;
 
-       if (!battery->flags.alarm_present)
+       if (!battery->alarm_present)
                return -ENODEV;
 
        arg0.integer.value = alarm;
 
-       status =
-           acpi_evaluate_object(acpi_battery_handle(battery), "_BTP",
-                                &arg_list, NULL);
+       mutex_lock(&battery->lock);
+       status = acpi_evaluate_object(acpi_battery_handle(battery), "_BTP",
+                                     &arg_list, NULL);
+       mutex_unlock(&battery->lock);
        if (ACPI_FAILURE(status))
                return -ENODEV;
 
@@ -354,22 +314,21 @@ static int acpi_battery_init_alarm(struct acpi_battery *battery)
        int result = 0;
        acpi_status status = AE_OK;
        acpi_handle handle = NULL;
-       struct acpi_battery_info *bif = battery->bif_data.pointer;
        unsigned long alarm = battery->alarm;
 
        /* See if alarms are supported, and if so, set default */
 
        status = acpi_get_handle(acpi_battery_handle(battery), "_BTP", &handle);
        if (ACPI_SUCCESS(status)) {
-               battery->flags.alarm_present = 1;
-               if (!alarm && bif) {
-                       alarm = bif->design_capacity_warning;
+               battery->alarm_present = 1;
+               if (!alarm) {
+                       alarm = battery->design_capacity_warning;
                }
                result = acpi_battery_set_alarm(battery, alarm);
                if (result)
                        goto end;
        } else {
-               battery->flags.alarm_present = 0;
+               battery->alarm_present = 0;
        }
 
       end:
@@ -385,7 +344,7 @@ static int acpi_battery_init_update(struct acpi_battery *battery)
        if (result)
                return result;
 
-       battery->flags.battery_present_prev = acpi_battery_present(battery);
+       battery->present_prev = acpi_battery_present(battery);
 
        if (acpi_battery_present(battery)) {
                result = acpi_battery_get_info(battery);
@@ -411,7 +370,7 @@ static int acpi_battery_update(struct acpi_battery *battery,
                update = 1;
        }
 
-       if (battery->flags.init_update) {
+       if (battery->init_update) {
                result = acpi_battery_init_update(battery);
                if (result)
                        goto end;
@@ -420,8 +379,8 @@ static int acpi_battery_update(struct acpi_battery *battery,
                result = acpi_battery_get_status(battery);
                if (result)
                        goto end;
-               if ((!battery->flags.battery_present_prev & acpi_battery_present(battery))
-                   || (battery->flags.battery_present_prev & !acpi_battery_present(battery))) {
+               if ((!battery->present_prev & acpi_battery_present(battery))
+                   || (battery->present_prev & !acpi_battery_present(battery))) {
                        result = acpi_battery_init_update(battery);
                        if (result)
                                goto end;
@@ -433,7 +392,7 @@ static int acpi_battery_update(struct acpi_battery *battery,
 
       end:
 
-       battery->flags.init_update = (result != 0);
+       battery->init_update = (result != 0);
 
        *update_result_ptr = update_result;
 
@@ -444,19 +403,19 @@ static void acpi_battery_notify_update(struct acpi_battery *battery)
 {
        acpi_battery_get_status(battery);
 
-       if (battery->flags.init_update) {
+       if (battery->init_update) {
                return;
        }
 
-       if ((!battery->flags.battery_present_prev &
+       if ((!battery->present_prev &
             acpi_battery_present(battery)) ||
-           (battery->flags.battery_present_prev &
+           (battery->present_prev &
             !acpi_battery_present(battery))) {
-               battery->flags.init_update = 1;
+               battery->init_update = 1;
        } else {
-               battery->flags.update[ACPI_BATTERY_INFO] = 1;
-               battery->flags.update[ACPI_BATTERY_STATE] = 1;
-               battery->flags.update[ACPI_BATTERY_ALARM] = 1;
+               battery->update[ACPI_BATTERY_INFO] = 1;
+               battery->update[ACPI_BATTERY_STATE] = 1;
+               battery->update[ACPI_BATTERY_ALARM] = 1;
        }
 }
 
@@ -469,7 +428,6 @@ static struct proc_dir_entry *acpi_battery_dir;
 static int acpi_battery_print_info(struct seq_file *seq, int result)
 {
        struct acpi_battery *battery = seq->private;
-       struct acpi_battery_info *bif = NULL;
        char *units = "?";
 
        if (result)
@@ -482,30 +440,23 @@ static int acpi_battery_print_info(struct seq_file *seq, int result)
                goto end;
        }
 
-       bif = battery->bif_data.pointer;
-       if (!bif) {
-               ACPI_EXCEPTION((AE_INFO, AE_ERROR, "BIF buffer is NULL"));
-               result = -ENODEV;
-               goto end;
-       }
-
        /* Battery Units */
 
        units = acpi_battery_power_units(battery);
 
-       if (bif->design_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->design_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "design capacity:         unknown\n");
        else
                seq_printf(seq, "design capacity:         %d %sh\n",
-                          (u32) bif->design_capacity, units);
+                          (u32) battery->design_capacity, units);
 
-       if (bif->last_full_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->last_full_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "last full capacity:      unknown\n");
        else
                seq_printf(seq, "last full capacity:      %d %sh\n",
-                          (u32) bif->last_full_capacity, units);
+                          (u32) battery->last_full_capacity, units);
 
-       switch ((u32) bif->battery_technology) {
+       switch ((u32) battery->technology) {
        case 0:
                seq_printf(seq, "battery technology:      non-rechargeable\n");
                break;
@@ -517,23 +468,23 @@ static int acpi_battery_print_info(struct seq_file *seq, int result)
                break;
        }
 
-       if (bif->design_voltage == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->design_voltage == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "design voltage:          unknown\n");
        else
                seq_printf(seq, "design voltage:          %d mV\n",
-                          (u32) bif->design_voltage);
+                          (u32) battery->design_voltage);
        seq_printf(seq, "design capacity warning: %d %sh\n",
-                  (u32) bif->design_capacity_warning, units);
+                  (u32) battery->design_capacity_warning, units);
        seq_printf(seq, "design capacity low:     %d %sh\n",
-                  (u32) bif->design_capacity_low, units);
+                  (u32) battery->design_capacity_low, units);
        seq_printf(seq, "capacity granularity 1:  %d %sh\n",
-                  (u32) bif->battery_capacity_granularity_1, units);
+                  (u32) battery->capacity_granularity_1, units);
        seq_printf(seq, "capacity granularity 2:  %d %sh\n",
-                  (u32) bif->battery_capacity_granularity_2, units);
-       seq_printf(seq, "model number:            %s\n", bif->model_number);
-       seq_printf(seq, "serial number:           %s\n", bif->serial_number);
-       seq_printf(seq, "battery type:            %s\n", bif->battery_type);
-       seq_printf(seq, "OEM info:                %s\n", bif->oem_info);
+                  (u32) battery->capacity_granularity_2, units);
+       seq_printf(seq, "model number:            %s\n", battery->model_number);
+       seq_printf(seq, "serial number:           %s\n", battery->serial_number);
+       seq_printf(seq, "battery type:            %s\n", battery->type);
+       seq_printf(seq, "OEM info:                %s\n", battery->oem_info);
 
       end:
 
@@ -546,7 +497,6 @@ static int acpi_battery_print_info(struct seq_file *seq, int result)
 static int acpi_battery_print_state(struct seq_file *seq, int result)
 {
        struct acpi_battery *battery = seq->private;
-       struct acpi_battery_state *bst = NULL;
        char *units = "?";
 
        if (result)
@@ -559,50 +509,43 @@ static int acpi_battery_print_state(struct seq_file *seq, int result)
                goto end;
        }
 
-       bst = battery->bst_data.pointer;
-       if (!bst) {
-               ACPI_EXCEPTION((AE_INFO, AE_ERROR, "BST buffer is NULL"));
-               result = -ENODEV;
-               goto end;
-       }
-
        /* Battery Units */
 
        units = acpi_battery_power_units(battery);
 
-       if (!(bst->state & 0x04))
+       if (!(battery->state & 0x04))
                seq_printf(seq, "capacity state:          ok\n");
        else
                seq_printf(seq, "capacity state:          critical\n");
 
-       if ((bst->state & 0x01) && (bst->state & 0x02)) {
+       if ((battery->state & 0x01) && (battery->state & 0x02)) {
                seq_printf(seq,
                           "charging state:          charging/discharging\n");
-       } else if (bst->state & 0x01)
+       } else if (battery->state & 0x01)
                seq_printf(seq, "charging state:          discharging\n");
-       else if (bst->state & 0x02)
+       else if (battery->state & 0x02)
                seq_printf(seq, "charging state:          charging\n");
        else {
                seq_printf(seq, "charging state:          charged\n");
        }
 
-       if (bst->present_rate == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->present_rate == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "present rate:            unknown\n");
        else
                seq_printf(seq, "present rate:            %d %s\n",
-                          (u32) bst->present_rate, units);
+                          (u32) battery->present_rate, units);
 
-       if (bst->remaining_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->remaining_capacity == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "remaining capacity:      unknown\n");
        else
                seq_printf(seq, "remaining capacity:      %d %sh\n",
-                          (u32) bst->remaining_capacity, units);
+                          (u32) battery->remaining_capacity, units);
 
-       if (bst->present_voltage == ACPI_BATTERY_VALUE_UNKNOWN)
+       if (battery->present_voltage == ACPI_BATTERY_VALUE_UNKNOWN)
                seq_printf(seq, "present voltage:         unknown\n");
        else
                seq_printf(seq, "present voltage:         %d mV\n",
-                          (u32) bst->present_voltage);
+                          (u32) battery->present_voltage);
 
       end:
 
@@ -658,8 +601,6 @@ acpi_battery_write_alarm(struct file *file,
        if (!battery || (count > sizeof(alarm_string) - 1))
                return -EINVAL;
 
-       mutex_lock(&battery->mutex);
-
        result = acpi_battery_update(battery, 1, &update_result);
        if (result) {
                result = -ENODEV;
@@ -689,9 +630,6 @@ acpi_battery_write_alarm(struct file *file,
 
        if (!result)
                result = count;
-
-       mutex_unlock(&battery->mutex);
-
        return result;
 }
 
@@ -714,10 +652,8 @@ static int acpi_battery_read(int fid, struct seq_file *seq)
        int update_result = ACPI_BATTERY_NONE_UPDATE;
        int update = 0;
 
-       mutex_lock(&battery->mutex);
-
        update = (get_seconds() - battery->update_time[fid] >= update_time);
-       update = (update | battery->flags.update[fid]);
+       update = (update | battery->update[fid]);
 
        result = acpi_battery_update(battery, update, &update_result);
        if (result)
@@ -732,8 +668,7 @@ static int acpi_battery_read(int fid, struct seq_file *seq)
       end:
        result = acpi_read_funcs[fid].print(seq, result);
        acpi_battery_check_result(battery, result);
-       battery->flags.update[fid] = result;
-       mutex_unlock(&battery->mutex);
+       battery->update[fid] = result;
        return result;
 }
 
@@ -900,20 +835,16 @@ static int acpi_battery_add(struct acpi_device *device)
        if (!battery)
                return -ENOMEM;
 
-       mutex_init(&battery->mutex);
-
-       mutex_lock(&battery->mutex);
-
        battery->device = device;
        strcpy(acpi_device_name(device), ACPI_BATTERY_DEVICE_NAME);
        strcpy(acpi_device_class(device), ACPI_BATTERY_CLASS);
        acpi_driver_data(device) = battery;
-
+       mutex_init(&battery->lock);
        result = acpi_battery_get_status(battery);
        if (result)
                goto end;
 
-       battery->flags.init_update = 1;
+       battery->init_update = 1;
 
        result = acpi_battery_add_fs(device);
        if (result)
@@ -939,8 +870,6 @@ static int acpi_battery_add(struct acpi_device *device)
                kfree(battery);
        }
 
-       mutex_unlock(&battery->mutex);
-
        return result;
 }
 
@@ -954,22 +883,12 @@ static int acpi_battery_remove(struct acpi_device *device, int type)
 
        battery = acpi_driver_data(device);
 
-       mutex_lock(&battery->mutex);
-
        status = acpi_remove_notify_handler(device->handle,
                                            ACPI_ALL_NOTIFY,
                                            acpi_battery_notify);
 
        acpi_battery_remove_fs(device);
-
-       kfree(battery->bif_data.pointer);
-
-       kfree(battery->bst_data.pointer);
-
-       mutex_unlock(&battery->mutex);
-
-       mutex_destroy(&battery->mutex);
-
+       mutex_destroy(&battery->lock);
        kfree(battery);
 
        return 0;
@@ -985,7 +904,7 @@ static int acpi_battery_resume(struct acpi_device *device)
 
        battery = device->driver_data;
 
-       battery->flags.init_update = 1;
+       battery->init_update = 1;
 
        return 0;
 }