On Wed, 2012-07-18 at 15:41 -0600, Toshi Kani wrote: > On Wed, 2012-07-18 at 14:21 -0700, Joe Perches wrote: > > On Wed, 2012-07-18 at 14:40 -0600, Toshi Kani wrote: > > > This patch introduces acpi_pr_<level>(), where <level> is a message > > > level such as err/warn/info, to support improved logging messages > > > for ACPI, esp. in hotplug operations. acpi_pr_<level>() appends > > > "ACPI" prefix and ACPI object path to the messages. This improves > > > diagnostics in hotplug operations since it identifies an object that > > > caused an issue in a log file. [] > > I'd be tempted to instead make the calls more like > > other <subsystem>_<level> uses and rename these to > > acpi_<level> and change the existing acpi_info to > > another name. [] > I agree with you. Unfortunately, the ACPI CA (ACPI FW interpreter) > already uses them for its internal-use as follows, so I needed to come > up with some other name... Hence, acpi_pr_<level>. > > /* > * 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_WARNING(plist) acpi_warning plist > #define ACPI_EXCEPTION(plist) acpi_exception plist > #define ACPI_ERROR(plist) acpi_error plist > #define ACPI_DEBUG_OBJECT(obj,l,i) acpi_ex_do_debug_object(obj,l,i) I wouldn't have a problem renaming a few of those to something like: #define ACPI_INFO(plist) acpi_old_info plist #define ACPI_WARNING(plist) acpi_old_warning plist #define ACPI_ERROR(plist) acpi_old_error plist The acpi folk might though. > > Other than that, seems fine to me. > Great! Can I consider it as Ack? :) Fix the kfree first. I rarely ack stuff as other people generally have to pick up the changes and I think acks are overrated. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
