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