iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer

Hello everyone,

Version 5 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 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         |  13 +-
 drivers/iio/accel/kionix-kx022a.c             | 312 ++++++++++++++----
 drivers/iio/accel/kionix-kx022a.h             | 110 +++++-
 6 files changed, 404 insertions(+), 75 deletions(-)

2.30.2

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

---
v5:
v4:
v3:
- no changes

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(-)

index 986df1a6ff0a..034b69614416 100644
@@ -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 <[email protected]>
 
 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

Remove blank lines pointed out by the checkpatch script

---
v5:
v4:
- no changes

v3:
- no changes, this patch is introduced in the v2

 drivers/iio/accel/kionix-kx022a.c | 2 --
 1 file changed, 2 deletions(-)

index f98393d74666..ff8aa7b9568e 100644
@@ -341,7 +341,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)

@@ -1121,7 +1120,6 @@ int kx022a_probe_internal(struct device *dev)
 	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,

2.30.2

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.

---
v5:
v4:
- no changes

v3:
- changed from 'unsupported' to 'unknown'
- removed the opening bracket

v2:
- no changes, this patch is introduced in the v2

 drivers/iio/accel/kionix-kx022a.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

index ff8aa7b9568e..494e81ba1da9 100644
@@ -1036,10 +1036,8 @@ int kx022a_probe_internal(struct device *dev)
 	if (ret)
 		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;
-	}
+	if (chip_id != KX022A_ID)
+		dev_warn(dev, "unknown device 0x%x\n", chip_id);
 
 	irq = fwnode_irq_get_byname(fwnode, "INT1");
 	if (irq > 0) {

2.30.2

Add the missing i2c device id.

---
v5:
v4:
- no changes

v3:
- no changes, this patch is introduced in the v2

 drivers/iio/accel/kionix-kx022a-i2c.c | 7 +++++++
 1 file changed, 7 insertions(+)

index e6fd02d931b6..b5a85ce3a891 100644
@@ -30,6 +30,12 @@ static int kx022a_i2c_probe(struct i2c_client *i2c)
 	return kx022a_probe_internal(dev);
 }
 
+static const struct i2c_device_id kx022a_i2c_id[] = {
+	{ .name = "kx022a" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id);
+
 static const struct of_device_id kx022a_of_match[] = {
 	{ .compatible = "kionix,kx022a", },
 	{ }

@@ -42,6 +48,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);
 

2.30.2

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.

---
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 |  11 ++-
 drivers/iio/accel/kionix-kx022a.c     | 116 ++++++++++++++++----------
 drivers/iio/accel/kionix-kx022a.h     |  52 +++++++++++-
 4 files changed, 144 insertions(+), 52 deletions(-)

index b5a85ce3a891..c59558fc6493 100644
@@ -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);

index 9cd047f7b346..1e4b9d4b4b8d 100644
@@ -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,24 @@ 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);
+
+	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);

2.30.2

...

With Best Regards,
Andy Shevchenko

Hello Andy,

I will add the check for the SPI case

I used the bloat-o-meter on the two .ko files (this version and the the one where chip_info after regamp) Is this what you asked for ?

add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-20 (-20)

Function                                     old     new   delta
kx132_get_fifo_bytes                         148     144      -4
kx022a_trigger_set_state                     352     348      -4
kx022a_read_raw                              380     376      -4
kx022a_buffer_predisable                     248     244      -4
kx022a_buffer_postenable                     296     292      -4

Total: Before=11244, After=11224, chg -0.18%

--
Kind Regards
Mehdi Djait

...

Yes. I assume the old one is the current as in this patch? If so, you know what to do in the next version :-)

With Best Regards,
Andy Shevchenko

Hello Andy,

Yes

Yes, I will change it in the next version :)

--
Kind Regards
Mehdi Djait

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

---
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(-)

index 92cc42b2aaaa..4eed16cd3200 100644
@@ -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)
+		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);
 	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);
 

index 0e5026019213..c9f9aee7e597 100644
@@ -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

...

With Best Regards,
Andy Shevchenko

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

---
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(-)

index b6b45d359f28..d8cc6e6f2bb9 100644
@@ -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"

index c59558fc6493..0f4729c55d29 100644
@@ -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);

index 1e4b9d4b4b8d..3fb673ffcdeb 100644
@@ -35,12 +35,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);

2.30.2

...

With Best Regards,
Andy Shevchenko