Hi Jonathan, On 2/16/24 12:35, Jonathan Cameron wrote: > On Thu, 15 Feb 2024 10:19:52 -0800 > J Lo <jlobue10@xxxxxxxxx> wrote: > >> From: Jonathan LoBue <jlobue10@xxxxxxxxx> > > Hi Jonathan > > Cover letter should always include at least a short overview of what > the patch is doing. > > Long term this solution may be a pain to maintain. > The reasoning is the DT path where we have moved over time to allow > for fallback compatibles (same concept exists in ACPI even if it is > little used) to be used even if we don't recognise a ID read from > the chip. The intent being to allow old kernels to work with new > devices where they really are backwards compatible. > > If that gets fixed in these drivers, we will have to explicitly > exclude ACPI IDs. > > Hopefully we'll pick up such issues in review though so this should be fine. > > I'd like input from Hans though on whether this solution of duplicating > the IDs generally works out longer term and is appropriate here. The BOSC0200 ACPI ID is used in a lot of devices and the ACPI tables of these devices are not under our control. So we really have no other option. Having 2 different drivers match/bind to the same (ACPI or other) id/device is not unheard of. As long as the probe() method then figures out it is not the right device and cleanly exits then this is fine. I'll run a test with patch 2/2 + the bmi323 driver enabled on a device with a BOSC0200 ACPI id which does actually need the bmc150 driver to make sure that the bmi323 driver properly refuses to bind there. I do see that both drivers write to a reset-register before reading the id register and those registers are different ... Looking at the registers used for reset then on the bmc150 it seems that the bmi323 code is writing the last bytes of the fifo which should be fine. And when the bmc150 code is trying to reset a bmi323 it is writing to the BMI323_FEAT_IO_STATUS_REG. Since the bmi323 driver does a reset itself and programs that register during init I guess that should be fine to since the value written by bmc150's probe() will be overwritten. I'll get back to with test results of letting the bmi323 driver probe a BOSC0200 (*) device, before the bmc150 driver probes it and see if things still work then. Regards, Hans *) which will typically be a BMA250E >> Changes since v4: >> - Fixed comment location in bmc150. >> - Fixed signed off by portion. >> >> Jonathan LoBue (2): >> iio: accel: bmc150: Duplicate ACPI entries >> iio: imu: bmi323: Add and enable ACPI Match Table >> >> drivers/iio/accel/bmc150-accel-i2c.c | 13 +++++++++++++ >> drivers/iio/imu/bmi323/bmi323_i2c.c | 20 ++++++++++++++++++++ >> 2 files changed, 33 insertions(+) >> -- >> 2.43.0 >
