Re: [PATCH 0/4] ACPI: hotplug messages improvement

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

On Wed, 2012-07-25 at 12:45 +0900, Yasuaki Ishimatsu wrote:
> Hi Toshi,
> 
> 2012/07/19 5:40, Toshi Kani wrote:
> > This patchset improves logging messages for ACPI CPU, Memory, and
> > Container hotplug notify handlers.  The patchset introduces a set of
> > new macro interfaces, acpi_pr_<level>(), and updates the notify
> > handlers to use them.  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.
> >
> 
> The log message looks good to me.

Hi Yasuaki,

Thanks for reviewing!

> But I could not understand when to use it instead of pr_{warn, info, ...}
> or ACPI_{WARNING, INFO, ...}. Do you have the policy?

acpi_pr_<level>() is used when ACPI device path is used to identify an
ACPI object for the message, such as error message to the object.  The
usage model is similar to dev_<level>(), which appends device object to
the message.  acpi_pr_<level>() is intended for ACPI drivers, and can be
used when device object is not created / valid, such as the case for
ACPI hotplug handlers.

pr_<level>() is the regular printk() interfaces with message level, and
continues to be used when ACPI device path does not have to be appended
to the message.  I expect non-error messages (such as boot-up messages)
continue to use pr_<level>().

ACPI_[WARNING|INFO|ERROR]() are ACPICA internal interfaces, and are not
intended for ACPI drivers.

Additionally, ACPI drivers can also use dev_<level>() when device object
is valid.  You find such examples in patch 2/4.

I will add more descriptions to the patchset.


> > ---
> > This patchset applies on top of the patch below.
> > 
> > [PATCH] ACPI: Add ACPI CPU hot-remove support
> > http://marc.info/?l=linux-acpi&m=134098193327362&w=2
> > 
> > ---
> > Toshi Kani (4):
> >   ACPI: Add acpi_pr_<level>() interfaces
> >   ACPI: Update CPU hotplug messages
> >   ACPI: Update Memory hotplug messages
> >   ACPI: Update Container hotplug messages
> 
> I think you need update other component, which are driver/acpi/{acpi_pad.c,
> battery.c, button.c}. Do you have the plan to update them?

I won't be ready to include them in this round, but will look at them
and change them later as necessary.  I need to make sure that I can test
these drivers when making such changes.

Thanks,
-Toshi


> 
> Thanks,
> Yasuaki Ishimatsu
> 
> > ---
> >   drivers/acpi/acpi_memhotplug.c  |   24 ++++++++++++------------
> >   drivers/acpi/container.c        |    6 +++---
> >   drivers/acpi/processor_driver.c |   36 +++++++++++++++++++++---------------
> >   drivers/acpi/utils.c            |   32 ++++++++++++++++++++++++++++++++
> >   include/acpi/acpi_bus.h         |   18 ++++++++++++++++++
> >   5 files changed, 86 insertions(+), 30 deletions(-)
> > 
> 
>