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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 14 Feb 2024 17:39:53 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, Feb 14, 2024 at 5:07 PM Jonathan LoBue <jlobue10@gmail.com> wrote:
> > 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:  
> 
> ...
> 
> > > > 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.  
> 
> This is fine and needed. My comment was about the actual placement of
> the comment (should be immediately before the ID entry and not
> detached from it.
> 
> ...
> 
> > > > + * 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...
> > ?  
> 
> Yes.
> 
> > > > + * 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.  
> 
> ...
> 
> > > >  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.  
> 
> In my p.o.v. it's not stylic as we refer to the exact ID and having
> comment detached is, besides being unusual, may go outdated too
> quickly as code is being grown and developed. So, I really want it to
> be closer to the ID entry.

Yes, please send a v3 with it next to the relevant ID.
Also dont send new versions in reply to old ones.
For IIO patches at least, a new thread every time please.

>