RE: [RFC PATCH] acpi: Use a more normal logging style for ACPI_<LEVEL> calls
From: Moore, Robert
Date: Tue Mar 29 2016 - 11:34:47 EST
On the other hand, we removed AE_INFO from ACPI_INFO a while back, and this may remove the necessity for the double parens.
> -----Original Message-----
> From: Devel [mailto:devel-bounces@xxxxxxxxxx] On Behalf Of Moore, Robert
> Sent: Tuesday, March 29, 2016 8:27 AM
> To: Joe Perches; Zheng, Lv; Wysocki, Rafael J; Len Brown
> Cc: linux-acpi@xxxxxxxxxxxxxxx; linux-kernel; devel@xxxxxxxxxx
> Subject: Re: [Devel] [RFC PATCH] acpi: Use a more normal logging style for
> ACPI_<LEVEL> calls
>
> Probably not, because ACPI_INFO is a public (and documented) macro, and
> ACPICA is used in many operating systems.
>
> The double parens are there to allow for variable-length arguments, I
> believe.
>
> Bob
>
>
> > -----Original Message-----
> > From: Joe Perches [mailto:joe@xxxxxxxxxxx]
> > Sent: Monday, March 28, 2016 8:30 PM
> > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown
> > Cc: linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx; linux-kernel
> > Subject: [RFC PATCH] acpi: Use a more normal logging style for
> > ACPI_<LEVEL> calls
> >
> > This is just an example of a conversion of ACPI_INFO to a more typical
> > kernel use style. All of the other ACPI_<LEVEL> calls would also need
> > conversion.
> >
> > Almost all logging functions and macros in the kernel are lower case
> > and nearly all use formats with terminating newlines.
> >
> > ACPI uses upper case macros and no terminating newline in the format.
> >
> > Some of the uses though _do_ have newlines. This can cause undesired
> > newlines in dmesg output.
> >
> > Also, the ACPI_<LEVEL> macros use a somewhat odd and unpleasant style
> > with double parentheses.
> >
> > Convert this to a lower case macro, add terminating newlines to
> > formats and remove the unnecessary extra parentheses.
> >
> > Rename the logging function to _acpi_info and have the acpi_info
> > macros call this _acpi_info function.
> >
> > Remove the newline from the _acpi_info call.
> >
> > This means that all calls to acpi_info are complete and it is not
> > possible for any other message to be interleaved into this message.
> >
> > Miscellanea:
> >
> > o Coalesce formats
> > o Realign arguments
> > o Coalesce arguments even if > 80 columns
> > ---
> > drivers/acpi/acpica/dbcmds.c | 4 ++--
> > drivers/acpi/acpica/dsmethod.c | 6 ++----
> > drivers/acpi/acpica/dsobject.c | 5 ++---
> > drivers/acpi/acpica/evgpeblk.c | 8 ++++----
> > drivers/acpi/acpica/evgpeinit.c | 2 +-
> > drivers/acpi/acpica/exconfig.c | 4 ++--
> > drivers/acpi/acpica/nseval.c | 4 ++--
> > drivers/acpi/acpica/tbinstal.c | 15 +++++++--------
> > drivers/acpi/acpica/tbprint.c | 36
> > ++++++++++++++++--------------------
> > drivers/acpi/acpica/tbutils.c | 3 ++-
> > drivers/acpi/acpica/tbxfload.c | 5 +++--
> > drivers/acpi/acpica/uttrack.c | 2 +-
> > drivers/acpi/acpica/utxferror.c | 7 +++----
> > include/acpi/acoutput.h | 6 +++---
> > include/acpi/acpixf.h | 2 +-
> > 15 files changed, 51 insertions(+), 58 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dbcmds.c
> > b/drivers/acpi/acpica/dbcmds.c index 772178c..1ef21be 100644
> > --- a/drivers/acpi/acpica/dbcmds.c
> > +++ b/drivers/acpi/acpica/dbcmds.c
> > @@ -348,8 +348,8 @@ void acpi_db_display_table_info(char *table_arg)
> > } else {
> > /* If the pointer is null, the table has been unloaded
> */
> >
> > - ACPI_INFO(("%4.4s - Table has been unloaded",
> > - table_desc->signature.ascii));
> > + acpi_info("%4.4s - Table has been unloaded\n",
> > + table_desc->signature.ascii);
> > }
> > }
> > }
> > diff --git a/drivers/acpi/acpica/dsmethod.c
> > b/drivers/acpi/acpica/dsmethod.c index 1982310..4730b0a 100644
> > --- a/drivers/acpi/acpica/dsmethod.c
> > +++ b/drivers/acpi/acpica/dsmethod.c
> > @@ -809,10 +809,8 @@ acpi_ds_terminate_control_method(union
> > acpi_operand_object *method_desc,
> > if (method_desc->method.
> > info_flags & ACPI_METHOD_SERIALIZED_PENDING) {
> > if (walk_state) {
> > - ACPI_INFO(("Marking method %4.4s as Serialized "
> > - "because of AE_ALREADY_EXISTS error",
> > - walk_state->method_node->name.
> > - ascii));
> > + acpi_info("Marking method %4.4s as Serialized
> > because of AE_ALREADY_EXISTS error\n",
> > + walk_state->method_node->name.ascii);
> > }
> >
> > /*
> > diff --git a/drivers/acpi/acpica/dsobject.c
> > b/drivers/acpi/acpica/dsobject.c index a91de2b..6787274 100644
> > --- a/drivers/acpi/acpica/dsobject.c
> > +++ b/drivers/acpi/acpica/dsobject.c
> > @@ -524,9 +524,8 @@ acpi_ds_build_internal_package_obj(struct
> > acpi_walk_state *walk_state,
> > arg = arg->common.next;
> > }
> >
> > - ACPI_INFO(("Actual Package length (%u) is larger than "
> > - "NumElements field (%u), truncated",
> > - i, element_count));
> > + acpi_info("Actual Package length (%u) is larger than
> > NumElements field (%u), truncated\n",
> > + i, element_count);
> > } else if (i < element_count) {
> > /*
> > * Arg list (elements) was exhausted, but we did not reach
> > num_elements count.
> > diff --git a/drivers/acpi/acpica/evgpeblk.c
> > b/drivers/acpi/acpica/evgpeblk.c index 447fa1c..2244ed8 100644
> > --- a/drivers/acpi/acpica/evgpeblk.c
> > +++ b/drivers/acpi/acpica/evgpeblk.c
> > @@ -499,10 +499,10 @@ acpi_ev_initialize_gpe_block(struct
> > acpi_gpe_xrupt_info *gpe_xrupt_info,
> > }
> >
> > if (gpe_enabled_count) {
> > - ACPI_INFO(("Enabled %u GPEs in block %02X to %02X",
> > - gpe_enabled_count, (u32)gpe_block->block_base_number,
> > - (u32)(gpe_block->block_base_number +
> > - (gpe_block->gpe_count - 1))));
> > + acpi_info("Enabled %u GPEs in block %02X to %02X\n",
> > + gpe_enabled_count, (u32)gpe_block->block_base_number,
> > + (u32)(gpe_block->block_base_number +
> > + gpe_block->gpe_count - 1));
> > }
> >
> > gpe_block->initialized = TRUE;
> > diff --git a/drivers/acpi/acpica/evgpeinit.c
> > b/drivers/acpi/acpica/evgpeinit.c index 7dc7547..3a6d717 100644
> > --- a/drivers/acpi/acpica/evgpeinit.c
> > +++ b/drivers/acpi/acpica/evgpeinit.c
> > @@ -281,7 +281,7 @@ void acpi_ev_update_gpes(acpi_owner_id
> table_owner_id)
> > }
> >
> > if (walk_info.count) {
> > - ACPI_INFO(("Enabled %u new GPEs", walk_info.count));
> > + acpi_info("Enabled %u new GPEs\n", walk_info.count);
> > }
> >
> > (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
> > diff --git a/drivers/acpi/acpica/exconfig.c
> > b/drivers/acpi/acpica/exconfig.c index f741613..99916fe 100644
> > --- a/drivers/acpi/acpica/exconfig.c
> > +++ b/drivers/acpi/acpica/exconfig.c
> > @@ -252,7 +252,7 @@ acpi_ex_load_table_op(struct acpi_walk_state
> > *walk_state,
> >
> > status = acpi_get_table_by_index(table_index, &table);
> > if (ACPI_SUCCESS(status)) {
> > - ACPI_INFO(("Dynamic OEM Table Load:"));
> > + acpi_info("Dynamic OEM Table Load:\n");
> > acpi_tb_print_table_header(0, table);
> > }
> >
> > @@ -472,7 +472,7 @@ acpi_ex_load_op(union acpi_operand_object
> > *obj_desc,
> >
> > /* Install the new table into the local data structures */
> >
> > - ACPI_INFO(("Dynamic OEM Table Load:"));
> > + acpi_info("Dynamic OEM Table Load:\n");
> > (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
> >
> > status = acpi_tb_install_standard_table(ACPI_PTR_TO_PHYSADDR(table),
> > diff --git a/drivers/acpi/acpica/nseval.c
> > b/drivers/acpi/acpica/nseval.c index 5d59cfc..18777f0 100644
> > --- a/drivers/acpi/acpica/nseval.c
> > +++ b/drivers/acpi/acpica/nseval.c
> > @@ -378,8 +378,8 @@ void acpi_ns_exec_module_code_list(void)
> > acpi_ut_remove_reference(prev);
> > }
> >
> > - ACPI_INFO(("Executed %u blocks of module-level executable AML code",
> > - method_count));
> > + acpi_info("Executed %u blocks of module-level executable AML
> > code\n",
> > + method_count);
> >
> > ACPI_FREE(info);
> > acpi_gbl_module_code_list = NULL;
> > diff --git a/drivers/acpi/acpica/tbinstal.c
> > b/drivers/acpi/acpica/tbinstal.c index 4dc6108..28f2ab7 100644
> > --- a/drivers/acpi/acpica/tbinstal.c
> > +++ b/drivers/acpi/acpica/tbinstal.c
> > @@ -267,9 +267,9 @@
> > acpi_tb_install_standard_table(acpi_physical_address
> > address,
> > if (!reload &&
> > acpi_gbl_disable_ssdt_table_install &&
> > ACPI_COMPARE_NAME(&new_table_desc.signature, ACPI_SIG_SSDT)) {
> > - ACPI_INFO(("Ignoring installation of %4.4s at %8.8X%8.8X",
> > - new_table_desc.signature.ascii,
> > - ACPI_FORMAT_UINT64(address)));
> > + acpi_info("Ignoring installation of %4.4s at %8.8X%8.8X\n",
> > + new_table_desc.signature.ascii,
> > + ACPI_FORMAT_UINT64(address));
> > goto release_and_exit;
> > }
> >
> > @@ -431,11 +431,10 @@ finish_override:
> > return;
> > }
> >
> > - ACPI_INFO(("%4.4s 0x%8.8X%8.8X"
> > - " %s table override, new table: 0x%8.8X%8.8X",
> > - old_table_desc->signature.ascii,
> > - ACPI_FORMAT_UINT64(old_table_desc->address),
> > - override_type,
> > ACPI_FORMAT_UINT64(new_table_desc.address)));
> > + acpi_info("%4.4s 0x%8.8X%8.8X %s table override, new table:
> > 0x%8.8X%8.8X\n",
> > + old_table_desc->signature.ascii,
> > + ACPI_FORMAT_UINT64(old_table_desc->address),
> > + override_type, ACPI_FORMAT_UINT64(new_table_desc.address));
> >
> > /* We can now uninstall the original table */
> >
> > diff --git a/drivers/acpi/acpica/tbprint.c
> > b/drivers/acpi/acpica/tbprint.c index 26d61db..030d610 100644
> > --- a/drivers/acpi/acpica/tbprint.c
> > +++ b/drivers/acpi/acpica/tbprint.c
> > @@ -132,9 +132,9 @@ acpi_tb_print_table_header(acpi_physical_address
> > address,
> >
> > /* FACS only has signature and length fields */
> >
> > - ACPI_INFO(("%-4.4s 0x%8.8X%8.8X %06X",
> > - header->signature, ACPI_FORMAT_UINT64(address),
> > - header->length));
> > + acpi_info("%-4.4s 0x%8.8X%8.8X %06X\n",
> > + header->signature, ACPI_FORMAT_UINT64(address),
> > + header->length);
> > } else if (ACPI_VALIDATE_RSDP_SIG(header->signature)) {
> >
> > /* RSDP has no common fields */
> > @@ -144,28 +144,24 @@ acpi_tb_print_table_header(acpi_physical_address
> > address,
> > ACPI_OEM_ID_SIZE);
> > acpi_tb_fix_string(local_header.oem_id, ACPI_OEM_ID_SIZE);
> >
> > - ACPI_INFO(("RSDP 0x%8.8X%8.8X %06X (v%.2d %-6.6s)",
> > - ACPI_FORMAT_UINT64(address),
> > - (ACPI_CAST_PTR(struct acpi_table_rsdp, header)->
> > - revision >
> > - 0) ? ACPI_CAST_PTR(struct acpi_table_rsdp,
> > - header)->length : 20,
> > - ACPI_CAST_PTR(struct acpi_table_rsdp,
> > - header)->revision,
> > - local_header.oem_id));
> > + acpi_info("RSDP 0x%8.8X%8.8X %06X (v%.2d %-6.6s)\n",
> > + ACPI_FORMAT_UINT64(address),
> > + ACPI_CAST_PTR(struct acpi_table_rsdp, header)-
> > >revision > 0 ?
> > + ACPI_CAST_PTR(struct acpi_table_rsdp, header)->length
> > : 20,
> > + ACPI_CAST_PTR(struct acpi_table_rsdp, header)-
> > >revision,
> > + local_header.oem_id);
> > } else {
> > /* Standard ACPI table with full common header */
> >
> > acpi_tb_cleanup_table_header(&local_header, header);
> >
> > - ACPI_INFO(("%-4.4s 0x%8.8X%8.8X"
> > - " %06X (v%.2d %-6.6s %-8.8s %08X %-4.4s %08X)",
> > - local_header.signature, ACPI_FORMAT_UINT64(address),
> > - local_header.length, local_header.revision,
> > - local_header.oem_id, local_header.oem_table_id,
> > - local_header.oem_revision,
> > - local_header.asl_compiler_id,
> > - local_header.asl_compiler_revision));
> > + acpi_info("%-4.4s 0x%8.8X%8.8X %06X (v%.2d %-6.6s %-8.8s %08X
> > %-4.4s %08X)\n",
> > + local_header.signature, ACPI_FORMAT_UINT64(address),
> > + local_header.length, local_header.revision,
> > + local_header.oem_id, local_header.oem_table_id,
> > + local_header.oem_revision,
> > + local_header.asl_compiler_id,
> > + local_header.asl_compiler_revision);
> > }
> > }
> >
> > diff --git a/drivers/acpi/acpica/tbutils.c
> > b/drivers/acpi/acpica/tbutils.c index 9240c76..8f64b38 100644
> > --- a/drivers/acpi/acpica/tbutils.c
> > +++ b/drivers/acpi/acpica/tbutils.c
> > @@ -174,7 +174,8 @@ struct acpi_table_header *acpi_tb_copy_dsdt(u32
> > table_index)
> > ACPI_TABLE_ORIGIN_INTERNAL_VIRTUAL,
> > new_table);
> >
> > - ACPI_INFO(("Forced DSDT copy: length 0x%05X copied locally, original
> > unmapped", new_table->length));
> > + acpi_info("Forced DSDT copy: length 0x%05X copied locally, original
> > unmapped\n",
> > + new_table->length);
> >
> > return (new_table);
> > }
> > diff --git a/drivers/acpi/acpica/tbxfload.c
> > b/drivers/acpi/acpica/tbxfload.c index 3151968..0827d17 100644
> > --- a/drivers/acpi/acpica/tbxfload.c
> > +++ b/drivers/acpi/acpica/tbxfload.c
> > @@ -240,7 +240,8 @@ acpi_status acpi_tb_load_namespace(void)
> > }
> >
> > if (!tables_failed) {
> > - ACPI_INFO(("%u ACPI AML tables successfully acquired and
> > loaded\n", tables_loaded));
> > + acpi_info("%u ACPI AML tables successfully acquired and
> > loaded\n",
> > + tables_loaded);
> > } else {
> > ACPI_ERROR((AE_INFO,
> > "%u table load failures, %u successful", @@ -333,7
> > +334,7 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
> >
> > /* Install the table and load it into the namespace */
> >
> > - ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
> > + acpi_info("Host-directed Dynamic ACPI Table Load:\n");
> > (void)acpi_ut_acquire_mutex(ACPI_MTX_TABLES);
> >
> > status = acpi_tb_install_standard_table(ACPI_PTR_TO_PHYSADDR(table),
> > diff --git a/drivers/acpi/acpica/uttrack.c
> > b/drivers/acpi/acpica/uttrack.c index 60c406a..b1d6f4a 100644
> > --- a/drivers/acpi/acpica/uttrack.c
> > +++ b/drivers/acpi/acpica/uttrack.c
> > @@ -712,7 +712,7 @@ void acpi_ut_dump_allocations(u32 component, const
> > char *module)
> > /* Print summary */
> >
> > if (!num_outstanding) {
> > - ACPI_INFO(("No outstanding allocations"));
> > + acpi_info("No outstanding allocations\n");
> > } else {
> > ACPI_ERROR((AE_INFO, "%u(0x%X) Outstanding allocations",
> > num_outstanding, num_outstanding)); diff --git
> > a/drivers/acpi/acpica/utxferror.c b/drivers/acpi/acpica/utxferror.c
> > index
> > d9f15cb..e9a80c2 100644
> > --- a/drivers/acpi/acpica/utxferror.c
> > +++ b/drivers/acpi/acpica/utxferror.c
> > @@ -161,7 +161,7 @@ ACPI_EXPORT_SYMBOL(acpi_warning)
> >
> >
> > /*********************************************************************
> > ***
> > *******
> > *
> > - * FUNCTION: acpi_info
> > + * FUNCTION: _acpi_info
> > *
> > * PARAMETERS: module_name - Caller's module name (for error
> > output)
> > * line_number - Caller's line number (for error
> > output) @@ -175,7 +175,7 @@ ACPI_EXPORT_SYMBOL(acpi_warning)
> > * TBD: module_name and line_number args are not needed, should be
> > removed.
> > *
> >
> > **********************************************************************
> > **
> > ******/
> > -void ACPI_INTERNAL_VAR_XFACE acpi_info(const char *format, ...)
> > +void ACPI_INTERNAL_VAR_XFACE _acpi_info(const char *format, ...)
> > {
> > va_list arg_list;
> >
> > @@ -184,13 +184,12 @@ void ACPI_INTERNAL_VAR_XFACE acpi_info(const
> > char *format, ...)
> >
> > va_start(arg_list, format);
> > acpi_os_vprintf(format, arg_list);
> > - acpi_os_printf("\n");
> > va_end(arg_list);
> >
> > ACPI_MSG_REDIRECT_END;
> > }
> >
> > -ACPI_EXPORT_SYMBOL(acpi_info)
> > +ACPI_EXPORT_SYMBOL(_acpi_info)
> >
> >
> > /*********************************************************************
> > ***
> > *******
> > *
> > diff --git a/include/acpi/acoutput.h b/include/acpi/acoutput.h index
> > 34f601e..4812992 100644
> > --- a/include/acpi/acoutput.h
> > +++ b/include/acpi/acoutput.h
> > @@ -223,10 +223,9 @@
> >
> > /*
> > * Error reporting. Callers module and line number are inserted by
> > AE_INFO,
> > - * the plist contains a set of parens to allow variable-length lists.
> > * These macros are used for both the debug and non-debug versions of
> > the code.
> > */
> > -#define ACPI_INFO(plist) acpi_info plist
> > +#define acpi_info(fmt, ...) _acpi_info(fmt, ##__VA_ARGS__)
> > #define ACPI_WARNING(plist) acpi_warning plist
> > #define ACPI_EXCEPTION(plist) acpi_exception plist
> > #define ACPI_ERROR(plist) acpi_error plist @@ -238,7
> > +237,8 @@
> >
> > /* No error messages */
> >
> > -#define ACPI_INFO(plist)
> > +#define acpi_info(fmt, ...) \
> > + do { if (0) _acpi_info(fmt, ##__VA_ARGS__); } while (0)
> > #define ACPI_WARNING(plist)
> > #define ACPI_EXCEPTION(plist)
> > #define ACPI_ERROR(plist)
> > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index
> > 1755697..f5f96ac 100644
> > --- a/include/acpi/acpixf.h
> > +++ b/include/acpi/acpixf.h
> > @@ -899,7 +899,7 @@ ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(3)
> > const char *format, ...))
> > ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
> > void ACPI_INTERNAL_VAR_XFACE
> > - acpi_info(const char *format, ...))
> > + _acpi_info(const char *format, ...))
> > ACPI_MSG_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(3)
> > void ACPI_INTERNAL_VAR_XFACE
> > acpi_bios_error(const char *module_name,
> _______________________________________________
> Devel mailing list
> Devel@xxxxxxxxxx
> https://lists.acpica.org/mailman/listinfo/devel