[PATCH v6 0/2] Add bmi323 support for ASUS ROG ALLY

[PATCH v6 0/2] Add bmi323 support for ASUS ROG ALLY

[Jonathan LoBue]

From: Jonathan LoBue <jlobue10@gmail.com>

This patch series adds comments about a duplicate ACPI identifier
between devices using bmc150 and bmi323. This series also adds the ACPI
match table for devices using bmi323 to allow those devices to load the
proper driver.

Changes since v5:
- Updated patch titles
- Add patch description to cover letter

Jonathan LoBue (2):
  iio: accel: bmc150: Document duplicate ACPI entries with bmi323 driver
  iio: imu: bmi323: Add ACPI Match Table

 drivers/iio/accel/bmc150-accel-i2c.c | 13 +++++++++++++
 drivers/iio/imu/bmi323/bmi323_i2c.c  | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+)
--
2.43.0

[PATCH v6 1/2] iio: accel: bmc150: Document duplicate ACPI entries with bmi323 driver

[Jonathan LoBue]

From: jlobue10 <jlobue10@gmail.com>

Adds a description of the duplicate ACPI identifier issue
between devices using bmc150 and bmi323.

Reviewed-by: Andy Shevchenko <andy.shevchenko@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>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
 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..353271e23235 100644
--- a/drivers/iio/accel/bmc150-accel-i2c.c
+++ b/drivers/iio/accel/bmc150-accel-i2c.c
@@ -224,6 +224,19 @@ static const struct acpi_device_id bmc150_accel_acpi_match[] = {
 	{"BMA250E"},
 	{"BMC150A"},
 	{"BMI055A"},
+	/*
+ 	 * 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"},
 	{"BSBA0150"},
 	{"DUAL250E"},
-- 
2.43.0

[PATCH v6 2/2] iio: imu: bmi323: Add ACPI Match Table

[Jonathan LoBue]

From: jlobue10 <jlobue10@gmail.com>

Adds the ACPI match table for ASUS ROG ALLY to load the bmi323
driver with an ACPI match of "BOSC0200", and a comment about duplicate
ACPI identifiers between devices using the bmc150 and bmi323 chips.

Reviewed-by: Andy Shevchenko <andy.shevchenko@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>
Signed-off-by: Jonathan LoBue <jlobue10@gmail.com>
---
 drivers/iio/imu/bmi323/bmi323_i2c.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/iio/imu/bmi323/bmi323_i2c.c b/drivers/iio/imu/bmi323/bmi323_i2c.c
index 20a8001b9956..f7550503e47e 100644
--- a/drivers/iio/imu/bmi323/bmi323_i2c.c
+++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
@@ -93,6 +93,25 @@ static int bmi323_i2c_probe(struct i2c_client *i2c)
 	return bmi323_core_probe(dev);
 }
 
+static const struct acpi_device_id bmi323_acpi_match[] = {
+	/*
+ 	 * The "BOSC0200" identifier used here is not unique to bmi323 devices.
+ 	 * The same "BOSC0200" identifier is found in the ACPI tables of devices
+ 	 * using the bmc150 chip. This creates a conflict with duplicate ACPI
+ 	 * identifiers which multiple drivers want to use. If a non-bmi323 device
+ 	 * starts to load with this "BOSC0200" ACPI match here, then the chip
+ 	 * ID check portion should fail because the chip IDs received (via i2c) are
+ 	 * unique between bmc150 and bmi323 and the driver should relinquish the
+ 	 * device. If and when a different driver (such as bmc150) starts to load
+ 	 * with the "BOSC0200" ACPI match, a short reset should ensure that the
+ 	 * device is not in a bad state during that driver initialization. This
+ 	 * device reset does occur in both the bmi323 and bmc150 init sequences.
+ 	 */
+	{ "BOSC0200" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, bmi323_acpi_match);
+
 static const struct i2c_device_id bmi323_i2c_ids[] = {
 	{ "bmi323" },
 	{ }
@@ -109,6 +128,7 @@ static struct i2c_driver bmi323_i2c_driver = {
 	.driver = {
 		.name = "bmi323",
 		.of_match_table = bmi323_of_i2c_match,
+		.acpi_match_table = bmi323_acpi_match,
 	},
 	.probe = bmi323_i2c_probe,
 	.id_table = bmi323_i2c_ids,
-- 
2.43.0

Re: [PATCH v6 0/2] Add bmi323 support for ASUS ROG ALLY

[Hans de Goede]

Hi,

On 2/16/24 19:16, Jonathan LoBue wrote:
> From: Jonathan LoBue <jlobue10@gmail.com>
> 
> This patch series adds comments about a duplicate ACPI identifier
> between devices using bmc150 and bmi323. This series also adds the ACPI
> match table for devices using bmi323 to allow those devices to load the
> proper driver.
> 
> Changes since v5:
> - Updated patch titles
> - Add patch description to cover letter

I have just tested this on a Chuwi Hi13 tablet with
a BOSC0200 accel. I blacklisted the bmc150 driver so that
the bmi323 driver would load first. The bmi323 driver
"successfully" fails to probe and after manually loading
the bmc150 driver (to fake a module loading order of
the bmi323 driver loading before the bmc150) the accel
works properly.

So from my pov this series is good to go:

Tested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

Re: [PATCH v6 0/2] Add bmi323 support for ASUS ROG ALLY

[Jonathan Cameron]

On Fri, 16 Feb 2024 19:45:07 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 2/16/24 19:16, Jonathan LoBue wrote:
> > From: Jonathan LoBue <jlobue10@gmail.com>
> > 
> > This patch series adds comments about a duplicate ACPI identifier
> > between devices using bmc150 and bmi323. This series also adds the ACPI
> > match table for devices using bmi323 to allow those devices to load the
> > proper driver.
> > 
> > Changes since v5:
> > - Updated patch titles
> > - Add patch description to cover letter  
> 
> I have just tested this on a Chuwi Hi13 tablet with
> a BOSC0200 accel. I blacklisted the bmc150 driver so that
> the bmi323 driver would load first. The bmi323 driver
> "successfully" fails to probe and after manually loading
> the bmc150 driver (to fake a module loading order of
> the bmi323 driver loading before the bmc150) the accel
> works properly.
> 
> So from my pov this series is good to go:
> 
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> Regards,
> 
Thanks.  I've tweaked these a fair bit for minor things whilst applying.
1) Author (From: in the email) didn't match sign off. Given address was the
   same I think I'm fine to just fix that up.
2) All the lines had a leading space.  Deleted those.
3) quite a few lines went just over 80 chars - I've rewrapped the comments
   to avoid this.

