On Sun, Jan 8, 2023 at 3:40 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Sat, 7 Jan 2023 19:51:59 -0800 > Derek John Clark <derekjohn.clark@xxxxxxxxx> wrote: > > > Greetings, > > > > I have identified a trend where handheld PC manufacturers, primarily > > from China, are not properly identifying the IMU's in their device > > DSDT's. I was hoping to start a dialogue on possible remediation at > > the kernel level as I only imagine this issue compounding further. I > > have an idea of the solution, but considering the amount of work for > > it I at least wanted to see what the general thoughts of the > > maintainers were first. My apologies for the length here, I have done > > quite a bit of investigation before reaching out as I wanted to have > > all my ducks in a row. > > > > Background: > > The first instance was AYANEO with their founders edition & 2021 > > models using 10EC5280 instead of BMI0160. A kernel patch was submitted > > a while back that ultimately stalled for this being considered > > incorrect. For context: > > https://lore.kernel.org/all/Yfqv8V6fZBnG5J5H@xxxxxxxxxxxxxxxxxx/ > > > > Unfortunately the issue has only compounded with time. GPD has been > > shipping their WinMax2 with 10EC5280 in the DSDT, and the early GPD > > Win4 prototypes sent out are using the same despite it actually having > > a BMI0260, which isn't currently supported in the kernel. > > > > Three other manufacturers are using BMI0160 in their DSDT when they > > have a BMI0260. Specifically, the Aya Neo AIR Pro (5825u model), OXP > > Mini Pro, and AOKZOE A1 are all guilty of this. Unfortunately this IMU > > isn't a complete drop in replacement. While it uses some of the same > > registers, many are different. I won't go too into the weeds, but for > > this discussion the most important is that the CHIP_ID uses the same > > registers but is reported 0x27 on the BMI0260's while the BMI0160's > > are 0x1d. Based on all these findings I don't believe it is possible > > to have a separate bmi260 driver. > > > > Remediation: > > I have reached out to the aforementioned companies about releasing > > updated BIOS for each of the respective models to correct the DSDT > > ID's. AYANEO and GPD seem receptive as they have each provided beta > > BIOS for testing. > > Great. Hopefully that means that the scope of problem devices from > these manufacturers is not going to grow too much going forwards! > > > There are still plenty of devices in the wild > > however that will likely not be fixed, and there's no telling what > > will be created in the future considering the rapid growth in this > > market. OneNet forwarded the concern to the OXP/AOK engineering team > > but have not provided comment at this time. > > > > I think the only reliable way to resolve this effectively would be to > > utilize the IMU reported chip ID to actually identify which device is > > present, using the ACPI ID as an entry point for the driver. This > > would likely require some redesign of the driver, wherein all common > > functionality between chips would exist in the existing bmi160_core to > > ensure backwards compatibility. This would essentially become the > > entry point where all three ACPI IDs trigger a read of the CHIP_ID > > before splitting off to load IMU specific registers and functions. > > Whilst it's previously been more common to see a mess on this front for > DT, we do have drivers that use the 'compatible' as more of a hint than > anything else. The approach being. > > 1) Check for the chip ID matching the expected. All good use that. > 2) Check for a match against other known IDs. Print a warning message > but use the values for the right device. > 3) If no match, use the values for the compatible originally given. > The reasoning behind this is that new devices might come along that > are truely backwards compatible, but include extra features. > We want them to work with the supported features out of the box > so the dts includes a fallback compatible to the simpler old device. > > I have no problem with doing similar for ACPI. IIRC it even has the > equivalent of fallback compatibles to cover that 3rd case. > For the ACPI case we might have to deny certain fallbacks if we know > they are shipping in devices and are broken (sigh). Or we only use > the fallback route if the _CID is provided - which is the right way > to do compatible handling in ACPI anyway. Sounds good. I don't have a timeline right now but I'll start working on this. Thank you for the feedback. > The bigger issue here was that the manufacturers were using the > ID space of a realtech pci device. I'm not so worried if they just > break things in their own ID spaces but potentially breaking > things for someone else is a big problem. > > *Sigh*. Doing this stuff right in the first place is so easy and > the specs are free to anyone etc... >From my experience, these manufacturers don't care about doing things the right way that much. I've only been able to pressure them into making changes because their pet projects are Linux based (AyaNeoOS and a GPD Manjaro fork). Telling them they need to make these changes to support their own initiatives has been more effective than trying to explain why they shouldn't break convention. It is interesting though that two separate companies are using the same incorrect ID. It would seem they are either stealing from each other, or they used the same supplier with bad data. > Thanks for your hard work tracking down people who can hopefully > improve what they do next time around. > > Jonathan > > This is similar to how ChromiumOS manages these two IMU's, which > > incidentally I'm using that driver as the primary source of > > information about the 260 since Bosch haven't released a comprehensive > > datasheet on it. > > > > Link for the curious, see accelgyro_bmi_x files for reference: > > https://chromium.googlesource.com/chromiumos/platform/ec/+/master/driver > > > > Thank you for your time, I await your feedback. >