Re: [PATCH] iio: accel: mxc4005: add support for mxc6655

[Christian Oder]

Hi Jonathan,

I tested the sensor on a Chuwi Hi10 X and only went by what I've seen in other
commits before[1].

I just ran another test to see what entry is necessary and it appears the sensor
still works when removing the i2c entry, but is not working anymore when
removing the ACPI match. I got the ACPI IDs from udevadm info -e[2].
Would that mean, that I should remove the i2c entry given it's working fine
with ACPI on its own then, or am I missing something?

I'm also successfully using the ACPI ID for the touchscreen orientation quirk
in systemd[3].

> Adding an explicit DT binding table would also be
> good if that is method you are using to probe this (or PRP0001
> from acpi which uses the dt bindings table)

Frankly, I have no idea how to do that or if that would still be required when
using ACPI. Can you point me in a rough direction in case it's still needed?

Regards,
Christian

---

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/iio/accel/mxc6255.c?h=v5.6.15&id=06777c562a50a09c4a2becfb2bf63c762a45df17

[2]
P: /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
L: 0
E: DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:22/MXC6655:00
E: SUBSYSTEM=acpi
E: MODALIAS=acpi:MXC6655:MXC6655:
E: USEC_INITIALIZED=5319671
E: ID_VENDOR_FROM_DATABASE=The Linux Foundation

P: /devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
L: 0
E: DEVPATH=/devices/pci0000:00/0000:00:16.0/i2c_designware.0/i2c-0/i2c-MXC6655:00
E: SUBSYSTEM=i2c
E: MODALIAS=acpi:MXC6655:MXC6655:

[3]
https://github.com/systemd/systemd/commit/5e0676c2cad60b1ea029b9bfb9737e1967abb93a

On Sun, May 31, 2020 at 12:30 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Fri, 29 May 2020 22:05:49 +0200
> Christian Oder <me@xxxxxxxxxx> wrote:
>
> > The mxc6655 is fully working with the existing mxc4005 driver.
> > Add support for it.
> >
> > Signed-off-by: Christian Oder <me@xxxxxxxxxx>
>
> One query on ACPI bindings.  What is there already may
> be missleading :(
>
>
> > ---
> >  drivers/iio/accel/mxc4005.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/iio/accel/mxc4005.c b/drivers/iio/accel/mxc4005.c
> > index 3d5bea651923..3b8614352cb4 100644
> > --- a/drivers/iio/accel/mxc4005.c
> > +++ b/drivers/iio/accel/mxc4005.c
> > @@ -474,12 +474,14 @@ static int mxc4005_probe(struct i2c_client *client,
> >
> >  static const struct acpi_device_id mxc4005_acpi_match[] = {
> >       {"MXC4005",     0},
> > +     {"MXC6655",     0},
>
> Do we have a reference for these ACPI bindings?  While they may seem
> obvious, memsic don't have a registered PNP or ACPI ID that I can
> find.  If these are in the wild (i.e. in shipping firmware) then we
> can take them as defacto bindings, otherwise we should avoid making
> them so by putting them in the driver.
>
> Quite a few similar bindings got in a while back that I should have
> noticed, but I wasn't so familiar with ACPI back then.  Some
> scrubbing of these has gone on recently, but there are lots still
> left in IIO.
>
> If we aren't sure, then drop the ACPI addition and just leave the
> i2c one below.  Adding an explicit DT binding table would also be
> good if that is method you are using to probe this (or PRP0001
> from acpi which uses the dt bindings table)
>
> Thanks,
>
> Jonathan
>
>
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(acpi, mxc4005_acpi_match);
> >
> >  static const struct i2c_device_id mxc4005_id[] = {
> >       {"mxc4005",     0},
> > +     {"mxc6655",     0},
> >       { },
> >  };
> >  MODULE_DEVICE_TABLE(i2c, mxc4005_id);
>