| Message ID | 20221213171536.1880089-3-han.xu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | FXLS8967AF and FXLS8974CF support | expand |
On Tue, 13 Dec 2022 11:15:33 -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> > > --- > changes in v2 > - change chip info orders > --- > 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 98811e4e16bb..c3589c3084ee 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 > > /* Raw temp channel offset */ > #define FXLS8962AF_TEMP_CENTER_VAL 25 > @@ -765,6 +766,12 @@ static const struct fxls8962af_chip_info fxls_chip_info_table[] = { > .channels = fxls8962af_channels, > .num_channels = ARRAY_SIZE(fxls8962af_channels), > }, > + [fxls8967af] = { > + .chip_id = FXLS8967AF_DEVICE_ID, > + .name = "fxls8967af", > + .channels = fxls8962af_channels, > + .num_channels = ARRAY_SIZE(fxls8962af_channels), So far all the channels /num_channels in these have pointed to same place. Not a lot of point in having the complexity if it's not used. I'd like to see that dropped unless we know it is going to be used in a patch set you expect to post soon after this one. > + }, > [fxls8974cf] = { > .chip_id = FXLS8974CF_DEVICE_ID, > .name = "fxls8974cf", > diff --git a/drivers/iio/accel/fxls8962af-i2c.c b/drivers/iio/accel/fxls8962af-i2c.c > index 17dd56756ff9..a8944b255a28 100644 > --- a/drivers/iio/accel/fxls8962af-i2c.c > +++ b/drivers/iio/accel/fxls8962af-i2c.c > @@ -30,6 +30,7 @@ static int fxls8962af_probe(struct i2c_client *client) > static const struct i2c_device_id fxls8962af_id[] = { > { "fxls8962af", fxls8962af }, > { "fxls8964af", fxls8964af }, > + { "fxls8967af", fxls8967af }, As in previous patch, driver_data isn't used anyway so drop it. > { "fxls8974cf", fxls8974cf }, > {} > }; > @@ -38,6 +39,7 @@ MODULE_DEVICE_TABLE(i2c, fxls8962af_id); > static const struct of_device_id fxls8962af_of_match[] = { > { .compatible = "nxp,fxls8962af" }, > { .compatible = "nxp,fxls8964af" }, > + { .compatible = "nxp,fxls8967af" }, > { .compatible = "nxp,fxls8974cf" }, > {} > }; > diff --git a/drivers/iio/accel/fxls8962af.h b/drivers/iio/accel/fxls8962af.h > index 45c7e57412e0..ba33eb2b807d 100644 > --- a/drivers/iio/accel/fxls8962af.h > +++ b/drivers/iio/accel/fxls8962af.h > @@ -11,6 +11,7 @@ struct device; > enum { > fxls8962af, > fxls8964af, > + fxls8967af, > fxls8974cf, > }; >
On Wed, 14 Dec 2022 10:54:00 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 14/12/2022 10:32, Jonathan Cameron wrote: > > On Tue, 13 Dec 2022 19:53:30 +0100 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 13/12/2022 18:15, Han Xu wrote: > >>> Add this new device entry in the driver id table. > >>> > >>> Signed-off-by: Han Xu <han.xu@nxp.com> > >>> > >>> --- > >>> changes in v2 > >>> - change chip info orders > >>> --- > >>> 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 98811e4e16bb..c3589c3084ee 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 > >>> > >>> /* Raw temp channel offset */ > >>> #define FXLS8962AF_TEMP_CENTER_VAL 25 > >>> @@ -765,6 +766,12 @@ static const struct fxls8962af_chip_info fxls_chip_info_table[] = { > >>> .channels = fxls8962af_channels, > >>> .num_channels = ARRAY_SIZE(fxls8962af_channels), > >>> }, > >>> + [fxls8967af] = { > >>> + .chip_id = FXLS8967AF_DEVICE_ID, > >>> + .name = "fxls8967af", > >>> + .channels = fxls8962af_channels, > >>> + .num_channels = ARRAY_SIZE(fxls8962af_channels), > >>> + }, > >>> [fxls8974cf] = { > >>> .chip_id = FXLS8974CF_DEVICE_ID, > >>> .name = "fxls8974cf", > >>> diff --git a/drivers/iio/accel/fxls8962af-i2c.c b/drivers/iio/accel/fxls8962af-i2c.c > >>> index 17dd56756ff9..a8944b255a28 100644 > >>> --- a/drivers/iio/accel/fxls8962af-i2c.c > >>> +++ b/drivers/iio/accel/fxls8962af-i2c.c > >>> @@ -30,6 +30,7 @@ static int fxls8962af_probe(struct i2c_client *client) > >>> static const struct i2c_device_id fxls8962af_id[] = { > >>> { "fxls8962af", fxls8962af }, > >>> { "fxls8964af", fxls8964af }, > >>> + { "fxls8967af", fxls8967af }, > >>> { "fxls8974cf", fxls8974cf }, > >>> {} > >>> }; > >>> @@ -38,6 +39,7 @@ MODULE_DEVICE_TABLE(i2c, fxls8962af_id); > >>> static const struct of_device_id fxls8962af_of_match[] = { > >>> { .compatible = "nxp,fxls8962af" }, > >>> { .compatible = "nxp,fxls8964af" }, > >>> + { .compatible = "nxp,fxls8967af" }, > >>> { .compatible = "nxp,fxls8974cf" }, > >> > >> This is confusing. The I2C ID table has driver data, but OF ID table > >> hasn't. So are they compatible or not? > > > > Due to some evilness in i2c that 'works' as long as the two arrays have > > matching entries. As a general rule we prefer to have the data in both, check > > the firmware table first and only then fallback to i2c_device_id data on the > > basis it is less fragile. > > > > The evilness in i2c is that the search for match data will use the dt compatible > > stripped of the vendor prefix and string match that against the i2c_device_id table. > > > > Nice to clean this up, but not necessarily in this series (fine if it is though!) > > OK, so in fact devices are not fully compatible - I got mislead by OF > table. I'll comment in bindings about it. I actually took a look at the driver today. It's not using the driver_data anyway. It does the better option of searching for a match based on the WHO_AM_I anyway. So best option is a precursor patch dropping the driver_data from the i2c_device_id table. Oops. I was guilty of making assumptions and didn't previously check. It could have been used as described, but wasn't. > > Best regards, > Krzysztof >
diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c index 98811e4e16bb..c3589c3084ee 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 /* Raw temp channel offset */ #define FXLS8962AF_TEMP_CENTER_VAL 25 @@ -765,6 +766,12 @@ static const struct fxls8962af_chip_info fxls_chip_info_table[] = { .channels = fxls8962af_channels, .num_channels = ARRAY_SIZE(fxls8962af_channels), }, + [fxls8967af] = { + .chip_id = FXLS8967AF_DEVICE_ID, + .name = "fxls8967af", + .channels = fxls8962af_channels, + .num_channels = ARRAY_SIZE(fxls8962af_channels), + }, [fxls8974cf] = { .chip_id = FXLS8974CF_DEVICE_ID, .name = "fxls8974cf", diff --git a/drivers/iio/accel/fxls8962af-i2c.c b/drivers/iio/accel/fxls8962af-i2c.c index 17dd56756ff9..a8944b255a28 100644 --- a/drivers/iio/accel/fxls8962af-i2c.c +++ b/drivers/iio/accel/fxls8962af-i2c.c @@ -30,6 +30,7 @@ static int fxls8962af_probe(struct i2c_client *client) static const struct i2c_device_id fxls8962af_id[] = { { "fxls8962af", fxls8962af }, { "fxls8964af", fxls8964af }, + { "fxls8967af", fxls8967af }, { "fxls8974cf", fxls8974cf }, {} }; @@ -38,6 +39,7 @@ MODULE_DEVICE_TABLE(i2c, fxls8962af_id); static const struct of_device_id fxls8962af_of_match[] = { { .compatible = "nxp,fxls8962af" }, { .compatible = "nxp,fxls8964af" }, + { .compatible = "nxp,fxls8967af" }, { .compatible = "nxp,fxls8974cf" }, {} }; diff --git a/drivers/iio/accel/fxls8962af.h b/drivers/iio/accel/fxls8962af.h index 45c7e57412e0..ba33eb2b807d 100644 --- a/drivers/iio/accel/fxls8962af.h +++ b/drivers/iio/accel/fxls8962af.h @@ -11,6 +11,7 @@ struct device; enum { fxls8962af, fxls8964af, + fxls8967af, fxls8974cf, };
Add this new device entry in the driver id table. Signed-off-by: Han Xu <han.xu@nxp.com> --- changes in v2 - change chip info orders --- 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(+)