[PATCH v2 0/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[PATCH v2 0/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Mehdi Djait]

Hello everyone,

Second version for adding support for the kx132-1211 accelerometer

KX132 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 accelerometer is very similair 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 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 (5):
  dt-bindings: iio: Add KX132-1211 accelerometer
  iio: accel: kionix-kx022a: Warn on failed matches and assume
    compatibility
  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         |  22 +-
 drivers/iio/accel/kionix-kx022a-spi.c         |  23 +-
 drivers/iio/accel/kionix-kx022a.c             | 325 +++++++++++++-----
 drivers/iio/accel/kionix-kx022a.h             | 143 +++++++-
 6 files changed, 426 insertions(+), 107 deletions(-)

-- 
2.30.2

[PATCH v2 1/5] dt-bindings: iio: Add KX132-1211 accelerometer

[Mehdi Djait]

Extend the kionix,kx022a.yaml file to support the kx132-1211 device

Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
v2:
- made the device name more specific from "kx132" to "kx132-1211"
- removed the output data-rates mentioned and replaced them with "variable 
output data-rates"

 .../devicetree/bindings/iio/accel/kionix,kx022a.yaml | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/kionix,kx022a.yaml b/Documentation/devicetree/bindings/iio/accel/kionix,kx022a.yaml
index 986df1a6ff0a..034b69614416 100644
--- a/Documentation/devicetree/bindings/iio/accel/kionix,kx022a.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/kionix,kx022a.yaml
@@ -4,19 +4,21 @@
 $id: http://devicetree.org/schemas/iio/accel/kionix,kx022a.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: ROHM/Kionix KX022A Accelerometer
+title: ROHM/Kionix KX022A and KX132-1211 Accelerometers
 
 maintainers:
   - Matti Vaittinen <mazziesaccount@gmail.com>
 
 description: |
-  KX022A is a 3-axis accelerometer supporting +/- 2G, 4G, 8G and 16G ranges,
-  output data-rates from 0.78Hz to 1600Hz and a hardware-fifo buffering.
-  KX022A can be accessed either via I2C or SPI.
+  KX022A and KX132-1211 are 3-axis accelerometers supporting +/- 2G, 4G, 8G and
+  16G ranges, variable output data-rates and a hardware-fifo buffering.
+  KX022A and KX132-1211 can be accessed either via I2C or SPI.
 
 properties:
   compatible:
-    const: kionix,kx022a
+    enum:
+      - kionix,kx022a
+      - kionix,kx132-1211
 
   reg:
     maxItems: 1
-- 
2.30.2

[PATCH v2 2/5] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility

[Mehdi Djait]

Avoid error returns on a failure to match and instead just warn with
assumption that we have a correct dt-binding telling us that
some new device with a different ID is backwards compatible.

Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
v2:
- no changes, this patch is introduced in the v2

 drivers/iio/accel/kionix-kx022a.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index f98393d74666..70530005cad3 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -1038,9 +1038,7 @@ int kx022a_probe_internal(struct device *dev)
 		return dev_err_probe(dev, ret, "Failed to access sensor\n");
 
 	if (chip_id != KX022A_ID) {
-		dev_err(dev, "unsupported device 0x%x\n", chip_id);
-		return -EINVAL;
-	}
+		dev_warn(dev, "unsupported device 0x%x\n", chip_id);
 
 	irq = fwnode_irq_get_byname(fwnode, "INT1");
 	if (irq > 0) {
-- 
2.30.2

[PATCH v2 3/5] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

[Mehdi Djait]

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

Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
v2:
- mentioned the introduction of the i2c_device_id table in the commit
- get i2c_/spi_get_device_id only with 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 |  20 ++++-
 drivers/iio/accel/kionix-kx022a-spi.c |  21 ++++--
 drivers/iio/accel/kionix-kx022a.c     | 102 ++++++++++++++++----------
 drivers/iio/accel/kionix-kx022a.h     |  54 +++++++++++++-
 4 files changed, 146 insertions(+), 51 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
index e6fd02d931b6..ce299d0446f7 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,16 +23,28 @@ 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;
+	}
+
+	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", .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);
@@ -42,6 +55,7 @@ static struct i2c_driver kx022a_i2c_driver = {
 		.of_match_table = kx022a_of_match,
 	  },
 	.probe_new    = kx022a_i2c_probe,
+	.id_table     = kx022a_i2c_id,
 };
 module_i2c_driver(kx022a_i2c_driver);
 
diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
index 9cd047f7b346..7bc81588e40e 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,28 @@ static int kx022a_spi_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
-	regmap = devm_regmap_init_spi(spi, &kx022a_regmap);
+	chip_info = device_get_match_data(&spi->dev);
+	if (!chip_info) {
+		const struct spi_device_id *id = spi_get_device_id(spi);
+		chip_info = (const struct kx022a_chip_info *)id->driver_data;
+	}
+
+	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" },
+static const struct spi_device_id kx022a_spi_id[] = {
+	{ .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
 	{ }
 };
-MODULE_DEVICE_TABLE(spi, kx022a_id);
+MODULE_DEVICE_TABLE(spi, kx022a_spi_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);
@@ -48,7 +55,7 @@ static struct spi_driver kx022a_spi_driver = {
 		.of_match_table = kx022a_of_match,
 	},
 	.probe = kx022a_spi_probe,
-	.id_table = kx022a_id,
+	.id_table = kx022a_spi_id,
 };
 module_spi_driver(kx022a_spi_driver);
 
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 70530005cad3..7f9a2c29790b 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,7 +149,6 @@ 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;
@@ -208,7 +207,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 +219,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 +230,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 +331,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);
@@ -403,7 +402,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);
@@ -424,7 +423,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);
@@ -446,7 +445,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);
 }
 
