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

[Jonathan Cameron]

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