Re: [PATCH 1/4] ACPI: Add acpi_pr_() interfaces

Joe Perches
2012-07-18T21:54:16Z

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