Re: [PATCH v1 2/8] ACPI: bus: Rework printing debug messages on _OSC errors

Jonathan Cameron
2025-12-19T12:33:24Z

On Thu, 18 Dec 2025 21:35:27 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Instead of using one function, acpi_print_osc_error(), for printing a
> debug message and dumping the _OSC request data in one go, use
> acpi_handle_debug() directly for printing messages and a separate
> function called acpi_dump_osc_data() for dumping the _OSC request data
> after printing one or more of them.
>
> This allows the message printing in the _OSC handling code to be
> organized so that the messages printed by it are easier to parse.
Hi Rafael,

Perhaps an example of the print to motivate this change clearly?
The absence of a guid on the error string line for instance may
or may not bother people. It's there in the dump but that comes
after the error print I think.

Thanks,

Jonathan

>
> Also, use %pUL for UUID printing instead of printing UUIDs as strings
> and include the revision number into the dumped _OSC request data.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/bus.c | 35 ++++++++++++++++-------------------
> 1 file changed, 16 insertions(+), 19 deletions(-)
>
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -180,18 +180,15 @@ void acpi_bus_detach_private_data(acpi_h
> }
> EXPORT_SYMBOL_GPL(acpi_bus_detach_private_data);
>
> -static void acpi_print_osc_error(acpi_handle handle,
> - struct acpi_osc_context *context, char *error)
> +static void acpi_dump_osc_data(acpi_handle handle, const guid_t *guid, int rev,
> + struct acpi_buffer *cap)
> {
> + u32 *capbuf = cap->pointer;
> int i;
>
> - acpi_handle_debug(handle, "(%s): %s\n", context->uuid_str, error);
> -
> - pr_debug("_OSC request data:");
> - for (i = 0; i < context->cap.length; i += sizeof(u32))
> - pr_debug(" %x", *((u32 *)(context->cap.pointer + i)));
> -
> - pr_debug("\n");
> + for (i = 0; i < cap->length / sizeof(u32); i++)
> + acpi_handle_debug(handle, "(%pUL, %d): capabilities DWORD %i: [%08x]\n",
> + guid, rev, i, capbuf[i]);
> }
>
> #define OSC_ERROR_MASK (OSC_REQUEST_ERROR | OSC_INVALID_UUID_ERROR | \
> @@ -239,8 +236,8 @@ acpi_status acpi_run_osc(acpi_handle han
> 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");
> + acpi_handle_debug(handle, "_OSC evaluation returned wrong type");
> + acpi_dump_osc_data(handle, &guid, context->rev, &context->cap);
> status = AE_TYPE;
> goto out_kfree;
> }
> @@ -252,18 +249,18 @@ acpi_status acpi_run_osc(acpi_handle han
>
> if (errors) {
> if (errors & OSC_REQUEST_ERROR)
> - acpi_print_osc_error(handle, context,
> - "_OSC request failed");
> + acpi_handle_debug(handle, "_OSC request failed");
> +
> if (errors & OSC_INVALID_UUID_ERROR)
> - acpi_print_osc_error(handle, context,
> - "_OSC invalid UUID");
> + acpi_handle_debug(handle, "_OSC invalid UUID");
> +
> if (errors & OSC_INVALID_REVISION_ERROR)
> - acpi_print_osc_error(handle, context,
> - "_OSC invalid revision");
> + acpi_handle_debug(handle, "_OSC invalid revision");
> +
> if (errors & OSC_CAPABILITIES_MASK_ERROR)
> - acpi_print_osc_error(handle, context,
> - "_OSC capability bits masked");
> + acpi_handle_debug(handle, "_OSC capability bits masked");
>
> + acpi_dump_osc_data(handle, &guid, context->rev, &context->cap);
> status = AE_ERROR;
> goto out_kfree;
> }
>
>
>
>