@@ -489,7 +488,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;
 
@@ -504,7 +503,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;
 
@@ -531,8 +530,8 @@ 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;
+	if (val > data->chip_info->fifo_length)
+		val = data->chip_info->fifo_length;
 
 	mutex_lock(&data->mutex);
 	data->watermark = val;
@@ -593,7 +592,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,
@@ -680,7 +679,7 @@ 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,
+	ret = regmap_noinc_read(data->regmap, data->chip_info->buf_read,
 				&buffer[0], fifo_bytes);
 	if (ret)
 		goto renable_out;
@@ -733,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);
 }
 
@@ -771,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;
@@ -812,7 +811,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;
@@ -858,7 +857,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;
@@ -906,7 +905,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;
 	}
@@ -959,7 +958,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;
 
@@ -969,7 +968,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);
@@ -979,14 +978,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");
@@ -996,7 +995,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;
@@ -1023,6 +1046,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
@@ -1033,24 +1057,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, "unsupported 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;
@@ -1059,9 +1083,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..3c31e7d88f78 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,7 +76,57 @@
 
 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_smp_lvl_mask:	buffer sample level mask
+ * @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;
+	u16 buf_smp_lvl_mask;
+	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

[PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Mehdi Djait]

Since Kionix accelerometers use various numbers of bits to report data, a
device-specific function is required.
Move the driver's private data to the header file to support the new function.
Make the allocation of the "buffer" array in the fifo_flush function dynamic
and more generic.

Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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 | 72 +++++++++++++------------------
 drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
 2 files changed, 66 insertions(+), 43 deletions(-)

diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 7f9a2c29790b..1c81ea1657b9 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
 	.cache_type = REGCACHE_RBTREE,
 };
 
-struct kx022a_data {
-	struct regmap *regmap;
-	struct iio_trigger *trig;
-	struct device *dev;
-	struct iio_mount_matrix orientation;
-	int64_t timestamp, old_timestamp;
-
-	int irq;
-	int inc_reg;
-	int ien_reg;
-
-	unsigned int state;
-	unsigned int odr_ns;
-
-	bool trigger_enabled;
-	/*
-	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
-	 * middle of a configuration, or when the fifo is enabled. Also,
-	 * protect the data stored/retrieved from this structure from
-	 * concurrent accesses.
-	 */
-	struct mutex mutex;
-	u8 watermark;
-
-	/* 3 x 16bit accel data + timestamp */
-	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
-	struct {
-		__le16 channels[3];
-		s64 ts __aligned(8);
-	} scan;
 };
 
 static const struct iio_mount_matrix *
@@ -340,7 +310,6 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
 		dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
 
 	return ret;
-
 }
 
 static int kx022a_turn_off_lock(struct kx022a_data *data)
@@ -595,34 +564,50 @@ 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)
+{
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret, fifo_bytes;
+
+	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;
+
+	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);
-	__le16 buffer[KX022A_FIFO_LENGTH * 3];
+	__le16 *buffer;
 	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;
