diff mbox series

[2/2] iio: imu: lsm6dsx: Add ACPI mount matrix retrieval

Message ID 20230130201018.981024-3-jic23@kernel.org (mailing list archive)
State Accepted
Headers show
Series iio: imu: lsx6dsx: support SMO8B30 ACPI ID. | expand

Commit Message

Jonathan Cameron Jan. 30, 2023, 8:10 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

DSDT ROTM method seen in the wild with SMO8B30 _HID.
Making assumption it is similar to that used for bmc150 plus
information from Darrell that the rotation is out by 90 degrees at boot.

Method (ROTM, 0, NotSerialized)
{
    Name (RBUF, Package (0x03)
    {
        "0 -1 0",
        "1 0 0",
        "0 0 1"
    })
    Return (RBUF) /* \_SB_.PCI0.I2C5.DEV_.ROTM.RBUF */
}

Reported-by: Darrell Kavanagh <darrell.kavanagh@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
Untested.  Fingers crossed.

We could factor this out if it keeps turning up.  The bmc150 varient
is more complex as multiple method IDs exist depending on the configuration.

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 65 +++++++++++++++++++-
 1 file changed, 62 insertions(+), 3 deletions(-)

Comments

Darrell Kavanagh Jan. 30, 2023, 8:11 p.m. UTC | #1
Here's the device info:

** Model information
sys_vendor: LENOVO
product_name: 82AT
product_version: IdeaPad Duet 3 10IGL5
chassis_vendor: LENOVO
chassis_version: IdeaPad Duet 3 10IGL5
bios_vendor: LENOVO
bios_version: EQCN35WW
board_vendor: LENOVO
board_name: LNVNB161216
board_version: SDK0R32802 WIN

I'll try to adapt the full patches to my Debian 6.1 source for
testing, and report back to you.

Darrell

On Mon, 30 Jan 2023 at 19:56, Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> DSDT ROTM method seen in the wild with SMO8B30 _HID.
> Making assumption it is similar to that used for bmc150 plus
> information from Darrell that the rotation is out by 90 degrees at boot.
>
> Method (ROTM, 0, NotSerialized)
> {
>     Name (RBUF, Package (0x03)
>     {
>         "0 -1 0",
>         "1 0 0",
>         "0 0 1"
>     })
>     Return (RBUF) /* \_SB_.PCI0.I2C5.DEV_.ROTM.RBUF */
> }
>
> Reported-by: Darrell Kavanagh <darrell.kavanagh@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> Untested.  Fingers crossed.
>
> We could factor this out if it keeps turning up.  The bmc150 varient
> is more complex as multiple method IDs exist depending on the configuration.
>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 65 +++++++++++++++++++-
>  1 file changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 3f6060c64f32..3c2d67f8d9b7 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -56,6 +56,7 @@
>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/iio/events.h>
>  #include <linux/iio/iio.h>
> @@ -2602,6 +2603,61 @@ static int st_lsm6dsx_init_regulators(struct device *dev)
>         return 0;
>  }
>
> +static bool lsm6dsx_get_acpi_mount_matrix(struct device *dev,
> +                                         struct iio_mount_matrix *orientation)
> +{
> +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +       struct acpi_device *adev = ACPI_COMPANION(dev);
> +       union acpi_object *obj, *elements;
> +       acpi_status status;
> +       int i, j, val[3];
> +       char *str;
> +
> +       if (!has_acpi_companion(dev))
> +               return -EINVAL;
> +
> +       if (!acpi_has_method(adev->handle, "ROTM"))
> +               return -EINVAL;
> +
> +       status = acpi_evaluate_object(adev->handle, "ROTM", NULL, &buffer);
> +       if (ACPI_FAILURE(status)) {
> +               dev_warn(dev, "Failed to get ACPI mount matrix: %d\n", status);
> +               return -EINVAL;
> +       }
> +
> +       obj = buffer.pointer;
> +       if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3)
> +               goto unknown_format;
> +
> +       elements = obj->package.elements;
> +       for (i = 0; i < 3; i++) {
> +               if (elements[i].type != ACPI_TYPE_STRING)
> +                       goto unknown_format;
> +
> +               str = elements[i].string.pointer;
> +               if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3)
> +                       goto unknown_format;
> +
> +               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: goto unknown_format;
> +                       }
> +                       orientation->rotation[i * 3 + j] = str;
> +               }
> +       }
> +
> +       kfree(buffer.pointer);
> +       return 0;
> +
> +unknown_format:
> +       dev_warn(dev, "Unknown ACPI mount matrix format, ignoring\n");
> +       kfree(buffer.pointer);
> +       return -EINVAL;
> +}
> +
>  int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>                      struct regmap *regmap)
>  {
> @@ -2676,9 +2732,12 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>                         return err;
>         }
>
> -       err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> -       if (err)
> -               return err;
> +       err = lsm6dsx_get_acpi_mount_matrix(hw->dev, &hw->orientation);
> +       if (err) {
> +               err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> +               if (err)
> +                       return err;
> +       }
>
>         for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
>                 if (!hw->iio_devs[i])
> --
> 2.39.1
>
Jonathan Cameron Feb. 5, 2023, 2:23 p.m. UTC | #2
On Mon, 30 Jan 2023 20:11:07 +0000
Darrell Kavanagh <darrell.kavanagh@gmail.com> wrote:

