[V2] ACPI/Battery: Add a _BIX quirk for NEC LZ750/LS

Commit Message

lan,Tianyu Jan. 15, 2014, 2:42 p.m. UTC

On 01/15/2014 06:17 AM, Rafael J. Wysocki wrote:
> On Tuesday, January 14, 2014 09:24:06 PM Matthew Garrett wrote:
>> On Tue, Jan 14, 2014 at 10:37:02PM +0100, Rafael J. Wysocki wrote:
>>> On Tuesday, January 14, 2014 04:06:01 PM Matthew Garrett wrote:
>>>> On Mon, Jan 06, 2014 at 11:25:53PM +0100, Rafael J. Wysocki wrote:
>>>>
>>>>> Queued up as a fix for 3.13 (I fixed up the indentation).
>>>>
>>>> Ah, sorry, I missed this chunk of the thread. If the system provides
>>>> valid _BIF data then we should possibly just fall back to that rather
>>>> than adding another quirk table.
>>>
>>> The problem is to know that _BIX is broken.  If we could figure that out
>>> upfront, we woulnd't need the quirk table in any case.
>>
>> It's obvious that it is in this case - the package is the wrong size.
>
> Then Tianyu should be able to come up with a better fix relatively easily. :-)
>
Hi Rafael&Matthew:

	I think we can evaluate _BIX before setting ACPI_BATTERY_XINFO_PRESENT 
flag. CA routine(acpi_ns_check_package) will check the package size and 
return error code when there is wrong size package.

Something like this.

Comments

Matthew Garrett Jan. 15, 2014, 2:47 p.m. UTC | #1

On Wed, Jan 15, 2014 at 10:42:31PM +0800, Lan Tianyu wrote:
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index fbf1ace..e98fa83 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -770,7 +770,7 @@ static int acpi_battery_add(struct acpi_device *device)
>         device->driver_data = battery;
>         mutex_init(&battery->lock);
>         mutex_init(&battery->sysfs_lock);
> -       if (acpi_has_method(battery->device->handle, "_BIX"))
> +       if (acpi_evaluate_object(device->handle, "_BIX", NULL, &buffer);)
>                 set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);

Doesn't acpi_evaluate_object() return 0 on success? I think:

