Re: [PATCH v4 1/2] iio: accel: bmc150: Duplicate ACPI entries

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 2/14/24 23:59, Jonathan LoBue wrote:
> This patch adds a description of the duplicate ACPI identifier issue
> between devices using bmc150 and bmi323.
> 
> With the remarks below,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@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 <benato.denis96@gmail.com>
> Signed-off-by: Denis Benato <benato.denis96@gmail.com>
> Co-developed-by: Antheas Kapenekakis <lkml@antheas.dev>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/iio/accel/bmc150-accel-i2c.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/iio/accel/bmc150-accel-i2c.c b/drivers/iio/accel/bmc150-accel-i2c.c
> index ee1ba134ad42..df7d48a35e41 100644
> --- a/drivers/iio/accel/bmc150-accel-i2c.c
> +++ b/drivers/iio/accel/bmc150-accel-i2c.c
> @@ -15,6 +15,19 @@
>  
>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id bmc150_acpi_dual_accel_ids[] = {
> +	/*
> + 	 * The "BOSC0200" identifier used here is not unique to devices using
> + 	 * bmc150. The same "BOSC0200" identifier is found in the ACPI tables of
> + 	 * the ASUS ROG ALLY and Ayaneo AIR Plus which both use a Bosch BMI323
> + 	 * chip. This creates a conflict with duplicate ACPI identifiers which
> + 	 * multiple drivers want to use. Fortunately, when the bmc150 driver
> + 	 * starts to load on the ASUS ROG ALLY, the chip ID check portion fails
> + 	 * (correctly) because the chip IDs received (via i2c) are unique between
> + 	 * bmc150 and bmi323 and a dmesg output similar to this:
> + 	 * "bmc150_accel_i2c i2c-BOSC0200:00: Invalid chip 0" can be seen.
> + 	 * This allows the bmi323 driver to take over for ASUS ROG ALLY, and
> + 	 * other devices using the bmi323 chip.
> + 	 */
>  	{"BOSC0200"},
>  	{"DUAL250E"},
>  	{ }

Sorry for not noticing this sooner:

The BOSC0200 ACPI id is listed twice in this file.

The listing you are adding the comment to is for ACPI ids which are known
to be used in yoga-style (360° hinges) 2-in-1s which may have 2 accelerometers
1 in the bottom/keyboard half and 1 in the display half.

The bmc150_acpi_dual_accel_ids list indicates that this might be the case
and enables some extra code for this.

The actual listing of ACPI ids used to determine if the driver should
bind is the bmc150_accel_acpi_match[] list lower in the file and IMHO
that is the list where this comment should go.

Regards,

Hans