From: Bjorn Helgaas <bhelgaas@google.com> To: Toshi Kani <toshi.kani@hp.com> Cc: lenb@kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] ACPI: Add _OST support for ACPI CPU hotplug Date: Thu, 26 Apr 2012 09:22:59 -0600 [thread overview] Message-ID: <CAErSpo5n22TLo+U3f_zHdkZSG=e7x4bySFPWtxT+ghYyenGc6A@mail.gmail.com> (raw) In-Reply-To: <1334096510-17319-4-git-send-email-toshi.kani@hp.com> On Tue, Apr 10, 2012 at 4:21 PM, Toshi Kani <toshi.kani@hp.com> wrote: > Changed acpi_processor_hotplug_notify() to call ACPI _OST method > when ACPI-based CPU hotplug operation completed. > > Signed-off-by: Toshi Kani <toshi.kani@hp.com> > --- > drivers/acpi/processor_driver.c | 28 ++++++++++++++++++++-------- > 1 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > index 0734086..5d9b5a8 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -701,9 +701,9 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > { > struct acpi_processor *pr; > struct acpi_device *device = NULL; > + u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */ > int result; > > - > switch (event) { > case ACPI_NOTIFY_BUS_CHECK: > case ACPI_NOTIFY_DEVICE_CHECK: > @@ -715,14 +715,18 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > if (!is_processor_present(handle)) > break; > > - if (acpi_bus_get_device(handle, &device)) { > - result = acpi_processor_device_add(handle, &device); > - if (result) > - printk(KERN_ERR PREFIX > - "Unable to add the device\n"); > + if (!acpi_bus_get_device(handle, &device)) > + break; > + > + result = acpi_processor_device_add(handle, &device); > + if (result) { > + printk(KERN_ERR PREFIX "Unable to add the device\n"); You didn't add this problem, but since you're touching the code, I'll point it out. This message will look to the user like: ACPI: Unable to add the device which is useless. The user has no idea what device we're talking about or why we can't add it, so he can't *do* anything with the message. Almost every printk of a constant string has this problem. Using dev_err() instead of printk() would help a lot, as would including the error code (result). Of course, this would be a separate patch that would update all the messages in the driver at the same time. > break; > } > + > + ost_code = ACPI_OST_SC_SUCCESS; > break; > + > case ACPI_NOTIFY_EJECT_REQUEST: > ACPI_DEBUG_PRINT((ACPI_DB_INFO, > "received ACPI_NOTIFY_EJECT_REQUEST\n")); > @@ -736,15 +740,23 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > if (!pr) { > printk(KERN_ERR PREFIX > "Driver data is NULL, dropping EJECT\n"); > - return; > + break; > } > + > + /* REVISIT: update when eject is supported */ > + ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED; > break; > + > default: > ACPI_DEBUG_PRINT((ACPI_DB_INFO, > "Unsupported event [0x%x]\n", event)); > - break; > + > + /* non-hotplug event; possibly handled by other handler */ > + return; > } > > + /* Inform firmware that the hotplug operation has completed */ > + (void) acpi_evaluate_ost(handle, event, ost_code, NULL); > return; > } > > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <bhelgaas@google.com> To: Toshi Kani <toshi.kani@hp.com> Cc: lenb@kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] ACPI: Add _OST support for ACPI CPU hotplug Date: Thu, 26 Apr 2012 09:22:59 -0600 [thread overview] Message-ID: <CAErSpo5n22TLo+U3f_zHdkZSG=e7x4bySFPWtxT+ghYyenGc6A@mail.gmail.com> (raw) In-Reply-To: <1334096510-17319-4-git-send-email-toshi.kani@hp.com> On Tue, Apr 10, 2012 at 4:21 PM, Toshi Kani <toshi.kani@hp.com> wrote: > Changed acpi_processor_hotplug_notify() to call ACPI _OST method > when ACPI-based CPU hotplug operation completed. > > Signed-off-by: Toshi Kani <toshi.kani@hp.com> > --- > drivers/acpi/processor_driver.c | 28 ++++++++++++++++++++-------- > 1 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > index 0734086..5d9b5a8 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -701,9 +701,9 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > { > struct acpi_processor *pr; > struct acpi_device *device = NULL; > + u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE; /* default */ > int result; > > - > switch (event) { > case ACPI_NOTIFY_BUS_CHECK: > case ACPI_NOTIFY_DEVICE_CHECK: > @@ -715,14 +715,18 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > if (!is_processor_present(handle)) > break; > > - if (acpi_bus_get_device(handle, &device)) { > - result = acpi_processor_device_add(handle, &device); > - if (result) > - printk(KERN_ERR PREFIX > - "Unable to add the device\n"); > + if (!acpi_bus_get_device(handle, &device)) > + break; > + > + result = acpi_processor_device_add(handle, &device); > + if (result) { > + printk(KERN_ERR PREFIX "Unable to add the device\n"); You didn't add this problem, but since you're touching the code, I'll point it out. This message will look to the user like: ACPI: Unable to add the device which is useless. The user has no idea what device we're talking about or why we can't add it, so he can't *do* anything with the message. Almost every printk of a constant string has this problem. Using dev_err() instead of printk() would help a lot, as would including the error code (result). Of course, this would be a separate patch that would update all the messages in the driver at the same time. > break; > } > + > + ost_code = ACPI_OST_SC_SUCCESS; > break; > + > case ACPI_NOTIFY_EJECT_REQUEST: > ACPI_DEBUG_PRINT((ACPI_DB_INFO, > "received ACPI_NOTIFY_EJECT_REQUEST\n")); > @@ -736,15 +740,23 @@ static void acpi_processor_hotplug_notify(acpi_handle handle, > if (!pr) { > printk(KERN_ERR PREFIX > "Driver data is NULL, dropping EJECT\n"); > - return; > + break; > } > + > + /* REVISIT: update when eject is supported */ > + ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED; > break; > + > default: > ACPI_DEBUG_PRINT((ACPI_DB_INFO, > "Unsupported event [0x%x]\n", event)); > - break; > + > + /* non-hotplug event; possibly handled by other handler */ > + return; > } > > + /* Inform firmware that the hotplug operation has completed */ > + (void) acpi_evaluate_ost(handle, event, ost_code, NULL); > return; > } > > -- > 1.7.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-04-26 15:23 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-04-10 22:21 [PATCH 0/5] ACPI: Add _OST support for ACPI hotplug Toshi Kani 2012-04-10 22:21 ` [PATCH 1/4] ACPI: Set hotplug _OST support bit to _OSC Toshi Kani 2012-04-26 15:16 ` Bjorn Helgaas 2012-04-26 15:16 ` Bjorn Helgaas 2012-04-26 17:10 ` Toshi Kani 2012-04-27 22:05 ` Toshi Kani 2012-05-02 21:20 ` Toshi Kani 2012-04-10 22:21 ` [PATCH 2/4] ACPI: Add acpi_evaluate_ost() for calling _OST Toshi Kani 2012-04-10 22:21 ` [PATCH 3/4] ACPI: Add _OST support for ACPI CPU hotplug Toshi Kani 2012-04-26 15:22 ` Bjorn Helgaas [this message] 2012-04-26 15:22 ` Bjorn Helgaas 2012-04-26 17:20 ` Toshi Kani 2012-04-10 22:21 ` [PATCH 4/4] ACPI: Add _OST support for ACPI memory hotplug Toshi Kani 2012-04-11 16:33 ` [PATCH 0/5] ACPI: Add _OST support for ACPI hotplug Shuah Khan 2012-04-11 18:50 ` Toshi Kani 2012-04-12 14:19 ` Shuah Khan 2012-04-13 14:24 ` Jiang Liu 2012-04-13 15:23 ` Shuah Khan 2012-04-13 16:05 ` Toshi Kani 2012-04-13 18:34 ` Shuah Khan 2012-04-16 18:24 ` Toshi Kani
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAErSpo5n22TLo+U3f_zHdkZSG=e7x4bySFPWtxT+ghYyenGc6A@mail.gmail.com' \ --to=bhelgaas@google.com \ --cc=lenb@kernel.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=toshi.kani@hp.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.