> Here's the device info:
> 
> ** Model information
> sys_vendor: LENOVO
> product_name: 82AT
> product_version: IdeaPad Duet 3 10IGL5
> chassis_vendor: LENOVO
> chassis_version: IdeaPad Duet 3 10IGL5
> bios_vendor: LENOVO
> bios_version: EQCN35WW
> board_vendor: LENOVO
> board_name: LNVNB161216
> board_version: SDK0R32802 WIN
> 
> I'll try to adapt the full patches to my Debian 6.1 source for
> testing, and report back to you.
> 
> Darrell

Hi Darrel, 

Once the other thread has reached a conclusion, if you could give
a Tested-by tag or reply here with what is wrong, that would be great.

Due to bad timing this may well have to wait for next cycle now, but it
would be good to at least get it queued up for mainline.

Jonathan

> 
> On Mon, 30 Jan 2023 at 19:56, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > DSDT ROTM method seen in the wild with SMO8B30 _HID.
> > Making assumption it is similar to that used for bmc150 plus
> > information from Darrell that the rotation is out by 90 degrees at boot.
> >
> > Method (ROTM, 0, NotSerialized)
> > {
> >     Name (RBUF, Package (0x03)
> >     {
> >         "0 -1 0",
> >         "1 0 0",
> >         "0 0 1"
> >     })
> >     Return (RBUF) /* \_SB_.PCI0.I2C5.DEV_.ROTM.RBUF */
> > }
> >
> > Reported-by: Darrell Kavanagh <darrell.kavanagh@gmail.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > ---
> > Untested.  Fingers crossed.
> >
> > We could factor this out if it keeps turning up.  The bmc150 varient
> > is more complex as multiple method IDs exist depending on the configuration.
> >
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 65 +++++++++++++++++++-
> >  1 file changed, 62 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index 3f6060c64f32..3c2d67f8d9b7 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -56,6 +56,7 @@
> >
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> > +#include <linux/acpi.h>
> >  #include <linux/delay.h>
> >  #include <linux/iio/events.h>
> >  #include <linux/iio/iio.h>
> > @@ -2602,6 +2603,61 @@ static int st_lsm6dsx_init_regulators(struct device *dev)
> >         return 0;
> >  }
> >
> > +static bool lsm6dsx_get_acpi_mount_matrix(struct device *dev,
> > +                                         struct iio_mount_matrix *orientation)
> > +{
> > +       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > +       struct acpi_device *adev = ACPI_COMPANION(dev);
> > +       union acpi_object *obj, *elements;
> > +       acpi_status status;
> > +       int i, j, val[3];
> > +       char *str;
> > +
> > +       if (!has_acpi_companion(dev))
> > +               return -EINVAL;
> > +
> > +       if (!acpi_has_method(adev->handle, "ROTM"))
> > +               return -EINVAL;
> > +
> > +       status = acpi_evaluate_object(adev->handle, "ROTM", NULL, &buffer);
> > +       if (ACPI_FAILURE(status)) {
> > +               dev_warn(dev, "Failed to get ACPI mount matrix: %d\n", status);
> > +               return -EINVAL;
> > +       }
> > +
> > +       obj = buffer.pointer;
> > +       if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3)
> > +               goto unknown_format;
> > +
> > +       elements = obj->package.elements;
> > +       for (i = 0; i < 3; i++) {
> > +               if (elements[i].type != ACPI_TYPE_STRING)
> > +                       goto unknown_format;
> > +
> > +               str = elements[i].string.pointer;
> > +               if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3)
> > +                       goto unknown_format;
> > +
> > +               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: goto unknown_format;
> > +                       }
> > +                       orientation->rotation[i * 3 + j] = str;
> > +               }
> > +       }
> > +
> > +       kfree(buffer.pointer);
> > +       return 0;
> > +
> > +unknown_format:
> > +       dev_warn(dev, "Unknown ACPI mount matrix format, ignoring\n");
> > +       kfree(buffer.pointer);
> > +       return -EINVAL;
> > +}
> > +
> >  int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
> >                      struct regmap *regmap)
> >  {
> > @@ -2676,9 +2732,12 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
> >                         return err;
> >         }
> >
> > -       err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> > -       if (err)
> > -               return err;
> > +       err = lsm6dsx_get_acpi_mount_matrix(hw->dev, &hw->orientation);
> > +       if (err) {
> > +               err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> > +               if (err)
> > +                       return err;
> > +       }
> >
> >         for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> >                 if (!hw->iio_devs[i])
> > --
> > 2.39.1
> >
Darrell Kavanagh Feb. 6, 2023, 12:49 a.m. UTC | #3
On Sun, 5 Feb 2023 at 14:09, Jonathan Cameron <jic23@kernel.org> wrote:
>
> Hi Darrel,
>
> Once the other thread has reached a conclusion, if you could give
> a Tested-by tag or reply here with what is wrong, that would be great.
>
> Due to bad timing this may well have to wait for next cycle now, but it
> would be good to at least get it queued up for mainline.
>
> Jonathan
>

