On Wed, 17 Apr 2024 18:46:14 +0200 Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx> 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;
