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]

Date:   Sun, 31 May 2020 11:30:05 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Christian Oder <me@...elf5.de>
Cc:     myself5@...bonrom.org, 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

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);