2023-06-13 10:31:03

by Mehdi Djait

[permalink] [raw]
Subject: [PATCH v6 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

Hello everyone,

Version 6 for adding support for the kx132-1211 accelerometer

KX132-1211 accelerometer is a sensor which:
- supports G-ranges of (+/-) 2, 4, 8, and 16G
- can be connected to I2C or SPI
- has internal HW FIFO buffer
- supports various ODRs (output data rates)

The KX132-1211 accelerometer is very similar to the KX022A.
One key difference is number of bits to report the number of data bytes that
have been stored in the buffer: 8 bits for KX022A vs 10 bits for
KX132-1211.

Changes in v6:
- check for availability of chip_info for the SPI case
- changed the order of elements in the kx022a_data struct to save memory

Changes in v5:
- moved the "kfree" call to match the reverse of what happens in
kx022a_fifo_enable() as suggested by Matti and Jonathan
- used min_t, checked for availability of chip_info and moved the
position of u16 buf_smp_lvl_mask as suggested by Andy
- introduced buf_smp_lvl_mask in Patch 7 as suggested by Jonathan

Changes in v4:
- moved the allocation of the fifo_buffer to kx022a_fifo_enable and
kx022a_fifo_disable
- some fixes to the regmap ranges of kx132-1211

Changes in v3:
- added two new patches by separating the addition of the
i2c_device_id table and the removal of blank lines from other
unrelated changes
- fixes a warning detected by the kernel test robot
- made all the changes related the chip_info in one patch

Changes in v2:
- added a new patch for warning when the device_id match fails in the
probe function
- added a new patch for the function that retrieves the number of bytes
in the buffer
- added a change to the Kconfig file in the patch adding the support
for the kx132-1211
- various fixes and modifications listed under each patch

Mehdi Djait (7):
dt-bindings: iio: Add KX132-1211 accelerometer
iio: accel: kionix-kx022a: Remove blank lines
iio: accel: kionix-kx022a: Warn on failed matches and assume
compatibility
iio: accel: kionix-kx022a: Add an i2c_device_id table
iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
iio: accel: kionix-kx022a: Add a function to retrieve number of bytes
in buffer
iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

.../bindings/iio/accel/kionix,kx022a.yaml | 12 +-
drivers/iio/accel/Kconfig | 8 +-
drivers/iio/accel/kionix-kx022a-i2c.c | 24 +-
drivers/iio/accel/kionix-kx022a-spi.c | 15 +-
drivers/iio/accel/kionix-kx022a.c | 312 ++++++++++++++----
drivers/iio/accel/kionix-kx022a.h | 110 +++++-
6 files changed, 406 insertions(+), 75 deletions(-)

--
2.30.2



2023-06-13 10:31:13

by Mehdi Djait

[permalink] [raw]
Subject: [PATCH v6 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

Since Kionix accelerometers use various numbers of bits to report data, a
device-specific function is required.
Implement the function as a callback in the device-specific chip_info structure

Reviewed-by: Matti Vaittinen <[email protected]>
Signed-off-by: Mehdi Djait <[email protected]>
---
v6:
- directly return KX022A_FIFO_MAX_BYTES as suggested by Andy

v5:
- no changes

v4:
- removed the comment about "bogus value from i2c"
- removed regmap_get_device(data->regmap); dev is present in the
driver's private data

v3:
- no changes

v2:
- separated this change from the chip_info introduction and made it a patch in v2
- changed the function from generic implementation for to device-specific one
- removed blank lines pointed out by checkpatch
- changed the allocation of the "buffer" array in __kx022a_fifo_flush

drivers/iio/accel/kionix-kx022a.c | 28 ++++++++++++++++++----------
drivers/iio/accel/kionix-kx022a.h | 4 ++++
2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 0bab3f109e4e..0b0f14dca17f 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -596,26 +596,33 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
}

+static int kx022a_get_fifo_bytes(struct kx022a_data *data)
+{
+ int ret, fifo_bytes;
+
+ ret = regmap_read(data->regmap, KX022A_REG_BUF_STATUS_1, &fifo_bytes);
+ if (ret) {
+ dev_err(data->dev, "Error reading buffer status\n");
+ return ret;
+ }
+
+ if (fifo_bytes == KX022A_FIFO_FULL_VALUE)
+ return KX022A_FIFO_MAX_BYTES;
+
+ return fifo_bytes;
+}
+
static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
bool irq)
{
struct kx022a_data *data = iio_priv(idev);
- struct device *dev = regmap_get_device(data->regmap);
uint64_t sample_period;
int count, fifo_bytes;
bool renable = false;
int64_t tstamp;
int ret, i;

- ret = regmap_read(data->regmap, KX022A_REG_BUF_STATUS_1, &fifo_bytes);
- if (ret) {
- dev_err(dev, "Error reading buffer status\n");
- return ret;
- }
-
- /* Let's not overflow if we for some reason get bogus value from i2c */
- if (fifo_bytes == KX022A_FIFO_FULL_VALUE)
- fifo_bytes = KX022A_FIFO_MAX_BYTES;
+ fifo_bytes = data->chip_info->get_fifo_bytes(data);

if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
@@ -1023,6 +1030,7 @@ const struct kx022a_chip_info kx022a_chip_info = {
.inc5 = KX022A_REG_INC5,
.inc6 = KX022A_REG_INC6,
.xout_l = KX022A_REG_XOUT_L,
+ .get_fifo_bytes = kx022a_get_fifo_bytes,
};
EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);

diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
index 0e5026019213..c9f9aee7e597 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,6 +76,8 @@

struct device;

+struct kx022a_data;
+
/**
* struct kx022a_chip_info - Kionix accelerometer chip specific information
*
@@ -99,6 +101,7 @@ struct device;
* @inc5: interrupt control register 5
* @inc6: interrupt control register 6
* @xout_l: x-axis output least significant byte
+ * @get_fifo_bytes: function pointer to get number of bytes in the FIFO buffer
*/
struct kx022a_chip_info {
const char *name;
@@ -121,6 +124,7 @@ struct kx022a_chip_info {
u8 inc5;
u8 inc6;
u8 xout_l;
+ int (*get_fifo_bytes)(struct kx022a_data *);
};

int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info);
--
2.30.2


2023-06-13 10:32:24

by Mehdi Djait

[permalink] [raw]
Subject: [PATCH v6 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

Add the chip_info structure to the driver's private data to hold all
the device specific infos.
Refactor the kx022a driver implementation to make it more generic and
extensible.

Signed-off-by: Mehdi Djait <[email protected]>
---
v6:
- placed chip_info after the regmap elemnt in kx022a_data struct to save
memory as suggested by Andy
- added a check for the availability of chip_info for the SPI case as
suggested by Andy

v5:
- moved the "kfree" call to match the reverse of what happens in
kx022a_fifo_enable() as suggested by Matti and Jonathan
- used min_t, checked for availability of chip_info as suggested by Andy

v4:
- allocating and freeing the buffer moved to the kx022a_fifo{enable,
disable} functions
- used the spi_get_device_match_data helper function

v3:
- added the change of the buffer's allocation in the __kx022a_fifo_flush
to this patch
- added the chip_info to the struct kx022a_data

v2:
- mentioned the introduction of the i2c_device_id table in the commit
- get i2c_/spi_get_device_id only when device get match fails
- removed the generic KX_define
- removed the kx022a_device_type enum
- added comments for the chip_info struct elements
- fixed errors pointed out by the kernel test robot

drivers/iio/accel/kionix-kx022a-i2c.c | 17 +++-
drivers/iio/accel/kionix-kx022a-spi.c | 13 ++-
drivers/iio/accel/kionix-kx022a.c | 116 ++++++++++++++++----------
drivers/iio/accel/kionix-kx022a.h | 52 +++++++++++-
4 files changed, 146 insertions(+), 52 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
index b5a85ce3a891..c59558fc6493 100644
--- a/drivers/iio/accel/kionix-kx022a-i2c.c
+++ b/drivers/iio/accel/kionix-kx022a-i2c.c
@@ -15,6 +15,7 @@
static int kx022a_i2c_probe(struct i2c_client *i2c)
{
struct device *dev = &i2c->dev;
+ const struct kx022a_chip_info *chip_info;
struct regmap *regmap;

if (!i2c->irq) {
@@ -22,22 +23,30 @@ static int kx022a_i2c_probe(struct i2c_client *i2c)
return -EINVAL;
}

- regmap = devm_regmap_init_i2c(i2c, &kx022a_regmap);
+ chip_info = device_get_match_data(&i2c->dev);
+ if (!chip_info) {
+ const struct i2c_device_id *id = i2c_client_get_device_id(i2c);
+ chip_info = (const struct kx022a_chip_info *)id->driver_data;
+ if (!chip_info)
+ return -EINVAL;
+ }
+
+ regmap = devm_regmap_init_i2c(i2c, chip_info->regmap_config);
if (IS_ERR(regmap))
return dev_err_probe(dev, PTR_ERR(regmap),
"Failed to initialize Regmap\n");

- return kx022a_probe_internal(dev);
+ return kx022a_probe_internal(dev, chip_info);
}

static const struct i2c_device_id kx022a_i2c_id[] = {
- { .name = "kx022a" },
+ { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id);

static const struct of_device_id kx022a_of_match[] = {
- { .compatible = "kionix,kx022a", },
+ { .compatible = "kionix,kx022a", .data = &kx022a_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 9cd047f7b346..896b57866fc9 100644
--- a/drivers/iio/accel/kionix-kx022a-spi.c
+++ b/drivers/iio/accel/kionix-kx022a-spi.c
@@ -15,6 +15,7 @@
static int kx022a_spi_probe(struct spi_device *spi)
{
struct device *dev = &spi->dev;
+ const struct kx022a_chip_info *chip_info;
struct regmap *regmap;

if (!spi->irq) {
@@ -22,22 +23,26 @@ static int kx022a_spi_probe(struct spi_device *spi)
return -EINVAL;
}

- regmap = devm_regmap_init_spi(spi, &kx022a_regmap);
+ chip_info = spi_get_device_match_data(spi);
+ if (!chip_info)
+ return -EINVAL;
+
+ regmap = devm_regmap_init_spi(spi, chip_info->regmap_config);
if (IS_ERR(regmap))
return dev_err_probe(dev, PTR_ERR(regmap),
"Failed to initialize Regmap\n");

- return kx022a_probe_internal(dev);
+ return kx022a_probe_internal(dev, chip_info);
}

static const struct spi_device_id kx022a_id[] = {
- { "kx022a" },
+ { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
{ }
};
MODULE_DEVICE_TABLE(spi, kx022a_id);

static const struct of_device_id kx022a_of_match[] = {
- { .compatible = "kionix,kx022a", },
+ { .compatible = "kionix,kx022a", .data = &kx022a_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 494e81ba1da9..0bab3f109e4e 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -48,7 +48,7 @@ enum {
KX022A_STATE_FIFO,
};

-/* Regmap configs */
+/* kx022a Regmap configs */
static const struct regmap_range kx022a_volatile_ranges[] = {
{
.range_min = KX022A_REG_XHP_L,
@@ -138,7 +138,7 @@ static const struct regmap_access_table kx022a_nir_regs = {
.n_yes_ranges = ARRAY_SIZE(kx022a_noinc_read_ranges),
};

-const struct regmap_config kx022a_regmap = {
+static const struct regmap_config kx022a_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
.volatile_table = &kx022a_volatile_regs,
@@ -149,10 +149,10 @@ const struct regmap_config kx022a_regmap = {
.max_register = KX022A_MAX_REGISTER,
.cache_type = REGCACHE_RBTREE,
};
-EXPORT_SYMBOL_NS_GPL(kx022a_regmap, IIO_KX022A);

struct kx022a_data {
struct regmap *regmap;
+ const struct kx022a_chip_info *chip_info;
struct iio_trigger *trig;
struct device *dev;
struct iio_mount_matrix orientation;
@@ -175,6 +175,8 @@ struct kx022a_data {
struct mutex mutex;
u8 watermark;

+ __le16 *fifo_buffer;
+
/* 3 x 16bit accel data + timestamp */
__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
struct {
@@ -208,7 +210,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
{ }
};

-#define KX022A_ACCEL_CHAN(axis, index) \
+#define KX022A_ACCEL_CHAN(axis, reg, index) \
{ \
.type = IIO_ACCEL, \
.modified = 1, \
@@ -220,7 +222,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
BIT(IIO_CHAN_INFO_SCALE) | \
BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.ext_info = kx022a_ext_info, \
- .address = KX022A_REG_##axis##OUT_L, \
+ .address = reg, \
.scan_index = index, \
.scan_type = { \
.sign = 's', \
@@ -231,9 +233,9 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
}

static const struct iio_chan_spec kx022a_channels[] = {
- KX022A_ACCEL_CHAN(X, 0),
- KX022A_ACCEL_CHAN(Y, 1),
- KX022A_ACCEL_CHAN(Z, 2),
+ KX022A_ACCEL_CHAN(X, KX022A_REG_XOUT_L, 0),
+ KX022A_ACCEL_CHAN(Y, KX022A_REG_YOUT_L, 1),
+ KX022A_ACCEL_CHAN(Z, KX022A_REG_ZOUT_L, 2),
IIO_CHAN_SOFT_TIMESTAMP(3),
};

@@ -332,10 +334,10 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
int ret;

if (on)
- ret = regmap_set_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_set_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_PC1);
else
- ret = regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_clear_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_PC1);
if (ret)
dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
@@ -402,7 +404,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
break;

ret = regmap_update_bits(data->regmap,
- KX022A_REG_ODCNTL,
+ data->chip_info->odcntl,
KX022A_MASK_ODR, n);
data->odr_ns = kx022a_odrs[n];
kx022a_turn_on_unlock(data);
@@ -423,7 +425,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
if (ret)
break;

- ret = regmap_update_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_update_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_GSEL,
n << KX022A_GSEL_SHIFT);
kx022a_turn_on_unlock(data);
@@ -445,7 +447,7 @@ static int kx022a_fifo_set_wmi(struct kx022a_data *data)

threshold = data->watermark;

- return regmap_update_bits(data->regmap, KX022A_REG_BUF_CNTL1,
+ return regmap_update_bits(data->regmap, data->chip_info->buf_cntl1,
KX022A_MASK_WM_TH, threshold);
}

@@ -488,7 +490,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
return ret;

case IIO_CHAN_INFO_SAMP_FREQ:
- ret = regmap_read(data->regmap, KX022A_REG_ODCNTL, &regval);
+ ret = regmap_read(data->regmap, data->chip_info->odcntl, &regval);
if (ret)
return ret;

@@ -503,7 +505,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
return IIO_VAL_INT_PLUS_MICRO;

case IIO_CHAN_INFO_SCALE:
- ret = regmap_read(data->regmap, KX022A_REG_CNTL, &regval);
+ ret = regmap_read(data->regmap, data->chip_info->cntl, &regval);
if (ret < 0)
return ret;

@@ -530,8 +532,7 @@ static int kx022a_set_watermark(struct iio_dev *idev, unsigned int val)
{
struct kx022a_data *data = iio_priv(idev);

- if (val > KX022A_FIFO_LENGTH)
- val = KX022A_FIFO_LENGTH;
+ val = min_t(unsigned int, data->chip_info->fifo_length, val);

mutex_lock(&data->mutex);
data->watermark = val;
@@ -592,7 +593,7 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
*/
data->timestamp = 0;

- return regmap_write(data->regmap, KX022A_REG_BUF_CLEAR, 0x0);
+ return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
}

static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
@@ -600,7 +601,6 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
{
struct kx022a_data *data = iio_priv(idev);
struct device *dev = regmap_get_device(data->regmap);
- __le16 buffer[KX022A_FIFO_LENGTH * 3];
uint64_t sample_period;
int count, fifo_bytes;
bool renable = false;
@@ -679,13 +679,13 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
}

fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
- ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
- &buffer[0], fifo_bytes);
+ ret = regmap_noinc_read(data->regmap, data->chip_info->buf_read,
+ data->fifo_buffer, fifo_bytes);
if (ret)
goto renable_out;

for (i = 0; i < count; i++) {
- __le16 *sam = &buffer[i * 3];
+ __le16 *sam = &data->fifo_buffer[i * 3];
__le16 *chs;
int bit;

@@ -732,10 +732,10 @@ static const struct iio_info kx022a_info = {
static int kx022a_set_drdy_irq(struct kx022a_data *data, bool en)
{
if (en)
- return regmap_set_bits(data->regmap, KX022A_REG_CNTL,
+ return regmap_set_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_DRDY);

- return regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
+ return regmap_clear_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_DRDY);
}

@@ -770,7 +770,7 @@ static int kx022a_fifo_disable(struct kx022a_data *data)
if (ret)
goto unlock_out;

- ret = regmap_clear_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+ ret = regmap_clear_bits(data->regmap, data->chip_info->buf_cntl2,
KX022A_MASK_BUF_EN);
if (ret)
goto unlock_out;
@@ -779,6 +779,8 @@ static int kx022a_fifo_disable(struct kx022a_data *data)

kx022a_drop_fifo_contents(data);

+ kfree(data->fifo_buffer);
+
return kx022a_turn_on_unlock(data);

unlock_out:
@@ -801,6 +803,11 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
{
int ret;

+ data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
+ KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
+ if (!data->fifo_buffer)
+ return -ENOMEM;
+
ret = kx022a_turn_off_lock(data);
if (ret)
return ret;
@@ -811,7 +818,7 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
goto unlock_out;

/* Enable buffer */
- ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+ ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
KX022A_MASK_BUF_EN);
if (ret)
goto unlock_out;
@@ -857,7 +864,7 @@ static irqreturn_t kx022a_trigger_handler(int irq, void *p)
struct kx022a_data *data = iio_priv(idev);
int ret;

- ret = regmap_bulk_read(data->regmap, KX022A_REG_XOUT_L, data->buffer,
+ ret = regmap_bulk_read(data->regmap, data->chip_info->xout_l, data->buffer,
KX022A_FIFO_SAMPLES_SIZE_BYTES);
if (ret < 0)
goto err_read;
@@ -905,7 +912,7 @@ static irqreturn_t kx022a_irq_thread_handler(int irq, void *private)
if (data->state & KX022A_STATE_FIFO) {
int ok;

- ok = __kx022a_fifo_flush(idev, KX022A_FIFO_LENGTH, true);
+ ok = __kx022a_fifo_flush(idev, data->chip_info->fifo_length, true);
if (ok > 0)
ret = IRQ_HANDLED;
}
@@ -958,7 +965,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
int ret, val;

/* Reset the senor */
- ret = regmap_write(data->regmap, KX022A_REG_CNTL2, KX022A_MASK_SRST);
+ ret = regmap_write(data->regmap, data->chip_info->cntl2, KX022A_MASK_SRST);
if (ret)
return ret;

@@ -968,7 +975,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
*/
msleep(1);

- ret = regmap_read_poll_timeout(data->regmap, KX022A_REG_CNTL2, val,
+ ret = regmap_read_poll_timeout(data->regmap, data->chip_info->cntl2, val,
!(val & KX022A_MASK_SRST),
KX022A_SOFT_RESET_WAIT_TIME_US,
KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US);
@@ -978,14 +985,14 @@ static int kx022a_chip_init(struct kx022a_data *data)
return ret;
}

- ret = regmap_reinit_cache(data->regmap, &kx022a_regmap);
+ ret = regmap_reinit_cache(data->regmap, data->chip_info->regmap_config);
if (ret) {
dev_err(data->dev, "Failed to reinit reg cache\n");
return ret;
}

/* set data res 16bit */
- ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+ ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
KX022A_MASK_BRES16);
if (ret) {
dev_err(data->dev, "Failed to set data resolution\n");
@@ -995,7 +1002,31 @@ static int kx022a_chip_init(struct kx022a_data *data)
return kx022a_prepare_irq_pin(data);
}

-int kx022a_probe_internal(struct device *dev)
+const struct kx022a_chip_info kx022a_chip_info = {
+ .name = "kx022-accel",
+ .regmap_config = &kx022a_regmap_config,
+ .channels = kx022a_channels,
+ .num_channels = ARRAY_SIZE(kx022a_channels),
+ .fifo_length = KX022A_FIFO_LENGTH,
+ .who = KX022A_REG_WHO,
+ .id = KX022A_ID,
+ .cntl = KX022A_REG_CNTL,
+ .cntl2 = KX022A_REG_CNTL2,
+ .odcntl = KX022A_REG_ODCNTL,
+ .buf_cntl1 = KX022A_REG_BUF_CNTL1,
+ .buf_cntl2 = KX022A_REG_BUF_CNTL2,
+ .buf_clear = KX022A_REG_BUF_CLEAR,
+ .buf_status1 = KX022A_REG_BUF_STATUS_1,
+ .buf_read = KX022A_REG_BUF_READ,
+ .inc1 = KX022A_REG_INC1,
+ .inc4 = KX022A_REG_INC4,
+ .inc5 = KX022A_REG_INC5,
+ .inc6 = KX022A_REG_INC6,
+ .xout_l = KX022A_REG_XOUT_L,
+};
+EXPORT_SYMBOL_NS_GPL(kx022a_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"};
struct iio_trigger *indio_trig;
@@ -1022,6 +1053,7 @@ int kx022a_probe_internal(struct device *dev)
return -ENOMEM;

data = iio_priv(idev);
+ data->chip_info = chip_info;

/*
* VDD is the analog and digital domain voltage supply and
@@ -1032,24 +1064,24 @@ int kx022a_probe_internal(struct device *dev)
if (ret && ret != -ENODEV)
return dev_err_probe(dev, ret, "failed to enable regulator\n");

- ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
+ ret = regmap_read(regmap, chip_info->who, &chip_id);
if (ret)
return dev_err_probe(dev, ret, "Failed to access sensor\n");

- if (chip_id != KX022A_ID)
+ if (chip_id != chip_info->id)
dev_warn(dev, "unknown device 0x%x\n", chip_id);

irq = fwnode_irq_get_byname(fwnode, "INT1");
if (irq > 0) {
- data->inc_reg = KX022A_REG_INC1;
- data->ien_reg = KX022A_REG_INC4;
+ data->inc_reg = chip_info->inc1;
+ data->ien_reg = chip_info->inc4;
} else {
irq = fwnode_irq_get_byname(fwnode, "INT2");
if (irq <= 0)
return dev_err_probe(dev, irq, "No suitable IRQ\n");

- data->inc_reg = KX022A_REG_INC5;
- data->ien_reg = KX022A_REG_INC6;
+ data->inc_reg = chip_info->inc5;
+ data->ien_reg = chip_info->inc6;
}

data->regmap = regmap;
@@ -1058,9 +1090,9 @@ int kx022a_probe_internal(struct device *dev)
data->odr_ns = KX022A_DEFAULT_PERIOD_NS;
mutex_init(&data->mutex);

- idev->channels = kx022a_channels;
- idev->num_channels = ARRAY_SIZE(kx022a_channels);
- idev->name = "kx022-accel";
+ idev->channels = chip_info->channels;
+ idev->num_channels = chip_info->num_channels;
+ idev->name = chip_info->name;
idev->info = &kx022a_info;
idev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
idev->available_scan_masks = kx022a_scan_masks;
diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
index 12424649d438..0e5026019213 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,7 +76,55 @@

struct device;

-int kx022a_probe_internal(struct device *dev);
-extern const struct regmap_config kx022a_regmap;
+/**
+ * struct kx022a_chip_info - Kionix accelerometer chip specific information
+ *
+ * @name: name of the device
+ * @regmap_config: pointer to register map configuration
+ * @channels: pointer to iio_chan_spec array
+ * @num_channels: number of iio_chan_spec channels
+ * @fifo_length: number of 16-bit samples in a full buffer
+ * @who: WHO_AM_I register
+ * @id: WHO_AM_I register value
+ * @cntl: control register 1
+ * @cntl2: control register 2
+ * @odcntl: output data control register
+ * @buf_cntl1: buffer control register 1
+ * @buf_cntl2: buffer control register 2
+ * @buf_clear: buffer clear register
+ * @buf_status1: buffer status register 1
+ * @buf_read: buffer read register
+ * @inc1: interrupt control register 1
+ * @inc4: interrupt control register 4
+ * @inc5: interrupt control register 5
+ * @inc6: interrupt control register 6
+ * @xout_l: x-axis output least significant byte
+ */
+struct kx022a_chip_info {
+ const char *name;
+ const struct regmap_config *regmap_config;
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+ unsigned int fifo_length;
+ u8 who;
+ u8 id;
+ u8 cntl;
+ u8 cntl2;
+ u8 odcntl;
+ u8 buf_cntl1;
+ u8 buf_cntl2;
+ u8 buf_clear;
+ u8 buf_status1;
+ u8 buf_read;
+ u8 inc1;
+ u8 inc4;
+ u8 inc5;
+ u8 inc6;
+ u8 xout_l;
+};
+
+int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info);
+
+extern const struct kx022a_chip_info kx022a_chip_info;

#endif
--
2.30.2


2023-06-13 11:11:18

by Mehdi Djait

[permalink] [raw]
Subject: [PATCH v6 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

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
Acked-by: Matti Vaittinen <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Mehdi Djait <[email protected]>
---
v6:
- no changes

v5:
- moved the position of u16 buf_smp_lvl_mask as suggested by Andy
- introduced buf_smp_lvl_mask in Patch 7 as suggested by Jonathan

v4:
- added KX132_REG_CNTL5 to the volatile ranges
- added the kionix reserved regs to the read_only ranges
- removed KX132_REG_MAN_WAKEUP from the write_only ranges

v3:
- fixed the warning of the kernel test robot in kx132_get_fifo_bytes
(invalid assignment: &=, left side has type restricted __le16
right side has type unsigned short)

v2:
- mentioned the kx132-1211 in the Kconfig
- added a kx132-specific get_fifo_bytes function
- changed the device name from "kx132" to "kx132-1211

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 | 162 ++++++++++++++++++++++++++
drivers/iio/accel/kionix-kx022a.h | 54 +++++++++
5 files changed, 224 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 c59558fc6493..0f4729c55d29 100644
--- a/drivers/iio/accel/kionix-kx022a-i2c.c
+++ b/drivers/iio/accel/kionix-kx022a-i2c.c
@@ -41,12 +41,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 896b57866fc9..c7766492bcd9 100644
--- a/drivers/iio/accel/kionix-kx022a-spi.c
+++ b/drivers/iio/accel/kionix-kx022a-spi.c
@@ -37,12 +37,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 0b0f14dca17f..9b8304978d84 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -150,6 +150,117 @@ 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_CNTL5,
+ .range_max = KX132_REG_CNTL5,
+ }, {
+ .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,
+ }, {
+ /* Kionix reserved registers: should not be written */
+ .range_min = 0x28,
+ .range_max = 0x28,
+ }, {
+ .range_min = 0x35,
+ .range_max = 0x36,
+ }, {
+ .range_min = 0x3c,
+ .range_max = 0x48,
+ }, {
+ .range_min = 0x4e,
+ .range_max = 0x5c,
+ }, {
+ .range_min = 0x77,
+ .range_max = 0x7f,
+ },
+};
+
+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_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 {
struct regmap *regmap;
const struct kx022a_chip_info *chip_info;
@@ -239,6 +350,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
@@ -612,6 +730,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
return fifo_bytes;
}

+static int kx132_get_fifo_bytes(struct kx022a_data *data)
+{
+ __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(data->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)
{
@@ -1034,6 +1170,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 c9f9aee7e597..ea6202d29303 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_REG_CNTL5 0x1f
+#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_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;
@@ -86,6 +137,7 @@ struct kx022a_data;
* @channels: pointer to iio_chan_spec array
* @num_channels: number of iio_chan_spec channels
* @fifo_length: number of 16-bit samples in a full buffer
+ * @buf_smp_lvl_mask: buffer sample level mask
* @who: WHO_AM_I register
* @id: WHO_AM_I register value
* @cntl: control register 1
@@ -109,6 +161,7 @@ struct kx022a_chip_info {
const struct iio_chan_spec *channels;
unsigned int num_channels;
unsigned int fifo_length;
+ u16 buf_smp_lvl_mask;
u8 who;
u8 id;
u8 cntl;
@@ -130,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
--
2.30.2


2023-06-13 11:12:45

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH v6 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

Hi again Mehdi,

Just one thing still crossed to my mind...

On 6/13/23 13: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
> Acked-by: Matti Vaittinen <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Mehdi Djait <[email protected]>
> ---


> +static int kx132_get_fifo_bytes(struct kx022a_data *data)
> +{
> + __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(data->dev, "Error reading buffer status\n");
> + return ret;
> + }
> +
> + fifo_bytes = le16_to_cpu(buf_status);
> + fifo_bytes &= data->chip_info->buf_smp_lvl_mask;

Does this mask guarantee the fifo_bytes can't be bigger than the
allocated buffer no matter what is read from I2C?

I am asking this because it's not uncommon to see some noise in I2C, or
read 0xff 0xff, ... if IC is for example resetting. Hence, I would sleep
a tad better if I knew the fifo_bytes read from IC is not 100% trusted -
but is verified against known maximum number of samples.

This does not matter with the KX022A because the size of max number of
samples exceeds the value indicated by 0xff (the 8bit SMP_LVL register).

Please correct me if I am wrong but it seems to me we use 10bits of data
from smp_lvl register on KX132. Hence, the maximum value if all bits are
set is 1023. This equals to 1023/6bytes samples - which is roughly 170.
I think the FIFO size was defined to be maximum of 86 samples on KX132 -
which means that if we have some bogus value from I2C, we may overwrite
allocated buffer. Hence we might want to ensure the fifo_bytes is never
bigger than the fifo_length * KX022A_FIFO_SAMPLES_SIZE_BYTES - no matter
what we read from I2C.

> +
> + return fifo_bytes;
> +}
> +

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2023-06-13 11:13:33

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH v6 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

On 6/13/23 13:22, Mehdi Djait wrote:
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
>
> Signed-off-by: Mehdi Djait <[email protected]>
Acked-by: Matti Vaittinen <[email protected]>

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~


2023-07-08 18:30:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v6 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

On Tue, 13 Jun 2023 12:22:33 +0200
Mehdi Djait <[email protected]> wrote:

> Hello everyone,
>
> Version 6 for adding support for the kx132-1211 accelerometer
>
> KX132-1211 accelerometer is a sensor which:
> - supports G-ranges of (+/-) 2, 4, 8, and 16G
> - can be connected to I2C or SPI
> - has internal HW FIFO buffer
> - supports various ODRs (output data rates)
>
> The KX132-1211 accelerometer is very similar to the KX022A.
> One key difference is number of bits to report the number of data bytes that
> have been stored in the buffer: 8 bits for KX022A vs 10 bits for
> KX132-1211.
>
Just though I'd say that the series looks fine to me. I'm just waiting
on reply to Matti's query on the last patch before applying it.

Thanks,

Jonathan


> Changes in v6:
> - check for availability of chip_info for the SPI case
> - changed the order of elements in the kx022a_data struct to save memory
>
> Changes in v5:
> - moved the "kfree" call to match the reverse of what happens in
> kx022a_fifo_enable() as suggested by Matti and Jonathan
> - used min_t, checked for availability of chip_info and moved the
> position of u16 buf_smp_lvl_mask as suggested by Andy
> - introduced buf_smp_lvl_mask in Patch 7 as suggested by Jonathan
>
> Changes in v4:
> - moved the allocation of the fifo_buffer to kx022a_fifo_enable and
> kx022a_fifo_disable
> - some fixes to the regmap ranges of kx132-1211
>
> Changes in v3:
> - added two new patches by separating the addition of the
> i2c_device_id table and the removal of blank lines from other
> unrelated changes
> - fixes a warning detected by the kernel test robot
> - made all the changes related the chip_info in one patch
>
> Changes in v2:
> - added a new patch for warning when the device_id match fails in the
> probe function
> - added a new patch for the function that retrieves the number of bytes
> in the buffer
> - added a change to the Kconfig file in the patch adding the support
> for the kx132-1211
> - various fixes and modifications listed under each patch
>
> Mehdi Djait (7):
> dt-bindings: iio: Add KX132-1211 accelerometer
> iio: accel: kionix-kx022a: Remove blank lines
> iio: accel: kionix-kx022a: Warn on failed matches and assume
> compatibility
> iio: accel: kionix-kx022a: Add an i2c_device_id table
> iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
> iio: accel: kionix-kx022a: Add a function to retrieve number of bytes
> in buffer
> iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
>
> .../bindings/iio/accel/kionix,kx022a.yaml | 12 +-
> drivers/iio/accel/Kconfig | 8 +-
> drivers/iio/accel/kionix-kx022a-i2c.c | 24 +-
> drivers/iio/accel/kionix-kx022a-spi.c | 15 +-
> drivers/iio/accel/kionix-kx022a.c | 312 ++++++++++++++----
> drivers/iio/accel/kionix-kx022a.h | 110 +++++-
> 6 files changed, 406 insertions(+), 75 deletions(-)
>


2023-07-14 16:11:58

by Mehdi Djait

[permalink] [raw]
Subject: Re: [PATCH v6 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

Hello Jonathan,

On Sat, Jul 08, 2023 at 05:23:36PM +0100, Jonathan Cameron wrote:
> On Tue, 13 Jun 2023 12:22:33 +0200
> Mehdi Djait <[email protected]> wrote:
>
> > Hello everyone,
> >
> > Version 6 for adding support for the kx132-1211 accelerometer
> >
> > KX132-1211 accelerometer is a sensor which:
> > - supports G-ranges of (+/-) 2, 4, 8, and 16G
> > - can be connected to I2C or SPI
> > - has internal HW FIFO buffer
> > - supports various ODRs (output data rates)
> >
> > The KX132-1211 accelerometer is very similar to the KX022A.
> > One key difference is number of bits to report the number of data bytes that
> > have been stored in the buffer: 8 bits for KX022A vs 10 bits for
> > KX132-1211.
> >
> Just though I'd say that the series looks fine to me. I'm just waiting
> on reply to Matti's query on the last patch before applying it.

I will send a v7 to address the comment of Matti.

--
Kind Regards
Mehdi Djait