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

[Jonathan LoBue <jlobue10@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

On Wednesday, February 14, 2024 1:35:56 AM PST Andy Shevchenko wrote:
> On Wed, Feb 14, 2024 at 12:38 AM Jonathan LoBue <jlobue10@gmail.com> 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>
> (carry the tag if you send a new version)
> 
> ...

Will do.

> > Comment describing the duplicate ACPI identifier issue has been added
> > before the "BOSC0200" entry here.
> 
> Hmm...

You asked for a changelog after the cutter, although it really seems
unnecessary to me here as it's repetitive in nature with comment above.

> > +/*
> > + * The "BOSC0200" ACPI identifier used here in the bmc150 driver is not
> 
> s/ACPI//
> s/in the bmc150 driver//
> 

So update the first sentence in the comment to be:

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) 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.
> > + */
> > +
> >  #ifdef CONFIG_ACPI
> >  static const struct acpi_device_id bmc150_acpi_dual_accel_ids[] = {
> 
> ...it should be here. But don't resend, let's Jonathan to decide in
> case he won't amend this when applying.
> 
> >         {"BOSC0200"},

This seems to be a stylistic preference on whether or not to include this
long comment inside of the ACPI match table or not. Stylistically, my
preference would be to include it directly above the match table and not
inside of it. I will wait for Jonathan Cameron's comments about what to do
here.

Best Regards,
Jon LoBue

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]