Re: [PATCH 1/2] iio: accel: add the new entry in driver for FXLS8967AF

[Jonathan Cameron <jic23@kernel.org>]

On Wed,  7 Dec 2022 10:20:44 -0600
Han Xu <han.xu@nxp.com> wrote:

> Add this new device entry in the driver id table.
> 
> Signed-off-by: Han Xu <han.xu@nxp.com>
Hi Han,

I went to apply this and discovered the FXLs8974CF_DEVICE_ID etc
isn't yet present in the upstream driver.  Have a I missed a patch
set somewhere?

One comment inline.


> ---
>  drivers/iio/accel/fxls8962af-core.c | 7 +++++++
>  drivers/iio/accel/fxls8962af-i2c.c  | 2 ++
>  drivers/iio/accel/fxls8962af.h      | 1 +
>  3 files changed, 10 insertions(+)
> 
> diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
> index 17a6f4f4e06c..ddeb1b153499 100644
> --- a/drivers/iio/accel/fxls8962af-core.c
> +++ b/drivers/iio/accel/fxls8962af-core.c
> @@ -127,6 +127,7 @@
>  #define FXLS8962AF_DEVICE_ID			0x62
>  #define FXLS8964AF_DEVICE_ID			0x84
>  #define FXLS8974CF_DEVICE_ID			0x86
> +#define FXLS8967AF_DEVICE_ID			0x87
Fine with these being in numeric value order...
>  
>  /* Raw temp channel offset */
>  #define FXLS8962AF_TEMP_CENTER_VAL		25
> @@ -772,6 +773,12 @@ static const struct fxls8962af_chip_info fxls_chip_info_table[] = {
>  		.channels = fxls8962af_channels,
>  		.num_channels = ARRAY_SIZE(fxls8962af_channels),
>  	},
> +	[fxls8967af] = {

I'd rather everything else was in 'alphabetical order as anyone looking down
the lists will expect that ordering rather than one based on IDs that they
probably won't have reason to know.

> +		.chip_id = FXLS8967AF_DEVICE_ID,
> +		.name = "fxls8967af",
> +		.channels = fxls8962af_channels,
> +		.num_channels = ARRAY_SIZE(fxls8962af_channels),
> +	},
>  };
>  
>  static const struct iio_info fxls8962af_info = {
> diff --git a/drivers/iio/accel/fxls8962af-i2c.c b/drivers/iio/accel/fxls8962af-i2c.c
> index 4a755a39d702..fd21f524e04f 100644
> --- a/drivers/iio/accel/fxls8962af-i2c.c
> +++ b/drivers/iio/accel/fxls8962af-i2c.c
> @@ -31,6 +31,7 @@ static const struct i2c_device_id fxls8962af_id[] = {
>  	{ "fxls8962af", fxls8962af },
>  	{ "fxls8964af", fxls8964af },
>  	{ "fxls8974cf", fxls8974cf },
> +	{ "fxls8967af", fxls8967af },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, fxls8962af_id);
> @@ -39,6 +40,7 @@ static const struct of_device_id fxls8962af_of_match[] = {
>  	{ .compatible = "nxp,fxls8962af" },
>  	{ .compatible = "nxp,fxls8964af" },
>  	{ .compatible = "nxp,fxls8974cf" },
> +	{ .compatible = "nxp,fxls8967af" },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, fxls8962af_of_match);
> diff --git a/drivers/iio/accel/fxls8962af.h b/drivers/iio/accel/fxls8962af.h
> index 45c7e57412e0..7de924d15694 100644
> --- a/drivers/iio/accel/fxls8962af.h
> +++ b/drivers/iio/accel/fxls8962af.h
> @@ -12,6 +12,7 @@ enum {
>  	fxls8962af,
>  	fxls8964af,
>  	fxls8974cf,
> +	fxls8967af,
>  };
>  
>  int fxls8962af_core_probe(struct device *dev, struct regmap *regmap, int irq);