Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752668AbcC2Peo (ORCPT ); Tue, 29 Mar 2016 11:34:44 -0400 Received: from mga14.intel.com ([192.55.52.115]:8495 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751877AbcC2Pem convert rfc822-to-8bit (ORCPT ); Tue, 29 Mar 2016 11:34:42 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,411,1455004800"; d="scan'208";a="773916303" From: "Moore, Robert" To: "Moore, Robert" , Joe Perches , "Zheng, Lv" , "Wysocki, Rafael J" , Len Brown CC: "linux-acpi@vger.kernel.org" , linux-kernel , "devel@acpica.org" Subject: RE: [RFC PATCH] acpi: Use a more normal logging style for ACPI_ calls Thread-Topic: [RFC PATCH] acpi: Use a more normal logging style for ACPI_ calls Thread-Index: AQHRiWtYgz+muFARtkyMNyQ54xfnyJ9wi4RwgAACe1A= Date: Tue, 29 Mar 2016 15:34:39 +0000 Message-ID: <94F2FBAB4432B54E8AACC7DFDE6C92E37E4553F8@ORSMSX110.amr.corp.intel.com> References: <1459222225.25110.58.camel@perches.com> <94F2FBAB4432B54E8AACC7DFDE6C92E37E4553D5@ORSMSX110.amr.corp.intel.com> In-Reply-To: <94F2FBAB4432B54E8AACC7DFDE6C92E37E4553D5@ORSMSX110.amr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYThlM2QxNGUtNmNmZS00MmViLWJhNmQtYjc4ODE4NThiYzBhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik03SHpQNDlYb25aS1NtTjhuTDUwWFRsSGt2dGw5NlBpMXF0QTBHNXlEem89In0= x-ctpclassification: CTP_IC x-originating-ip: [10.22.254.140] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17471 Lines: 459 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@acpica.org] 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@vger.kernel.org; linux-kernel; devel@acpica.org > Subject: Re: [Devel] [RFC PATCH] acpi: Use a more normal logging style for > ACPI_ 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@perches.com] > > Sent: Monday, March 28, 2016 8:30 PM > > To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown > > Cc: linux-acpi@vger.kernel.org; devel@acpica.org; linux-kernel > > Subject: [RFC PATCH] acpi: Use a more normal logging style for > > ACPI_ calls > > > > This is just an example of a conversion of ACPI_INFO to a more typical > > kernel use style. All of the other ACPI_ 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_ 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@acpica.org > https://lists.acpica.org/mailman/listinfo/devel