On Wed, Dec 23, 2009 at 10:57:19AM +0800, Yinghai Lu wrote: > Shaohua Li wrote: > > On Wed, Dec 23, 2009 at 10:09:04AM +0800, Yinghai Lu wrote: > >> On Tue, Dec 22, 2009 at 5:38 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: > >>> Shaohua Li wrote: > >>>> On Tue, Dec 22, 2009 at 06:08:57PM +0800, Yinghai Lu wrote: > >>>>> Ingo Molnar wrote: > >>>>>> * Shaohua Li <shaohua.li@xxxxxxxxx> wrote: > >>>>>> > >>>>>>> On Fri, Dec 18, 2009 at 09:28:50AM +0800, Yinghai Lu wrote: > >>>>>>>> On Wed, Dec 16, 2009 at 12:06 PM, Len Brown <lenb@xxxxxxxxxx> wrote: > >>>>>>>>> Hi Linus, > >>>>>>>>> > >>>>>>>>> please pull from: > >>>>>>>>> > >>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git release > >>>>>>>>> > >>>>>>>> ;.. > >>>>>>>>> Shaohua Li (3): > >>>>>>>>> ? ? ?ACPI: Add a generic API for _OSC -v2 > >>>>>>>>> ? ? ?ACPI: cleanup pci_root _OSC code. > >>>>>>>>> ? ? ?ACPI: Add platform-wide _OSC support. > >>>>>>>> it seems these three patches broke the _OSC on my intel new systems. > >>>>>>>> > >>>>>>>> revert them fix the problem with AER and pciehp and etc > >>>>>>> can you give more details? I just cleaned up the _OSC code for AER and > >>>>>>> pciehp, no function changes. > >>>>>> Famous last words ;-) > >>>>>> > >>>>>> Yinghai, i suspect Shaohua needs the kind of info you'd need if you tried to > >>>>>> fix it: acpidump, before/after debug boot log, a description of what goes bad, > >>>>>> etc. > >>>>> the so called clean up, change the ret length checking. > >>>>> > >>>>> - if (!output.length) > >>>>> - return AE_NULL_OBJECT; > >>>>> - > >>>>> > >>>>> + /* return buffer should have the same length as cap buffer */ > >>>>> + if (context->ret.length != context->cap.length) > >>>>> + return AE_NULL_OBJECT; > >>>> Wield BIOS. ACPI spec does mention the return buffer have the same length. > >>>> Does changing the check back make the issue go away? > >>> change to > >>> if (context->ret.length < context->cap.length) > >>> > >>> make AER work, but pciehp still fail. > > Can you try below patch please? Looks the returned acpi buffer is a two-tiled buffer. > > Strange is it doesn't fail at my hand. > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > > index 65f7e33..0c1ad31 100644 > > --- a/drivers/acpi/bus.c > > +++ b/drivers/acpi/bus.c > > @@ -397,6 +397,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) > > union acpi_object *out_obj; > > u8 uuid[16]; > > u32 errors; > > + struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL}; > > > > if (!context) > > return AE_ERROR; > > @@ -419,16 +420,16 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) > > in_params[3].buffer.length = context->cap.length; > > in_params[3].buffer.pointer = context->cap.pointer; > > > > - status = acpi_evaluate_object(handle, "_OSC", &input, &context->ret); > > + status = acpi_evaluate_object(handle, "_OSC", &input, &output); > > if (ACPI_FAILURE(status)) > > return status; > > > > - /* return buffer should have the same length as cap buffer */ > > - if (context->ret.length != context->cap.length) > > + if (!output.length) > > return AE_NULL_OBJECT; > > > > - out_obj = context->ret.pointer; > > - if (out_obj->type != ACPI_TYPE_BUFFER) { > > + out_obj = output.pointer; > > + if (out_obj->type != ACPI_TYPE_BUFFER > > + || out_obj->buffer.length != context->cap.length) { > > acpi_print_osc_error(handle, context, > > "_OSC evaluation returned wrong type"); > > status = AE_TYPE; > > @@ -457,11 +458,20 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) > > goto out_kfree; > > } > > out_success: > > - return AE_OK; > > + context->ret.length = out_obj->buffer.length; > > + context->ret.pointer = kmalloc(context->ret.length, GFP_KERNEL); > > + if (!context->ret.pointer) { > > + status = AE_NO_MEMORY; > > + goto out_kfree; > > + } > > + memcpy(context->ret.pointer, out_obj->buffer.pointer, > > + context->ret.length); > > + status = AE_OK; > > > > out_kfree: > > - kfree(context->ret.pointer); > > - context->ret.pointer = NULL; > > + kfree(output.pointer); > > + if (status != AE_OK) > > + context->ret.pointer = NULL; > > return status; > > } > > EXPORT_SYMBOL(acpi_run_osc); > > aer and pciehp work again with this patch. Len, this is the patch with describtion, please check and apply. Executing _OSC returns a buffer, which has an acpi object in it. Don't directly returns the buffer, instead, we return the acpi object's buffer. This fixes a regression since caller of acpi_run_osc expects an acpi object's buffer returned. Tested-by: Yinghai Lu <yinghai@xxxxxxxxxx> Signed-off-by: Shaohua Li <shaohua.li@xxxxxxxxx> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 65f7e33..0c1ad31 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -397,6 +397,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) union acpi_object *out_obj; u8 uuid[16]; u32 errors; + struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL}; if (!context) return AE_ERROR; @@ -419,16 +420,16 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) in_params[3].buffer.length = context->cap.length; in_params[3].buffer.pointer = context->cap.pointer; - status = acpi_evaluate_object(handle, "_OSC", &input, &context->ret); + status = acpi_evaluate_object(handle, "_OSC", &input, &output); if (ACPI_FAILURE(status)) return status; - /* return buffer should have the same length as cap buffer */ - if (context->ret.length != context->cap.length) + if (!output.length) return AE_NULL_OBJECT; - out_obj = context->ret.pointer; - if (out_obj->type != ACPI_TYPE_BUFFER) { + out_obj = output.pointer; + if (out_obj->type != ACPI_TYPE_BUFFER + || out_obj->buffer.length != context->cap.length) { acpi_print_osc_error(handle, context, "_OSC evaluation returned wrong type"); status = AE_TYPE; @@ -457,11 +458,20 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) goto out_kfree; } out_success: - return AE_OK; + context->ret.length = out_obj->buffer.length; + context->ret.pointer = kmalloc(context->ret.length, GFP_KERNEL); + if (!context->ret.pointer) { + status = AE_NO_MEMORY; + goto out_kfree; + } + memcpy(context->ret.pointer, out_obj->buffer.pointer, + context->ret.length); + status = AE_OK; out_kfree: - kfree(context->ret.pointer); - context->ret.pointer = NULL; + kfree(output.pointer); + if (status != AE_OK) + context->ret.pointer = NULL; return status; } EXPORT_SYMBOL(acpi_run_osc); -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html