Anyhow, with those tweaks, applied via the slow path (it's not a fix as such
as the right driver was never loaded) but I'm fine with a request for a backport
of this to older kernels after this is upstream (next merge window).

Some of those would have been caught be scripts/checkpatch.pl - make sure you
run that and either fix ever warning or have a good reason why not (there are
often good reasons so don't take that script as a rule book!)

Applied to the togreg branch of iio.git but initially only pushed out as testing
for 0-day to get a head start checking for anything we missed.

Thanks,

Jonathan

> Hans
> 
> 
> 
> 

Re: [PATCH v6 0/2] Add bmi323 support for ASUS ROG ALLY

[Jonathan LoBue]

Jonathan,

Thank you. Yes I saw my error on the from lines in v6,
but I thought that would've been a silly reason for a v7.
A better reason might have been to add the reviewed
and tested by lines for Hans.

Yes I think the comment lines went over once indentation
was added. Sorry about that and thank you for the
adjustments.

Thank you all in this thread for your patience, help and
guidance, and hopefully if I submit more patches in the
future, it will go much more smoothly.

Best Regards,
Jon LoBue

On Sat, Feb 17, 2024 at 9:22 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 16 Feb 2024 19:45:07 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>
> > Hi,
> >
> > On 2/16/24 19:16, Jonathan LoBue wrote:
> > > From: Jonathan LoBue <jlobue10@gmail.com>
> > >
> > > This patch series adds comments about a duplicate ACPI identifier
> > > between devices using bmc150 and bmi323. This series also adds the ACPI
> > > match table for devices using bmi323 to allow those devices to load the
> > > proper driver.
> > >
> > > Changes since v5:
> > > - Updated patch titles
> > > - Add patch description to cover letter
> >
> > I have just tested this on a Chuwi Hi13 tablet with
> > a BOSC0200 accel. I blacklisted the bmc150 driver so that
> > the bmi323 driver would load first. The bmi323 driver
> > "successfully" fails to probe and after manually loading
> > the bmc150 driver (to fake a module loading order of
> > the bmi323 driver loading before the bmc150) the accel
> > works properly.
> >
> > So from my pov this series is good to go:
> >
> > Tested-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >
> > Regards,
> >
> Thanks.  I've tweaked these a fair bit for minor things whilst applying.
> 1) Author (From: in the email) didn't match sign off. Given address was the
>    same I think I'm fine to just fix that up.
> 2) All the lines had a leading space.  Deleted those.
> 3) quite a few lines went just over 80 chars - I've rewrapped the comments
>    to avoid this.
>
> Anyhow, with those tweaks, applied via the slow path (it's not a fix as such
> as the right driver was never loaded) but I'm fine with a request for a backport
> of this to older kernels after this is upstream (next merge window).
>
> Some of those would have been caught be scripts/checkpatch.pl - make sure you
> run that and either fix ever warning or have a good reason why not (there are
> often good reasons so don't take that script as a rule book!)
>
> Applied to the togreg branch of iio.git but initially only pushed out as testing
> for 0-day to get a head start checking for anything we missed.
>
> Thanks,
>
> Jonathan
>
> > Hans
> >
> >
> >
> >
>