if (ACPI_SUCESS(acpi_evaluate_object(device->handle, "_BIX", NULL, 
&buffer))

But maybe we should check for existence first and give an FW_BUG message 
to indicate an invalid _BIX?

lan,Tianyu Jan. 15, 2014, 3:06 p.m. UTC | #2

On 01/15/2014 10:47 PM, Matthew Garrett wrote:
> On Wed, Jan 15, 2014 at 10:42:31PM +0800, Lan Tianyu wrote:
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index fbf1ace..e98fa83 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -770,7 +770,7 @@ static int acpi_battery_add(struct acpi_device *device)
>>          device->driver_data = battery;
>>          mutex_init(&battery->lock);
>>          mutex_init(&battery->sysfs_lock);
>> -       if (acpi_has_method(battery->device->handle, "_BIX"))
>> +       if (acpi_evaluate_object(device->handle, "_BIX", NULL, &buffer);)
>>                  set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
>
> Doesn't acpi_evaluate_object() return 0 on success? I think:
>
> if (ACPI_SUCESS(acpi_evaluate_object(device->handle, "_BIX", NULL,
> &buffer))
>

Yes, Sorry for oops.

> But maybe we should check for existence first and give an FW_BUG message
> to indicate an invalid _BIX?

Yes, this is a good idea.

Another point, the acpi_evaluate_object should return different error 
code for these two cases(no _BIX and wrong size.). I wonder whether we 
can use the error code to determine it belong which case?
>

Moore, Robert Jan. 15, 2014, 4:48 p.m. UTC | #3

SWYgYW4gb2JqZWN0IGRvZXMgbm90IGV4aXN0LCBBRV9OT1RfRk9VTkQgaXMgcmV0dXJuZWQgYnkg
ZXZhbHVhdGVfb2JqZWN0Lg0KDQpJZiB0aGUgcGFja2FnZSBpcyBlbXB0eSBvciBoYXMgaW5zdWZm
aWNpZW50IGVsZW1lbnRzLCBzb21ldGhpbmcgbGlrZSBBRV9BTUxfT1BFUkFORF9WQUxVRSBpcyBy
ZXR1cm5lZC4NCg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IExhbiwg
VGlhbnl1DQo+IFNlbnQ6IFdlZG5lc2RheSwgSmFudWFyeSAxNSwgMjAxNCA3OjA3IEFNDQo+IFRv
OiBNYXR0aGV3IEdhcnJldHQNCj4gQ2M6IFJhZmFlbCBKLiBXeXNvY2tpOyBEbWl0cnkgVG9yb2to
b3Y7IGxlbmJAa2VybmVsLm9yZzsgbGludXgtDQo+IGFjcGlAdmdlci5rZXJuZWwub3JnOyBsaW51
eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBmY3JAYWRpbmV0LmNvbS51eTsNCj4gbEBkb3JpbGVv
Lm9yZzsgWmhlbmcsIEx2OyBNb29yZSwgUm9iZXJ0DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggVjJd
IEFDUEkvQmF0dGVyeTogQWRkIGEgX0JJWCBxdWlyayBmb3IgTkVDIExaNzUwL0xTDQo+IA0KPiBP
biAwMS8xNS8yMDE0IDEwOjQ3IFBNLCBNYXR0aGV3IEdhcnJldHQgd3JvdGU6DQo+ID4gT24gV2Vk
LCBKYW4gMTUsIDIwMTQgYXQgMTA6NDI6MzFQTSArMDgwMCwgTGFuIFRpYW55dSB3cm90ZToNCj4g
Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYWNwaS9iYXR0ZXJ5LmMgYi9kcml2ZXJzL2FjcGkvYmF0
dGVyeS5jIGluZGV4DQo+ID4+IGZiZjFhY2UuLmU5OGZhODMgMTAwNjQ0DQo+ID4+IC0tLSBhL2Ry
aXZlcnMvYWNwaS9iYXR0ZXJ5LmMNCj4gPj4gKysrIGIvZHJpdmVycy9hY3BpL2JhdHRlcnkuYw0K
PiA+PiBAQCAtNzcwLDcgKzc3MCw3IEBAIHN0YXRpYyBpbnQgYWNwaV9iYXR0ZXJ5X2FkZChzdHJ1
Y3QgYWNwaV9kZXZpY2UNCj4gKmRldmljZSkNCj4gPj4gICAgICAgICAgZGV2aWNlLT5kcml2ZXJf
ZGF0YSA9IGJhdHRlcnk7DQo+ID4+ICAgICAgICAgIG11dGV4X2luaXQoJmJhdHRlcnktPmxvY2sp
Ow0KPiA+PiAgICAgICAgICBtdXRleF9pbml0KCZiYXR0ZXJ5LT5zeXNmc19sb2NrKTsNCj4gPj4g
LSAgICAgICBpZiAoYWNwaV9oYXNfbWV0aG9kKGJhdHRlcnktPmRldmljZS0+aGFuZGxlLCAiX0JJ
WCIpKQ0KPiA+PiArICAgICAgIGlmIChhY3BpX2V2YWx1YXRlX29iamVjdChkZXZpY2UtPmhhbmRs
ZSwgIl9CSVgiLCBOVUxMLA0KPiA+PiArICZidWZmZXIpOykNCj4gPj4gICAgICAgICAgICAgICAg
ICBzZXRfYml0KEFDUElfQkFUVEVSWV9YSU5GT19QUkVTRU5ULA0KPiA+PiAmYmF0dGVyeS0+Zmxh
Z3MpOw0KPiA+DQo+ID4gRG9lc24ndCBhY3BpX2V2YWx1YXRlX29iamVjdCgpIHJldHVybiAwIG9u
IHN1Y2Nlc3M/IEkgdGhpbms6DQo+ID4NCj4gPiBpZiAoQUNQSV9TVUNFU1MoYWNwaV9ldmFsdWF0
ZV9vYmplY3QoZGV2aWNlLT5oYW5kbGUsICJfQklYIiwgTlVMTCwNCj4gPiAmYnVmZmVyKSkNCj4g
Pg0KPiANCj4gWWVzLCBTb3JyeSBmb3Igb29wcy4NCj4gDQo+ID4gQnV0IG1heWJlIHdlIHNob3Vs
ZCBjaGVjayBmb3IgZXhpc3RlbmNlIGZpcnN0IGFuZCBnaXZlIGFuIEZXX0JVRw0KPiA+IG1lc3Nh
Z2UgdG8gaW5kaWNhdGUgYW4gaW52YWxpZCBfQklYPw0KPiANCj4gWWVzLCB0aGlzIGlzIGEgZ29v
ZCBpZGVhLg0KPiANCj4gQW5vdGhlciBwb2ludCwgdGhlIGFjcGlfZXZhbHVhdGVfb2JqZWN0IHNo
b3VsZCByZXR1cm4gZGlmZmVyZW50IGVycm9yIGNvZGUNCj4gZm9yIHRoZXNlIHR3byBjYXNlcyhu
byBfQklYIGFuZCB3cm9uZyBzaXplLikuIEkgd29uZGVyIHdoZXRoZXIgd2UgY2FuIHVzZQ0KPiB0
aGUgZXJyb3IgY29kZSB0byBkZXRlcm1pbmUgaXQgYmVsb25nIHdoaWNoIGNhc2U/DQo+ID4NCj4g
DQo+IA0KPiAtLQ0KPiBCZXN0IFJlZ2FyZHMNCj4gVGlhbnl1IExhbg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index fbf1ace..e98fa83 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -770,7 +770,7 @@  static int acpi_battery_add(struct acpi_device *device)
         device->driver_data = battery;
         mutex_init(&battery->lock);
         mutex_init(&battery->sysfs_lock);
-       if (acpi_has_method(battery->device->handle, "_BIX"))
+       if (acpi_evaluate_object(device->handle, "_BIX", NULL, &buffer);)
                 set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
         result = acpi_battery_update(battery);
         if (result)