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-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
