Re: [PATCH v3 2/4] ACPI: Update CPU hotplug messages

[Bjorn Helgaas]

On Thu, Jul 26, 2012 at 8:39 PM, Toshi Kani <toshi.kani@xxxxxx> wrote:
> On Thu, 2012-07-26 at 13:23 -0600, Bjorn Helgaas wrote:
>> On Wed, Jul 25, 2012 at 5:12 PM, Toshi Kani <toshi.kani@xxxxxx> wrote:

>> > @@ -560,8 +565,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
>> >          */
>> >         if (per_cpu(processor_device_array, pr->id) != NULL &&
>> >             per_cpu(processor_device_array, pr->id) != device) {
>> > -               printk(KERN_WARNING "BIOS reported wrong ACPI id "
>> > -                       "for the processor\n");
>> > +               pr_warn("BIOS reported wrong ACPI id for the processor\n");
>>
>> And this.
>
> Changed to use dev_warn().

Is there additional information you could print here, like the pr->id?
 I don't understand the data structures here, so maybe there isn't.

>> > @@ -727,17 +731,19 @@ static void acpi_processor_hotplug_notify(acpi_handle handle,
>> >                                   "received ACPI_NOTIFY_EJECT_REQUEST\n"));
>> >
>> >                 if (acpi_bus_get_device(handle, &device)) {
>> > -                       pr_err(PREFIX "Device don't exist, dropping EJECT\n");
>> > +                       acpi_pr_err(handle,
>> > +                               "Device don't exist, dropping EJECT\n");
>> >                         break;
>> >                 }
>> >                 if (!acpi_driver_data(device)) {
>> > -                       pr_err(PREFIX "Driver data is NULL, dropping EJECT\n");
>> > +                       acpi_pr_err(handle,
>> > +                               "Driver data is NULL, dropping EJECT\n");
>>
>> And this.
>
> No change since it is called directly from the handler.

True, but by this point, we have a valid acpi_device *, don't we?  We
called acpi_driver_data(device), which requires "device" to be valid.

>> >                         break;
>> >                 }
>> >
>> >                 ej_event = kmalloc(sizeof(*ej_event), GFP_KERNEL);
>> >                 if (!ej_event) {
>> > -                       pr_err(PREFIX "No memory, dropping EJECT\n");
>> > +                       acpi_pr_err(handle, "No memory, dropping EJECT\n");
>>
>> And this.
>
> No change since it is called directly from the handler.
>
>> >                         break;
>> >                 }
>> >
>> > @@ -847,7 +853,7 @@ static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr)
>> >          * and do it when the CPU gets online the first time
>> >          * TBD: Cleanup above functions and try to do this more elegant.
>> >          */
>> > -       printk(KERN_INFO "CPU %d got hotplugged\n", pr->id);
>> > +       pr_info("CPU %d got hotplugged\n", pr->id);
>>
>> And this.  The caller (acpi_processor_get_info()) has an acpi_device
>> *, so we should be able to use it here.
>
> I think pr_info() is fine since it is a normal message and already has
> CPU number in the message.

Is there another message that correlates the device name
("ACPI0007:xx") with the CPU number?  That correlation seems useful.
My mindset is that a driver should *always* use dev_<level>() when
possible, but I won't belabor the point.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/