| Message ID | 1471546104-28520-1-git-send-email-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 18, 2016 at 11:48 AM, Vishal Verma <vishal.l.verma@intel.com> wrote: > The nfit driver had an acpi event notification handler, but it never > would've worked because we weren't setting the > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. Let's update the changelog to be helpful for someone implementing a backport or taking this back to a -stable branch. Something like: Subject: acpi, nfit: fix event notifications Commit 209851649dc4 "acpi: nfit: Add support for hot-add" added support for _FIT notifications, but it neglected to set the ACPI_DRIVER_ALL_NOTIFY_EVENTS flag that acpi_bus_notify() uses to gate notification delivery. Fixes: 209851649dc4 ("acpi: nfit: Add support for hot-add")
On 8/18/2016 3:48 PM, Dan Williams wrote: > On Thu, Aug 18, 2016 at 11:48 AM, Vishal Verma <vishal.l.verma@intel.com> wrote: >> The nfit driver had an acpi event notification handler, but it never >> would've worked because we weren't setting the >> ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. > > Let's update the changelog to be helpful for someone implementing a > backport or taking this back to a -stable branch. Something like: > > Subject: acpi, nfit: fix event notifications > > Commit 209851649dc4 "acpi: nfit: Add support for hot-add" added > support for _FIT notifications, but it neglected to set the > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag that acpi_bus_notify() uses to gate > notification delivery. While we're at it, should we update the notifier function to explicitly check for event 0x80 before re-evaluating the _FIT? I'm thinking about some time in the future when there might be more than one event. -- ljk > > Fixes: 209851649dc4 ("acpi: nfit: Add support for hot-add") > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm >
On Thu, Aug 18, 2016 at 12:52 PM, Linda Knippers <linda.knippers@hpe.com> wrote: > > > On 8/18/2016 3:48 PM, Dan Williams wrote: >> On Thu, Aug 18, 2016 at 11:48 AM, Vishal Verma <vishal.l.verma@intel.com> wrote: >>> The nfit driver had an acpi event notification handler, but it never >>> would've worked because we weren't setting the >>> ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. >> >> Let's update the changelog to be helpful for someone implementing a >> backport or taking this back to a -stable branch. Something like: >> >> Subject: acpi, nfit: fix event notifications >> >> Commit 209851649dc4 "acpi: nfit: Add support for hot-add" added >> support for _FIT notifications, but it neglected to set the >> ACPI_DRIVER_ALL_NOTIFY_EVENTS flag that acpi_bus_notify() uses to gate >> notification delivery. > > While we're at it, should we update the notifier function to explicitly check > for event 0x80 before re-evaluating the _FIT? I'm thinking about some time > in the future when there might be more than one event. Yes, good idea.
On Thu, 2016-08-18 at 12:54 -0700, Dan Williams wrote: > On Thu, Aug 18, 2016 at 12:52 PM, Linda Knippers <linda.knippers@hpe.c > om> wrote: > > > > > > > > On 8/18/2016 3:48 PM, Dan Williams wrote: > > > > > > On Thu, Aug 18, 2016 at 11:48 AM, Vishal Verma <vishal.l.verma@int > > > el.com> wrote: > > > > > > > > The nfit driver had an acpi event notification handler, but it > > > > never > > > > would've worked because we weren't setting the > > > > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. > > > > > > Let's update the changelog to be helpful for someone implementing > > > a > > > backport or taking this back to a -stable branch. Something like: > > > > > > Subject: acpi, nfit: fix event notifications > > > > > > Commit 209851649dc4 "acpi: nfit: Add support for hot-add" added > > > support for _FIT notifications, but it neglected to set the > > > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag that acpi_bus_notify() uses to > > > gate > > > notification delivery. > > > > While we're at it, should we update the notifier function to > > explicitly check > > for event 0x80 before re-evaluating the _FIT? I'm thinking about > > some time > > in the future when there might be more than one event. > > Yes, good idea. Sounds good, part of the same patch or separately?
On Thu, Aug 18, 2016 at 1:22 PM, Verma, Vishal L <vishal.l.verma@intel.com> wrote: > On Thu, 2016-08-18 at 12:54 -0700, Dan Williams wrote: >> On Thu, Aug 18, 2016 at 12:52 PM, Linda Knippers <linda.knippers@hpe.c >> om> wrote: >> > >> > >> > >> > On 8/18/2016 3:48 PM, Dan Williams wrote: >> > > >> > > On Thu, Aug 18, 2016 at 11:48 AM, Vishal Verma <vishal.l.verma@int >> > > el.com> wrote: >> > > > >> > > > The nfit driver had an acpi event notification handler, but it >> > > > never >> > > > would've worked because we weren't setting the >> > > > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. >> > > >> > > Let's update the changelog to be helpful for someone implementing >> > > a >> > > backport or taking this back to a -stable branch. Something like: >> > > >> > > Subject: acpi, nfit: fix event notifications >> > > >> > > Commit 209851649dc4 "acpi: nfit: Add support for hot-add" added >> > > support for _FIT notifications, but it neglected to set the >> > > ACPI_DRIVER_ALL_NOTIFY_EVENTS flag that acpi_bus_notify() uses to >> > > gate >> > > notification delivery. >> > >> > While we're at it, should we update the notifier function to >> > explicitly check >> > for event 0x80 before re-evaluating the _FIT? I'm thinking about >> > some time >> > in the future when there might be more than one event. >> >> Yes, good idea. > > Sounds good, part of the same patch or separately? Separate. In general, if you would write the word "also" in a patch description, that's an indication to make a separate patch.
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 80cc7c0..5d33d81 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -2731,6 +2731,7 @@ MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids); static struct acpi_driver acpi_nfit_driver = { .name = KBUILD_MODNAME, .ids = acpi_nfit_ids, + .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS, .ops = { .add = acpi_nfit_add, .remove = acpi_nfit_remove,
The nfit driver had an acpi event notification handler, but it never would've worked because we weren't setting the ACPI_DRIVER_ALL_NOTIFY_EVENTS flag in acpi_driver. Cc: <stable@vger.kernel.org> Cc: <linux-acpi@vger.kernel.org> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- drivers/acpi/nfit/core.c | 1 + 1 file changed, 1 insertion(+)