-	}
+	/* 3 axis, 2 bytes of data for each of the axis */
+	buffer = kmalloc(data->chip_info->fifo_length * 6, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
 
-	/* 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");
 
 	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
-	if (!count)
+	if (!count) {
+		kfree(buffer);
 		return 0;
+	}
 
 	/*
 	 * If we are being called from IRQ handler we know the stored timestamp
@@ -704,6 +689,7 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
 	if (renable)
 		enable_irq(data->irq);
 
+	kfree(buffer);
 	return ret;
 }
 
@@ -1016,6 +1002,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);
 
@@ -1143,7 +1130,6 @@ int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chi
 	if (ret)
 		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
 
-
 	ret = devm_iio_trigger_register(dev, indio_trig);
 	if (ret)
 		return dev_err_probe(data->dev, ret,
diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
index 3c31e7d88f78..43e32e688258 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -11,6 +11,8 @@
 #include <linux/bits.h>
 #include <linux/regmap.h>
 
+#include <linux/iio/iio.h>
+
 #define KX022A_REG_WHO		0x0f
 #define KX022A_ID		0xc8
 
@@ -76,6 +78,39 @@
 
 struct device;
 
+struct kx022a_data {
+	const struct kx022a_chip_info *chip_info;
+	struct regmap *regmap;
+	struct iio_trigger *trig;
+	struct device *dev;
+	struct iio_mount_matrix orientation;
+	int64_t timestamp, old_timestamp;
+
+	int irq;
+	int inc_reg;
+	int ien_reg;
+
+	unsigned int state;
+	unsigned int odr_ns;
+
+	bool trigger_enabled;
+	/*
+	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
+	 * middle of a configuration, or when the fifo is enabled. Also,
+	 * protect the data stored/retrieved from this structure from
+	 * concurrent accesses.
+	 */
+	struct mutex mutex;
+	u8 watermark;
+
+	/* 3 x 16bit accel data + timestamp */
+	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
+	struct {
+		__le16 channels[3];
+		s64 ts __aligned(8);
+	} scan;
+};
+
 /**
  * struct kx022a_chip_info - Kionix accelerometer chip specific information
  *
@@ -100,6 +135,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;
@@ -123,6 +159,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

[PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Mehdi Djait]

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.

Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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     | 145 ++++++++++++++++++++++++++
 drivers/iio/accel/kionix-kx022a.h     |  52 +++++++++
 5 files changed, 205 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 7bc81588e40e..8441d1444b9d 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_spi_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_spi_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 1c81ea1657b9..f058e266d189 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -150,6 +150,99 @@ 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,
 };
 
 static const struct iio_mount_matrix *
@@ -206,6 +299,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
@@ -582,6 +682,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;
+	}
+
+	buf_status &= data->chip_info->buf_smp_lvl_mask;
+	fifo_bytes = le16_to_cpu(buf_status);
+
+	return fifo_bytes;
+}
+
 static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
 			       bool irq)
 {
@@ -1006,6 +1125,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 43e32e688258..c79d9895921e 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,6 +76,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 {
@@ -165,5 +216,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

Re: [PATCH v2 1/5] dt-bindings: iio: Add KX132-1211 accelerometer

[Matti Vaittinen]

On 4/20/23 23:22, Mehdi Djait wrote:
> Extend the kionix,kx022a.yaml file to support the kx132-1211 device
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>

Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>

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

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

Re: [PATCH v2 2/5] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility

[Matti Vaittinen]

On 4/20/23 23:22, Mehdi Djait wrote:
> Avoid error returns on a failure to match and instead just warn with
> assumption that we have a correct dt-binding telling us that
> some new device with a different ID is backwards compatible.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> v2:
> - no changes, this patch is introduced in the v2
> 
>   drivers/iio/accel/kionix-kx022a.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index f98393d74666..70530005cad3 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -1038,9 +1038,7 @@ int kx022a_probe_internal(struct device *dev)
>   		return dev_err_probe(dev, ret, "Failed to access sensor\n");
>   
>   	if (chip_id != KX022A_ID) {
> -		dev_err(dev, "unsupported device 0x%x\n", chip_id);
> -		return -EINVAL;
> -	}
> +		dev_warn(dev, "unsupported device 0x%x\n", chip_id);

Just a 'nit' - no need to re-spin the series for this if there is no 
other changes requested.

Maybe a slightly better wording here would be "unknown device"? If I am 
not mistaken the code proceeds because device is assumed to be supported.

Jonathan, do you think this change is worth backporting? If yes, then we 
might need a Fixes tag.

Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>

>   
>   	irq = fwnode_irq_get_byname(fwnode, "INT1");
>   	if (irq > 0) {

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

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

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Matti Vaittinen]

Hi Mehdi,

Thanks for the v2!

On 4/20/23 23:22, Mehdi Djait wrote:
> Since Kionix accelerometers use various numbers of bits to report data, a
> device-specific function is required.

I think this is the right approach. Thanks for adding this 
device-specific function.

> Move the driver's private data to the header file to support the new function.

Hmm. Why this move is necessary? I didn't immediately spot this struct 
being used outside this C-file. I'd rather saw the struct in C-file if 
possible.

> Make the allocation of the "buffer" array in the fifo_flush function dynamic
> and more generic.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> v2:
> - separated this change from the chip_info introduction and made it a patch in v2

I am unsure if this separation was needed. I'd only separate the "naming 
changes" which bring no changes to code flow, and then patches which are 
fixes and need to be backported (to minimize backporting effort and 
impact to stable branches). Well, I am fine with this separation though, 
seems like I am just making a noise here :).

> - 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 | 72 +++++++++++++------------------
>   drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
>   2 files changed, 66 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 7f9a2c29790b..1c81ea1657b9 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
>   	.cache_type = REGCACHE_RBTREE,
>   };

Yours,
	-- Matti

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

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

Re: [PATCH v2 3/5] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

[Matti Vaittinen]

Hi Mehdi,

Thanks for working on this driver :) Much appreciated!

On 4/20/23 23:22, Mehdi Djait wrote:
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
> Introduce an i2c_device_id table.
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> v2:
> - mentioned the introduction of the i2c_device_id table in the commit

Maybe adding the i2c_device_id table could be done in a separate patch 
with a Fixes tag? That would help backporting (and I think changes like 
this are worth it).

> - get i2c_/spi_get_device_id only with 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 |  20 ++++-
>   drivers/iio/accel/kionix-kx022a-spi.c |  21 ++++--
>   drivers/iio/accel/kionix-kx022a.c     | 102 ++++++++++++++++----------
>   drivers/iio/accel/kionix-kx022a.h     |  54 +++++++++++++-
>   4 files changed, 146 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> index e6fd02d931b6..ce299d0446f7 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,16 +23,28 @@ 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;
> +	}
> +
> +	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", .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);
> @@ -42,6 +55,7 @@ static struct i2c_driver kx022a_i2c_driver = {
>   		.of_match_table = kx022a_of_match,
>   	  },
>   	.probe_new    = kx022a_i2c_probe,
> +	.id_table     = kx022a_i2c_id,
>   };
>   module_i2c_driver(kx022a_i2c_driver);
>   
> diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
> index 9cd047f7b346..7bc81588e40e 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,28 @@ static int kx022a_spi_probe(struct spi_device *spi)
>   		return -EINVAL;
>   	}
>   
> -	regmap = devm_regmap_init_spi(spi, &kx022a_regmap);
> +	chip_info = device_get_match_data(&spi->dev);
> +	if (!chip_info) {
> +		const struct spi_device_id *id = spi_get_device_id(spi);
> +		chip_info = (const struct kx022a_chip_info *)id->driver_data;
> +	}
> +
> +	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" },
> +static const struct spi_device_id kx022a_spi_id[] = {

Did you have a reason to change the struct name? Please, keep the 
original name if there is no reason to change, it helps decreasing the 
size of the patch...

> +	{ .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
>   	{ }
>   };
> -MODULE_DEVICE_TABLE(spi, kx022a_id);
> +MODULE_DEVICE_TABLE(spi, kx022a_spi_id);

...this would not need to be changed if original name was kept...

>   
>   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);
> @@ -48,7 +55,7 @@ static struct spi_driver kx022a_spi_driver = {
>   		.of_match_table = kx022a_of_match,
>   	},
>   	.probe = kx022a_spi_probe,
> -	.id_table = kx022a_id,
> +	.id_table = kx022a_spi_id,

...and this neither.

>   };
>   module_spi_driver(kx022a_spi_driver);
>   
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 70530005cad3..7f9a2c29790b 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,7 +149,6 @@ 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;
> @@ -208,7 +207,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
>   	{ }
>   };

Does this compile? Shouldn't the chip_info be added in the struct 
kx022a_data?

>   
> -#define KX022A_ACCEL_CHAN(axis, index)				\
> +#define KX022A_ACCEL_CHAN(axis, reg, index)			\
>   {								\
>   	.type = IIO_ACCEL,					\
>   	.modified = 1,						\
> @@ -220,7 +219,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 +230,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 +331,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);

Hm, do you have the "chip_info" member added in kx022a_data? I didn't 
spot it from this patch.

Yours,
	-- Matti

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

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

Re: [PATCH v2 1/5] dt-bindings: iio: Add KX132-1211 accelerometer

[Krzysztof Kozlowski]

On 20/04/2023 22:22, Mehdi Djait wrote:
> Extend the kionix,kx022a.yaml file to support the kx132-1211 device
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> v2:
> - made the device name more specific from "kx132" to "kx132-1211"
> - removed the output data-rates mentioned and replaced them with "variable 
> output data-rates"

You missed Rob (and maybe other maintainers). I think he does not care,
but anyway - why not using just get_maintainers.pl? It does entire job
for you, no need to figure out any addresses.



Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[kernel test robot]

Hi Mehdi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on next-20230421]
[cannot apply to linus/master v6.3-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mehdi-Djait/dt-bindings-iio-Add-KX132-1211-accelerometer/20230421-042531
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/cef09595632a40eff8a0864fea2e0eb6653930a5.1682019544.git.mehdi.djait.k%40gmail.com
patch subject: [PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
config: mips-randconfig-s031-20230421 (https://download.01.org/0day-ci/archive/20230422/202304220729.FCofPRvH-lkp@intel.com/config)
compiler: mips64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/38837f58c549d688da8cb7cfb1aea0bd65a12548
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Mehdi-Djait/dt-bindings-iio-Add-KX132-1211-accelerometer/20230421-042531
        git checkout 38837f58c549d688da8cb7cfb1aea0bd65a12548
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304220729.FCofPRvH-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/accel/kionix-kx022a.c:698:20: sparse: sparse: invalid assignment: &=
>> drivers/iio/accel/kionix-kx022a.c:698:20: sparse:    left side has type restricted __le16
>> drivers/iio/accel/kionix-kx022a.c:698:20: sparse:    right side has type unsigned short

vim +698 drivers/iio/accel/kionix-kx022a.c

   684	
   685	static int kx132_get_fifo_bytes(struct kx022a_data *data)
   686	{
   687		struct device *dev = regmap_get_device(data->regmap);
   688		__le16 buf_status;
   689		int ret, fifo_bytes;
   690	
   691		ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
   692				       &buf_status, sizeof(buf_status));
   693		if (ret) {
   694			dev_err(dev, "Error reading buffer status\n");
   695			return ret;
   696		}
   697	
 > 698		buf_status &= data->chip_info->buf_smp_lvl_mask;
   699		fifo_bytes = le16_to_cpu(buf_status);
   700	
   701		return fifo_bytes;
   702	}
   703	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Andy Shevchenko]

On Sat, Apr 22, 2023 at 07:19:44AM +0800, kernel test robot wrote:
> Hi Mehdi,
> 
> kernel test robot noticed the following build warnings:

I believe it's not just a warning, it's a full functional error in the code.

>    686	{
>    687		struct device *dev = regmap_get_device(data->regmap);
>    688		__le16 buf_status;
>    689		int ret, fifo_bytes;
>    690	
>    691		ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
>    692				       &buf_status, sizeof(buf_status));
>    693		if (ret) {
>    694			dev_err(dev, "Error reading buffer status\n");
>    695			return ret;
>    696		}
>    697	
>  > 698		buf_status &= data->chip_info->buf_smp_lvl_mask;
>    699		fifo_bytes = le16_to_cpu(buf_status);

You need to mask in the same endianess space, i.o.w. either on CPU or device side.

I believe you wanted to have fifo_bytes to be masked, but I'm not sure.

>    701		return fifo_bytes;
>    702	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/5] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility

[Jonathan Cameron]

On Fri, 21 Apr 2023 06:44:28 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 4/20/23 23:22, Mehdi Djait wrote:
> > Avoid error returns on a failure to match and instead just warn with
> > assumption that we have a correct dt-binding telling us that
> > some new device with a different ID is backwards compatible.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > ---
> > v2:
> > - no changes, this patch is introduced in the v2
> > 
> >   drivers/iio/accel/kionix-kx022a.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> > index f98393d74666..70530005cad3 100644
> > --- a/drivers/iio/accel/kionix-kx022a.c
> > +++ b/drivers/iio/accel/kionix-kx022a.c
> > @@ -1038,9 +1038,7 @@ int kx022a_probe_internal(struct device *dev)
> >   		return dev_err_probe(dev, ret, "Failed to access sensor\n");
> >   
> >   	if (chip_id != KX022A_ID) {
> > -		dev_err(dev, "unsupported device 0x%x\n", chip_id);
> > -		return -EINVAL;
> > -	}
> > +		dev_warn(dev, "unsupported device 0x%x\n", chip_id);  
> 
> Just a 'nit' - no need to re-spin the series for this if there is no 
> other changes requested.
> 
> Maybe a slightly better wording here would be "unknown device"? If I am 
> not mistaken the code proceeds because device is assumed to be supported.
> 
> Jonathan, do you think this change is worth backporting? If yes, then we 
> might need a Fixes tag.

We haven't backported similar cases as far as I know. 
It's fine to request it explicitly gets picked up for stable if we decide
we care later.

I'll tidy up above if I take this version..

Jonathan

> 
> Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> >   
> >   	irq = fwnode_irq_get_byname(fwnode, "INT1");
> >   	if (irq > 0) {  
> 

Re: [PATCH v2 2/5] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility

[Jonathan Cameron]

On Thu, 20 Apr 2023 22:22:02 +0200
Mehdi Djait <mehdi.djait.k@gmail.com> wrote:

> Avoid error returns on a failure to match and instead just warn with
> assumption that we have a correct dt-binding telling us that
> some new device with a different ID is backwards compatible.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> v2:
> - no changes, this patch is introduced in the v2
> 
>  drivers/iio/accel/kionix-kx022a.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index f98393d74666..70530005cad3 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -1038,9 +1038,7 @@ int kx022a_probe_internal(struct device *dev)
>  		return dev_err_probe(dev, ret, "Failed to access sensor\n");
>  
>  	if (chip_id != KX022A_ID) {
> -		dev_err(dev, "unsupported device 0x%x\n", chip_id);
> -		return -EINVAL;
> -	}
> +		dev_warn(dev, "unsupported device 0x%x\n", chip_id);

Try building this ;)  You have remove the closing bracket but kept the opening
one.

Jonathan

>  
>  	irq = fwnode_irq_get_byname(fwnode, "INT1");
>  	if (irq > 0) {

Re: [PATCH v2 3/5] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

[Jonathan Cameron]

On Fri, 21 Apr 2023 09:19:32 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi Mehdi,
> 
> Thanks for working on this driver :) Much appreciated!
> 
> On 4/20/23 23:22, Mehdi Djait wrote:
> > Refactor the kx022a driver implementation to make it more generic and
> > extensible.
> > Introduce an i2c_device_id table.
> > Add the chip_info structure to the driver's private data to hold all
> > the device specific infos.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > ---
> > v2:
> > - mentioned the introduction of the i2c_device_id table in the commit  
> 
> Maybe adding the i2c_device_id table could be done in a separate patch 
> with a Fixes tag? That would help backporting (and I think changes like 
> this are worth it).

Is it technically a fix?  I thought it was but turned out my reasoning only
applied to spi.   Agreed it would be nice as a separate patch though.
It's not directly related to the rest of what is happening here.

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Jonathan Cameron]

On Fri, 21 Apr 2023 09:14:10 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi Mehdi,
> 
> Thanks for the v2!
> 
> On 4/20/23 23:22, Mehdi Djait wrote:
> > Since Kionix accelerometers use various numbers of bits to report data, a
> > device-specific function is required.  
> 
> I think this is the right approach. Thanks for adding this 
> device-specific function.
> 
> > Move the driver's private data to the header file to support the new function.  
> 
> Hmm. Why this move is necessary? I didn't immediately spot this struct 
> being used outside this C-file. I'd rather saw the struct in C-file if 
> possible.
> 
> > Make the allocation of the "buffer" array in the fifo_flush function dynamic
> > and more generic.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > ---
> > v2:
> > - separated this change from the chip_info introduction and made it a patch in v2  
> 
> I am unsure if this separation was needed. I'd only separate the "naming 
> changes" which bring no changes to code flow, and then patches which are 
> fixes and need to be backported (to minimize backporting effort and 
> impact to stable branches). Well, I am fine with this separation though, 
> seems like I am just making a noise here :).

I prefer them split.  I want to see a noop patch for refactors. It's easier
to quickly check it really makes no functional change.  Adding new
structures for a new device just adds noise to checking that.


> 
> > - 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 | 72 +++++++++++++------------------
> >   drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
> >   2 files changed, 66 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> > index 7f9a2c29790b..1c81ea1657b9 100644
> > --- a/drivers/iio/accel/kionix-kx022a.c
> > +++ b/drivers/iio/accel/kionix-kx022a.c
> > @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
> >   	.cache_type = REGCACHE_RBTREE,
> >   };  
> 
> Yours,
> 	-- Matti
> 

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Jonathan Cameron]

On Sat, 22 Apr 2023 18:36:26 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 21 Apr 2023 09:14:10 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
> > Hi Mehdi,
> > 
> > Thanks for the v2!
> > 
> > On 4/20/23 23:22, Mehdi Djait wrote:  
> > > Since Kionix accelerometers use various numbers of bits to report data, a
> > > device-specific function is required.    
> > 
> > I think this is the right approach. Thanks for adding this 
> > device-specific function.
> >   
> > > Move the driver's private data to the header file to support the new function.    
> > 
> > Hmm. Why this move is necessary? I didn't immediately spot this struct 
> > being used outside this C-file. I'd rather saw the struct in C-file if 
> > possible.
> >   
> > > Make the allocation of the "buffer" array in the fifo_flush function dynamic
> > > and more generic.
> > > 
> > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > > ---
> > > v2:
> > > - separated this change from the chip_info introduction and made it a patch in v2    
> > 
> > I am unsure if this separation was needed. I'd only separate the "naming 
> > changes" which bring no changes to code flow, and then patches which are 
> > fixes and need to be backported (to minimize backporting effort and 
> > impact to stable branches). Well, I am fine with this separation though, 
> > seems like I am just making a noise here :).  
> 
> I prefer them split.  I want to see a noop patch for refactors. It's easier
> to quickly check it really makes no functional change.  Adding new
> structures for a new device just adds noise to checking that.
Oops.  I got the patches confused. Ignore that comment.

> 
> 
> >   
> > > - 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 | 72 +++++++++++++------------------
> > >   drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
> > >   2 files changed, 66 insertions(+), 43 deletions(-)
> > > 
> > > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> > > index 7f9a2c29790b..1c81ea1657b9 100644
> > > --- a/drivers/iio/accel/kionix-kx022a.c
> > > +++ b/drivers/iio/accel/kionix-kx022a.c
> > > @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
> > >   	.cache_type = REGCACHE_RBTREE,
> > >   };    
> > 
> > Yours,
> > 	-- Matti
> >   
> 

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Jonathan Cameron]

On Thu, 20 Apr 2023 22:22:04 +0200
Mehdi Djait <mehdi.djait.k@gmail.com> wrote:

> Since Kionix accelerometers use various numbers of bits to report data, a
> device-specific function is required.
> Move the driver's private data to the header file to support the new function.
> Make the allocation of the "buffer" array in the fifo_flush function dynamic
> and more generic.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>

This results in some fifo_length changes in here and some in the previous patch.
Either keep it fixed for first patch, then make those changes and the ones you have
here in a single patch, or if that's hard maybe a single patch is cleaner.

I'd only expect the stuff about bytes in buffer to be in this patch.

Jonathan



> ---
> 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 | 72 +++++++++++++------------------
>  drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
>  2 files changed, 66 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 7f9a2c29790b..1c81ea1657b9 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
>  	.cache_type = REGCACHE_RBTREE,
>  };
>  
> -struct kx022a_data {
> -	struct regmap *regmap;
> -	struct iio_trigger *trig;
> -	struct device *dev;
> -	struct iio_mount_matrix orientation;
> -	int64_t timestamp, old_timestamp;
> -
> -	int irq;
> -	int inc_reg;
> -	int ien_reg;
> -
> -	unsigned int state;
> -	unsigned int odr_ns;
> -
> -	bool trigger_enabled;
> -	/*
> -	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
> -	 * middle of a configuration, or when the fifo is enabled. Also,
> -	 * protect the data stored/retrieved from this structure from
> -	 * concurrent accesses.
> -	 */
> -	struct mutex mutex;
> -	u8 watermark;
> -
> -	/* 3 x 16bit accel data + timestamp */
> -	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
> -	struct {
> -		__le16 channels[3];
> -		s64 ts __aligned(8);
> -	} scan;
>  };
>  
>  static const struct iio_mount_matrix *
> @@ -340,7 +310,6 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
>  		dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
>  
>  	return ret;
> -

