Re: [2.624-rc1 regression] lost battery information

From: Alexey Starikovskiy
Date: Sat Oct 27 2007 - 11:20:43 EST

Andrey Borzenkov wrote:
> On Saturday 27 October 2007, Alexey Starikovskiy wrote:
>> Andrey,
>> Please try the attached patch. I choose to do snprintf() instead of direct
>> copy, as your previous message showed empty OEM type.
>>
>
> Not quite. Now I get
>
> OEM info: 0
Ok, I was hoping to see some number starting with 0, which would be printed
as empty string...
>
> while before I got empty string. If I read acpi_extract_package correctly, it
> actually interpreted integer as string without any conversion. Which in this
> case obviously gave us empty string (integer being 0). I'd prefer to remain
> compatible.
As you wish... :) Please check the attached patch.
>
> also
>
> {pts/1}% cat /sys/class/power_supply/BAT1/manufacturer
> 0
>
> which is rather weird manufacturer name :)
>
>> Thanks,
>> Alex.
>>
>> Andrey Borzenkov wrote:
>>> On Friday 26 October 2007, Alexey Starikovskiy wrote:
>>>> Your cat's "Bad address" means -EFAULT, according to "man errno".
>>>> Please apply this patch to see what exactly failed...
>>> [ 1191.471572] ACPI: element[12]->type = 1, expected string
>>> [ 1196.640065] ACPI: element[12]->type = 1, expected string
>>> [ 1199.479773] ACPI: element[12]->type = 1, expected string
>>> [ 1199.745435] ACPI: element[12]->type = 1, expected string
>>>
>>> it is "OEM type". For reference here is _BIF from my DSDT:
>>>
>>> Method (_BIF, 0, NotSerialized)
>>> {
>>> Name (BUFF, Package (0x0D) {})
>>> Store (0x00, Index (BUFF, 0x00))
>>> Store (\_SB.MEM.BDV2, Local2)
>>> Multiply (\_SB.MEM.BDC2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x01))
>>> Multiply (\_SB.MEM.BLF2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x02))
>>> Store (\_SB.MEM.BTC2, Index (BUFF, 0x03))
>>> Store (\_SB.MEM.BDV2, Index (BUFF, 0x04))
>>> Multiply (\_SB.MEM.BCW2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x05))
>>> Multiply (\_SB.MEM.BCL2, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x06))
>>> Multiply (\_SB.MEM.BG12, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x07))
>>> Multiply (\_SB.MEM.BG22, Local2, Local0)
>>> Divide (Local0, 0x03E8, Local1, Local0)
>>> Store (Local0, Index (BUFF, 0x08))
>>> Store (\_SB.MEM.BMN2, Index (BUFF, 0x09))
>>> Store (\_SB.MEM.BSN2, Index (BUFF, 0x0A))
>>> Store (\_SB.MEM.BTP2, Index (BUFF, 0x0B))
>>> Store (\_SB.MEM.BOI2, Index (BUFF, 0x0C))
>>> Return (BUFF)
>>> }
>>>
>>> This is behaviour change. Previous battery.c used generic
>>> acpi_extract_package which allowed (allows) for object of type integer
>>> when string is requested:
>>>
>>> case ACPI_TYPE_INTEGER:
>>> switch (format_string[i]) {
>>> case 'N':
>>> size_required += sizeof(acpi_integer);
>>> tail_offset += sizeof(acpi_integer);
>>> break;
>>> case 'S':
>>> size_required +=
>>> sizeof(char *) + sizeof(acpi_integer)
>>> + sizeof(char);
>>> tail_offset += sizeof(char *);
>>> break;
>>>
>>> while current battery.c:extract_package fails:
>>>
>>> if (offsets[i].mode) {
>>> if (element->type != ACPI_TYPE_STRING &&
>>> element->type != ACPI_TYPE_BUFFER) {
>>> printk (KERN_ERR PREFIX "element[%d]->type = %x, expected string\n", i,
>>> element->type);
>>> return -EFAULT;
>>> }
>>>
>>> well, while it could be BIOS fault this happily worked before ... This is
>>> obviously also the reason why I do not have anything in /sys
>>>
>>> Fans, could you check whether you have the same issue using test patch?
>
>

ACPI: Battery: Allow extract string from integer

From: Alexey Starikovskiy <astarikovskiy@xxxxxxx>

Some machines return integer instead of expected string.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@xxxxxxx>
---

drivers/acpi/battery.c | 25 +++++++++++++++----------
1 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 02a396d..6c06879 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -260,7 +260,7 @@ static int extract_package(struct acpi_battery *battery,
union acpi_object *package,
struct acpi_offsets *offsets, int num)
{
- int i, *x;
+ int i;
union acpi_object *element;
if (package->type != ACPI_TYPE_PACKAGE)
return -EFAULT;
@@ -269,16 +269,21 @@ static int extract_package(struct acpi_battery *battery,
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);
+ u8 *ptr = (u8 *)battery + offsets[i].offset;
+ if (element->type == ACPI_TYPE_STRING ||
+ element->type == ACPI_TYPE_BUFFER)
+ strncpy(ptr, element->string.pointer, 32);
+ else if (element->type == ACPI_TYPE_INTEGER) {
+ strncpy(ptr, (u8 *)&element->integer.value,
+ sizeof(acpi_integer));
+ ptr[sizeof(acpi_integer)] = 0;
+ } else return -EFAULT;
} else {
- if (element->type != ACPI_TYPE_INTEGER)
- return -EFAULT;
- x = (int *)((u8 *)battery + offsets[i].offset);
- *x = element->integer.value;
+ if (element->type == ACPI_TYPE_INTEGER) {
+ int *x = (int *)((u8 *)battery +
+ offsets[i].offset);
+ *x = element->integer.value;
+ } else return -EFAULT;
}
}
return 0;