| Message ID | 593798a44c8ba45f969b86aa29e172d59065958c.1682373451.git.mehdi.djait.k@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer | expand |
On 4/25/23 01:22, Mehdi Djait wrote: > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > ranges from ±2G to ±16G, digital output through I²C/SPI. > Add support for basic accelerometer features such as reading acceleration > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> > --- > drivers/iio/accel/Kconfig | 8 +- > drivers/iio/accel/kionix-kx022a-i2c.c | 2 + > drivers/iio/accel/kionix-kx022a-spi.c | 2 + > drivers/iio/accel/kionix-kx022a.c | 147 ++++++++++++++++++++++++++ > drivers/iio/accel/kionix-kx022a.h | 52 +++++++++ > 5 files changed, 207 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index b6b45d359f28..d8cc6e6f2bb9 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -418,8 +418,8 @@ config IIO_KX022A_SPI > select IIO_KX022A > select REGMAP_SPI > help > - Enable support for the Kionix KX022A digital tri-axis > - accelerometer connected to I2C interface. > + Enable support for the Kionix KX022A, KX132-1211 digital tri-axis > + accelerometers connected to SPI interface. > > config IIO_KX022A_I2C > tristate "Kionix KX022A tri-axis digital accelerometer I2C interface" > @@ -427,8 +427,8 @@ config IIO_KX022A_I2C > select IIO_KX022A > select REGMAP_I2C > help > - Enable support for the Kionix KX022A digital tri-axis > - accelerometer connected to I2C interface. > + Enable support for the Kionix KX022A, KX132-1211 digital tri-axis > + accelerometers connected to I2C interface. > > config KXSD9 > tristate "Kionix KXSD9 Accelerometer Driver" > diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c > index ce299d0446f7..4ea28d2482ec 100644 > --- a/drivers/iio/accel/kionix-kx022a-i2c.c > +++ b/drivers/iio/accel/kionix-kx022a-i2c.c > @@ -39,12 +39,14 @@ static int kx022a_i2c_probe(struct i2c_client *i2c) > > static const struct i2c_device_id kx022a_i2c_id[] = { > { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info }, > + { .name = "kx132-1211", .driver_data = (kernel_ulong_t)&kx132_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id); > > static const struct of_device_id kx022a_of_match[] = { > { .compatible = "kionix,kx022a", .data = &kx022a_chip_info }, > + { .compatible = "kionix,kx132-1211", .data = &kx132_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(of, kx022a_of_match); > diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c > index b84503e24510..b755b2b395ed 100644 > --- a/drivers/iio/accel/kionix-kx022a-spi.c > +++ b/drivers/iio/accel/kionix-kx022a-spi.c > @@ -39,12 +39,14 @@ static int kx022a_spi_probe(struct spi_device *spi) > > static const struct spi_device_id kx022a_id[] = { > { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info }, > + { .name = "kx132-1211", .driver_data = (kernel_ulong_t)&kx132_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(spi, kx022a_id); > > static const struct of_device_id kx022a_of_match[] = { > { .compatible = "kionix,kx022a", .data = &kx022a_chip_info }, > + { .compatible = "kionix,kx132-1211", .data = &kx132_chip_info }, > { } > }; > MODULE_DEVICE_TABLE(of, kx022a_of_match); > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c > index 4a31d17c1f22..a6808ab12162 100644 > --- a/drivers/iio/accel/kionix-kx022a.c > +++ b/drivers/iio/accel/kionix-kx022a.c > @@ -150,6 +150,101 @@ static const struct regmap_config kx022a_regmap_config = { > .cache_type = REGCACHE_RBTREE, > }; > > +/* Regmap configs kx132 */ > +static const struct regmap_range kx132_volatile_ranges[] = { > + { > + .range_min = KX132_REG_XADP_L, > + .range_max = KX132_REG_COTR, > + }, { > + .range_min = KX132_REG_TSCP, > + .range_max = KX132_REG_INT_REL, > + }, { > + /* The reset bit will be cleared by sensor */ > + .range_min = KX132_REG_CNTL2, > + .range_max = KX132_REG_CNTL2, > + }, { > + .range_min = KX132_REG_BUF_STATUS_1, > + .range_max = KX132_REG_BUF_READ, > + }, > +}; Maybe the CNTL5 should also be added to volatile table as it has "self clearing" bits? I didn't go through all the registers to see if there are more. > + > +static const struct regmap_access_table kx132_volatile_regs = { > + .yes_ranges = &kx132_volatile_ranges[0], > + .n_yes_ranges = ARRAY_SIZE(kx132_volatile_ranges), > +}; > + > +static const struct regmap_range kx132_precious_ranges[] = { > + { > + .range_min = KX132_REG_INT_REL, > + .range_max = KX132_REG_INT_REL, > + }, > +}; > + > +static const struct regmap_access_table kx132_precious_regs = { > + .yes_ranges = &kx132_precious_ranges[0], > + .n_yes_ranges = ARRAY_SIZE(kx132_precious_ranges), > +}; > + > +static const struct regmap_range kx132_read_only_ranges[] = { > + { > + .range_min = KX132_REG_XADP_L, > + .range_max = KX132_REG_INT_REL, > + }, { > + .range_min = KX132_REG_BUF_STATUS_1, > + .range_max = KX132_REG_BUF_STATUS_2, > + }, { > + .range_min = KX132_REG_BUF_READ, > + .range_max = KX132_REG_BUF_READ, > + }, > +}; Do you think adding the "Kionix reserved" registers to read-only ranges would make things safer or is there a reason to keep them writable? I think the data-sheet states these should not be written to. > + > +static const struct regmap_access_table kx132_ro_regs = { > + .no_ranges = &kx132_read_only_ranges[0], > + .n_no_ranges = ARRAY_SIZE(kx132_read_only_ranges), > +}; > + > +static const struct regmap_range kx132_write_only_ranges[] = { > + { > + .range_min = KX132_REG_MAN_WAKE, > + .range_max = KX132_REG_MAN_WAKE, Why the WUFC is write-only? Also, I don't think the KX022A_REG_MAN_WAKE and KX132_REG_MAN_WAKE reflect same functionality. The naming of the define may be slightly misleading. > + }, { > + .range_min = KX132_REG_SELF_TEST, > + .range_max = KX132_REG_SELF_TEST, > + }, { > + .range_min = KX132_REG_BUF_CLEAR, > + .range_max = KX132_REG_BUF_CLEAR, > + }, > +}; > + > +static const struct regmap_access_table kx132_wo_regs = { > + .no_ranges = &kx132_write_only_ranges[0], > + .n_no_ranges = ARRAY_SIZE(kx132_write_only_ranges), > +}; > + > +static const struct regmap_range kx132_noinc_read_ranges[] = { > + { > + .range_min = KX132_REG_BUF_READ, > + .range_max = KX132_REG_BUF_READ, > + }, > +}; > + > +static const struct regmap_access_table kx132_nir_regs = { > + .yes_ranges = &kx132_noinc_read_ranges[0], > + .n_yes_ranges = ARRAY_SIZE(kx132_noinc_read_ranges), > +}; > + > +static const struct regmap_config kx132_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .volatile_table = &kx132_volatile_regs, > + .rd_table = &kx132_wo_regs, > + .wr_table = &kx132_ro_regs, > + .rd_noinc_table = &kx132_nir_regs, > + .precious_table = &kx132_precious_regs, > + .max_register = KX132_MAX_REGISTER, > + .cache_type = REGCACHE_RBTREE, > +}; > + > struct kx022a_data { > const struct kx022a_chip_info *chip_info; > struct regmap *regmap; > @@ -237,6 +332,13 @@ static const struct iio_chan_spec kx022a_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(3), > }; > > +static const struct iio_chan_spec kx132_channels[] = { > + KX022A_ACCEL_CHAN(X, KX132_REG_XOUT_L, 0), > + KX022A_ACCEL_CHAN(Y, KX132_REG_YOUT_L, 1), > + KX022A_ACCEL_CHAN(Z, KX132_REG_ZOUT_L, 2), > + IIO_CHAN_SOFT_TIMESTAMP(3), > +}; > + > /* > * The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the > * Linux CPUs can handle without dropping samples. Also, the low power mode is > @@ -613,6 +715,25 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data) > return fifo_bytes; > } > > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > +{ > + struct device *dev = regmap_get_device(data->regmap); > + __le16 buf_status; > + int ret, fifo_bytes; > + > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > + &buf_status, sizeof(buf_status)); > + if (ret) { > + dev_err(dev, "Error reading buffer status\n"); > + return ret; > + } > + > + fifo_bytes = le16_to_cpu(buf_status); > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; This is probably just my limitation but I've hard time thinking how this works out on BE machines. It'd be much easier for me to understand this if the data was handled as two u8 values and mask was applied before endianes conversion. (Eg - untested pseudo code follows; __le16 buf_status; u8 *reg_data; ... ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, &buf_status, sizeof(buf_status)); ... reg_data = (u8 *)&buf_status; /* Clear the unused bits form 2.nd reg */ reg_data[1] = reg_data[i] & MASK_SMP_LVL_REG_HIGH_BITS; /* Convert to CPU endianess */ fifo_bytes = le16_to_cpu(buf_status); Well, others may have different view on this :) > + > + return fifo_bytes; > +} > + > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > bool irq) > { > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > }; > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > +const struct kx022a_chip_info kx132_chip_info = { > + .name = "kx132-1211", > + .regmap_config = &kx132_regmap_config, > + .channels = kx132_channels, > + .num_channels = ARRAY_SIZE(kx132_channels), > + .fifo_length = KX132_FIFO_LENGTH, > + .who = KX132_REG_WHO, > + .id = KX132_ID, > + .cntl = KX132_REG_CNTL, > + .cntl2 = KX132_REG_CNTL2, > + .odcntl = KX132_REG_ODCNTL, > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > + .buf_clear = KX132_REG_BUF_CLEAR, > + .buf_status1 = KX132_REG_BUF_STATUS_1, > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, > + .buf_read = KX132_REG_BUF_READ, > + .inc1 = KX132_REG_INC1, > + .inc4 = KX132_REG_INC4, > + .inc5 = KX132_REG_INC5, > + .inc6 = KX132_REG_INC6, > + .xout_l = KX132_REG_XOUT_L, > + .get_fifo_bytes = kx132_get_fifo_bytes, > +}; > +EXPORT_SYMBOL_NS_GPL(kx132_chip_info, IIO_KX022A); > + > int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info) > { > static const char * const regulator_names[] = {"io-vdd", "vdd"}; > diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h > index f043767067b7..1f4135cf20eb 100644 > --- a/drivers/iio/accel/kionix-kx022a.h > +++ b/drivers/iio/accel/kionix-kx022a.h > @@ -74,6 +74,57 @@ > #define KX022A_REG_SELF_TEST 0x60 > #define KX022A_MAX_REGISTER 0x60 > > +#define KX132_REG_WHO 0x13 > +#define KX132_ID 0x3d > + > +#define KX132_FIFO_LENGTH 86 > + > +#define KX132_REG_CNTL 0x1b > +#define KX132_REG_CNTL2 0x1c > +#define KX132_MASK_RES BIT(6) > +#define KX132_GSEL_2 0x0 > +#define KX132_GSEL_4 BIT(3) > +#define KX132_GSEL_8 BIT(4) > +#define KX132_GSEL_16 GENMASK(4, 3) > + > +#define KX132_REG_INS2 0x17 > +#define KX132_MASK_INS2_WMI BIT(5) > + > +#define KX132_REG_XADP_L 0x02 > +#define KX132_REG_XOUT_L 0x08 > +#define KX132_REG_YOUT_L 0x0a > +#define KX132_REG_ZOUT_L 0x0c > +#define KX132_REG_COTR 0x12 > +#define KX132_REG_TSCP 0x14 > +#define KX132_REG_INT_REL 0x1a > + > +#define KX132_REG_ODCNTL 0x21 > + > +#define KX132_REG_BTS_WUF_TH 0x4a > +#define KX132_REG_MAN_WAKE 0x4d As mentioned elsewhere, I don't think this is functionally same as KX022A_REG_MAN_WAKE. > + > +#define KX132_REG_BUF_CNTL1 0x5e > +#define KX132_REG_BUF_CNTL2 0x5f > +#define KX132_REG_BUF_STATUS_1 0x60 > +#define KX132_REG_BUF_STATUS_2 0x61 > +#define KX132_MASK_BUF_SMP_LVL GENMASK(9, 0) > +#define KX132_REG_BUF_CLEAR 0x62 > +#define KX132_REG_BUF_READ 0x63 > +#define KX132_ODR_SHIFT 3 > +#define KX132_FIFO_MAX_WMI_TH 86 > + > +#define KX132_REG_INC1 0x22 > +#define KX132_REG_INC5 0x26 > +#define KX132_REG_INC6 0x27 > +#define KX132_IPOL_LOW 0 > +#define KX132_IPOL_HIGH KX022A_MASK_IPOL > +#define KX132_ITYP_PULSE KX022A_MASK_ITYP > + > +#define KX132_REG_INC4 0x25 > + > +#define KX132_REG_SELF_TEST 0x5d > +#define KX132_MAX_REGISTER 0x76 > + > struct device; > > struct kx022a_data; > @@ -132,5 +183,6 @@ struct kx022a_chip_info { > int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info); > > extern const struct kx022a_chip_info kx022a_chip_info; > +extern const struct kx022a_chip_info kx132_chip_info; > > #endif Thanks for adding this support! Yours, -- Matti
On Tue, 25 Apr 2023 00:22:27 +0200 Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > ranges from ±2G to ±16G, digital output through I²C/SPI. > Add support for basic accelerometer features such as reading acceleration > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> Two tiny things inline. > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > +{ > + struct device *dev = regmap_get_device(data->regmap); > + __le16 buf_status; > + int ret, fifo_bytes; > + > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > + &buf_status, sizeof(buf_status)); > + if (ret) { > + dev_err(dev, "Error reading buffer status\n"); > + return ret; > + } > + > + fifo_bytes = le16_to_cpu(buf_status); > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; Slight preference for FIELD_GET() as it saves me checking the mask includes lowest bits. > + > + return fifo_bytes; > +} > + > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > bool irq) > { > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > }; > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > +const struct kx022a_chip_info kx132_chip_info = { > + .name = "kx132-1211", > + .regmap_config = &kx132_regmap_config, > + .channels = kx132_channels, > + .num_channels = ARRAY_SIZE(kx132_channels), > + .fifo_length = KX132_FIFO_LENGTH, > + .who = KX132_REG_WHO, > + .id = KX132_ID, > + .cntl = KX132_REG_CNTL, > + .cntl2 = KX132_REG_CNTL2, > + .odcntl = KX132_REG_ODCNTL, > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > + .buf_clear = KX132_REG_BUF_CLEAR, > + .buf_status1 = KX132_REG_BUF_STATUS_1, > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, There are some things in here (typically where the define isn't used anywhere else) where I think it would be easier to follow if the value was listed here. Masks and IDs for example. > + .buf_read = KX132_REG_BUF_READ, > + .inc1 = KX132_REG_INC1, > + .inc4 = KX132_REG_INC4, > + .inc5 = KX132_REG_INC5, > + .inc6 = KX132_REG_INC6, > + .xout_l = KX132_REG_XOUT_L, > + .get_fifo_bytes = kx132_get_fifo_bytes, > +}; > +EXPORT_SYMBOL_NS_GPL(kx132_chip_info, IIO_KX022A); > + > int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info) > { > static const char * const regulator_names[] = {"io-vdd", "vdd"};
> > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > > +{ > > + struct device *dev = regmap_get_device(data->regmap); > > + __le16 buf_status; > > + int ret, fifo_bytes; > > + > > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > > + &buf_status, sizeof(buf_status)); > > + if (ret) { > > + dev_err(dev, "Error reading buffer status\n"); > > + return ret; > > + } > > + > > + fifo_bytes = le16_to_cpu(buf_status); > > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; > > This is probably just my limitation but I've hard time thinking how this > works out on BE machines. It'd be much easier for me to understand this > if the data was handled as two u8 values and mask was applied before > endianes conversion. (Eg - untested pseudo code follows; > > __le16 buf_status; > u8 *reg_data; > > ... > > ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > &buf_status, sizeof(buf_status)); > ... > > reg_data = (u8 *)&buf_status; > > /* Clear the unused bits form 2.nd reg */ > reg_data[1] = reg_data[i] & MASK_SMP_LVL_REG_HIGH_BITS; > > /* Convert to CPU endianess */ > fifo_bytes = le16_to_cpu(buf_status); > > Well, others may have different view on this :) :) I go the other way. It's less obvious to me that it is appropriate to apply le16_to_cpu(buf_status) after applying a mask to some bits. The moment that is appropriate, then we certainly hope a single mask application is as well. I think treating it as a 16 bit register is appropriate, in particular as the field is described as SMP_LEV[9:0] on the datasheet (of course there are datasheets that do that for unconnected sets of bits so this doesn't always work ;) Jonathan
Hello Jonathan, On Mon, May 01, 2023 at 03:56:45PM +0100, Jonathan Cameron wrote: > On Tue, 25 Apr 2023 00:22:27 +0200 > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > > ranges from ±2G to ±16G, digital output through I²C/SPI. > > Add support for basic accelerometer features such as reading acceleration > > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> > > Two tiny things inline. > > > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > > +{ > > + struct device *dev = regmap_get_device(data->regmap); > > + __le16 buf_status; > > + int ret, fifo_bytes; > > + > > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > > + &buf_status, sizeof(buf_status)); > > + if (ret) { > > + dev_err(dev, "Error reading buffer status\n"); > > + return ret; > > + } > > + > > + fifo_bytes = le16_to_cpu(buf_status); > > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; > > Slight preference for FIELD_GET() as it saves me checking the mask includes > lowest bits. This will mean I have the remove the chip_info member buf_smp_lvl_mask and use KX132_MASK_BUF_SMP_LVL directly because otherwise the __builtin_constant_p function will cause an error when building. Check: https://elixir.bootlin.com/linux/latest/source/include/linux/bitfield.h#L65 I can change it to FIELD_GET() if you want to. > > > > + > > + return fifo_bytes; > > +} > > + > > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > > bool irq) > > { > > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > > }; > > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > > > +const struct kx022a_chip_info kx132_chip_info = { > > + .name = "kx132-1211", > > + .regmap_config = &kx132_regmap_config, > > + .channels = kx132_channels, > > + .num_channels = ARRAY_SIZE(kx132_channels), > > + .fifo_length = KX132_FIFO_LENGTH, > > + .who = KX132_REG_WHO, > > + .id = KX132_ID, > > + .cntl = KX132_REG_CNTL, > > + .cntl2 = KX132_REG_CNTL2, > > + .odcntl = KX132_REG_ODCNTL, > > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > > + .buf_clear = KX132_REG_BUF_CLEAR, > > + .buf_status1 = KX132_REG_BUF_STATUS_1, > > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, > > There are some things in here (typically where the define isn't used > anywhere else) where I think it would be easier to follow if the > value was listed here. Masks and IDs for example. > After removing buf_smp_lvl_mask, which members will be easier to understand (besides id) ? -- Kind Regards Mehdi Djait
On Fri, 5 May 2023 20:11:33 +0200 Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > Hello Jonathan, > > On Mon, May 01, 2023 at 03:56:45PM +0100, Jonathan Cameron wrote: > > On Tue, 25 Apr 2023 00:22:27 +0200 > > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > > > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > > > ranges from ±2G to ±16G, digital output through I²C/SPI. > > > Add support for basic accelerometer features such as reading acceleration > > > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > > > > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> > > > > Two tiny things inline. > > > > > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > > > +{ > > > + struct device *dev = regmap_get_device(data->regmap); > > > + __le16 buf_status; > > > + int ret, fifo_bytes; > > > + > > > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > > > + &buf_status, sizeof(buf_status)); > > > + if (ret) { > > > + dev_err(dev, "Error reading buffer status\n"); > > > + return ret; > > > + } > > > + > > > + fifo_bytes = le16_to_cpu(buf_status); > > > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; > > > > Slight preference for FIELD_GET() as it saves me checking the mask includes > > lowest bits. > > This will mean I have the remove the chip_info member buf_smp_lvl_mask > and use KX132_MASK_BUF_SMP_LVL directly because otherwise the > __builtin_constant_p function will cause an error when building. > Check: https://elixir.bootlin.com/linux/latest/source/include/linux/bitfield.h#L65 > > I can change it to FIELD_GET() if you want to. Good point. You could use le16_get_bits() though which I'm fairly sure will take a variable just fine. > > > > > > > > + > > > + return fifo_bytes; > > > +} > > > + > > > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > > > bool irq) > > > { > > > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > > > }; > > > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > > > > > +const struct kx022a_chip_info kx132_chip_info = { > > > + .name = "kx132-1211", > > > + .regmap_config = &kx132_regmap_config, > > > + .channels = kx132_channels, > > > + .num_channels = ARRAY_SIZE(kx132_channels), > > > + .fifo_length = KX132_FIFO_LENGTH, > > > + .who = KX132_REG_WHO, > > > + .id = KX132_ID, > > > + .cntl = KX132_REG_CNTL, > > > + .cntl2 = KX132_REG_CNTL2, > > > + .odcntl = KX132_REG_ODCNTL, > > > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > > > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > > > + .buf_clear = KX132_REG_BUF_CLEAR, > > > + .buf_status1 = KX132_REG_BUF_STATUS_1, > > > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, > > > > There are some things in here (typically where the define isn't used > > anywhere else) where I think it would be easier to follow if the > > value was listed here. Masks and IDs for example. > > > > After removing buf_smp_lvl_mask, which members will be easier to understand (besides id) ? I haven't gone through them. Length seems an obvious one. It's a number not a magic value. Register addresses might also be simpler if they aren't used elsewhere. Not really about understanding just about a define that adds nothing if all we do is assign it to a variable of the same name. > > -- > Kind Regards > Mehdi Djait
Hello Jonathan, On Sat, May 06, 2023 at 05:46:51PM +0100, Jonathan Cameron wrote: > On Fri, 5 May 2023 20:11:33 +0200 > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > Hello Jonathan, > > > > On Mon, May 01, 2023 at 03:56:45PM +0100, Jonathan Cameron wrote: > > > On Tue, 25 Apr 2023 00:22:27 +0200 > > > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > > > > > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > > > > ranges from ±2G to ±16G, digital output through I²C/SPI. > > > > Add support for basic accelerometer features such as reading acceleration > > > > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > > > > > > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > > > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> > > > > > > Two tiny things inline. > > > > > > > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > > > > +{ > > > > + struct device *dev = regmap_get_device(data->regmap); > > > > + __le16 buf_status; > > > > + int ret, fifo_bytes; > > > > + > > > > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > > > > + &buf_status, sizeof(buf_status)); > > > > + if (ret) { > > > > + dev_err(dev, "Error reading buffer status\n"); > > > > + return ret; > > > > + } > > > > + > > > > + fifo_bytes = le16_to_cpu(buf_status); > > > > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; > > > > > > Slight preference for FIELD_GET() as it saves me checking the mask includes > > > lowest bits. > > > > This will mean I have the remove the chip_info member buf_smp_lvl_mask > > and use KX132_MASK_BUF_SMP_LVL directly because otherwise the > > __builtin_constant_p function will cause an error when building. > > Check: https://elixir.bootlin.com/linux/latest/source/include/linux/bitfield.h#L65 > > > > I can change it to FIELD_GET() if you want to. > > Good point. You could use le16_get_bits() though which I'm fairly sure will take > a variable just fine. > I don't think it will work. From the commit log introducing the {type}_get_bits to <linux/bitfield.h> " Field specification must be a constant; __builtin_constant_p() doesn't have to be true for it, but compiler must be able to evaluate it at build time. If it cannot or if the value does not encode any bitfield, the build will fail. " Actually Geert Uytterhoeven tried to solve excatly this issue, but it seems that the patch was not accepted. Check: https://lore.kernel.org/linux-iio/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be/ So which solution would be the best: 1. Use directly KX132_MASK_BUF_SMP_LVL, the only reason I was trying to avoid this was to make this function generic enough for other chip variants 2. Copy the field_get() definition from drivers/clk/at91 or from the commit of Geert and use it in this driver 3. leave it as it is ? 4. another solution ? > > > > > > > > > > > > + > > > > + return fifo_bytes; > > > > +} > > > > + > > > > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > > > > bool irq) > > > > { > > > > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > > > > }; > > > > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > > > > > > > +const struct kx022a_chip_info kx132_chip_info = { > > > > + .name = "kx132-1211", > > > > + .regmap_config = &kx132_regmap_config, > > > > + .channels = kx132_channels, > > > > + .num_channels = ARRAY_SIZE(kx132_channels), > > > > + .fifo_length = KX132_FIFO_LENGTH, > > > > + .who = KX132_REG_WHO, > > > > + .id = KX132_ID, > > > > + .cntl = KX132_REG_CNTL, > > > > + .cntl2 = KX132_REG_CNTL2, > > > > + .odcntl = KX132_REG_ODCNTL, > > > > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > > > > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > > > > + .buf_clear = KX132_REG_BUF_CLEAR, > > > > + .buf_status1 = KX132_REG_BUF_STATUS_1, > > > > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, > > > > > > There are some things in here (typically where the define isn't used > > > anywhere else) where I think it would be easier to follow if the > > > value was listed here. Masks and IDs for example. > > > > > > > After removing buf_smp_lvl_mask, which members will be easier to understand (besides id) ? > > I haven't gone through them. Length seems an obvious one. It's a number not a magic value. > Register addresses might also be simpler if they aren't used elsewhere. > > Not really about understanding just about a define that adds nothing if all we do is > assign it to a variable of the same name. Do you have a strong opinion about this ? I would really prefer to leave it like this, for the following reasons: 1. If I directly use values here, I have to do it also in the previous patch where I introduce the chip_info for the kx022a -> this means I will have defines in the h file which are not used at all -> the defines should be deleted -> the patch will get unnecessarily bigger. I received multiple comments about removing unnecessary changes and reducing of the patch size when possible. 2. Consistency: having all the defines in one place really seems to be better for understanding IMO. I find the mix of values and defines in the chip-info a bit confusing, e.g., I will use the direct value for KX132_REG_CNTL but not for KX132_REG_CNTL2 because KX132_REG_CNTL2 is referenced in a regmap_range. -- Kind Regards Mehdi Djait
On Sun, 7 May 2023 16:56:55 +0200 Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > Hello Jonathan, > > On Sat, May 06, 2023 at 05:46:51PM +0100, Jonathan Cameron wrote: > > On Fri, 5 May 2023 20:11:33 +0200 > > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > > > Hello Jonathan, > > > > > > On Mon, May 01, 2023 at 03:56:45PM +0100, Jonathan Cameron wrote: > > > > On Tue, 25 Apr 2023 00:22:27 +0200 > > > > Mehdi Djait <mehdi.djait.k@gmail.com> wrote: > > > > > > > > > Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support > > > > > ranges from ±2G to ±16G, digital output through I²C/SPI. > > > > > Add support for basic accelerometer features such as reading acceleration > > > > > via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. > > > > > > > > > > Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf > > > > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> > > > > > > > > Two tiny things inline. > > > > > > > > > +static int kx132_get_fifo_bytes(struct kx022a_data *data) > > > > > +{ > > > > > + struct device *dev = regmap_get_device(data->regmap); > > > > > + __le16 buf_status; > > > > > + int ret, fifo_bytes; > > > > > + > > > > > + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, > > > > > + &buf_status, sizeof(buf_status)); > > > > > + if (ret) { > > > > > + dev_err(dev, "Error reading buffer status\n"); > > > > > + return ret; > > > > > + } > > > > > + > > > > > + fifo_bytes = le16_to_cpu(buf_status); > > > > > + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; > > > > > > > > Slight preference for FIELD_GET() as it saves me checking the mask includes > > > > lowest bits. > > > > > > This will mean I have the remove the chip_info member buf_smp_lvl_mask > > > and use KX132_MASK_BUF_SMP_LVL directly because otherwise the > > > __builtin_constant_p function will cause an error when building. > > > Check: https://elixir.bootlin.com/linux/latest/source/include/linux/bitfield.h#L65 > > > > > > I can change it to FIELD_GET() if you want to. > > > > Good point. You could use le16_get_bits() though which I'm fairly sure will take > > a variable just fine. > > > > I don't think it will work. > > From the commit log introducing the {type}_get_bits to <linux/bitfield.h> > " Field specification must be a constant; __builtin_constant_p() doesn't > have to be true for it, but compiler must be able to evaluate it at > build time. If it cannot or if the value does not encode any bitfield, > the build will fail. " > > Actually Geert Uytterhoeven tried to solve excatly this issue, but it > seems that the patch was not accepted. > Check: https://lore.kernel.org/linux-iio/3a54a6703879d10f08cf0275a2a69297ebd2b1d4.1637592133.git.geert+renesas@glider.be/ > > > So which solution would be the best: > > 1. Use directly KX132_MASK_BUF_SMP_LVL, the only reason I was trying to > avoid this was to make this function generic enough for other chip > variants > > 2. Copy the field_get() definition from drivers/clk/at91 or from the commit > of Geert and use it in this driver > > 3. leave it as it is ? This fine. Sorry for the diversion to nowhere! Jonathan > > 4. another solution ? > > > > > > > > > > > > > > > > > + > > > > > + return fifo_bytes; > > > > > +} > > > > > + > > > > > static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, > > > > > bool irq) > > > > > { > > > > > @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { > > > > > }; > > > > > EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); > > > > > > > > > > +const struct kx022a_chip_info kx132_chip_info = { > > > > > + .name = "kx132-1211", > > > > > + .regmap_config = &kx132_regmap_config, > > > > > + .channels = kx132_channels, > > > > > + .num_channels = ARRAY_SIZE(kx132_channels), > > > > > + .fifo_length = KX132_FIFO_LENGTH, > > > > > + .who = KX132_REG_WHO, > > > > > + .id = KX132_ID, > > > > > + .cntl = KX132_REG_CNTL, > > > > > + .cntl2 = KX132_REG_CNTL2, > > > > > + .odcntl = KX132_REG_ODCNTL, > > > > > + .buf_cntl1 = KX132_REG_BUF_CNTL1, > > > > > + .buf_cntl2 = KX132_REG_BUF_CNTL2, > > > > > + .buf_clear = KX132_REG_BUF_CLEAR, > > > > > + .buf_status1 = KX132_REG_BUF_STATUS_1, > > > > > + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, > > > > > > > > There are some things in here (typically where the define isn't used > > > > anywhere else) where I think it would be easier to follow if the > > > > value was listed here. Masks and IDs for example. > > > > > > > > > > After removing buf_smp_lvl_mask, which members will be easier to understand (besides id) ? > > > > I haven't gone through them. Length seems an obvious one. It's a number not a magic value. > > Register addresses might also be simpler if they aren't used elsewhere. > > > > Not really about understanding just about a define that adds nothing if all we do is > > assign it to a variable of the same name. > > Do you have a strong opinion about this ? > > I would really prefer to leave it like this, for the following reasons: > > 1. If I directly use values here, I have to do it also in the previous > patch where I introduce the chip_info for the kx022a -> this means I > will have defines in the h file which are not used at all -> the defines should > be deleted -> the patch will get unnecessarily bigger. I received > multiple comments about removing unnecessary changes and reducing of the > patch size when possible. > > 2. Consistency: having all the defines in one place really seems to be > better for understanding IMO. I find the mix of values and defines in > the chip-info a bit confusing, e.g., I will use the direct value for > KX132_REG_CNTL but not for KX132_REG_CNTL2 because KX132_REG_CNTL2 is > referenced in a regmap_range. > > -- > Kind Regards > Mehdi Djait
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index b6b45d359f28..d8cc6e6f2bb9 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -418,8 +418,8 @@ config IIO_KX022A_SPI select IIO_KX022A select REGMAP_SPI help - Enable support for the Kionix KX022A digital tri-axis - accelerometer connected to I2C interface. + Enable support for the Kionix KX022A, KX132-1211 digital tri-axis + accelerometers connected to SPI interface. config IIO_KX022A_I2C tristate "Kionix KX022A tri-axis digital accelerometer I2C interface" @@ -427,8 +427,8 @@ config IIO_KX022A_I2C select IIO_KX022A select REGMAP_I2C help - Enable support for the Kionix KX022A digital tri-axis - accelerometer connected to I2C interface. + Enable support for the Kionix KX022A, KX132-1211 digital tri-axis + accelerometers connected to I2C interface. config KXSD9 tristate "Kionix KXSD9 Accelerometer Driver" diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c index ce299d0446f7..4ea28d2482ec 100644 --- a/drivers/iio/accel/kionix-kx022a-i2c.c +++ b/drivers/iio/accel/kionix-kx022a-i2c.c @@ -39,12 +39,14 @@ static int kx022a_i2c_probe(struct i2c_client *i2c) static const struct i2c_device_id kx022a_i2c_id[] = { { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info }, + { .name = "kx132-1211", .driver_data = (kernel_ulong_t)&kx132_chip_info }, { } }; MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id); static const struct of_device_id kx022a_of_match[] = { { .compatible = "kionix,kx022a", .data = &kx022a_chip_info }, + { .compatible = "kionix,kx132-1211", .data = &kx132_chip_info }, { } }; MODULE_DEVICE_TABLE(of, kx022a_of_match); diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c index b84503e24510..b755b2b395ed 100644 --- a/drivers/iio/accel/kionix-kx022a-spi.c +++ b/drivers/iio/accel/kionix-kx022a-spi.c @@ -39,12 +39,14 @@ static int kx022a_spi_probe(struct spi_device *spi) static const struct spi_device_id kx022a_id[] = { { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info }, + { .name = "kx132-1211", .driver_data = (kernel_ulong_t)&kx132_chip_info }, { } }; MODULE_DEVICE_TABLE(spi, kx022a_id); static const struct of_device_id kx022a_of_match[] = { { .compatible = "kionix,kx022a", .data = &kx022a_chip_info }, + { .compatible = "kionix,kx132-1211", .data = &kx132_chip_info }, { } }; MODULE_DEVICE_TABLE(of, kx022a_of_match); diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c index 4a31d17c1f22..a6808ab12162 100644 --- a/drivers/iio/accel/kionix-kx022a.c +++ b/drivers/iio/accel/kionix-kx022a.c @@ -150,6 +150,101 @@ static const struct regmap_config kx022a_regmap_config = { .cache_type = REGCACHE_RBTREE, }; +/* Regmap configs kx132 */ +static const struct regmap_range kx132_volatile_ranges[] = { + { + .range_min = KX132_REG_XADP_L, + .range_max = KX132_REG_COTR, + }, { + .range_min = KX132_REG_TSCP, + .range_max = KX132_REG_INT_REL, + }, { + /* The reset bit will be cleared by sensor */ + .range_min = KX132_REG_CNTL2, + .range_max = KX132_REG_CNTL2, + }, { + .range_min = KX132_REG_BUF_STATUS_1, + .range_max = KX132_REG_BUF_READ, + }, +}; + +static const struct regmap_access_table kx132_volatile_regs = { + .yes_ranges = &kx132_volatile_ranges[0], + .n_yes_ranges = ARRAY_SIZE(kx132_volatile_ranges), +}; + +static const struct regmap_range kx132_precious_ranges[] = { + { + .range_min = KX132_REG_INT_REL, + .range_max = KX132_REG_INT_REL, + }, +}; + +static const struct regmap_access_table kx132_precious_regs = { + .yes_ranges = &kx132_precious_ranges[0], + .n_yes_ranges = ARRAY_SIZE(kx132_precious_ranges), +}; + +static const struct regmap_range kx132_read_only_ranges[] = { + { + .range_min = KX132_REG_XADP_L, + .range_max = KX132_REG_INT_REL, + }, { + .range_min = KX132_REG_BUF_STATUS_1, + .range_max = KX132_REG_BUF_STATUS_2, + }, { + .range_min = KX132_REG_BUF_READ, + .range_max = KX132_REG_BUF_READ, + }, +}; + +static const struct regmap_access_table kx132_ro_regs = { + .no_ranges = &kx132_read_only_ranges[0], + .n_no_ranges = ARRAY_SIZE(kx132_read_only_ranges), +}; + +static const struct regmap_range kx132_write_only_ranges[] = { + { + .range_min = KX132_REG_MAN_WAKE, + .range_max = KX132_REG_MAN_WAKE, + }, { + .range_min = KX132_REG_SELF_TEST, + .range_max = KX132_REG_SELF_TEST, + }, { + .range_min = KX132_REG_BUF_CLEAR, + .range_max = KX132_REG_BUF_CLEAR, + }, +}; + +static const struct regmap_access_table kx132_wo_regs = { + .no_ranges = &kx132_write_only_ranges[0], + .n_no_ranges = ARRAY_SIZE(kx132_write_only_ranges), +}; + +static const struct regmap_range kx132_noinc_read_ranges[] = { + { + .range_min = KX132_REG_BUF_READ, + .range_max = KX132_REG_BUF_READ, + }, +}; + +static const struct regmap_access_table kx132_nir_regs = { + .yes_ranges = &kx132_noinc_read_ranges[0], + .n_yes_ranges = ARRAY_SIZE(kx132_noinc_read_ranges), +}; + +static const struct regmap_config kx132_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .volatile_table = &kx132_volatile_regs, + .rd_table = &kx132_wo_regs, + .wr_table = &kx132_ro_regs, + .rd_noinc_table = &kx132_nir_regs, + .precious_table = &kx132_precious_regs, + .max_register = KX132_MAX_REGISTER, + .cache_type = REGCACHE_RBTREE, +}; + struct kx022a_data { const struct kx022a_chip_info *chip_info; struct regmap *regmap; @@ -237,6 +332,13 @@ static const struct iio_chan_spec kx022a_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(3), }; +static const struct iio_chan_spec kx132_channels[] = { + KX022A_ACCEL_CHAN(X, KX132_REG_XOUT_L, 0), + KX022A_ACCEL_CHAN(Y, KX132_REG_YOUT_L, 1), + KX022A_ACCEL_CHAN(Z, KX132_REG_ZOUT_L, 2), + IIO_CHAN_SOFT_TIMESTAMP(3), +}; + /* * The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the * Linux CPUs can handle without dropping samples. Also, the low power mode is @@ -613,6 +715,25 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data) return fifo_bytes; } +static int kx132_get_fifo_bytes(struct kx022a_data *data) +{ + struct device *dev = regmap_get_device(data->regmap); + __le16 buf_status; + int ret, fifo_bytes; + + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1, + &buf_status, sizeof(buf_status)); + if (ret) { + dev_err(dev, "Error reading buffer status\n"); + return ret; + } + + fifo_bytes = le16_to_cpu(buf_status); + fifo_bytes &= data->chip_info->buf_smp_lvl_mask; + + return fifo_bytes; +} + static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples, bool irq) { @@ -1036,6 +1157,32 @@ const struct kx022a_chip_info kx022a_chip_info = { }; EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A); +const struct kx022a_chip_info kx132_chip_info = { + .name = "kx132-1211", + .regmap_config = &kx132_regmap_config, + .channels = kx132_channels, + .num_channels = ARRAY_SIZE(kx132_channels), + .fifo_length = KX132_FIFO_LENGTH, + .who = KX132_REG_WHO, + .id = KX132_ID, + .cntl = KX132_REG_CNTL, + .cntl2 = KX132_REG_CNTL2, + .odcntl = KX132_REG_ODCNTL, + .buf_cntl1 = KX132_REG_BUF_CNTL1, + .buf_cntl2 = KX132_REG_BUF_CNTL2, + .buf_clear = KX132_REG_BUF_CLEAR, + .buf_status1 = KX132_REG_BUF_STATUS_1, + .buf_smp_lvl_mask = KX132_MASK_BUF_SMP_LVL, + .buf_read = KX132_REG_BUF_READ, + .inc1 = KX132_REG_INC1, + .inc4 = KX132_REG_INC4, + .inc5 = KX132_REG_INC5, + .inc6 = KX132_REG_INC6, + .xout_l = KX132_REG_XOUT_L, + .get_fifo_bytes = kx132_get_fifo_bytes, +}; +EXPORT_SYMBOL_NS_GPL(kx132_chip_info, IIO_KX022A); + int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info) { static const char * const regulator_names[] = {"io-vdd", "vdd"}; diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h index f043767067b7..1f4135cf20eb 100644 --- a/drivers/iio/accel/kionix-kx022a.h +++ b/drivers/iio/accel/kionix-kx022a.h @@ -74,6 +74,57 @@ #define KX022A_REG_SELF_TEST 0x60 #define KX022A_MAX_REGISTER 0x60 +#define KX132_REG_WHO 0x13 +#define KX132_ID 0x3d + +#define KX132_FIFO_LENGTH 86 + +#define KX132_REG_CNTL 0x1b +#define KX132_REG_CNTL2 0x1c +#define KX132_MASK_RES BIT(6) +#define KX132_GSEL_2 0x0 +#define KX132_GSEL_4 BIT(3) +#define KX132_GSEL_8 BIT(4) +#define KX132_GSEL_16 GENMASK(4, 3) + +#define KX132_REG_INS2 0x17 +#define KX132_MASK_INS2_WMI BIT(5) + +#define KX132_REG_XADP_L 0x02 +#define KX132_REG_XOUT_L 0x08 +#define KX132_REG_YOUT_L 0x0a +#define KX132_REG_ZOUT_L 0x0c +#define KX132_REG_COTR 0x12 +#define KX132_REG_TSCP 0x14 +#define KX132_REG_INT_REL 0x1a + +#define KX132_REG_ODCNTL 0x21 + +#define KX132_REG_BTS_WUF_TH 0x4a +#define KX132_REG_MAN_WAKE 0x4d + +#define KX132_REG_BUF_CNTL1 0x5e +#define KX132_REG_BUF_CNTL2 0x5f +#define KX132_REG_BUF_STATUS_1 0x60 +#define KX132_REG_BUF_STATUS_2 0x61 +#define KX132_MASK_BUF_SMP_LVL GENMASK(9, 0) +#define KX132_REG_BUF_CLEAR 0x62 +#define KX132_REG_BUF_READ 0x63 +#define KX132_ODR_SHIFT 3 +#define KX132_FIFO_MAX_WMI_TH 86 + +#define KX132_REG_INC1 0x22 +#define KX132_REG_INC5 0x26 +#define KX132_REG_INC6 0x27 +#define KX132_IPOL_LOW 0 +#define KX132_IPOL_HIGH KX022A_MASK_IPOL +#define KX132_ITYP_PULSE KX022A_MASK_ITYP + +#define KX132_REG_INC4 0x25 + +#define KX132_REG_SELF_TEST 0x5d +#define KX132_MAX_REGISTER 0x76 + struct device; struct kx022a_data; @@ -132,5 +183,6 @@ struct kx022a_chip_info { int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info); extern const struct kx022a_chip_info kx022a_chip_info; +extern const struct kx022a_chip_info kx132_chip_info; #endif
Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support ranges from ±2G to ±16G, digital output through I²C/SPI. Add support for basic accelerometer features such as reading acceleration via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ. Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com> --- drivers/iio/accel/Kconfig | 8 +- drivers/iio/accel/kionix-kx022a-i2c.c | 2 + drivers/iio/accel/kionix-kx022a-spi.c | 2 + drivers/iio/accel/kionix-kx022a.c | 147 ++++++++++++++++++++++++++ drivers/iio/accel/kionix-kx022a.h | 52 +++++++++ 5 files changed, 207 insertions(+), 4 deletions(-)