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
