On Tue, Oct 22, 2024 at 08:50:43AM -0700, Justin Weiss wrote: > Justin Weiss <justin@xxxxxxxxxxxxxxx> writes: ... > The ACPI IDs with device pointers are here: > > > +static const struct acpi_device_id bmi270_acpi_match[] = { > > + /* OrangePi NEO */ > > + { "BMI0260", (kernel_ulong_t)&bmi260_chip_info }, > > + /* GPD Win Mini, Aya Neo AIR Pro, OXP Mini Pro, etc. */ > > + { "BMI0160", (kernel_ulong_t)&bmi260_chip_info }, > > + /* GPD Win Max 2 */ > > + { "10EC5280", (kernel_ulong_t)&bmi260_chip_info }, > > + { } Cool! But please, keep them alphabetically ordered by ID. Can we push OrangePI NED to go and fix ACPI IDs eventually? > > +}; > > I pulled DSDT device excerpts for the GPD Win Mini (which uses the > BMI0160 ACPI ID, even though it has a bmi260) and the OrangePi NEO > (which uses the BMI0260 ACPI ID). > > I couldn't find a shipping device with a bmi260 using the 10EC5280 ACPI > ID. Some prototype devices with the bmi260 may have used them: > https://lore.kernel.org/all/CAFqHKTm2WRNkcSoBEE=oNbfu_9d9RagQHLydmv6q1=snO_MXyA@xxxxxxxxxxxxxx/ > > I can remove that ID from this changeset for now. Yes, please do not add anything that has no evidence of existence in the wild or approved vendor allocated ID. > GPD Win Mini: Add short parts of these to the commit message, or better split these to two patches each of them adding a new ID to the table. See below what I do want to see there (no need to have everything), i.e. I removed unneeded lines: > Device (BMI2) > { > Name (_ADR, Zero) // _ADR: Address My gosh, can this be fixed (seems rhetorical)? The _ADR must NOT be present together with _HID. It's against the ACPI specifications. > Name (_HID, "BMI0160") // _HID: Hardware ID > Name (_CID, "BMI0160") // _CID: Compatible ID > Name (_DDN, "Accelerometer") // _DDN: DOS Device Name > Name (_UID, One) // _UID: Unique ID > Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings > { > Name (RBUF, ResourceTemplate () > { > I2cSerialBusV2 (0x0068, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.I2CB", > 0x00, ResourceConsumer, , Exclusive, > ) > GpioInt (Edge, ActiveLow, Exclusive, PullDefault, 0x0000, > "\\_SB.GPIO", 0x00, ResourceConsumer, , > ) > { // Pin list > 0x008B > } > }) > Return (RBUF) /* \_SB_.I2CB.BMI2._CRS.RBUF */ > } ... > } > > OrangePi NEO: Same comments for this device. ... > > +static const struct acpi_device_id bmi270_acpi_match[] = { > > + { "BOSC0260", (kernel_ulong_t)&bmi260_chip_info }, > > + { } > > +}; > > I can't find any evidence of BOSC0260 being used, besides existing in > the Windows driver. As suggested in an earlier review, I added it here > to encourage people looking at this driver in the future to use the > correct ACPI ID. Are you official representative of Bosch or do you have a proof by the vendor that they allocated this ID? Otherwise we may NOT allocate IDs on their behalf and has not to be added. -- With Best Regards, Andy Shevchenko