On Tue, 22 Oct 2024 19:54:03 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> 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.
Fair point. The provenance of the driver is a little disconnected from Bosch:
https://ayaneo-1305909189.cos.accelerate.myqcloud.com/ayaneo_com/download/2023/UMDF2.0_BMI260_V1.0.23_5ID_signed_20H1.zip
Justin, if you have contacts at ayaneo, maybe they can confirm if the IDs come
from Bosch. Or maybe we can find someone at Bosch?
Jonathan
>
