diff mbox series

[v1,1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()

Message ID 20231002135458.2603293-1-andriy.shevchenko@linux.intel.com
State Accepted
Commit 26da9a8d279f30f1d0aa33cea0003a3d37fa051c
Headers show
Series [v1,1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed() | expand

Commit Message

Andy Shevchenko Oct. 2, 2023, 1:54 p.m. UTC
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(-)

Comments

Michal Wilczynski Oct. 2, 2023, 8:27 p.m. UTC | #1
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>
Andy Shevchenko Oct. 3, 2023, 8:48 a.m. UTC | #2
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!
Andy Shevchenko Oct. 19, 2023, 3:36 p.m. UTC | #3
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?
Dan Williams Oct. 20, 2023, 1:02 a.m. UTC | #4
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.
Dan Williams Oct. 20, 2023, 1:03 a.m. UTC | #5
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>
Andy Shevchenko Nov. 20, 2023, 3:03 p.m. UTC | #6
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?
Rafael J. Wysocki Nov. 20, 2023, 3:11 p.m. UTC | #7
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.
Andy Shevchenko Feb. 2, 2024, 3:49 p.m. UTC | #8
On Mon, Nov 20, 2023 at 07:19:44PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 20, 2023 at 04:11:54PM +0100, Rafael J. Wysocki wrote:
> > 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.
> 
> Would be nice, thank you!

Any news on this?
Rafael J. Wysocki Feb. 2, 2024, 4:33 p.m. UTC | #9
On Fri, Feb 2, 2024 at 4:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Nov 20, 2023 at 07:19:44PM +0200, Andy Shevchenko wrote:
> > On Mon, Nov 20, 2023 at 04:11:54PM +0100, Rafael J. Wysocki wrote:
> > > 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.
> >
> > Would be nice, thank you!
>
> Any news on this?

Fell through the cracks, sorry about that.

Applied now (as 6.9 material).

Thanks!
diff mbox series

Patch

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);