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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAO6HPN1OfyJBxGO-8-jL6BNqxRfZzDkOkytNLpqAJam=2MPJ8w@mail.gmail.com>
Date:   Sun, 31 May 2020 15:16:00 +0200
From:   Christian Oder <me@...elf5.de>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Christian Oder <me@...elf5.de>, Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Thomas Gleixner <tglx@...utronix.de>,
        Allison Randal <allison@...utok.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Chuhong Yuan <hslester96@...il.com>, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iio: accel: mxc4005: add support for mxc6655

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@...nel.org> wrote:
>
> On Fri, 29 May 2020 22:05:49 +0200
> Christian Oder <me@...elf5.de> wrote:
>
> > The mxc6655 is fully working with the existing mxc4005 driver.
> > Add support for it.
> >
> > Signed-off-by: Christian Oder <me@...elf5.de>
>
> 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);
>