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

Kenji Kaneshige
2009-02-06T04:30:11Z

Alex Chiang wrote:
> 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.
>

Thank you for 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"
>

I'll fix it. But this debug message would be removed if we use
acpi_evaluate_object_typed() instead of acpi_evaluate_object() as
Robert suggested.

>> + if (errors) {
>> + if (errors & OSC_REQUEST_ERROR)
>> + printk(KERN_DEBUG "_OSC request fails\n");
>
> "_OSC request failed\n"
>

I'll fix it.

>> + 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.
>

Ok, I think one line is better. So I'll bread the 80 column rule:)

Thanks,
Kenji Kaneshige