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

Posted by Rafael J. Wysocki 1 month, 2 weeks ago

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.

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;
 	}

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

Posted by Jonathan Cameron 1 month, 2 weeks ago

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;
>  	}
> 
> 
> 
>

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

Posted by Rafael J. Wysocki 1 month, 2 weeks ago

On Fri, Dec 19, 2025 at 1:33 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> 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.

Yes, it's there.

I guess I can add examples of "before" and "after" printouts to the
patch changelog.

> > 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;
> >       }