* [PATCH] Install Notify() handler before getting NFIT table
@ 2023-10-19 13:01 chenxiang
2023-10-19 15:43 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: chenxiang @ 2023-10-19 13:01 UTC (permalink / raw
To: michal.wilczynski, rafael; +Cc: linuxarm, linux-acpi, nvdimm, Xiang Chen
From: Xiang Chen <chenxiang66@hisilicon.com>
If there is no NFIT at startup, it will return 0 immediately in function
acpi_nfit_add() and will not install Notify() handler. If hotplugging
a nvdimm device later, it will not be identified as there is no Notify()
handler.
So move handler installing before getting NFI table in function
acpi_nfit_add() to avoid above issue.
Fixes: dcca12ab62a2 ("ACPI: NFIT: Install Notify() handler directly")
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
---
drivers/acpi/nfit/core.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 3826f49..9923855 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -3339,6 +3339,16 @@ static int acpi_nfit_add(struct acpi_device *adev)
acpi_size sz;
int rc = 0;
+ rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
+ acpi_nfit_notify, adev);
+ if (rc)
+ return rc;
+
+ rc = devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
+ adev);
+ if (rc)
+ return rc;
+
status = acpi_get_table(ACPI_SIG_NFIT, 0, &tbl);
if (ACPI_FAILURE(status)) {
/* The NVDIMM root device allows OS to trigger enumeration of
@@ -3386,17 +3396,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
if (rc)
return rc;
- rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
- if (rc)
- return rc;
-
- rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
- acpi_nfit_notify, adev);
- if (rc)
- return rc;
-
- return devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
- adev);
+ return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
}
static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
--
2.8.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Install Notify() handler before getting NFIT table
2023-10-19 13:01 [PATCH] Install Notify() handler before getting NFIT table chenxiang
@ 2023-10-19 15:43 ` Rafael J. Wysocki
2023-10-20 20:26 ` Ira Weiny
0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2023-10-19 15:43 UTC (permalink / raw
To: chenxiang, Dan Williams
Cc: michal.wilczynski, rafael, linuxarm, linux-acpi, nvdimm
On Thu, Oct 19, 2023 at 2:57 PM chenxiang <chenxiang66@hisilicon.com> wrote:
>
> From: Xiang Chen <chenxiang66@hisilicon.com>
>
> If there is no NFIT at startup, it will return 0 immediately in function
> acpi_nfit_add() and will not install Notify() handler. If hotplugging
> a nvdimm device later, it will not be identified as there is no Notify()
> handler.
Yes, this is a change in behavior that shouldn't have been made.
> So move handler installing before getting NFI table in function
> acpi_nfit_add() to avoid above issue.
And the fix is correct if I'm not mistaken.
I can still queue it up for 6.6 if that's fine with everyone. Dan?
> Fixes: dcca12ab62a2 ("ACPI: NFIT: Install Notify() handler directly")
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> ---
> drivers/acpi/nfit/core.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 3826f49..9923855 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -3339,6 +3339,16 @@ static int acpi_nfit_add(struct acpi_device *adev)
> acpi_size sz;
> int rc = 0;
>
> + rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> + acpi_nfit_notify, adev);
> + if (rc)
> + return rc;
> +
> + rc = devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> + adev);
> + if (rc)
> + return rc;
> +
> status = acpi_get_table(ACPI_SIG_NFIT, 0, &tbl);
> if (ACPI_FAILURE(status)) {
> /* The NVDIMM root device allows OS to trigger enumeration of
> @@ -3386,17 +3396,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
> if (rc)
> return rc;
>
> - rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> - if (rc)
> - return rc;
> -
> - rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> - acpi_nfit_notify, adev);
> - if (rc)
> - return rc;
> -
> - return devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> - adev);
> + return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> }
>
> static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
> --
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Install Notify() handler before getting NFIT table
2023-10-19 15:43 ` Rafael J. Wysocki
@ 2023-10-20 20:26 ` Ira Weiny
2023-10-22 10:53 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Ira Weiny @ 2023-10-20 20:26 UTC (permalink / raw
To: Rafael J. Wysocki, chenxiang, Williams, Dan J
Cc: Wilczynski, Michal, rafael@kernel.org, linuxarm@huawei.com,
linux-acpi@vger.kernel.org, nvdimm@lists.linux.dev
Rafael J. Wysocki wrote:
> On Thu, Oct 19, 2023 at 2:57 PM chenxiang <chenxiang66@hisilicon.com> wrote:
> >
> > From: Xiang Chen <chenxiang66@hisilicon.com>
> >
> > If there is no NFIT at startup, it will return 0 immediately in function
> > acpi_nfit_add() and will not install Notify() handler. If hotplugging
> > a nvdimm device later, it will not be identified as there is no Notify()
> > handler.
>
> Yes, this is a change in behavior that shouldn't have been made.
>
> > So move handler installing before getting NFI table in function
> > acpi_nfit_add() to avoid above issue.
>
> And the fix is correct if I'm not mistaken.
>
> I can still queue it up for 6.6 if that's fine with everyone. Dan?
That is fine with me. Vishal, Dave Jiang, and I are wrangling the nvdimm
tree these days. I've prepared 6.7 already so I'll ignore this.
Ira
>
> > Fixes: dcca12ab62a2 ("ACPI: NFIT: Install Notify() handler directly")
> > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> > ---
> > drivers/acpi/nfit/core.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > index 3826f49..9923855 100644
> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -3339,6 +3339,16 @@ static int acpi_nfit_add(struct acpi_device *adev)
> > acpi_size sz;
> > int rc = 0;
> >
> > + rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> > + acpi_nfit_notify, adev);
> > + if (rc)
> > + return rc;
> > +
> > + rc = devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> > + adev);
> > + if (rc)
> > + return rc;
> > +
> > status = acpi_get_table(ACPI_SIG_NFIT, 0, &tbl);
> > if (ACPI_FAILURE(status)) {
> > /* The NVDIMM root device allows OS to trigger enumeration of
> > @@ -3386,17 +3396,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
> > if (rc)
> > return rc;
> >
> > - rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> > - if (rc)
> > - return rc;
> > -
> > - rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> > - acpi_nfit_notify, adev);
> > - if (rc)
> > - return rc;
> > -
> > - return devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> > - adev);
> > + return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> > }
> >
> > static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
> > --
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Install Notify() handler before getting NFIT table
2023-10-20 20:26 ` Ira Weiny
@ 2023-10-22 10:53 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2023-10-22 10:53 UTC (permalink / raw
To: Ira Weiny
Cc: Rafael J. Wysocki, chenxiang, Williams, Dan J, Wilczynski, Michal,
linuxarm@huawei.com, linux-acpi@vger.kernel.org,
nvdimm@lists.linux.dev
On Fri, Oct 20, 2023 at 10:27 PM Ira Weiny <ira.weiny@intel.com> wrote:
>
> Rafael J. Wysocki wrote:
> > On Thu, Oct 19, 2023 at 2:57 PM chenxiang <chenxiang66@hisilicon.com> wrote:
> > >
> > > From: Xiang Chen <chenxiang66@hisilicon.com>
> > >
> > > If there is no NFIT at startup, it will return 0 immediately in function
> > > acpi_nfit_add() and will not install Notify() handler. If hotplugging
> > > a nvdimm device later, it will not be identified as there is no Notify()
> > > handler.
> >
> > Yes, this is a change in behavior that shouldn't have been made.
> >
> > > So move handler installing before getting NFI table in function
> > > acpi_nfit_add() to avoid above issue.
> >
> > And the fix is correct if I'm not mistaken.
> >
> > I can still queue it up for 6.6 if that's fine with everyone. Dan?
>
> That is fine with me. Vishal, Dave Jiang, and I are wrangling the nvdimm
> tree these days. I've prepared 6.7 already so I'll ignore this.
Applied now (with some minor edits in the subject and changelog).
Thanks!
> > > Fixes: dcca12ab62a2 ("ACPI: NFIT: Install Notify() handler directly")
> > > Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> > > ---
> > > drivers/acpi/nfit/core.c | 22 +++++++++++-----------
> > > 1 file changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > > index 3826f49..9923855 100644
> > > --- a/drivers/acpi/nfit/core.c
> > > +++ b/drivers/acpi/nfit/core.c
> > > @@ -3339,6 +3339,16 @@ static int acpi_nfit_add(struct acpi_device *adev)
> > > acpi_size sz;
> > > int rc = 0;
> > >
> > > + rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> > > + acpi_nfit_notify, adev);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + rc = devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> > > + adev);
> > > + if (rc)
> > > + return rc;
> > > +
> > > status = acpi_get_table(ACPI_SIG_NFIT, 0, &tbl);
> > > if (ACPI_FAILURE(status)) {
> > > /* The NVDIMM root device allows OS to trigger enumeration of
> > > @@ -3386,17 +3396,7 @@ static int acpi_nfit_add(struct acpi_device *adev)
> > > if (rc)
> > > return rc;
> > >
> > > - rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> > > - if (rc)
> > > - return rc;
> > > -
> > > - rc = acpi_dev_install_notify_handler(adev, ACPI_DEVICE_NOTIFY,
> > > - acpi_nfit_notify, adev);
> > > - if (rc)
> > > - return rc;
> > > -
> > > - return devm_add_action_or_reset(dev, acpi_nfit_remove_notify_handler,
> > > - adev);
> > > + return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc);
> > > }
> > >
> > > static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle)
> > > --
> >
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-22 10:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-19 13:01 [PATCH] Install Notify() handler before getting NFIT table chenxiang
2023-10-19 15:43 ` Rafael J. Wysocki
2023-10-20 20:26 ` Ira Weiny
2023-10-22 10:53 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).