LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] ACPI: DPTF: Fix reading of attributes
@ 2021-07-27 16:18 Srinivas Pandruvada
  2021-07-28 16:42 ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Srinivas Pandruvada @ 2021-07-27 16:18 UTC (permalink / raw)
  To: rjw, lenb; +Cc: linux-acpi, linux-kernel, srinivas.pandruvada

The current assumption that methods to read PCH FIVR attributes will
return integer, is not correct. There is no good way to return integer
as negative numbers are also valid.

These read methods return a package of integers. The first integer returns
status, which is 0 on success and any other value for failure. When the
returned status is zero, then the second integer returns the actual value.

This change fixes this issue by replacing acpi_evaluate_integer() with
acpi_evaluate_object() and use acpi_extract_package() to extract results.

Fixes: 2ce6324eadb01 ("ACPI: DPTF: Add PCH FIVR participant driver")
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: stable@vger.kernel.org # 5.10+
---
 drivers/acpi/dptf/dptf_pch_fivr.c | 51 ++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/dptf/dptf_pch_fivr.c b/drivers/acpi/dptf/dptf_pch_fivr.c
index 5fca18296bf6..550b9081fcbc 100644
--- a/drivers/acpi/dptf/dptf_pch_fivr.c
+++ b/drivers/acpi/dptf/dptf_pch_fivr.c
@@ -9,6 +9,42 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
+struct pch_fivr_resp {
+	u64 status;
+	u64 result;
+};
+
+static int pch_fivr_read(acpi_handle handle, char *method, struct pch_fivr_resp *fivr_resp)
+{
+	struct acpi_buffer resp = { sizeof(struct pch_fivr_resp), fivr_resp};
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer format = { sizeof("NN"), "NN" };
+	union acpi_object *obj;
+	acpi_status status;
+	int ret = -EFAULT;
+
+	status = acpi_evaluate_object(handle, method, NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		return ret;
+
+	obj = buffer.pointer;
+	if (!obj || obj->type != ACPI_TYPE_PACKAGE)
+		goto release_buffer;
+
+	status = acpi_extract_package(obj, &format, &resp);
+	if (ACPI_FAILURE(status))
+		goto release_buffer;
+
+	if (fivr_resp->status)
+		goto release_buffer;
+
+	ret = 0;
+
+release_buffer:
+	kfree(buffer.pointer);
+	return ret;
+}
+
 /*
  * Presentation of attributes which are defined for INT1045
  * They are:
@@ -23,15 +59,14 @@ static ssize_t name##_show(struct device *dev,\
 			   char *buf)\
 {\
 	struct acpi_device *acpi_dev = dev_get_drvdata(dev);\
-	unsigned long long val;\
-	acpi_status status;\
+	struct pch_fivr_resp fivr_resp;\
+	int status;\
 \
-	status = acpi_evaluate_integer(acpi_dev->handle, #method,\
-				       NULL, &val);\
-	if (ACPI_SUCCESS(status))\
-		return sprintf(buf, "%d\n", (int)val);\
-	else\
-		return -EINVAL;\
+	status = pch_fivr_read(acpi_dev->handle, #method, &fivr_resp);\
+	if (status)\
+		return status;\
+\
+	return sprintf(buf, "%llu\n", fivr_resp.result);\
 }
 
 #define PCH_FIVR_STORE(name, method) \
-- 
2.31.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ACPI: DPTF: Fix reading of attributes
  2021-07-27 16:18 [PATCH] ACPI: DPTF: Fix reading of attributes Srinivas Pandruvada
@ 2021-07-28 16:42 ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2021-07-28 16:42 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List

On Tue, Jul 27, 2021 at 6:18 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> The current assumption that methods to read PCH FIVR attributes will
> return integer, is not correct. There is no good way to return integer
> as negative numbers are also valid.
>
> These read methods return a package of integers. The first integer returns
> status, which is 0 on success and any other value for failure. When the
> returned status is zero, then the second integer returns the actual value.
>
> This change fixes this issue by replacing acpi_evaluate_integer() with
> acpi_evaluate_object() and use acpi_extract_package() to extract results.
>
> Fixes: 2ce6324eadb01 ("ACPI: DPTF: Add PCH FIVR participant driver")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: stable@vger.kernel.org # 5.10+

Applied as 5.14-rc material.

I'll apply the [2/2[ when this one is merged.

> ---
>  drivers/acpi/dptf/dptf_pch_fivr.c | 51 ++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/dptf/dptf_pch_fivr.c b/drivers/acpi/dptf/dptf_pch_fivr.c
> index 5fca18296bf6..550b9081fcbc 100644
> --- a/drivers/acpi/dptf/dptf_pch_fivr.c
> +++ b/drivers/acpi/dptf/dptf_pch_fivr.c
> @@ -9,6 +9,42 @@
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>
> +struct pch_fivr_resp {
> +       u64 status;
> +       u64 result;
> +};
> +
> +static int pch_fivr_read(acpi_handle handle, char *method, struct pch_fivr_resp *fivr_resp)
> +{
> +       struct acpi_buffer resp = { sizeof(struct pch_fivr_resp), fivr_resp};
> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       struct acpi_buffer format = { sizeof("NN"), "NN" };
> +       union acpi_object *obj;
> +       acpi_status status;
> +       int ret = -EFAULT;
> +
> +       status = acpi_evaluate_object(handle, method, NULL, &buffer);
> +       if (ACPI_FAILURE(status))
> +               return ret;
> +
> +       obj = buffer.pointer;
> +       if (!obj || obj->type != ACPI_TYPE_PACKAGE)
> +               goto release_buffer;
> +
> +       status = acpi_extract_package(obj, &format, &resp);
> +       if (ACPI_FAILURE(status))
> +               goto release_buffer;
> +
> +       if (fivr_resp->status)
> +               goto release_buffer;
> +
> +       ret = 0;
> +
> +release_buffer:
> +       kfree(buffer.pointer);
> +       return ret;
> +}
> +
>  /*
>   * Presentation of attributes which are defined for INT1045
>   * They are:
> @@ -23,15 +59,14 @@ static ssize_t name##_show(struct device *dev,\
>                            char *buf)\
>  {\
>         struct acpi_device *acpi_dev = dev_get_drvdata(dev);\
> -       unsigned long long val;\
> -       acpi_status status;\
> +       struct pch_fivr_resp fivr_resp;\
> +       int status;\
>  \
> -       status = acpi_evaluate_integer(acpi_dev->handle, #method,\
> -                                      NULL, &val);\
> -       if (ACPI_SUCCESS(status))\
> -               return sprintf(buf, "%d\n", (int)val);\
> -       else\
> -               return -EINVAL;\
> +       status = pch_fivr_read(acpi_dev->handle, #method, &fivr_resp);\
> +       if (status)\
> +               return status;\
> +\
> +       return sprintf(buf, "%llu\n", fivr_resp.result);\
>  }
>
>  #define PCH_FIVR_STORE(name, method) \
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-28 16:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 16:18 [PATCH] ACPI: DPTF: Fix reading of attributes Srinivas Pandruvada
2021-07-28 16:42 ` 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 NNTP newsgroup(s).