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

[Han Xu <han.xu@nxp.com>]

On 22/12/11 01:09PM, Jonathan Cameron wrote:
> 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.
> 

There is another local patch not upstreamed, just sent them out as a patch set.

> 
> > ---
> >  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.

fixed in v2.

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