From: Jonathan Cameron <jic23@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
Sean Rhodes <sean@starlabs.systems>,
linux-iio@vger.kernel.org, linux-acpi@vger.kernel.org,
"Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH 2/4] iio: accel: kxcjk-1013: Move ACPI ROTM parsing to new acpi-helpers.h
Date: Sat, 20 Apr 2024 12:13:45 +0100 [thread overview]
Message-ID: <20240420121345.26c2edf1@jic23-huawei> (raw)
In-Reply-To: <20240417164616.74651-3-hdegoede@redhat.com>
On Wed, 17 Apr 2024 18:46:14 +0200
Hans de Goede <hdegoede@redhat.com> wrote:
> The ACPI "ROTM" rotation matrix parsing code atm is already duplicated
> between bmc150-accel-core.c and kxcjk-1013.c and a third user of this is
> coming.
>
> Move the ROTM parsing from kxcjk-1013.c, which has slightly better error
> logging (and otherwise is 100% identical), into a new acpi-helpers.h file
> so that it can be shared.
>
> Other then moving the code the only 2 other changes are:
>
> 1. Rename the function to acpi_read_mount_matrix() to make clear this
> is a generic ACPI mount matrix read function.
> 2. Add a "char *acpi_method" parameter since some bmc150 dual-accel setups
> (360° hinges with 1 accel in kbd/base + 1 in display half) declare both
> accels in a single ACPI device with 2 different method names for
> the 2 matrices.
>
> Cc: Sean Rhodes <sean@starlabs.systems>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tempted to ask you to rename this to
acpi_non_standard_microsoft_extension_that_they_never_agreed_with_aswg_read_mount_matrix()
but meh, I'll cope with a reference to:
https://learn.microsoft.com/en-us/windows-hardware/drivers/sensors/sensors-acpi-entries
and a comment saying this is not part of the ACPI standard.
I'm not grumpy at all about companies who add non standard stuff without
at least reserving the ID space.
Why isn't it a _DSM (Device Specific Method) with a microsoft defined UUID? Harrumph.
+CC Rafael and linux-acpi as whilst this remains in IIO, it is named such that it
ends up in the acpi_* namespace. They may not care, but best to check!
Jonathan
> ---
> drivers/iio/accel/acpi-helpers.h | 76 ++++++++++++++++++++++++++++++++
> drivers/iio/accel/kxcjk-1013.c | 71 ++---------------------------
> 2 files changed, 79 insertions(+), 68 deletions(-)
> create mode 100644 drivers/iio/accel/acpi-helpers.h
>
> diff --git a/drivers/iio/accel/acpi-helpers.h b/drivers/iio/accel/acpi-helpers.h
> new file mode 100644
> index 000000000000..a4357925bf07
> --- /dev/null
> +++ b/drivers/iio/accel/acpi-helpers.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* ACPI helper functions for parsing ACPI rotation matrices */
> +
> +#include <linux/acpi.h>
> +#include <linux/dev_printk.h>
> +#include <linux/iio/iio.h>
> +#include <linux/sprintf.h>
> +
> +#ifdef CONFIG_ACPI
> +static inline bool acpi_read_mount_matrix(struct device *dev,
> + struct iio_mount_matrix *orientation,
> + char *acpi_method)
> +{
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + char *str;
> + union acpi_object *obj, *elements;
> + acpi_status status;
> + int i, j, val[3];
> + bool ret = false;
> +
> + if (!adev || !acpi_has_method(adev->handle, acpi_method))
> + return false;
> +
> + status = acpi_evaluate_object(adev->handle, acpi_method, NULL, &buffer);
> + if (ACPI_FAILURE(status)) {
> + dev_err(dev, "Failed to get ACPI mount matrix: %d\n", status);
> + return false;
> + }
> +
> + obj = buffer.pointer;
> + if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3) {
> + dev_err(dev, "Unknown ACPI mount matrix package format\n");
> + goto out_free_buffer;
> + }
> +
> + elements = obj->package.elements;
> + for (i = 0; i < 3; i++) {
> + if (elements[i].type != ACPI_TYPE_STRING) {
> + dev_err(dev, "Unknown ACPI mount matrix element format\n");
> + goto out_free_buffer;
> + }
> +
> + str = elements[i].string.pointer;
> + if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3) {
> + dev_err(dev, "Incorrect ACPI mount matrix string format\n");
> + goto out_free_buffer;
> + }
> +
> + for (j = 0; j < 3; j++) {
> + switch (val[j]) {
> + case -1: str = "-1"; break;
> + case 0: str = "0"; break;
> + case 1: str = "1"; break;
> + default:
> + dev_err(dev, "Invalid value in ACPI mount matrix: %d\n", val[j]);
> + goto out_free_buffer;
> + }
> + orientation->rotation[i * 3 + j] = str;
> + }
> + }
> +
> + ret = true;
> +
> +out_free_buffer:
> + kfree(buffer.pointer);
> + return ret;
> +}
> +#else
> +static inline bool acpi_read_mount_matrix(struct device *dev,
> + struct iio_mount_matrix *orientation,
> + char *acpi_method)
> +{
> + return false;
> +}
> +#endif
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index bb1660667bb0..7e19278491dc 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -24,6 +24,8 @@
> #include <linux/iio/triggered_buffer.h>
> #include <linux/iio/accel/kxcjk_1013.h>
>
> +#include "acpi-helpers.h"
> +
> #define KXCJK1013_DRV_NAME "kxcjk1013"
> #define KXCJK1013_IRQ_NAME "kxcjk1013_event"
>
> @@ -636,73 +638,6 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
> return 0;
> }
>
> -#ifdef CONFIG_ACPI
> -static bool kxj1009_apply_acpi_orientation(struct device *dev,
> - struct iio_mount_matrix *orientation)
> -{
> - struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> - struct acpi_device *adev = ACPI_COMPANION(dev);
> - char *str;
> - union acpi_object *obj, *elements;
> - acpi_status status;
> - int i, j, val[3];
> - bool ret = false;
> -
> - if (!adev || !acpi_has_method(adev->handle, "ROTM"))
> - return false;
> -
> - status = acpi_evaluate_object(adev->handle, "ROTM", NULL, &buffer);
> - if (ACPI_FAILURE(status)) {
> - dev_err(dev, "Failed to get ACPI mount matrix: %d\n", status);
> - return false;
> - }
> -
> - obj = buffer.pointer;
> - if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3) {
> - dev_err(dev, "Unknown ACPI mount matrix package format\n");
> - goto out_free_buffer;
> - }
> -
> - elements = obj->package.elements;
> - for (i = 0; i < 3; i++) {
> - if (elements[i].type != ACPI_TYPE_STRING) {
> - dev_err(dev, "Unknown ACPI mount matrix element format\n");
> - goto out_free_buffer;
> - }
> -
> - str = elements[i].string.pointer;
> - if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3) {
> - dev_err(dev, "Incorrect ACPI mount matrix string format\n");
> - goto out_free_buffer;
> - }
> -
> - for (j = 0; j < 3; j++) {
> - switch (val[j]) {
> - case -1: str = "-1"; break;
> - case 0: str = "0"; break;
> - case 1: str = "1"; break;
> - default:
> - dev_err(dev, "Invalid value in ACPI mount matrix: %d\n", val[j]);
> - goto out_free_buffer;
> - }
> - orientation->rotation[i * 3 + j] = str;
> - }
> - }
> -
> - ret = true;
> -
> -out_free_buffer:
> - kfree(buffer.pointer);
> - return ret;
> -}
> -#else
> -static bool kxj1009_apply_acpi_orientation(struct device *dev,
> - struct iio_mount_matrix *orientation)
> -{
> - return false;
> -}
> -#endif
> -
> static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
> {
> int ret;
> @@ -1533,7 +1468,7 @@ static int kxcjk1013_probe(struct i2c_client *client)
> } else {
> data->active_high_intr = true; /* default polarity */
>
> - if (!kxj1009_apply_acpi_orientation(&client->dev, &data->orientation)) {
> + if (!acpi_read_mount_matrix(&client->dev, &data->orientation, "ROTM")) {
> ret = iio_read_mount_matrix(&client->dev, &data->orientation);
> if (ret)
> return ret;
next prev parent reply other threads:[~2024-04-20 11:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 16:46 [PATCH 0/4] iio: accel: Share ACPI ROTM parsing between drivers and add it to mxc4005 Hans de Goede
2024-04-17 16:46 ` [PATCH 1/4] iio: accel: kxcjk-1013: Simplify ACPI ROTM mount matrix retreival Hans de Goede
2024-04-17 16:46 ` [PATCH 2/4] iio: accel: kxcjk-1013: Move ACPI ROTM parsing to new acpi-helpers.h Hans de Goede
2024-04-20 11:13 ` Jonathan Cameron [this message]
2024-04-22 9:17 ` Hans de Goede
2024-04-22 9:18 ` Hans de Goede
2024-04-22 17:06 ` Jonathan Cameron
2024-04-17 16:46 ` [PATCH 3/4] iio: bmc150-accel-core: Use acpi_read_mount_matrix() helper Hans de Goede
2024-04-17 16:46 ` [PATCH 4/4] iio: accel: mxc4005: Read orientation matrix from ACPI ROTM method Hans de Goede
2024-04-19 7:36 ` [PATCH 0/4] iio: accel: Share ACPI ROTM parsing between drivers and add it to mxc4005 Hans de Goede
2024-04-22 7:51 ` Andy Shevchenko
2024-04-22 7:55 ` Andy Shevchenko
2024-04-22 8:24 ` Hans de Goede
2024-04-22 11:33 ` Andy Shevchenko
2024-04-22 11:45 ` Hans de Goede
2024-04-22 12:28 ` Andy Shevchenko
2024-04-22 17:05 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240420121345.26c2edf1@jic23-huawei \
--to=jic23@kernel.org \
--cc=hdegoede@redhat.com \
--cc=lars@metafoo.de \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=sean@starlabs.systems \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.