* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 13:54 [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed() Andy Shevchenko
@ 2023-10-02 15:46 ` Dave Jiang
2023-10-02 20:27 ` Wilczynski, Michal
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2023-10-02 15:46 UTC (permalink / raw)
To: Andy Shevchenko, Michal Wilczynski, nvdimm, linux-acpi,
linux-kernel
Cc: Dan Williams, Vishal Verma, Ira Weiny, Rafael J. Wysocki,
Len Brown
On 10/2/23 06:54, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/acpi/nfit/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f96bf32cd368..280da408c02c 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
> if ((nfit_mem->dsm_mask & (1 << func)) == 0)
> return;
>
> - out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
> - if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> - || out_obj->buffer.length < sizeof(smart)) {
> + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, &in_obj, ACPI_TYPE_BUFFER);
> + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> dev_name(dev));
> ACPI_FREE(out_obj);
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 13:54 [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed() Andy Shevchenko
2023-10-02 15:46 ` Dave Jiang
@ 2023-10-02 20:27 ` Wilczynski, Michal
2023-10-03 8:48 ` Andy Shevchenko
2023-10-20 1:02 ` Dan Williams
2023-10-19 15:36 ` Andy Shevchenko
2023-10-20 1:03 ` Dan Williams
3 siblings, 2 replies; 11+ messages in thread
From: Wilczynski, Michal @ 2023-10-02 20:27 UTC (permalink / raw)
To: Andy Shevchenko, nvdimm, linux-acpi, linux-kernel
Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
Rafael J. Wysocki, Len Brown
On 10/2/2023 3:54 PM, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/acpi/nfit/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f96bf32cd368..280da408c02c 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
> if ((nfit_mem->dsm_mask & (1 << func)) == 0)
> return;
>
> - out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
> - if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> - || out_obj->buffer.length < sizeof(smart)) {
> + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, &in_obj, ACPI_TYPE_BUFFER);
This line is 90 characters long, wouldn't it be better to split it ?
> + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> dev_name(dev));
While at it maybe fix alignment ? :-)
> ACPI_FREE(out_obj);
Just nitpicks, functionally code seems correct to me.
Reviewed-by: Michal Wilczynski <michal.wilczynski@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 20:27 ` Wilczynski, Michal
@ 2023-10-03 8:48 ` Andy Shevchenko
2023-10-20 1:02 ` Dan Williams
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2023-10-03 8:48 UTC (permalink / raw)
To: Wilczynski, Michal
Cc: nvdimm, linux-acpi, linux-kernel, Dan Williams, Vishal Verma,
Dave Jiang, Ira Weiny, Rafael J. Wysocki, Len Brown
On Mon, Oct 02, 2023 at 10:27:02PM +0200, Wilczynski, Michal wrote:
> On 10/2/2023 3:54 PM, Andy Shevchenko wrote:
...
> > + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, &in_obj, ACPI_TYPE_BUFFER);
>
> This line is 90 characters long, wouldn't it be better to split it ?
I dunno it's a problem, but if people insist, I can redo that.
...
> > + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> > dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> > dev_name(dev));
>
> While at it maybe fix alignment ? :-)
I don't think it's in scope of this change.
> > ACPI_FREE(out_obj);
>
> Just nitpicks, functionally code seems correct to me.
> Reviewed-by: Michal Wilczynski <michal.wilczynski@intel.com>
Thank you!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 20:27 ` Wilczynski, Michal
2023-10-03 8:48 ` Andy Shevchenko
@ 2023-10-20 1:02 ` Dan Williams
1 sibling, 0 replies; 11+ messages in thread
From: Dan Williams @ 2023-10-20 1:02 UTC (permalink / raw)
To: Wilczynski, Michal, Andy Shevchenko, nvdimm, linux-acpi,
linux-kernel
Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
Rafael J. Wysocki, Len Brown
Wilczynski, Michal wrote:
>
>
> On 10/2/2023 3:54 PM, Andy Shevchenko wrote:
> > The acpi_evaluate_dsm_typed() provides a way to check the type of the
> > object evaluated by _DSM call. Use it instead of open coded variant.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > drivers/acpi/nfit/core.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> > index f96bf32cd368..280da408c02c 100644
> > --- a/drivers/acpi/nfit/core.c
> > +++ b/drivers/acpi/nfit/core.c
> > @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
> > if ((nfit_mem->dsm_mask & (1 << func)) == 0)
> > return;
> >
> > - out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj);
> > - if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> > - || out_obj->buffer.length < sizeof(smart)) {
> > + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, &in_obj, ACPI_TYPE_BUFFER);
>
> This line is 90 characters long, wouldn't it be better to split it ?
100 characters is allowed these days.
> > + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> > dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> > dev_name(dev));
>
> While at it maybe fix alignment ? :-)
Life is too short to fiddle with indentation. For old code leave it
alone, for new code just use clang-format.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 13:54 [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed() Andy Shevchenko
2023-10-02 15:46 ` Dave Jiang
2023-10-02 20:27 ` Wilczynski, Michal
@ 2023-10-19 15:36 ` Andy Shevchenko
2023-10-20 1:03 ` Dan Williams
3 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2023-10-19 15:36 UTC (permalink / raw)
To: Michal Wilczynski, nvdimm, linux-acpi, linux-kernel
Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
Rafael J. Wysocki, Len Brown
On Mon, Oct 02, 2023 at 04:54:58PM +0300, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
Dan, do you have any comments?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-02 13:54 [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed() Andy Shevchenko
` (2 preceding siblings ...)
2023-10-19 15:36 ` Andy Shevchenko
@ 2023-10-20 1:03 ` Dan Williams
2023-11-20 15:03 ` Andy Shevchenko
3 siblings, 1 reply; 11+ messages in thread
From: Dan Williams @ 2023-10-20 1:03 UTC (permalink / raw)
To: Andy Shevchenko, Michal Wilczynski, nvdimm, linux-acpi,
linux-kernel
Cc: Dan Williams, Vishal Verma, Dave Jiang, Ira Weiny,
Rafael J. Wysocki, Len Brown, Andy Shevchenko
Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
Looks good to me.
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-10-20 1:03 ` Dan Williams
@ 2023-11-20 15:03 ` Andy Shevchenko
2023-11-20 15:11 ` Rafael J. Wysocki
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2023-11-20 15:03 UTC (permalink / raw)
To: Dan Williams
Cc: Michal Wilczynski, nvdimm, linux-acpi, linux-kernel, Vishal Verma,
Dave Jiang, Ira Weiny, Rafael J. Wysocki, Len Brown
On Thu, Oct 19, 2023 at 06:03:28PM -0700, Dan Williams wrote:
> Andy Shevchenko wrote:
> > The acpi_evaluate_dsm_typed() provides a way to check the type of the
> > object evaluated by _DSM call. Use it instead of open coded variant.
>
> Looks good to me.
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Thank you!
Who is taking care of this? Rafael?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
2023-11-20 15:03 ` Andy Shevchenko
@ 2023-11-20 15:11 ` Rafael J. Wysocki
[not found] ` <ZVuVMNlfumQ4p6oM@smile.fi.intel.com>
0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2023-11-20 15:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dan Williams, Michal Wilczynski, nvdimm, linux-acpi, linux-kernel,
Vishal Verma, Dave Jiang, Ira Weiny, Rafael J. Wysocki, Len Brown
On Mon, Nov 20, 2023 at 4:03 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 19, 2023 at 06:03:28PM -0700, Dan Williams wrote:
> > Andy Shevchenko wrote:
> > > The acpi_evaluate_dsm_typed() provides a way to check the type of the
> > > object evaluated by _DSM call. Use it instead of open coded variant.
> >
> > Looks good to me.
> >
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
>
> Thank you!
>
> Who is taking care of this? Rafael?
I can apply it.
^ permalink raw reply [flat|nested] 11+ messages in thread