[PATCH v2.1 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
before printing one or more of them.

This avoids
 * dumping _OSC request data multiple times when there are
   multiple error bits set in the return buffer,
 * wrapping message lines on terminals with 80 character line width,
 * mixing up unrelated messages by printing full lines only,
and generally helps to make the messages 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.

This is how the debug printout looks like when the
OSC_REQUEST_ERROR and OSC_INVALID_REVISION_ERROR bits
are set in the return buffer before the change:

 ACPI: \_SB_: ACPI: (0811B06E-4A27-44F9-8D60-3CBBC22E7B48): _OSC request failed
 ACPI: _OSC request data:
 ACPI:  1
 ACPI:  2e7eff
 ACPI: 
 ACPI: \_SB_: ACPI: (0811B06E-4A27-44F9-8D60-3CBBC22E7B48): _OSC invalid revision
 ACPI: _OSC request data:
 ACPI:  1
 ACPI:  2e7eff
 ACPI:

and this is how it is going to look like afterward:

 ACPI: \_SB_: ACPI: _OSC: UUID: 0811B06E-4A27-44F9-8D60-3CBBC22E7B48, rev: 1
 ACPI: \_SB_: ACPI: _OSC: capabilities DWORD 0: [00000001]
 ACPI: \_SB_: ACPI: _OSC: capabilities DWORD 1: [002e7eff]
 ACPI: \_SB_: ACPI: _OSC: request failed
 ACPI: \_SB_: ACPI: _OSC: invalid revision

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2.1:
   * Dump _OSC request data before printing diagnostic messages.
   * Add example "before" and "after" debug output to the changelog (Jonathan).
   * Reformat messages to avoid crossing the 80 characters boundary.
   * Update the changelog.

---
 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,16 @@ 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");
+	acpi_handle_debug(handle, "_OSC: UUID: %pUL, rev: %d\n", guid, rev);
+	for (i = 0; i < cap->length / sizeof(u32); i++)
+		acpi_handle_debug(handle, "_OSC: capabilities DWORD %i: [%08x]\n",
+				  i, capbuf[i]);
 }
 
 #define OSC_ERROR_MASK 	(OSC_REQUEST_ERROR | OSC_INVALID_UUID_ERROR | \
@@ -239,8 +237,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_dump_osc_data(handle, &guid, context->rev, &context->cap);
+		acpi_handle_debug(handle, "_OSC: evaluation returned wrong type");
 		status = AE_TYPE;
 		goto out_kfree;
 	}
@@ -261,9 +259,9 @@ acpi_status acpi_run_osc(acpi_handle han
 		 */
 		bool fail = !!(capbuf[OSC_QUERY_DWORD] & OSC_QUERY_ENABLE);
 
+		acpi_dump_osc_data(handle, &guid, context->rev, &context->cap);
 		if (errors & OSC_INVALID_UUID_ERROR) {
-			acpi_print_osc_error(handle, context,
-				"_OSC invalid UUID");
+			acpi_handle_debug(handle, "_OSC: invalid UUID");
 			/*
 			 * Always fail if this bit is set because it means that
 			 * the request could not be processed.
@@ -272,14 +270,13 @@ acpi_status acpi_run_osc(acpi_handle han
 			goto out_kfree;
 		}
 		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_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");
 
 		if (fail) {
 			status = AE_ERROR;

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

Posted by Jonathan Cameron 1 month, 2 weeks ago

On Mon, 22 Dec 2025 20:11:08 +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
> before printing one or more of them.
> 
> This avoids
>  * dumping _OSC request data multiple times when there are
>    multiple error bits set in the return buffer,
>  * wrapping message lines on terminals with 80 character line width,
>  * mixing up unrelated messages by printing full lines only,
> and generally helps to make the messages 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.
> 
> This is how the debug printout looks like when the
> OSC_REQUEST_ERROR and OSC_INVALID_REVISION_ERROR bits
> are set in the return buffer before the change:
> 
>  ACPI: \_SB_: ACPI: (0811B06E-4A27-44F9-8D60-3CBBC22E7B48): _OSC request failed
>  ACPI: _OSC request data:
>  ACPI:  1
>  ACPI:  2e7eff
>  ACPI: 
>  ACPI: \_SB_: ACPI: (0811B06E-4A27-44F9-8D60-3CBBC22E7B48): _OSC invalid revision
>  ACPI: _OSC request data:
>  ACPI:  1
>  ACPI:  2e7eff
>  ACPI:
> 
> and this is how it is going to look like afterward:
> 
>  ACPI: \_SB_: ACPI: _OSC: UUID: 0811B06E-4A27-44F9-8D60-3CBBC22E7B48, rev: 1
>  ACPI: \_SB_: ACPI: _OSC: capabilities DWORD 0: [00000001]
>  ACPI: \_SB_: ACPI: _OSC: capabilities DWORD 1: [002e7eff]
>  ACPI: \_SB_: ACPI: _OSC: request failed
>  ACPI: \_SB_: ACPI: _OSC: invalid revision
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>