Re: [PATCH] iio: imu: bmi323: Support loading of bmi323 driver for ASUS ROG ALLY

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 09 Feb 2024 08:05:14 -0800
Jonathan LoBue <jlobue10@gmail.com> wrote:

> Due to an ACPI match of "BOSC0200" and existing gyro drivers, the ASUS ROG ALLY attempts to incorrectly load the bmc150 driver.
> This leaves the gyro inoperable for ASUS ROG ALLY. The correct gyro driver, bmi323, has already been upstreamed as part of the 6.8 kernel changes.
> In order to load the correct bmi323 driver for ASUS ROG ALLY's gyro, this patch uses a DMI match to unhook the ASUS ROG ALLY from loading the bmc150 driver.
> This unhooking is also added for the Ayaneo AIR Plus device, as requested by ChimeraOS devs.
> 

Please reformat as a patch as per the documentation for submitting patches.
Wrap the lines to 75 chars in the description.

> ---
The cut lines affect what git will pick up and I don't think that's your
intent.

More generally I'd just like to confirm I have understood the correctly.
We have two incompatible devices advertised with the same ACPI ID?
If anyone has contacts with Bosch or Asus can we chase down how this happened
and preferably point out to them that it causes problems if they
through device that don't have actually compatible register sets into
the same ID.

I assume this occurred because there is some hyrda of a driver on
windows that copes with all sorts of different Bosch devices.

It's a valid bosch ID - I've no idea how bosch issues these but
normal practice is one per device interface (so if a device register
compatible you can share an ID, not otherwise).  They have lots of
ID space (and can trivially get more if they need it)...

Solution wise, I'm not keen on having having a DMI check against
particular boards.  Possibly we can add another driver that
binds just to the BOSCH ID and does just enough querying of the part
ID (not the identify of the board) to figure out what it is and
kick of probing the right driver.

Andy, you see more of this mess I think than anyway, any thoughts on
how to handle this elegantly?

Jonathan


> 
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -10,6 +10,7 @@
>  #include <linux/delay.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> +#include <linux/dmi.h>
>  #include <linux/of_irq.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> @@ -1670,6 +1671,9 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
>  	struct iio_dev *indio_dev;
>  	int ret;
>  
> +	if (dmi_match(DMI_BOARD_NAME, "RC71L") || (dmi_match(DMI_BOARD_NAME, "AB05-AMD") && dmi_match(DMI_PRODUCT_NAME, "AIR Plus")))
> +		return -ENODEV; // Abort loading bmc150 for ASUS ROG ALLY, Ayaneo Air Plus
> +
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>  	if (!indio_dev)
>  		return -ENOMEM;
> 
> ---
> 
> Now, after this unhooking from bmc150, loading the correct bmi323 driver needs to occur. In order to accomplish this, an ACPI match table is added to bmi323.
> 
> ---
> 
> --- a/drivers/iio/imu/bmi323/bmi323_i2c.c
> +++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2023, Jagath Jog J <jagathjog1996@gmail.com>
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/i2c.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> @@ -93,6 +94,12 @@ static int bmi323_i2c_probe(struct i2c_c
>  	return bmi323_core_probe(dev);
>  }
>  
> +static const struct acpi_device_id bmi323_acpi_match[] = {
> +	{"BOSC0200"},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(acpi, bmi323_acpi_match);
> +
>  static const struct i2c_device_id bmi323_i2c_ids[] = {
>  	{ "bmi323" },
>  	{ }
> @@ -109,6 +116,7 @@ static struct i2c_driver bmi323_i2c_driv
>  	.driver = {
>  		.name = "bmi323",
>  		.of_match_table = bmi323_of_i2c_match,
> +		.acpi_match_table = ACPI_PTR(bmi323_acpi_match),
>  	},
>  	.probe = bmi323_i2c_probe,
>  	.id_table = bmi323_i2c_ids,
> 
> ---
> 
> Patching these two files in this manner successfully accomplishes unhooking the ASUS ROG ALLY from the bmc150 driver and loading of the bmi323 driver.
> 
> Best Regards,
> Jon LoBue
> 
> Co-developed-by: Jonathan LoBue <jlobue10@gmail.com>
> Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
> Co-developed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> Co-developed-by: Denis Benato <dbenato.denis96@gmail.com>
> Signed-off-by: Denis Benato <dbenato.denis96@gmail.com>
> Co-developed-by: Antheas Kapenekakis <lkml@antheas.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
As above, look up how to submit a kernel patch.

https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

Thanks,

Jonathan