On Sat, 10 Feb 2024 08:23:00 -0800
Jonathan LoBue <jlobue10@xxxxxxxxx> wrote:
> On Saturday, February 10, 2024 7:25:50 AM PST Jonathan Cameron wrote:
> > On Fri, 09 Feb 2024 08:05:14 -0800
> > Jonathan LoBue <jlobue10@xxxxxxxxx> wrote:
> >
> > > Due to an ACPI match of "BOSC0200" and existing gyro drivers, the ASUS ROG ALLY attempts to incorrectly load the bmc150 driver.
> > > This leaves the gyro inoperable for ASUS ROG ALLY. The correct gyro driver, bmi323, has already been upstreamed as part of the 6.8 kernel changes.
> > > In order to load the correct bmi323 driver for ASUS ROG ALLY's gyro, this patch uses a DMI match to unhook the ASUS ROG ALLY from loading the bmc150 driver.
> > > This unhooking is also added for the Ayaneo AIR Plus device, as requested by ChimeraOS devs.
> > >
> >
> > Please reformat as a patch as per the documentation for submitting patches.
> > Wrap the lines to 75 chars in the description.
> >
> > > ---
> > The cut lines affect what git will pick up and I don't think that's your
> > intent.
> >
> > More generally I'd just like to confirm I have understood the correctly.
> > We have two incompatible devices advertised with the same ACPI ID?
> > If anyone has contacts with Bosch or Asus can we chase down how this happened
> > and preferably point out to them that it causes problems if they
> > through device that don't have actually compatible register sets into
> > the same ID.
> >
> > I assume this occurred because there is some hyrda of a driver on
> > windows that copes with all sorts of different Bosch devices.
> >
> > It's a valid bosch ID - I've no idea how bosch issues these but
> > normal practice is one per device interface (so if a device register
> > compatible you can share an ID, not otherwise). They have lots of
> > ID space (and can trivially get more if they need it)...
> >
> > Solution wise, I'm not keen on having having a DMI check against
> > particular boards. Possibly we can add another driver that
> > binds just to the BOSCH ID and does just enough querying of the part
> > ID (not the identify of the board) to figure out what it is and
> > kick of probing the right driver.
> >
> > Andy, you see more of this mess I think than anyway, any thoughts on
> > how to handle this elegantly?
> >
> > Jonathan
> >
> >
> > >
> > > --- a/drivers/iio/accel/bmc150-accel-core.c
> > > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > > @@ -10,6 +10,7 @@
> > > #include <linux/delay.h>
> > > #include <linux/slab.h>
> > > #include <linux/acpi.h>
> > > +#include <linux/dmi.h>
> > > #include <linux/of_irq.h>
> > > #include <linux/pm.h>
> > > #include <linux/pm_runtime.h>
> > > @@ -1670,6 +1671,9 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
> > > struct iio_dev *indio_dev;
> > > int ret;
> > >
> > > + if (dmi_match(DMI_BOARD_NAME, "RC71L") || (dmi_match(DMI_BOARD_NAME, "AB05-AMD") && dmi_match(DMI_PRODUCT_NAME, "AIR Plus")))
> > > + return -ENODEV; // Abort loading bmc150 for ASUS ROG ALLY, Ayaneo Air Plus
> > > +
> > > indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > > if (!indio_dev)
> > > return -ENOMEM;
> > >
> > > ---
> > >
> > > Now, after this unhooking from bmc150, loading the correct bmi323 driver needs to occur. In order to accomplish this, an ACPI match table is added to bmi323.
> > >
> > > ---
> > >
> > > --- a/drivers/iio/imu/bmi323/bmi323_i2c.c
> > > +++ b/drivers/iio/imu/bmi323/bmi323_i2c.c
> > > @@ -5,6 +5,7 @@
> > > * Copyright (C) 2023, Jagath Jog J <jagathjog1996@xxxxxxxxx>
> > > */
> > >
> > > +#include <linux/acpi.h>
> > > #include <linux/i2c.h>
> > > #include <linux/mod_devicetable.h>
> > > #include <linux/module.h>
> > > @@ -93,6 +94,12 @@ static int bmi323_i2c_probe(struct i2c_c
> > > return bmi323_core_probe(dev);
> > > }
> > >
> > > +static const struct acpi_device_id bmi323_acpi_match[] = {
> > > + {"BOSC0200"},
> > > + { },
> > > +};
> > > +MODULE_DEVICE_TABLE(acpi, bmi323_acpi_match);
> > > +
> > > static const struct i2c_device_id bmi323_i2c_ids[] = {
> > > { "bmi323" },
> > > { }
> > > @@ -109,6 +116,7 @@ static struct i2c_driver bmi323_i2c_driv
> > > .driver = {
> > > .name = "bmi323",
> > > .of_match_table = bmi323_of_i2c_match,
> > > + .acpi_match_table = ACPI_PTR(bmi323_acpi_match),
> > > },
> > > .probe = bmi323_i2c_probe,
> > > .id_table = bmi323_i2c_ids,
> > >
> > > ---
> > >
> > > Patching these two files in this manner successfully accomplishes unhooking the ASUS ROG ALLY from the bmc150 driver and loading of the bmi323 driver.
> > >
> > > Best Regards,
> > > Jon LoBue
> > >
> > > Co-developed-by: Jonathan LoBue <jlobue10@xxxxxxxxx>
> > > Signed-off-by: Jonathan LoBue <jlobue10@xxxxxxxxx>
> > > Co-developed-by: Luke D. Jones <luke@xxxxxxxxxx>
> > > Signed-off-by: Luke D. Jones <luke@xxxxxxxxxx>
> > > Co-developed-by: Denis Benato <benato.denis96@xxxxxxxxx>
> > > Signed-off-by: Denis Benato <benato.denis96@xxxxxxxxx>
> > > Co-developed-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> > > Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
> > As above, look up how to submit a kernel patch.
> >
> > https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html
> >
> > Thanks,
> >
> > Jonathan
>
> Thank you for the feedback. I will work to fix the formatting. I must have made a mistake somewhere. I was referring to that exact website while doing this.
> The original source for the patch is here: https://github.com/jlobue10/ALLY_Nobara_fixes/raw/main/0003-iio-imu_Add_ROG_ALLY_bmi323-support.patch .
> Several Linux distros have been unofficially using it for a while now that I know of (Nobara, Bazzite, Manjaro, ChimeraOS soon).
> I will fix that in a future reply. I normally use git diff and perhaps made a mistake trying to do the diff -up method instead. I will fix it.
> I understand that other canonical parts may be missing too. This will also be fixed in a future reply.
>
> I completely agree with your assessment. This proposed patch is really meant as a temporary "band-aid" solution.
> There should be consensus reached about a better long term solution going forward.
> Needing to do the DMI match and abort the loading of the bmc150 driver for ASUS ROG ALLY is a symptom of a larger problem.
> The generic identifier "BOSC0200" is not adequate when referring to multiple different devices that the kernel wants to load, which then happens on a first come basis.
> I think a better identifier in this case would be something like "BMI323" but this decision should be left up to BOSCH (and ASUS in this case).
It needs to be a compliant ACPI ID. BOSC is Bosch's valid manufacturer ID.
The code after that tends to just be allocated by someone inside the company
who keeps a bit list of IDs (I have access to the list of HiSilicon HISIXXXX ones for
example but no idea how Bosch manages this.)
> The problem is that these identifiers don't really matter for the Windows' side drivers so manufacturers give them little to no extra thought or consideration.
> There needs to be some agreement or consensus reached by the manufacturers in this regard. This is a much larger problem with several gyro drivers.
> I've had this conversation before and confirmed the larger issue with some ChimeraOS devs. Thanks.
Sure. Please also wrap your emails to the mailing list to under 80 chars (roughly).
>
> Best Regards,
> Jon LoBue