Grumpy maintainer hat on:  I don't want to see white space changes in unrelated code
in a patch doing anything other than white space cleanup.

>  }
>  
>  static int kx022a_turn_off_lock(struct kx022a_data *data)
> @@ -595,34 +564,50 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
>  	return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
>  }
> 


>  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);
> -	__le16 buffer[KX022A_FIFO_LENGTH * 3];
> +	__le16 *buffer;
>  	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;
> -	}
> +	/* 3 axis, 2 bytes of data for each of the axis */
> +	buffer = kmalloc(data->chip_info->fifo_length * 6, GFP_KERNEL);

Split that 6 up into sizeof(*buffer) * 3 
Then just comment on 3 axis.  Or use a define for number of axis and drop the
comment entirely.

Also, this feels like it fitst better in previous patch.


> +	if (!buffer)
> +		return -ENOMEM;
>  
> -	/* 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);

Only this small part is what this patch claims to do.. (+ the callback of course).


>  
>  	if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
>  		dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
>  
>  	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
> -	if (!count)
> +	if (!count) {
> +		kfree(buffer);
>  		return 0;
> +	}
>  
>  	/*
>  	 * If we are being called from IRQ handler we know the stored timestamp
> @@ -704,6 +689,7 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
>  	if (renable)
>  		enable_irq(data->irq);
>  
> +	kfree(buffer);
>  	return ret;
>  }
>  
> @@ -1016,6 +1002,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);
>  
> @@ -1143,7 +1130,6 @@ int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chi
>  	if (ret)
>  		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
>  
> -

Another one.  If those predate your series, feel free to clean them up either
at the start or end of this series.


>  	ret = devm_iio_trigger_register(dev, indio_trig);
>  	if (ret)
>  		return dev_err_probe(data->dev, ret,
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> index 3c31e7d88f78..43e32e688258 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -11,6 +11,8 @@
>  #include <linux/bits.h>
>  #include <linux/regmap.h>
>  
> +#include <linux/iio/iio.h>
> +
>  #define KX022A_REG_WHO		0x0f
>  #define KX022A_ID		0xc8
>  
> @@ -76,6 +78,39 @@
>  
>  struct device;
>  
> +struct kx022a_data {
> +	const struct kx022a_chip_info *chip_info;
> +	struct regmap *regmap;
> +	struct iio_trigger *trig;
> +	struct device *dev;
> +	struct iio_mount_matrix orientation;
> +	int64_t timestamp, old_timestamp;
> +
> +	int irq;
> +	int inc_reg;
> +	int ien_reg;
> +
> +	unsigned int state;
> +	unsigned int odr_ns;
> +
> +	bool trigger_enabled;
> +	/*
> +	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
> +	 * middle of a configuration, or when the fifo is enabled. Also,
> +	 * protect the data stored/retrieved from this structure from
> +	 * concurrent accesses.
> +	 */
> +	struct mutex mutex;
> +	u8 watermark;
> +
> +	/* 3 x 16bit accel data + timestamp */
> +	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
> +	struct {
> +		__le16 channels[3];
> +		s64 ts __aligned(8);
> +	} scan;
> +};
> +
>  /**
>   * struct kx022a_chip_info - Kionix accelerometer chip specific information
>   *
> @@ -100,6 +135,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;
> @@ -123,6 +159,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);

Re: [PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Mehdi Djait]

Hello Andy,

thank you for the review.

On Sat, Apr 22, 2023 at 07:09:12PM +0300, Andy Shevchenko wrote:
> On Sat, Apr 22, 2023 at 07:19:44AM +0800, kernel test robot wrote:
> > Hi Mehdi,
> > 
> > kernel test robot noticed the following build warnings:
> 
> I believe it's not just a warning, it's a full functional error in the code.
> 
> >    686	{
> >    687		struct device *dev = regmap_get_device(data->regmap);
> >    688		__le16 buf_status;
> >    689		int ret, fifo_bytes;
> >    690	
> >    691		ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
> >    692				       &buf_status, sizeof(buf_status));
> >    693		if (ret) {
> >    694			dev_err(dev, "Error reading buffer status\n");
> >    695			return ret;
> >    696		}
> >    697	
> >  > 698		buf_status &= data->chip_info->buf_smp_lvl_mask;
> >    699		fifo_bytes = le16_to_cpu(buf_status);
> 
> You need to mask in the same endianess space, i.o.w. either on CPU or device side.
> 
> I believe you wanted to have fifo_bytes to be masked, but I'm not sure.

I wanted to read the registers buf_status_1 and buf_status_2 --> 16 bits
and mask the result of the read to get the bits 0..9 which is the
buf_status: the number of bytes in the buffer

This is due to my lack of experience, but I have a question:
If I don't get any warnings when testing, how should I go about this ? I
will obviously fix this, but this is for the future.

--
Kind Regards
Mehdi Djait

Re: [PATCH v2 2/5] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility

[Mehdi Djait]

Hello Jonathan,

On Sat, Apr 22, 2023 at 06:28:33PM +0100, Jonathan Cameron wrote:
> On Thu, 20 Apr 2023 22:22:02 +0200
> Mehdi Djait <mehdi.djait.k@gmail.com> wrote:
> 
> > Avoid error returns on a failure to match and instead just warn with
> > assumption that we have a correct dt-binding telling us that
> > some new device with a different ID is backwards compatible.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > ---
> > v2:
> > - no changes, this patch is introduced in the v2
> > 
> >  drivers/iio/accel/kionix-kx022a.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> > index f98393d74666..70530005cad3 100644
> > --- a/drivers/iio/accel/kionix-kx022a.c
> > +++ b/drivers/iio/accel/kionix-kx022a.c
> > @@ -1038,9 +1038,7 @@ int kx022a_probe_internal(struct device *dev)
> >  		return dev_err_probe(dev, ret, "Failed to access sensor\n");
> >  
> >  	if (chip_id != KX022A_ID) {
> > -		dev_err(dev, "unsupported device 0x%x\n", chip_id);
> > -		return -EINVAL;
> > -	}
> > +		dev_warn(dev, "unsupported device 0x%x\n", chip_id);
> 
> Try building this ;)  You have remove the closing bracket but kept the opening
> one.

Oops! I will fix this :)

--
Kind Regards
Mehdi Djait

Re: [PATCH v2 3/5] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

[Mehdi Djait]

Hello Matti and Jonathan,

On Sat, Apr 22, 2023 at 06:32:46PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Apr 2023 09:19:32 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
> > Hi Mehdi,
> > 
> > Thanks for working on this driver :) Much appreciated!
> > 
> > On 4/20/23 23:22, Mehdi Djait wrote:
> > > Refactor the kx022a driver implementation to make it more generic and
> > > extensible.
> > > Introduce an i2c_device_id table.
> > > Add the chip_info structure to the driver's private data to hold all
> > > the device specific infos.
> > > 
> > > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> > > ---
> > > v2:
> > > - mentioned the introduction of the i2c_device_id table in the commit  
> > 
> > Maybe adding the i2c_device_id table could be done in a separate patch 
> > with a Fixes tag? That would help backporting (and I think changes like 
> > this are worth it).
> 
> Is it technically a fix?  I thought it was but turned out my reasoning only
> applied to spi.   Agreed it would be nice as a separate patch though.
> It's not directly related to the rest of what is happening here.

I will make it a separate patch in the v3

--
Kind Regard
Mehdi Djait

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Mehdi Djait]

Hello everyone,

On Sat, Apr 22, 2023 at 06:46:53PM +0100, Jonathan Cameron wrote:
> On Thu, 20 Apr 2023 22:22:04 +0200
> Mehdi Djait <mehdi.djait.k@gmail.com> wrote:
> 
> > Since Kionix accelerometers use various numbers of bits to report data, a
> > device-specific function is required.
> > Move the driver's private data to the header file to support the new function.
> > Make the allocation of the "buffer" array in the fifo_flush function dynamic
> > and more generic.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> 
> This results in some fifo_length changes in here and some in the previous patch.
> Either keep it fixed for first patch, then make those changes and the ones you have
> here in a single patch, or if that's hard maybe a single patch is cleaner.
> 
> I'd only expect the stuff about bytes in buffer to be in this patch.
> 
> Jonathan

I will come up with a better solution in the v3

> 
> 
> 
> > ---
> > 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 | 72 +++++++++++++------------------
> >  drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
> >  2 files changed, 66 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> > index 7f9a2c29790b..1c81ea1657b9 100644
> > --- a/drivers/iio/accel/kionix-kx022a.c
> > +++ b/drivers/iio/accel/kionix-kx022a.c
> > @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
> >  	.cache_type = REGCACHE_RBTREE,
> >  };
> >  
> > -struct kx022a_data {
> > -	struct regmap *regmap;
> > -	struct iio_trigger *trig;
> > -	struct device *dev;
> > -	struct iio_mount_matrix orientation;
> > -	int64_t timestamp, old_timestamp;
> > -
> > -	int irq;
> > -	int inc_reg;
> > -	int ien_reg;
> > -
> > -	unsigned int state;
> > -	unsigned int odr_ns;
> > -
> > -	bool trigger_enabled;
> > -	/*
> > -	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
> > -	 * middle of a configuration, or when the fifo is enabled. Also,
> > -	 * protect the data stored/retrieved from this structure from
> > -	 * concurrent accesses.
> > -	 */
> > -	struct mutex mutex;
> > -	u8 watermark;
> > -
> > -	/* 3 x 16bit accel data + timestamp */
> > -	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
> > -	struct {
> > -		__le16 channels[3];
> > -		s64 ts __aligned(8);
> > -	} scan;
> >  };
> >  
> >  static const struct iio_mount_matrix *
> > @@ -340,7 +310,6 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
> >  		dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
> >  
> >  	return ret;
> > -
> 
> Grumpy maintainer hat on:  I don't want to see white space changes in unrelated code
> in a patch doing anything other than white space cleanup.

