On Wed, 14 Feb 2024 17:39:53 +0200 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Wed, Feb 14, 2024 at 5:07 PM Jonathan LoBue <jlobue10@xxxxxxxxx> 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@xxxxxxxxx> 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. >