Hi Jonathan,

I confirm that the patches work as expected, both on kernel 6.2.0-rc6
and backported to 6.1.0-2.

Tested-by: Darrell Kavanagh <darrell.kavanagh@gmail.org>

Darrell
Jonathan Cameron Feb. 18, 2023, 5:23 p.m. UTC | #4
On Mon, 6 Feb 2023 00:49:24 +0000
Darrell Kavanagh <darrell.kavanagh@gmail.com> wrote:

> On Sun, 5 Feb 2023 at 14:09, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > Hi Darrel,
> >
> > Once the other thread has reached a conclusion, if you could give
> > a Tested-by tag or reply here with what is wrong, that would be great.
> >
> > Due to bad timing this may well have to wait for next cycle now, but it
> > would be good to at least get it queued up for mainline.
> >
> > Jonathan
> >  
> 
> Hi Jonathan,
> 
> I confirm that the patches work as expected, both on kernel 6.2.0-rc6
> and backported to 6.1.0-2.
> 
> Tested-by: Darrell Kavanagh <darrell.kavanagh@gmail.org>
> 

Thanks.  I'm holding off on applying these to give Lorenzo a chance to
take a look.  They are too late for this cycle anyway so we have lots of
time.

Jonathan


> Darrell
Jonathan Cameron April 11, 2023, 5:34 p.m. UTC | #5
On Mon, 30 Jan 2023 20:10:18 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> DSDT ROTM method seen in the wild with SMO8B30 _HID.
> Making assumption it is similar to that used for bmc150 plus
> information from Darrell that the rotation is out by 90 degrees at boot.
> 
> Method (ROTM, 0, NotSerialized)
> {
>     Name (RBUF, Package (0x03)
>     {
>         "0 -1 0",
>         "1 0 0",
>         "0 0 1"
>     })
>     Return (RBUF) /* \_SB_.PCI0.I2C5.DEV_.ROTM.RBUF */
> }
> 
> Reported-by: Darrell Kavanagh <darrell.kavanagh@gmail.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Not my best work. 0-day pointed out a few things.

