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

[Toshi Kani <toshi.kani@hp.com>]

On Wed, 2012-07-18 at 14:54 -0700, Joe Perches wrote:
> 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.

Hi Joe,

ACPI CA is being developed by Intel as OS-neutral code, and is used by
multiple OSes including Linux.  So, I am not sure how easy to make such
changes.  I am copying to Lin Ming.


> > > Other than that, seems fine to me.
> > Great!  Can I consider it as Ack? :)
> 
> Fix the kfree first.

Please see my other email.  Do you think the check should be added
despite of the warning message?


> I rarely ack stuff as other people generally have to
> pick up the changes and I think acks are overrated.

That's fair enough.

Thanks!
-Toshi


> cheers, Joe
>