I will make a separate patch.

> 
> >  }
> >  
> >  static int kx022a_turn_off_lock(struct kx022a_data *data)
> > @@ -595,34 +564,50 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
> >  	return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
> >  }
> > 
> 
> 
> >  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);
> > -	__le16 buffer[KX022A_FIFO_LENGTH * 3];
> > +	__le16 *buffer;
> >  	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;
> > -	}
> > +	/* 3 axis, 2 bytes of data for each of the axis */
> > +	buffer = kmalloc(data->chip_info->fifo_length * 6, GFP_KERNEL);
> 
> Split that 6 up into sizeof(*buffer) * 3 
> Then just comment on 3 axis.  Or use a define for number of axis and drop the
> comment entirely.
> 
> Also, this feels like it fitst better in previous patch.

I will add this change to the previous patch

> 
> 
> > +	if (!buffer)
> > +		return -ENOMEM;
> >  
> > -	/* 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);
> 
> Only this small part is what this patch claims to do.. (+ the callback of course).
> 
> 
> >  
> >  	if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
> >  		dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
> >  
> >  	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
> > -	if (!count)
> > +	if (!count) {
> > +		kfree(buffer);
> >  		return 0;
> > +	}
> >  
> >  	/*
> >  	 * If we are being called from IRQ handler we know the stored timestamp
> > @@ -704,6 +689,7 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> >  	if (renable)
> >  		enable_irq(data->irq);
> >  
> > +	kfree(buffer);
> >  	return ret;
> >  }
> >  
> > @@ -1016,6 +1002,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);
> >  
> > @@ -1143,7 +1130,6 @@ int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chi
> >  	if (ret)
> >  		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
> >  
> > -
> 
> Another one.  If those predate your series, feel free to clean them up either
> at the start or end of this series.

I will

--
Kind Regards
Mehdi Djait

Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

[Mehdi Djait]

Hi Matti,

On Fri, Apr 21, 2023 at 09:14:10AM +0300, Matti Vaittinen wrote:
> Hi Mehdi,
> 
> Thanks for the v2!
> 
> On 4/20/23 23:22, Mehdi Djait wrote:
> > Since Kionix accelerometers use various numbers of bits to report data, a
> > device-specific function is required.
> 
> I think this is the right approach. Thanks for adding this device-specific
> function.
> 
> > Move the driver's private data to the header file to support the new function.
> 
> Hmm. Why this move is necessary? I didn't immediately spot this struct being
> used outside this C-file. I'd rather saw the struct in C-file if possible.

My bad, I will move it back

--
Kind Regards
Mehdi Djait

Re: [PATCH v2 5/5] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

[Mehdi Djait]

Hello again,

On Sun, Apr 23, 2023 at 10:57:00PM +0200, Mehdi Djait wrote:
> Hello Andy,
> 
> thank you for the review.
> 
> On Sat, Apr 22, 2023 at 07:09:12PM +0300, Andy Shevchenko wrote:
> > On Sat, Apr 22, 2023 at 07:19:44AM +0800, kernel test robot wrote:
> > > Hi Mehdi,
> > > 
> > > kernel test robot noticed the following build warnings:
> > 
> > I believe it's not just a warning, it's a full functional error in the code.
> > 
> > >    686	{
> > >    687		struct device *dev = regmap_get_device(data->regmap);
> > >    688		__le16 buf_status;
> > >    689		int ret, fifo_bytes;
> > >    690	
> > >    691		ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
> > >    692				       &buf_status, sizeof(buf_status));
> > >    693		if (ret) {
> > >    694			dev_err(dev, "Error reading buffer status\n");
> > >    695			return ret;
> > >    696		}
> > >    697	
> > >  > 698		buf_status &= data->chip_info->buf_smp_lvl_mask;
> > >    699		fifo_bytes = le16_to_cpu(buf_status);
> > 
> > You need to mask in the same endianess space, i.o.w. either on CPU or device side.
> > 
> > I believe you wanted to have fifo_bytes to be masked, but I'm not sure.
> 
> I wanted to read the registers buf_status_1 and buf_status_2 --> 16 bits
> and mask the result of the read to get the bits 0..9 which is the
> buf_status: the number of bytes in the buffer
> 
> This is due to my lack of experience, but I have a question:
> If I don't get any warnings when testing, how should I go about this ? I
> will obviously fix this, but this is for the future.

just ignore this question. I installed sparse and I am using it now when
building. 
 
--
Kind Regards
Mehdi Djait