> 
> ---
> Untested.  Fingers crossed.
> 
> We could factor this out if it keeps turning up.  The bmc150 varient
> is more complex as multiple method IDs exist depending on the configuration.
> 
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 65 +++++++++++++++++++-
>  1 file changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 3f6060c64f32..3c2d67f8d9b7 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -56,6 +56,7 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/acpi.h>
>  #include <linux/delay.h>
>  #include <linux/iio/events.h>
>  #include <linux/iio/iio.h>
> @@ -2602,6 +2603,61 @@ static int st_lsm6dsx_init_regulators(struct device *dev)
>  	return 0;
>  }
>  
> +static bool lsm6dsx_get_acpi_mount_matrix(struct device *dev,
int not bool.

Also needs ifdef CONFIG_ACPI protections as some of the acpi functions are not stubbed out.

I've tweaked those two things and pushed out again.
If there is anything else I'll probably drop this patch and come back to it
next cycle.



> +					  struct iio_mount_matrix *orientation)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_device *adev = ACPI_COMPANION(dev);
> +	union acpi_object *obj, *elements;
> +	acpi_status status;
> +	int i, j, val[3];
> +	char *str;
> +
> +	if (!has_acpi_companion(dev))
> +		return -EINVAL;
> +
> +	if (!acpi_has_method(adev->handle, "ROTM"))
> +		return -EINVAL;
> +
> +	status = acpi_evaluate_object(adev->handle, "ROTM", NULL, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		dev_warn(dev, "Failed to get ACPI mount matrix: %d\n", status);
> +		return -EINVAL;
> +	}
> +
> +	obj = buffer.pointer;
> +	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3)
> +		goto unknown_format;
> +
> +	elements = obj->package.elements;
> +	for (i = 0; i < 3; i++) {
> +		if (elements[i].type != ACPI_TYPE_STRING)
> +			goto unknown_format;
> +
> +		str = elements[i].string.pointer;
> +		if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3)
> +			goto unknown_format;
> +
> +		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: goto unknown_format;
> +			}
> +			orientation->rotation[i * 3 + j] = str;
> +		}
> +	}
> +
> +	kfree(buffer.pointer);
> +	return 0;
> +
> +unknown_format:
> +	dev_warn(dev, "Unknown ACPI mount matrix format, ignoring\n");
> +	kfree(buffer.pointer);
> +	return -EINVAL;
> +}
> +
>  int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>  		     struct regmap *regmap)
>  {
> @@ -2676,9 +2732,12 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
>  			return err;
>  	}
>  
> -	err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> -	if (err)
> -		return err;
> +	err = lsm6dsx_get_acpi_mount_matrix(hw->dev, &hw->orientation);
> +	if (err) {
> +		err = iio_read_mount_matrix(hw->dev, &hw->orientation);
> +		if (err)
> +			return err;
> +	}
>  
>  	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
>  		if (!hw->iio_devs[i])
diff mbox series

Patch

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 3f6060c64f32..3c2d67f8d9b7 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -56,6 +56,7 @@ 
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
@@ -2602,6 +2603,61 @@  static int st_lsm6dsx_init_regulators(struct device *dev)
 	return 0;
 }
 
+static bool lsm6dsx_get_acpi_mount_matrix(struct device *dev,
+					  struct iio_mount_matrix *orientation)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	union acpi_object *obj, *elements;
+	acpi_status status;
+	int i, j, val[3];
+	char *str;
+
+	if (!has_acpi_companion(dev))
+		return -EINVAL;
+
+	if (!acpi_has_method(adev->handle, "ROTM"))
+		return -EINVAL;
+
+	status = acpi_evaluate_object(adev->handle, "ROTM", NULL, &buffer);
+	if (ACPI_FAILURE(status)) {
+		dev_warn(dev, "Failed to get ACPI mount matrix: %d\n", status);
+		return -EINVAL;
+	}
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 3)
+		goto unknown_format;
+
+	elements = obj->package.elements;
+	for (i = 0; i < 3; i++) {
+		if (elements[i].type != ACPI_TYPE_STRING)
+			goto unknown_format;
+
+		str = elements[i].string.pointer;
+		if (sscanf(str, "%d %d %d", &val[0], &val[1], &val[2]) != 3)
+			goto unknown_format;
+
+		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: goto unknown_format;
+			}
+			orientation->rotation[i * 3 + j] = str;
+		}
+	}
+
+	kfree(buffer.pointer);
+	return 0;
+
+unknown_format:
+	dev_warn(dev, "Unknown ACPI mount matrix format, ignoring\n");
+	kfree(buffer.pointer);
+	return -EINVAL;
+}
+
 int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 		     struct regmap *regmap)
 {
@@ -2676,9 +2732,12 @@  int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
 			return err;
 	}
 
-	err = iio_read_mount_matrix(hw->dev, &hw->orientation);
-	if (err)
-		return err;
+	err = lsm6dsx_get_acpi_mount_matrix(hw->dev, &hw->orientation);
+	if (err) {
+		err = iio_read_mount_matrix(hw->dev, &hw->orientation);
+		if (err)
+			return err;
+	}
 
 	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
 		if (!hw->iio_devs[i])