RE: [PATCH 1/2] PCI/ACPI: move _OSC code to pci_root.c

Moore, Robert
2009-02-05T16:03:06Z

The acpi_evaluate_object_typed interface automatically handles the two error cases you check for:

1) Checks for the case where no object is returned
2) Checks for the correct return object type (buffer in this case)

Use of this interface would simplify and shrink the code.

Bob

>-----Original Message-----
>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>owner@vger.kernel.org] On Behalf Of Alex Chiang
>Sent: Thursday, February 05, 2009 7:50 AM
>To: Kenji Kaneshige
>Cc: Jesse Barnes; linux-pci@vger.kernel.org; linux acpi; Andrew Patterson
>Subject: Re: [PATCH 1/2] PCI/ACPI: move _OSC code to pci_root.c
>
>Hi Kenji-san,
>
>Sorry, I didn't read your patch in detail last time, and only saw
>the one wording change that Andrew suggested.
>
>Reading through now, I have a few more suggestions.
>
>> + status = acpi_evaluate_object(handle, "_OSC", &input, &output);
>> + if (ACPI_FAILURE(status))
>> + return status;
>> +
>> + if (!output.length)
>> + return AE_NULL_OBJECT;
>> +
>> + out_obj = output.pointer;
>> + if (out_obj->type != ACPI_TYPE_BUFFER) {
>> + printk(KERN_DEBUG "Evaluate _OSC returns wrong type\n");
>
>"_OSC evaluation returned wrong type\n"
>
>> + if (errors) {
>> + if (errors & OSC_REQUEST_ERROR)
>> + printk(KERN_DEBUG "_OSC request fails\n");
>
>"_OSC request failed\n"
>
>> + if (errors & OSC_INVALID_UUID_ERROR)
>> + printk(KERN_DEBUG "_OSC invalid UUID\n");
>> + if (errors & OSC_INVALID_REVISION_ERROR)
>> + printk(KERN_DEBUG "_OSC invalid revision\n");
>> + if (errors & OSC_CAPABILITIES_MASK_ERROR) {
>> + if (capbuf[OSC_QUERY_TYPE] & OSC_QUERY_ENABLE)
>> + goto out_success;
>> + printk(KERN_DEBUG"Firmware did not grant requested "
>> + "_OSC control\n");
>
>I prefer to see this message on one line (and break the 80 column
>rule) to make grepping easier. But this is a judgement call, and
>I'll let you decide.
>
>Thanks.
>
>/ac
>
>--
>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