* [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
@ 2023-05-26 14:30 Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 1/7] dt-bindings: iio: Add " Mehdi Djait
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, Mehdi Djait
Hello everyone,
Version 4 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 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 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 | 22 +-
drivers/iio/accel/kionix-kx022a-spi.c | 13 +-
drivers/iio/accel/kionix-kx022a.c | 315 ++++++++++++++----
drivers/iio/accel/kionix-kx022a.h | 109 +++++-
6 files changed, 404 insertions(+), 75 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 1/7] dt-bindings: iio: Add KX132-1211 accelerometer
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 2/7] iio: accel: kionix-kx022a: Remove blank lines Mehdi Djait
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, Mehdi Djait,
Krzysztof Kozlowski
Extend the kionix,kx022a.yaml file to support the kx132-1211 device
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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(-)
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/7] iio: accel: kionix-kx022a: Remove blank lines
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 1/7] dt-bindings: iio: Add " Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 3/7] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility Mehdi Djait
` (3 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, Mehdi Djait
Remove blank lines pointed out by the checkpatch script.
Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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(-)
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index f98393d74666..ff8aa7b9568e 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 3/7] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 1/7] dt-bindings: iio: Add " Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 2/7] iio: accel: kionix-kx022a: Remove blank lines Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table Mehdi Djait
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, 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.
Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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(-)
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index ff8aa7b9568e..494e81ba1da9 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
` (2 preceding siblings ...)
2023-05-26 14:30 ` [PATCH v4 3/7] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 16:35 ` Matti Vaittinen
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer Mehdi Djait
5 siblings, 1 reply; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, Mehdi Djait
Add the missing i2c device id.
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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(+)
diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
index e6fd02d931b6..b5a85ce3a891 100644
--- a/drivers/iio/accel/kionix-kx022a-i2c.c
+++ b/drivers/iio/accel/kionix-kx022a-i2c.c
@@ -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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
` (3 preceding siblings ...)
2023-05-26 14:30 ` [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 17:04 ` Matti Vaittinen
` (2 more replies)
2023-05-26 14:30 ` [PATCH v4 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer Mehdi Djait
5 siblings, 3 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, Mehdi Djait
Add the chip_info structure to the driver's private data to hold all
the device specific infos.
Refactor the kx022a driver implementation to make it more generic and
extensible.
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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 | 15 +++-
drivers/iio/accel/kionix-kx022a-spi.c | 11 ++-
drivers/iio/accel/kionix-kx022a.c | 118 +++++++++++++++++---------
drivers/iio/accel/kionix-kx022a.h | 53 +++++++++++-
4 files changed, 145 insertions(+), 52 deletions(-)
diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
index b5a85ce3a891..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,22 +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" },
+ { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id);
static const struct of_device_id kx022a_of_match[] = {
- { .compatible = "kionix,kx022a", },
+ { .compatible = "kionix,kx022a", .data = &kx022a_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, kx022a_of_match);
diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
index 9cd047f7b346..1e4b9d4b4b8d 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,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);
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 494e81ba1da9..69c22071c731 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,9 +149,9 @@ 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 {
+ const struct kx022a_chip_info *chip_info;
struct regmap *regmap;
struct iio_trigger *trig;
struct device *dev;
@@ -175,6 +175,8 @@ struct kx022a_data {
struct mutex mutex;
u8 watermark;
+ __le16 *fifo_buffer;
+
/* 3 x 16bit accel data + timestamp */
__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
struct {
@@ -208,7 +210,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
{ }
};
-#define KX022A_ACCEL_CHAN(axis, index) \
+#define KX022A_ACCEL_CHAN(axis, reg, index) \
{ \
.type = IIO_ACCEL, \
.modified = 1, \
@@ -220,7 +222,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
BIT(IIO_CHAN_INFO_SCALE) | \
BIT(IIO_CHAN_INFO_SAMP_FREQ), \
.ext_info = kx022a_ext_info, \
- .address = KX022A_REG_##axis##OUT_L, \
+ .address = reg, \
.scan_index = index, \
.scan_type = { \
.sign = 's', \
@@ -231,9 +233,9 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
}
static const struct iio_chan_spec kx022a_channels[] = {
- KX022A_ACCEL_CHAN(X, 0),
- KX022A_ACCEL_CHAN(Y, 1),
- KX022A_ACCEL_CHAN(Z, 2),
+ KX022A_ACCEL_CHAN(X, KX022A_REG_XOUT_L, 0),
+ KX022A_ACCEL_CHAN(Y, KX022A_REG_YOUT_L, 1),
+ KX022A_ACCEL_CHAN(Z, KX022A_REG_ZOUT_L, 2),
IIO_CHAN_SOFT_TIMESTAMP(3),
};
@@ -332,10 +334,10 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
int ret;
if (on)
- ret = regmap_set_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_set_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_PC1);
else
- ret = regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_clear_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_PC1);
if (ret)
dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
@@ -402,7 +404,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
break;
ret = regmap_update_bits(data->regmap,
- KX022A_REG_ODCNTL,
+ data->chip_info->odcntl,
KX022A_MASK_ODR, n);
data->odr_ns = kx022a_odrs[n];
kx022a_turn_on_unlock(data);
@@ -423,7 +425,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
if (ret)
break;
- ret = regmap_update_bits(data->regmap, KX022A_REG_CNTL,
+ ret = regmap_update_bits(data->regmap, data->chip_info->cntl,
KX022A_MASK_GSEL,
n << KX022A_GSEL_SHIFT);
kx022a_turn_on_unlock(data);
@@ -445,7 +447,7 @@ static int kx022a_fifo_set_wmi(struct kx022a_data *data)
threshold = data->watermark;
- return regmap_update_bits(data->regmap, KX022A_REG_BUF_CNTL1,
+ return regmap_update_bits(data->regmap, data->chip_info->buf_cntl1,
KX022A_MASK_WM_TH, threshold);
}
@@ -488,7 +490,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
return ret;
case IIO_CHAN_INFO_SAMP_FREQ:
- ret = regmap_read(data->regmap, KX022A_REG_ODCNTL, ®val);
+ ret = regmap_read(data->regmap, data->chip_info->odcntl, ®val);
if (ret)
return ret;
@@ -503,7 +505,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_SCALE:
- ret = regmap_read(data->regmap, KX022A_REG_CNTL, ®val);
+ ret = regmap_read(data->regmap, data->chip_info->cntl, ®val);
if (ret < 0)
return ret;
@@ -530,8 +532,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;
@@ -592,7 +594,7 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
*/
data->timestamp = 0;
- return regmap_write(data->regmap, KX022A_REG_BUF_CLEAR, 0x0);
+ return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
}
static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
@@ -600,7 +602,6 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
{
struct kx022a_data *data = iio_priv(idev);
struct device *dev = regmap_get_device(data->regmap);
- __le16 buffer[KX022A_FIFO_LENGTH * 3];
uint64_t sample_period;
int count, fifo_bytes;
bool renable = false;
@@ -679,13 +680,13 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
}
fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
- ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
- &buffer[0], fifo_bytes);
+ ret = regmap_noinc_read(data->regmap, data->chip_info->buf_read,
+ &data->fifo_buffer[0], fifo_bytes);
if (ret)
goto renable_out;
for (i = 0; i < count; i++) {
- __le16 *sam = &buffer[i * 3];
+ __le16 *sam = &data->fifo_buffer[i * 3];
__le16 *chs;
int bit;
@@ -732,10 +733,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);
}
@@ -762,6 +763,8 @@ static int kx022a_fifo_disable(struct kx022a_data *data)
{
int ret = 0;
+ kfree(data->fifo_buffer);
+
ret = kx022a_turn_off_lock(data);
if (ret)
return ret;
@@ -770,7 +773,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;
@@ -801,6 +804,12 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
{
int ret;
+ data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
+ KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
+
+ if (!data->fifo_buffer)
+ return -ENOMEM;
+
ret = kx022a_turn_off_lock(data);
if (ret)
return ret;
@@ -811,7 +820,7 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
goto unlock_out;
/* Enable buffer */
- ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+ ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
KX022A_MASK_BUF_EN);
if (ret)
goto unlock_out;
@@ -857,7 +866,7 @@ static irqreturn_t kx022a_trigger_handler(int irq, void *p)
struct kx022a_data *data = iio_priv(idev);
int ret;
- ret = regmap_bulk_read(data->regmap, KX022A_REG_XOUT_L, data->buffer,
+ ret = regmap_bulk_read(data->regmap, data->chip_info->xout_l, data->buffer,
KX022A_FIFO_SAMPLES_SIZE_BYTES);
if (ret < 0)
goto err_read;
@@ -905,7 +914,7 @@ static irqreturn_t kx022a_irq_thread_handler(int irq, void *private)
if (data->state & KX022A_STATE_FIFO) {
int ok;
- ok = __kx022a_fifo_flush(idev, KX022A_FIFO_LENGTH, true);
+ ok = __kx022a_fifo_flush(idev, data->chip_info->fifo_length, true);
if (ok > 0)
ret = IRQ_HANDLED;
}
@@ -958,7 +967,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
int ret, val;
/* Reset the senor */
- ret = regmap_write(data->regmap, KX022A_REG_CNTL2, KX022A_MASK_SRST);
+ ret = regmap_write(data->regmap, data->chip_info->cntl2, KX022A_MASK_SRST);
if (ret)
return ret;
@@ -968,7 +977,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
*/
msleep(1);
- ret = regmap_read_poll_timeout(data->regmap, KX022A_REG_CNTL2, val,
+ ret = regmap_read_poll_timeout(data->regmap, data->chip_info->cntl2, val,
!(val & KX022A_MASK_SRST),
KX022A_SOFT_RESET_WAIT_TIME_US,
KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US);
@@ -978,14 +987,14 @@ static int kx022a_chip_init(struct kx022a_data *data)
return ret;
}
- ret = regmap_reinit_cache(data->regmap, &kx022a_regmap);
+ ret = regmap_reinit_cache(data->regmap, data->chip_info->regmap_config);
if (ret) {
dev_err(data->dev, "Failed to reinit reg cache\n");
return ret;
}
/* set data res 16bit */
- ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+ ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
KX022A_MASK_BRES16);
if (ret) {
dev_err(data->dev, "Failed to set data resolution\n");
@@ -995,7 +1004,31 @@ static int kx022a_chip_init(struct kx022a_data *data)
return kx022a_prepare_irq_pin(data);
}
-int kx022a_probe_internal(struct device *dev)
+const struct kx022a_chip_info kx022a_chip_info = {
+ .name = "kx022-accel",
+ .regmap_config = &kx022a_regmap_config,
+ .channels = kx022a_channels,
+ .num_channels = ARRAY_SIZE(kx022a_channels),
+ .fifo_length = KX022A_FIFO_LENGTH,
+ .who = KX022A_REG_WHO,
+ .id = KX022A_ID,
+ .cntl = KX022A_REG_CNTL,
+ .cntl2 = KX022A_REG_CNTL2,
+ .odcntl = KX022A_REG_ODCNTL,
+ .buf_cntl1 = KX022A_REG_BUF_CNTL1,
+ .buf_cntl2 = KX022A_REG_BUF_CNTL2,
+ .buf_clear = KX022A_REG_BUF_CLEAR,
+ .buf_status1 = KX022A_REG_BUF_STATUS_1,
+ .buf_read = KX022A_REG_BUF_READ,
+ .inc1 = KX022A_REG_INC1,
+ .inc4 = KX022A_REG_INC4,
+ .inc5 = KX022A_REG_INC5,
+ .inc6 = KX022A_REG_INC6,
+ .xout_l = KX022A_REG_XOUT_L,
+};
+EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
+
+int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info)
{
static const char * const regulator_names[] = {"io-vdd", "vdd"};
struct iio_trigger *indio_trig;
@@ -1022,6 +1055,7 @@ int kx022a_probe_internal(struct device *dev)
return -ENOMEM;
data = iio_priv(idev);
+ data->chip_info = chip_info;
/*
* VDD is the analog and digital domain voltage supply and
@@ -1032,24 +1066,24 @@ int kx022a_probe_internal(struct device *dev)
if (ret && ret != -ENODEV)
return dev_err_probe(dev, ret, "failed to enable regulator\n");
- ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
+ ret = regmap_read(regmap, chip_info->who, &chip_id);
if (ret)
return dev_err_probe(dev, ret, "Failed to access sensor\n");
- if (chip_id != KX022A_ID)
+ if (chip_id != chip_info->id)
dev_warn(dev, "unknown device 0x%x\n", chip_id);
irq = fwnode_irq_get_byname(fwnode, "INT1");
if (irq > 0) {
- data->inc_reg = KX022A_REG_INC1;
- data->ien_reg = KX022A_REG_INC4;
+ data->inc_reg = chip_info->inc1;
+ data->ien_reg = chip_info->inc4;
} else {
irq = fwnode_irq_get_byname(fwnode, "INT2");
if (irq <= 0)
return dev_err_probe(dev, irq, "No suitable IRQ\n");
- data->inc_reg = KX022A_REG_INC5;
- data->ien_reg = KX022A_REG_INC6;
+ data->inc_reg = chip_info->inc5;
+ data->ien_reg = chip_info->inc6;
}
data->regmap = regmap;
@@ -1058,9 +1092,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..c23b6f03409e 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,7 +76,56 @@
struct device;
-int kx022a_probe_internal(struct device *dev);
-extern const struct regmap_config kx022a_regmap;
+/**
+ * struct kx022a_chip_info - Kionix accelerometer chip specific information
+ *
+ * @name: name of the device
+ * @regmap_config: pointer to register map configuration
+ * @channels: pointer to iio_chan_spec array
+ * @num_channels: number of iio_chan_spec channels
+ * @fifo_length: number of 16-bit samples in a full buffer
+ * @who: WHO_AM_I register
+ * @id: WHO_AM_I register value
+ * @cntl: control register 1
+ * @cntl2: control register 2
+ * @odcntl: output data control register
+ * @buf_cntl1: buffer control register 1
+ * @buf_cntl2: buffer control register 2
+ * @buf_clear: buffer clear register
+ * @buf_status1: buffer status register 1
+ * @buf_read: buffer read register
+ * @inc1: interrupt control register 1
+ * @inc4: interrupt control register 4
+ * @inc5: interrupt control register 5
+ * @inc6: interrupt control register 6
+ * @xout_l: x-axis output least significant byte
+ */
+struct kx022a_chip_info {
+ const char *name;
+ const struct regmap_config *regmap_config;
+ const struct iio_chan_spec *channels;
+ unsigned int num_channels;
+ unsigned int fifo_length;
+ u8 who;
+ u8 id;
+ u8 cntl;
+ u8 cntl2;
+ u8 odcntl;
+ u8 buf_cntl1;
+ u8 buf_cntl2;
+ u8 buf_clear;
+ u8 buf_status1;
+ 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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
` (4 preceding siblings ...)
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
@ 2023-05-26 14:30 ` Mehdi Djait
5 siblings, 0 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, 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
Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
v4:
- removed the comment about "bogus value from i2c"
- removed regmap_get_device(data->regmap); dev is present in the
driver's private data
v3:
- no changes
v2:
- separated this change from the chip_info introduction and made it a patch in v2
- changed the function from generic implementation for to device-specific one
- removed blank lines pointed out by checkpatch
- changed the allocation of the "buffer" array in __kx022a_fifo_flush
drivers/iio/accel/kionix-kx022a.c | 28 ++++++++++++++++++----------
drivers/iio/accel/kionix-kx022a.h | 4 ++++
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 69c22071c731..20f5965e878c 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -597,26 +597,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");
@@ -1025,6 +1032,7 @@ const struct kx022a_chip_info kx022a_chip_info = {
.inc5 = KX022A_REG_INC5,
.inc6 = KX022A_REG_INC6,
.xout_l = KX022A_REG_XOUT_L,
+ .get_fifo_bytes = kx022a_get_fifo_bytes,
};
EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
index c23b6f03409e..7792907534b5 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -76,6 +76,8 @@
struct device;
+struct kx022a_data;
+
/**
* struct kx022a_chip_info - Kionix accelerometer chip specific information
*
@@ -99,6 +101,7 @@ struct device;
* @inc5: interrupt control register 5
* @inc6: interrupt control register 6
* @xout_l: x-axis output least significant byte
+ * @get_fifo_bytes: function pointer to get number of bytes in the FIFO buffer
*/
struct kx022a_chip_info {
const char *name;
@@ -122,6 +125,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
[not found] <cover.1685111274.git.mehdi.djait.k@gmail.com>
@ 2023-05-26 14:30 ` Mehdi Djait
2023-05-26 14:42 ` Mehdi Djait
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:30 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, 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.
Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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 | 52 +++++++++
5 files changed, 222 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 1e4b9d4b4b8d..3fb673ffcdeb 100644
--- a/drivers/iio/accel/kionix-kx022a-spi.c
+++ b/drivers/iio/accel/kionix-kx022a-spi.c
@@ -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);
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 20f5965e878c..327412a8a395 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -150,6 +150,117 @@ static const struct regmap_config kx022a_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+/* Regmap configs kx132 */
+static const struct regmap_range kx132_volatile_ranges[] = {
+ {
+ .range_min = KX132_REG_XADP_L,
+ .range_max = KX132_REG_COTR,
+ }, {
+ .range_min = KX132_REG_TSCP,
+ .range_max = KX132_REG_INT_REL,
+ }, {
+ /* The reset bit will be cleared by sensor */
+ .range_min = KX132_REG_CNTL2,
+ .range_max = KX132_REG_CNTL2,
+ }, {
+ .range_min = KX132_REG_CNTL5,
+ .range_max = KX132_REG_CNTL5,
+ }, {
+ .range_min = KX132_REG_BUF_STATUS_1,
+ .range_max = KX132_REG_BUF_READ,
+ },
+};
+
+static const struct regmap_access_table kx132_volatile_regs = {
+ .yes_ranges = &kx132_volatile_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_volatile_ranges),
+};
+
+static const struct regmap_range kx132_precious_ranges[] = {
+ {
+ .range_min = KX132_REG_INT_REL,
+ .range_max = KX132_REG_INT_REL,
+ },
+};
+
+static const struct regmap_access_table kx132_precious_regs = {
+ .yes_ranges = &kx132_precious_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_precious_ranges),
+};
+
+static const struct regmap_range kx132_read_only_ranges[] = {
+ {
+ .range_min = KX132_REG_XADP_L,
+ .range_max = KX132_REG_INT_REL,
+ }, {
+ .range_min = KX132_REG_BUF_STATUS_1,
+ .range_max = KX132_REG_BUF_STATUS_2,
+ }, {
+ .range_min = KX132_REG_BUF_READ,
+ .range_max = KX132_REG_BUF_READ,
+ }, {
+ /* Kionix reserved registers: should not be written */
+ .range_min = 0x28,
+ .range_max = 0x28,
+ }, {
+ .range_min = 0x35,
+ .range_max = 0x36,
+ }, {
+ .range_min = 0x3c,
+ .range_max = 0x48,
+ }, {
+ .range_min = 0x4e,
+ .range_max = 0x5c,
+ }, {
+ .range_min = 0x77,
+ .range_max = 0x7f,
+ },
+};
+
+static const struct regmap_access_table kx132_ro_regs = {
+ .no_ranges = &kx132_read_only_ranges[0],
+ .n_no_ranges = ARRAY_SIZE(kx132_read_only_ranges),
+};
+
+static const struct regmap_range kx132_write_only_ranges[] = {
+ {
+ .range_min = KX132_REG_SELF_TEST,
+ .range_max = KX132_REG_SELF_TEST,
+ }, {
+ .range_min = KX132_REG_BUF_CLEAR,
+ .range_max = KX132_REG_BUF_CLEAR,
+ },
+};
+
+static const struct regmap_access_table kx132_wo_regs = {
+ .no_ranges = &kx132_write_only_ranges[0],
+ .n_no_ranges = ARRAY_SIZE(kx132_write_only_ranges),
+};
+
+static const struct regmap_range kx132_noinc_read_ranges[] = {
+ {
+ .range_min = KX132_REG_BUF_READ,
+ .range_max = KX132_REG_BUF_READ,
+ },
+};
+
+static const struct regmap_access_table kx132_nir_regs = {
+ .yes_ranges = &kx132_noinc_read_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_noinc_read_ranges),
+};
+
+static const struct regmap_config kx132_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &kx132_volatile_regs,
+ .rd_table = &kx132_wo_regs,
+ .wr_table = &kx132_ro_regs,
+ .rd_noinc_table = &kx132_nir_regs,
+ .precious_table = &kx132_precious_regs,
+ .max_register = KX132_MAX_REGISTER,
+ .cache_type = REGCACHE_RBTREE,
+};
+
struct kx022a_data {
const struct kx022a_chip_info *chip_info;
struct regmap *regmap;
@@ -239,6 +350,13 @@ static const struct iio_chan_spec kx022a_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(3),
};
+static const struct iio_chan_spec kx132_channels[] = {
+ KX022A_ACCEL_CHAN(X, KX132_REG_XOUT_L, 0),
+ KX022A_ACCEL_CHAN(Y, KX132_REG_YOUT_L, 1),
+ KX022A_ACCEL_CHAN(Z, KX132_REG_ZOUT_L, 2),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
/*
* The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the
* Linux CPUs can handle without dropping samples. Also, the low power mode is
@@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
return fifo_bytes;
}
+static int kx132_get_fifo_bytes(struct kx022a_data *data)
+{
+ __le16 buf_status;
+ int ret, fifo_bytes;
+
+ ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
+ &buf_status, sizeof(buf_status));
+ if (ret) {
+ dev_err(data->dev, "Error reading buffer status\n");
+ return ret;
+ }
+
+ fifo_bytes = le16_to_cpu(buf_status);
+ fifo_bytes &= data->chip_info->buf_smp_lvl_mask;
+
+ return fifo_bytes;
+}
+
static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
bool irq)
{
@@ -1036,6 +1172,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 7792907534b5..351867a95f8c 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -74,6 +74,57 @@
#define KX022A_REG_SELF_TEST 0x60
#define KX022A_MAX_REGISTER 0x60
+#define KX132_REG_WHO 0x13
+#define KX132_ID 0x3d
+
+#define KX132_FIFO_LENGTH 86
+
+#define KX132_REG_CNTL 0x1b
+#define KX132_REG_CNTL2 0x1c
+#define KX132_REG_CNTL5 0x1f
+#define KX132_MASK_RES BIT(6)
+#define KX132_GSEL_2 0x0
+#define KX132_GSEL_4 BIT(3)
+#define KX132_GSEL_8 BIT(4)
+#define KX132_GSEL_16 GENMASK(4, 3)
+
+#define KX132_REG_INS2 0x17
+#define KX132_MASK_INS2_WMI BIT(5)
+
+#define KX132_REG_XADP_L 0x02
+#define KX132_REG_XOUT_L 0x08
+#define KX132_REG_YOUT_L 0x0a
+#define KX132_REG_ZOUT_L 0x0c
+#define KX132_REG_COTR 0x12
+#define KX132_REG_TSCP 0x14
+#define KX132_REG_INT_REL 0x1a
+
+#define KX132_REG_ODCNTL 0x21
+
+#define KX132_REG_BTS_WUF_TH 0x4a
+
+#define KX132_REG_BUF_CNTL1 0x5e
+#define KX132_REG_BUF_CNTL2 0x5f
+#define KX132_REG_BUF_STATUS_1 0x60
+#define KX132_REG_BUF_STATUS_2 0x61
+#define KX132_MASK_BUF_SMP_LVL GENMASK(9, 0)
+#define KX132_REG_BUF_CLEAR 0x62
+#define KX132_REG_BUF_READ 0x63
+#define KX132_ODR_SHIFT 3
+#define KX132_FIFO_MAX_WMI_TH 86
+
+#define KX132_REG_INC1 0x22
+#define KX132_REG_INC5 0x26
+#define KX132_REG_INC6 0x27
+#define KX132_IPOL_LOW 0
+#define KX132_IPOL_HIGH KX022A_MASK_IPOL
+#define KX132_ITYP_PULSE KX022A_MASK_ITYP
+
+#define KX132_REG_INC4 0x25
+
+#define KX132_REG_SELF_TEST 0x5d
+#define KX132_MAX_REGISTER 0x76
+
struct device;
struct kx022a_data;
@@ -131,5 +182,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
2023-05-26 14:30 ` [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
@ 2023-05-26 14:42 ` Mehdi Djait
2023-05-26 17:13 ` Matti Vaittinen
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Mehdi Djait @ 2023-05-26 14:42 UTC (permalink / raw)
To: jic23, mazziesaccount
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree, 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.
Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
---
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 | 52 +++++++++
5 files changed, 222 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 1e4b9d4b4b8d..3fb673ffcdeb 100644
--- a/drivers/iio/accel/kionix-kx022a-spi.c
+++ b/drivers/iio/accel/kionix-kx022a-spi.c
@@ -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);
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
index 20f5965e878c..327412a8a395 100644
--- a/drivers/iio/accel/kionix-kx022a.c
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -150,6 +150,117 @@ static const struct regmap_config kx022a_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+/* Regmap configs kx132 */
+static const struct regmap_range kx132_volatile_ranges[] = {
+ {
+ .range_min = KX132_REG_XADP_L,
+ .range_max = KX132_REG_COTR,
+ }, {
+ .range_min = KX132_REG_TSCP,
+ .range_max = KX132_REG_INT_REL,
+ }, {
+ /* The reset bit will be cleared by sensor */
+ .range_min = KX132_REG_CNTL2,
+ .range_max = KX132_REG_CNTL2,
+ }, {
+ .range_min = KX132_REG_CNTL5,
+ .range_max = KX132_REG_CNTL5,
+ }, {
+ .range_min = KX132_REG_BUF_STATUS_1,
+ .range_max = KX132_REG_BUF_READ,
+ },
+};
+
+static const struct regmap_access_table kx132_volatile_regs = {
+ .yes_ranges = &kx132_volatile_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_volatile_ranges),
+};
+
+static const struct regmap_range kx132_precious_ranges[] = {
+ {
+ .range_min = KX132_REG_INT_REL,
+ .range_max = KX132_REG_INT_REL,
+ },
+};
+
+static const struct regmap_access_table kx132_precious_regs = {
+ .yes_ranges = &kx132_precious_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_precious_ranges),
+};
+
+static const struct regmap_range kx132_read_only_ranges[] = {
+ {
+ .range_min = KX132_REG_XADP_L,
+ .range_max = KX132_REG_INT_REL,
+ }, {
+ .range_min = KX132_REG_BUF_STATUS_1,
+ .range_max = KX132_REG_BUF_STATUS_2,
+ }, {
+ .range_min = KX132_REG_BUF_READ,
+ .range_max = KX132_REG_BUF_READ,
+ }, {
+ /* Kionix reserved registers: should not be written */
+ .range_min = 0x28,
+ .range_max = 0x28,
+ }, {
+ .range_min = 0x35,
+ .range_max = 0x36,
+ }, {
+ .range_min = 0x3c,
+ .range_max = 0x48,
+ }, {
+ .range_min = 0x4e,
+ .range_max = 0x5c,
+ }, {
+ .range_min = 0x77,
+ .range_max = 0x7f,
+ },
+};
+
+static const struct regmap_access_table kx132_ro_regs = {
+ .no_ranges = &kx132_read_only_ranges[0],
+ .n_no_ranges = ARRAY_SIZE(kx132_read_only_ranges),
+};
+
+static const struct regmap_range kx132_write_only_ranges[] = {
+ {
+ .range_min = KX132_REG_SELF_TEST,
+ .range_max = KX132_REG_SELF_TEST,
+ }, {
+ .range_min = KX132_REG_BUF_CLEAR,
+ .range_max = KX132_REG_BUF_CLEAR,
+ },
+};
+
+static const struct regmap_access_table kx132_wo_regs = {
+ .no_ranges = &kx132_write_only_ranges[0],
+ .n_no_ranges = ARRAY_SIZE(kx132_write_only_ranges),
+};
+
+static const struct regmap_range kx132_noinc_read_ranges[] = {
+ {
+ .range_min = KX132_REG_BUF_READ,
+ .range_max = KX132_REG_BUF_READ,
+ },
+};
+
+static const struct regmap_access_table kx132_nir_regs = {
+ .yes_ranges = &kx132_noinc_read_ranges[0],
+ .n_yes_ranges = ARRAY_SIZE(kx132_noinc_read_ranges),
+};
+
+static const struct regmap_config kx132_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &kx132_volatile_regs,
+ .rd_table = &kx132_wo_regs,
+ .wr_table = &kx132_ro_regs,
+ .rd_noinc_table = &kx132_nir_regs,
+ .precious_table = &kx132_precious_regs,
+ .max_register = KX132_MAX_REGISTER,
+ .cache_type = REGCACHE_RBTREE,
+};
+
struct kx022a_data {
const struct kx022a_chip_info *chip_info;
struct regmap *regmap;
@@ -239,6 +350,13 @@ static const struct iio_chan_spec kx022a_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(3),
};
+static const struct iio_chan_spec kx132_channels[] = {
+ KX022A_ACCEL_CHAN(X, KX132_REG_XOUT_L, 0),
+ KX022A_ACCEL_CHAN(Y, KX132_REG_YOUT_L, 1),
+ KX022A_ACCEL_CHAN(Z, KX132_REG_ZOUT_L, 2),
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
/*
* The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the
* Linux CPUs can handle without dropping samples. Also, the low power mode is
@@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
return fifo_bytes;
}
+static int kx132_get_fifo_bytes(struct kx022a_data *data)
+{
+ __le16 buf_status;
+ int ret, fifo_bytes;
+
+ ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
+ &buf_status, sizeof(buf_status));
+ if (ret) {
+ dev_err(data->dev, "Error reading buffer status\n");
+ return ret;
+ }
+
+ fifo_bytes = le16_to_cpu(buf_status);
+ fifo_bytes &= data->chip_info->buf_smp_lvl_mask;
+
+ return fifo_bytes;
+}
+
static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
bool irq)
{
@@ -1036,6 +1172,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 7792907534b5..351867a95f8c 100644
--- a/drivers/iio/accel/kionix-kx022a.h
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -74,6 +74,57 @@
#define KX022A_REG_SELF_TEST 0x60
#define KX022A_MAX_REGISTER 0x60
+#define KX132_REG_WHO 0x13
+#define KX132_ID 0x3d
+
+#define KX132_FIFO_LENGTH 86
+
+#define KX132_REG_CNTL 0x1b
+#define KX132_REG_CNTL2 0x1c
+#define KX132_REG_CNTL5 0x1f
+#define KX132_MASK_RES BIT(6)
+#define KX132_GSEL_2 0x0
+#define KX132_GSEL_4 BIT(3)
+#define KX132_GSEL_8 BIT(4)
+#define KX132_GSEL_16 GENMASK(4, 3)
+
+#define KX132_REG_INS2 0x17
+#define KX132_MASK_INS2_WMI BIT(5)
+
+#define KX132_REG_XADP_L 0x02
+#define KX132_REG_XOUT_L 0x08
+#define KX132_REG_YOUT_L 0x0a
+#define KX132_REG_ZOUT_L 0x0c
+#define KX132_REG_COTR 0x12
+#define KX132_REG_TSCP 0x14
+#define KX132_REG_INT_REL 0x1a
+
+#define KX132_REG_ODCNTL 0x21
+
+#define KX132_REG_BTS_WUF_TH 0x4a
+
+#define KX132_REG_BUF_CNTL1 0x5e
+#define KX132_REG_BUF_CNTL2 0x5f
+#define KX132_REG_BUF_STATUS_1 0x60
+#define KX132_REG_BUF_STATUS_2 0x61
+#define KX132_MASK_BUF_SMP_LVL GENMASK(9, 0)
+#define KX132_REG_BUF_CLEAR 0x62
+#define KX132_REG_BUF_READ 0x63
+#define KX132_ODR_SHIFT 3
+#define KX132_FIFO_MAX_WMI_TH 86
+
+#define KX132_REG_INC1 0x22
+#define KX132_REG_INC5 0x26
+#define KX132_REG_INC6 0x27
+#define KX132_IPOL_LOW 0
+#define KX132_IPOL_HIGH KX022A_MASK_IPOL
+#define KX132_ITYP_PULSE KX022A_MASK_ITYP
+
+#define KX132_REG_INC4 0x25
+
+#define KX132_REG_SELF_TEST 0x5d
+#define KX132_MAX_REGISTER 0x76
+
struct device;
struct kx022a_data;
@@ -131,5 +182,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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table
2023-05-26 14:30 ` [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table Mehdi Djait
@ 2023-05-26 16:35 ` Matti Vaittinen
0 siblings, 0 replies; 17+ messages in thread
From: Matti Vaittinen @ 2023-05-26 16:35 UTC (permalink / raw)
To: Mehdi Djait, jic23
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree
On 5/26/23 17:30, Mehdi Djait wrote:
> Add the missing i2c device id.
>
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> 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(+)
>
> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> index e6fd02d931b6..b5a85ce3a891 100644
> --- a/drivers/iio/accel/kionix-kx022a-i2c.c
> +++ b/drivers/iio/accel/kionix-kx022a-i2c.c
> @@ -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);
>
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
@ 2023-05-26 17:04 ` Matti Vaittinen
2023-05-28 16:49 ` Jonathan Cameron
2023-05-27 13:39 ` Andy Shevchenko
2023-05-28 16:55 ` Jonathan Cameron
2 siblings, 1 reply; 17+ messages in thread
From: Matti Vaittinen @ 2023-05-26 17:04 UTC (permalink / raw)
To: Mehdi Djait, jic23
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree
On 5/26/23 17:30, Mehdi Djait wrote:
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
>
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> 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 | 15 +++-
> drivers/iio/accel/kionix-kx022a-spi.c | 11 ++-
> drivers/iio/accel/kionix-kx022a.c | 118 +++++++++++++++++---------
> drivers/iio/accel/kionix-kx022a.h | 53 +++++++++++-
> 4 files changed, 145 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> index b5a85ce3a891..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,22 +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" },
> + { .name = "kx022a", .driver_data = (kernel_ulong_t)&kx022a_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, kx022a_i2c_id);
>
> static const struct of_device_id kx022a_of_match[] = {
> - { .compatible = "kionix,kx022a", },
> + { .compatible = "kionix,kx022a", .data = &kx022a_chip_info },
> { }
> };
> MODULE_DEVICE_TABLE(of, kx022a_of_match);
> diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
> index 9cd047f7b346..1e4b9d4b4b8d 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,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);
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 494e81ba1da9..69c22071c731 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,9 +149,9 @@ 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 {
> + const struct kx022a_chip_info *chip_info;
> struct regmap *regmap;
> struct iio_trigger *trig;
> struct device *dev;
> @@ -175,6 +175,8 @@ struct kx022a_data {
> struct mutex mutex;
> u8 watermark;
>
> + __le16 *fifo_buffer;
> +
> /* 3 x 16bit accel data + timestamp */
> __le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
> struct {
> @@ -208,7 +210,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
> { }
> };
>
> -#define KX022A_ACCEL_CHAN(axis, index) \
> +#define KX022A_ACCEL_CHAN(axis, reg, index) \
> { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -220,7 +222,7 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
> BIT(IIO_CHAN_INFO_SCALE) | \
> BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> .ext_info = kx022a_ext_info, \
> - .address = KX022A_REG_##axis##OUT_L, \
> + .address = reg, \
> .scan_index = index, \
> .scan_type = { \
> .sign = 's', \
> @@ -231,9 +233,9 @@ static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
> }
>
> static const struct iio_chan_spec kx022a_channels[] = {
> - KX022A_ACCEL_CHAN(X, 0),
> - KX022A_ACCEL_CHAN(Y, 1),
> - KX022A_ACCEL_CHAN(Z, 2),
> + KX022A_ACCEL_CHAN(X, KX022A_REG_XOUT_L, 0),
> + KX022A_ACCEL_CHAN(Y, KX022A_REG_YOUT_L, 1),
> + KX022A_ACCEL_CHAN(Z, KX022A_REG_ZOUT_L, 2),
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> @@ -332,10 +334,10 @@ static int kx022a_turn_on_off_unlocked(struct kx022a_data *data, bool on)
> int ret;
>
> if (on)
> - ret = regmap_set_bits(data->regmap, KX022A_REG_CNTL,
> + ret = regmap_set_bits(data->regmap, data->chip_info->cntl,
> KX022A_MASK_PC1);
> else
> - ret = regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
> + ret = regmap_clear_bits(data->regmap, data->chip_info->cntl,
> KX022A_MASK_PC1);
> if (ret)
> dev_err(data->dev, "Turn %s fail %d\n", str_on_off(on), ret);
> @@ -402,7 +404,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
> break;
>
> ret = regmap_update_bits(data->regmap,
> - KX022A_REG_ODCNTL,
> + data->chip_info->odcntl,
> KX022A_MASK_ODR, n);
> data->odr_ns = kx022a_odrs[n];
> kx022a_turn_on_unlock(data);
> @@ -423,7 +425,7 @@ static int kx022a_write_raw(struct iio_dev *idev,
> if (ret)
> break;
>
> - ret = regmap_update_bits(data->regmap, KX022A_REG_CNTL,
> + ret = regmap_update_bits(data->regmap, data->chip_info->cntl,
> KX022A_MASK_GSEL,
> n << KX022A_GSEL_SHIFT);
> kx022a_turn_on_unlock(data);
> @@ -445,7 +447,7 @@ static int kx022a_fifo_set_wmi(struct kx022a_data *data)
>
> threshold = data->watermark;
>
> - return regmap_update_bits(data->regmap, KX022A_REG_BUF_CNTL1,
> + return regmap_update_bits(data->regmap, data->chip_info->buf_cntl1,
> KX022A_MASK_WM_TH, threshold);
> }
>
> @@ -488,7 +490,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
> return ret;
>
> case IIO_CHAN_INFO_SAMP_FREQ:
> - ret = regmap_read(data->regmap, KX022A_REG_ODCNTL, ®val);
> + ret = regmap_read(data->regmap, data->chip_info->odcntl, ®val);
> if (ret)
> return ret;
>
> @@ -503,7 +505,7 @@ static int kx022a_read_raw(struct iio_dev *idev,
> return IIO_VAL_INT_PLUS_MICRO;
>
> case IIO_CHAN_INFO_SCALE:
> - ret = regmap_read(data->regmap, KX022A_REG_CNTL, ®val);
> + ret = regmap_read(data->regmap, data->chip_info->cntl, ®val);
> if (ret < 0)
> return ret;
>
> @@ -530,8 +532,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;
> @@ -592,7 +594,7 @@ static int kx022a_drop_fifo_contents(struct kx022a_data *data)
> */
> data->timestamp = 0;
>
> - return regmap_write(data->regmap, KX022A_REG_BUF_CLEAR, 0x0);
> + return regmap_write(data->regmap, data->chip_info->buf_clear, 0x0);
> }
>
> static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> @@ -600,7 +602,6 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> {
> struct kx022a_data *data = iio_priv(idev);
> struct device *dev = regmap_get_device(data->regmap);
> - __le16 buffer[KX022A_FIFO_LENGTH * 3];
> uint64_t sample_period;
> int count, fifo_bytes;
> bool renable = false;
> @@ -679,13 +680,13 @@ static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> }
>
> fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
> - ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
> - &buffer[0], fifo_bytes);
> + ret = regmap_noinc_read(data->regmap, data->chip_info->buf_read,
> + &data->fifo_buffer[0], fifo_bytes);
> if (ret)
> goto renable_out;
>
> for (i = 0; i < count; i++) {
> - __le16 *sam = &buffer[i * 3];
> + __le16 *sam = &data->fifo_buffer[i * 3];
> __le16 *chs;
> int bit;
>
> @@ -732,10 +733,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);
> }
>
> @@ -762,6 +763,8 @@ static int kx022a_fifo_disable(struct kx022a_data *data)
> {
> int ret = 0;
>
> + kfree(data->fifo_buffer);
Should we have the kfree only after the sensor is disabled? I wonder if
we in theory have here a time window where the buffer is freed but the
measurement is still running and - with a lots of bad luck - can result
measurement being written to a freed buffer? Perhaps move the kfree to
be done only after the measurement has been stopped?
Other than that, this is looking good to me.
> +
> ret = kx022a_turn_off_lock(data);
> if (ret)
> return ret;
> @@ -770,7 +773,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;
> @@ -801,6 +804,12 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
> {
> int ret;
>
> + data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
> + KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
> +
> + if (!data->fifo_buffer)
> + return -ENOMEM;
> +
> ret = kx022a_turn_off_lock(data);
> if (ret)
> return ret;
> @@ -811,7 +820,7 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
> goto unlock_out;
>
> /* Enable buffer */
> - ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
> + ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
> KX022A_MASK_BUF_EN);
> if (ret)
> goto unlock_out;
> @@ -857,7 +866,7 @@ static irqreturn_t kx022a_trigger_handler(int irq, void *p)
> struct kx022a_data *data = iio_priv(idev);
> int ret;
>
> - ret = regmap_bulk_read(data->regmap, KX022A_REG_XOUT_L, data->buffer,
> + ret = regmap_bulk_read(data->regmap, data->chip_info->xout_l, data->buffer,
> KX022A_FIFO_SAMPLES_SIZE_BYTES);
> if (ret < 0)
> goto err_read;
> @@ -905,7 +914,7 @@ static irqreturn_t kx022a_irq_thread_handler(int irq, void *private)
> if (data->state & KX022A_STATE_FIFO) {
> int ok;
>
> - ok = __kx022a_fifo_flush(idev, KX022A_FIFO_LENGTH, true);
> + ok = __kx022a_fifo_flush(idev, data->chip_info->fifo_length, true);
> if (ok > 0)
> ret = IRQ_HANDLED;
> }
> @@ -958,7 +967,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
> int ret, val;
>
> /* Reset the senor */
> - ret = regmap_write(data->regmap, KX022A_REG_CNTL2, KX022A_MASK_SRST);
> + ret = regmap_write(data->regmap, data->chip_info->cntl2, KX022A_MASK_SRST);
> if (ret)
> return ret;
>
> @@ -968,7 +977,7 @@ static int kx022a_chip_init(struct kx022a_data *data)
> */
> msleep(1);
>
> - ret = regmap_read_poll_timeout(data->regmap, KX022A_REG_CNTL2, val,
> + ret = regmap_read_poll_timeout(data->regmap, data->chip_info->cntl2, val,
> !(val & KX022A_MASK_SRST),
> KX022A_SOFT_RESET_WAIT_TIME_US,
> KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US);
> @@ -978,14 +987,14 @@ static int kx022a_chip_init(struct kx022a_data *data)
> return ret;
> }
>
> - ret = regmap_reinit_cache(data->regmap, &kx022a_regmap);
> + ret = regmap_reinit_cache(data->regmap, data->chip_info->regmap_config);
> if (ret) {
> dev_err(data->dev, "Failed to reinit reg cache\n");
> return ret;
> }
>
> /* set data res 16bit */
> - ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
> + ret = regmap_set_bits(data->regmap, data->chip_info->buf_cntl2,
> KX022A_MASK_BRES16);
> if (ret) {
> dev_err(data->dev, "Failed to set data resolution\n");
> @@ -995,7 +1004,31 @@ static int kx022a_chip_init(struct kx022a_data *data)
> return kx022a_prepare_irq_pin(data);
> }
>
> -int kx022a_probe_internal(struct device *dev)
> +const struct kx022a_chip_info kx022a_chip_info = {
> + .name = "kx022-accel",
> + .regmap_config = &kx022a_regmap_config,
> + .channels = kx022a_channels,
> + .num_channels = ARRAY_SIZE(kx022a_channels),
> + .fifo_length = KX022A_FIFO_LENGTH,
> + .who = KX022A_REG_WHO,
> + .id = KX022A_ID,
> + .cntl = KX022A_REG_CNTL,
> + .cntl2 = KX022A_REG_CNTL2,
> + .odcntl = KX022A_REG_ODCNTL,
> + .buf_cntl1 = KX022A_REG_BUF_CNTL1,
> + .buf_cntl2 = KX022A_REG_BUF_CNTL2,
> + .buf_clear = KX022A_REG_BUF_CLEAR,
> + .buf_status1 = KX022A_REG_BUF_STATUS_1,
> + .buf_read = KX022A_REG_BUF_READ,
> + .inc1 = KX022A_REG_INC1,
> + .inc4 = KX022A_REG_INC4,
> + .inc5 = KX022A_REG_INC5,
> + .inc6 = KX022A_REG_INC6,
> + .xout_l = KX022A_REG_XOUT_L,
> +};
> +EXPORT_SYMBOL_NS_GPL(kx022a_chip_info, IIO_KX022A);
> +
> +int kx022a_probe_internal(struct device *dev, const struct kx022a_chip_info *chip_info)
> {
> static const char * const regulator_names[] = {"io-vdd", "vdd"};
> struct iio_trigger *indio_trig;
> @@ -1022,6 +1055,7 @@ int kx022a_probe_internal(struct device *dev)
> return -ENOMEM;
>
> data = iio_priv(idev);
> + data->chip_info = chip_info;
>
> /*
> * VDD is the analog and digital domain voltage supply and
> @@ -1032,24 +1066,24 @@ int kx022a_probe_internal(struct device *dev)
> if (ret && ret != -ENODEV)
> return dev_err_probe(dev, ret, "failed to enable regulator\n");
>
> - ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
> + ret = regmap_read(regmap, chip_info->who, &chip_id);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to access sensor\n");
>
> - if (chip_id != KX022A_ID)
> + if (chip_id != chip_info->id)
> dev_warn(dev, "unknown device 0x%x\n", chip_id);
>
> irq = fwnode_irq_get_byname(fwnode, "INT1");
> if (irq > 0) {
> - data->inc_reg = KX022A_REG_INC1;
> - data->ien_reg = KX022A_REG_INC4;
> + data->inc_reg = chip_info->inc1;
> + data->ien_reg = chip_info->inc4;
> } else {
> irq = fwnode_irq_get_byname(fwnode, "INT2");
> if (irq <= 0)
> return dev_err_probe(dev, irq, "No suitable IRQ\n");
>
> - data->inc_reg = KX022A_REG_INC5;
> - data->ien_reg = KX022A_REG_INC6;
> + data->inc_reg = chip_info->inc5;
> + data->ien_reg = chip_info->inc6;
> }
>
> data->regmap = regmap;
> @@ -1058,9 +1092,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..c23b6f03409e 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -76,7 +76,56 @@
>
> struct device;
>
> -int kx022a_probe_internal(struct device *dev);
> -extern const struct regmap_config kx022a_regmap;
> +/**
> + * struct kx022a_chip_info - Kionix accelerometer chip specific information
> + *
> + * @name: name of the device
> + * @regmap_config: pointer to register map configuration
> + * @channels: pointer to iio_chan_spec array
> + * @num_channels: number of iio_chan_spec channels
> + * @fifo_length: number of 16-bit samples in a full buffer
> + * @who: WHO_AM_I register
> + * @id: WHO_AM_I register value
> + * @cntl: control register 1
> + * @cntl2: control register 2
> + * @odcntl: output data control register
> + * @buf_cntl1: buffer control register 1
> + * @buf_cntl2: buffer control register 2
> + * @buf_clear: buffer clear register
> + * @buf_status1: buffer status register 1
> + * @buf_read: buffer read register
> + * @inc1: interrupt control register 1
> + * @inc4: interrupt control register 4
> + * @inc5: interrupt control register 5
> + * @inc6: interrupt control register 6
> + * @xout_l: x-axis output least significant byte
> + */
> +struct kx022a_chip_info {
> + const char *name;
> + const struct regmap_config *regmap_config;
> + const struct iio_chan_spec *channels;
> + unsigned int num_channels;
> + unsigned int fifo_length;
> + u8 who;
> + u8 id;
> + u8 cntl;
> + u8 cntl2;
> + u8 odcntl;
> + u8 buf_cntl1;
> + u8 buf_cntl2;
> + u8 buf_clear;
> + u8 buf_status1;
> + 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
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
2023-05-26 14:30 ` [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:42 ` Mehdi Djait
@ 2023-05-26 17:13 ` Matti Vaittinen
2023-05-27 8:24 ` Andy Shevchenko
2023-05-28 16:59 ` Jonathan Cameron
3 siblings, 0 replies; 17+ messages in thread
From: Matti Vaittinen @ 2023-05-26 17:13 UTC (permalink / raw)
To: Mehdi Djait, jic23
Cc: krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt, lars,
linux-iio, linux-kernel, devicetree
On 5/26/23 17:30, Mehdi Djait wrote:
> Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support
> ranges from ±2G to ±16G, digital output through I²C/SPI.
> Add support for basic accelerometer features such as reading acceleration
> via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ.
>
> Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
Acked-by: Matti Vaittinen <mazziesaccount@gmail.com>
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
2023-05-26 14:30 ` [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:42 ` Mehdi Djait
2023-05-26 17:13 ` Matti Vaittinen
@ 2023-05-27 8:24 ` Andy Shevchenko
2023-05-28 16:59 ` Jonathan Cameron
3 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2023-05-27 8:24 UTC (permalink / raw)
To: Mehdi Djait
Cc: jic23, mazziesaccount, krzysztof.kozlowski+dt, robh+dt, lars,
linux-iio, linux-kernel, devicetree
On Fri, May 26, 2023 at 04:42:33PM +0200, Mehdi Djait wrote:
> Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support
> ranges from ±2G to ±16G, digital output through I²C/SPI.
> Add support for basic accelerometer features such as reading acceleration
> via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ.
Well written patch, thank you!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
> ---
> 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 | 52 +++++++++
> 5 files changed, 222 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 1e4b9d4b4b8d..3fb673ffcdeb 100644
> --- a/drivers/iio/accel/kionix-kx022a-spi.c
> +++ b/drivers/iio/accel/kionix-kx022a-spi.c
> @@ -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);
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 20f5965e878c..327412a8a395 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -150,6 +150,117 @@ static const struct regmap_config kx022a_regmap_config = {
> .cache_type = REGCACHE_RBTREE,
> };
>
> +/* Regmap configs kx132 */
> +static const struct regmap_range kx132_volatile_ranges[] = {
> + {
> + .range_min = KX132_REG_XADP_L,
> + .range_max = KX132_REG_COTR,
> + }, {
> + .range_min = KX132_REG_TSCP,
> + .range_max = KX132_REG_INT_REL,
> + }, {
> + /* The reset bit will be cleared by sensor */
> + .range_min = KX132_REG_CNTL2,
> + .range_max = KX132_REG_CNTL2,
> + }, {
> + .range_min = KX132_REG_CNTL5,
> + .range_max = KX132_REG_CNTL5,
> + }, {
> + .range_min = KX132_REG_BUF_STATUS_1,
> + .range_max = KX132_REG_BUF_READ,
> + },
> +};
> +
> +static const struct regmap_access_table kx132_volatile_regs = {
> + .yes_ranges = &kx132_volatile_ranges[0],
> + .n_yes_ranges = ARRAY_SIZE(kx132_volatile_ranges),
> +};
> +
> +static const struct regmap_range kx132_precious_ranges[] = {
> + {
> + .range_min = KX132_REG_INT_REL,
> + .range_max = KX132_REG_INT_REL,
> + },
> +};
> +
> +static const struct regmap_access_table kx132_precious_regs = {
> + .yes_ranges = &kx132_precious_ranges[0],
> + .n_yes_ranges = ARRAY_SIZE(kx132_precious_ranges),
> +};
> +
> +static const struct regmap_range kx132_read_only_ranges[] = {
> + {
> + .range_min = KX132_REG_XADP_L,
> + .range_max = KX132_REG_INT_REL,
> + }, {
> + .range_min = KX132_REG_BUF_STATUS_1,
> + .range_max = KX132_REG_BUF_STATUS_2,
> + }, {
> + .range_min = KX132_REG_BUF_READ,
> + .range_max = KX132_REG_BUF_READ,
> + }, {
> + /* Kionix reserved registers: should not be written */
> + .range_min = 0x28,
> + .range_max = 0x28,
> + }, {
> + .range_min = 0x35,
> + .range_max = 0x36,
> + }, {
> + .range_min = 0x3c,
> + .range_max = 0x48,
> + }, {
> + .range_min = 0x4e,
> + .range_max = 0x5c,
> + }, {
> + .range_min = 0x77,
> + .range_max = 0x7f,
> + },
> +};
> +
> +static const struct regmap_access_table kx132_ro_regs = {
> + .no_ranges = &kx132_read_only_ranges[0],
> + .n_no_ranges = ARRAY_SIZE(kx132_read_only_ranges),
> +};
> +
> +static const struct regmap_range kx132_write_only_ranges[] = {
> + {
> + .range_min = KX132_REG_SELF_TEST,
> + .range_max = KX132_REG_SELF_TEST,
> + }, {
> + .range_min = KX132_REG_BUF_CLEAR,
> + .range_max = KX132_REG_BUF_CLEAR,
> + },
> +};
> +
> +static const struct regmap_access_table kx132_wo_regs = {
> + .no_ranges = &kx132_write_only_ranges[0],
> + .n_no_ranges = ARRAY_SIZE(kx132_write_only_ranges),
> +};
> +
> +static const struct regmap_range kx132_noinc_read_ranges[] = {
> + {
> + .range_min = KX132_REG_BUF_READ,
> + .range_max = KX132_REG_BUF_READ,
> + },
> +};
> +
> +static const struct regmap_access_table kx132_nir_regs = {
> + .yes_ranges = &kx132_noinc_read_ranges[0],
> + .n_yes_ranges = ARRAY_SIZE(kx132_noinc_read_ranges),
> +};
> +
> +static const struct regmap_config kx132_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = &kx132_volatile_regs,
> + .rd_table = &kx132_wo_regs,
> + .wr_table = &kx132_ro_regs,
> + .rd_noinc_table = &kx132_nir_regs,
> + .precious_table = &kx132_precious_regs,
> + .max_register = KX132_MAX_REGISTER,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> struct kx022a_data {
> const struct kx022a_chip_info *chip_info;
> struct regmap *regmap;
> @@ -239,6 +350,13 @@ static const struct iio_chan_spec kx022a_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(3),
> };
>
> +static const struct iio_chan_spec kx132_channels[] = {
> + KX022A_ACCEL_CHAN(X, KX132_REG_XOUT_L, 0),
> + KX022A_ACCEL_CHAN(Y, KX132_REG_YOUT_L, 1),
> + KX022A_ACCEL_CHAN(Z, KX132_REG_ZOUT_L, 2),
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> /*
> * The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the
> * Linux CPUs can handle without dropping samples. Also, the low power mode is
> @@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
> return fifo_bytes;
> }
>
> +static int kx132_get_fifo_bytes(struct kx022a_data *data)
> +{
> + __le16 buf_status;
> + int ret, fifo_bytes;
> +
> + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
> + &buf_status, sizeof(buf_status));
> + if (ret) {
> + dev_err(data->dev, "Error reading buffer status\n");
> + return ret;
> + }
> +
> + fifo_bytes = le16_to_cpu(buf_status);
> + fifo_bytes &= data->chip_info->buf_smp_lvl_mask;
> +
> + return fifo_bytes;
> +}
> +
> static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> bool irq)
> {
> @@ -1036,6 +1172,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 7792907534b5..351867a95f8c 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -74,6 +74,57 @@
> #define KX022A_REG_SELF_TEST 0x60
> #define KX022A_MAX_REGISTER 0x60
>
> +#define KX132_REG_WHO 0x13
> +#define KX132_ID 0x3d
> +
> +#define KX132_FIFO_LENGTH 86
> +
> +#define KX132_REG_CNTL 0x1b
> +#define KX132_REG_CNTL2 0x1c
> +#define KX132_REG_CNTL5 0x1f
> +#define KX132_MASK_RES BIT(6)
> +#define KX132_GSEL_2 0x0
> +#define KX132_GSEL_4 BIT(3)
> +#define KX132_GSEL_8 BIT(4)
> +#define KX132_GSEL_16 GENMASK(4, 3)
> +
> +#define KX132_REG_INS2 0x17
> +#define KX132_MASK_INS2_WMI BIT(5)
> +
> +#define KX132_REG_XADP_L 0x02
> +#define KX132_REG_XOUT_L 0x08
> +#define KX132_REG_YOUT_L 0x0a
> +#define KX132_REG_ZOUT_L 0x0c
> +#define KX132_REG_COTR 0x12
> +#define KX132_REG_TSCP 0x14
> +#define KX132_REG_INT_REL 0x1a
> +
> +#define KX132_REG_ODCNTL 0x21
> +
> +#define KX132_REG_BTS_WUF_TH 0x4a
> +
> +#define KX132_REG_BUF_CNTL1 0x5e
> +#define KX132_REG_BUF_CNTL2 0x5f
> +#define KX132_REG_BUF_STATUS_1 0x60
> +#define KX132_REG_BUF_STATUS_2 0x61
> +#define KX132_MASK_BUF_SMP_LVL GENMASK(9, 0)
> +#define KX132_REG_BUF_CLEAR 0x62
> +#define KX132_REG_BUF_READ 0x63
> +#define KX132_ODR_SHIFT 3
> +#define KX132_FIFO_MAX_WMI_TH 86
> +
> +#define KX132_REG_INC1 0x22
> +#define KX132_REG_INC5 0x26
> +#define KX132_REG_INC6 0x27
> +#define KX132_IPOL_LOW 0
> +#define KX132_IPOL_HIGH KX022A_MASK_IPOL
> +#define KX132_ITYP_PULSE KX022A_MASK_ITYP
> +
> +#define KX132_REG_INC4 0x25
> +
> +#define KX132_REG_SELF_TEST 0x5d
> +#define KX132_MAX_REGISTER 0x76
> +
> struct device;
>
> struct kx022a_data;
> @@ -131,5 +182,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
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
2023-05-26 17:04 ` Matti Vaittinen
@ 2023-05-27 13:39 ` Andy Shevchenko
2023-05-28 16:55 ` Jonathan Cameron
2 siblings, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2023-05-27 13:39 UTC (permalink / raw)
To: Mehdi Djait
Cc: jic23, mazziesaccount, krzysztof.kozlowski+dt, robh+dt, lars,
linux-iio, linux-kernel, devicetree
On Fri, May 26, 2023 at 04:30:46PM +0200, Mehdi Djait wrote:
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
...
> - 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;
> + }
And if still no chip_info available?..
> + 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");
...
> - if (val > KX022A_FIFO_LENGTH)
> - val = KX022A_FIFO_LENGTH;
> + if (val > data->chip_info->fifo_length)
> + val = data->chip_info->fifo_length;
min()/min_t() ?
...
> + ret = regmap_noinc_read(data->regmap, data->chip_info->buf_read,
> + &data->fifo_buffer[0], fifo_bytes);
data->fifo_buffer will suffice.
> if (ret)
> goto renable_out;
...
> + data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
> + KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
> +
Redundant blank line.
> + if (!data->fifo_buffer)
> + return -ENOMEM;
...
> +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;
Here is the gap since it's not a packed structure. Can we avoid it?
> + u16 buf_smp_lvl_mask;
> + u8 buf_read;
> + u8 inc1;
> + u8 inc4;
> + u8 inc5;
> + u8 inc6;
> + u8 xout_l;
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
2023-05-26 17:04 ` Matti Vaittinen
@ 2023-05-28 16:49 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-05-28 16:49 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Mehdi Djait, krzysztof.kozlowski+dt, andriy.shevchenko, robh+dt,
lars, linux-iio, linux-kernel, devicetree
...
> > @@ -762,6 +763,8 @@ static int kx022a_fifo_disable(struct kx022a_data *data)
> > {
> > int ret = 0;
> >
> > + kfree(data->fifo_buffer);
>
> Should we have the kfree only after the sensor is disabled? I wonder if
> we in theory have here a time window where the buffer is freed but the
> measurement is still running and - with a lots of bad luck - can result
> measurement being written to a freed buffer? Perhaps move the kfree to
> be done only after the measurement has been stopped?
>
> Other than that, this is looking good to me.
>
Agreed. Even if it's not a bug as such, it is better to keep the order
of this function as close as possible to the reverse of what happens in
*_fifo_enabled() as easier to reason about if that's the case.
> > +
> > ret = kx022a_turn_off_lock(data);
> > if (ret)
> > return ret;
> > @@ -770,7 +773,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;
> > @@ -801,6 +804,12 @@ static int kx022a_fifo_enable(struct kx022a_data *data)
> > {
> > int ret;
> >
> > + data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
> > + KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
> > +
> > + if (!data->fifo_buffer)
> > + return -ENOMEM;
> > +
> > ret = kx022a_turn_off_lock(data);
> > if (ret)
> > return ret;
> > @@ -811,7 +820,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;
...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
2023-05-26 17:04 ` Matti Vaittinen
2023-05-27 13:39 ` Andy Shevchenko
@ 2023-05-28 16:55 ` Jonathan Cameron
2 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-05-28 16:55 UTC (permalink / raw)
To: Mehdi Djait
Cc: mazziesaccount, krzysztof.kozlowski+dt, andriy.shevchenko,
robh+dt, lars, linux-iio, linux-kernel, devicetree
On Fri, 26 May 2023 16:30:46 +0200
Mehdi Djait <mehdi.djait.k@gmail.com> wrote:
> Add the chip_info structure to the driver's private data to hold all
> the device specific infos.
> Refactor the kx022a driver implementation to make it more generic and
> extensible.
>
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
Hi Mehdi,
Just one trivial comment from me about missing docs / unused element.
Thanks,
Jonathan
...
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> index 12424649d438..c23b6f03409e 100644
> --- a/drivers/iio/accel/kionix-kx022a.h
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -76,7 +76,56 @@
>
> 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
Missing entry for buf_smp_lvl_mask.
Mind you it's also not used in this patch and only seems to be used in the
get_fifo_bytes() function in patch 7, so might not be needed at all?
If it is, then introduce it in patch 7 where we can see how it is used.
> + * @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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer
2023-05-26 14:30 ` [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
` (2 preceding siblings ...)
2023-05-27 8:24 ` Andy Shevchenko
@ 2023-05-28 16:59 ` Jonathan Cameron
3 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-05-28 16:59 UTC (permalink / raw)
To: Mehdi Djait
Cc: mazziesaccount, krzysztof.kozlowski+dt, andriy.shevchenko,
robh+dt, lars, linux-iio, linux-kernel, devicetree
On Fri, 26 May 2023 16:30:48 +0200
Mehdi Djait <mehdi.djait.k@gmail.com> wrote:
> Kionix KX132-1211 is a tri-axis 16-bit accelerometer that can support
> ranges from ±2G to ±16G, digital output through I²C/SPI.
> Add support for basic accelerometer features such as reading acceleration
> via IIO using raw reads, triggered buffer (data-ready), or the WMI IRQ.
>
> Datasheet: https://kionixfs.azureedge.net/en/document/KX132-1211-Technical-Reference-Manual-Rev-5.0.pdf
> Signed-off-by: Mehdi Djait <mehdi.djait.k@gmail.com>
Hi Mehdi,
Comment is a follow on from the earlier one about buf_smp_lvl_mask.
Jonathan
> /*
> * The sensor HW can support ODR up to 1600 Hz, which is beyond what most of the
> * Linux CPUs can handle without dropping samples. Also, the low power mode is
> @@ -613,6 +731,24 @@ static int kx022a_get_fifo_bytes(struct kx022a_data *data)
> return fifo_bytes;
> }
>
> +static int kx132_get_fifo_bytes(struct kx022a_data *data)
> +{
> + __le16 buf_status;
> + int ret, fifo_bytes;
> +
> + ret = regmap_bulk_read(data->regmap, data->chip_info->buf_status1,
> + &buf_status, sizeof(buf_status));
> + if (ret) {
> + dev_err(data->dev, "Error reading buffer status\n");
> + return ret;
> + }
> +
> + fifo_bytes = le16_to_cpu(buf_status);
> + fifo_bytes &= data->chip_info->buf_smp_lvl_mask;
See below.
> +
> + return fifo_bytes;
> +}
> +
> static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> bool irq)
> {
> @@ -1036,6 +1172,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,
As per earlier comment I think this is only set for this device...
> + .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,
and only used in this device specific callback.
Probably can just use it directly in the callback.
> +};
> +EXPORT_SYMBOL_NS_GPL(kx132_chip_info, IIO_KX022A);
> +
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-05-28 16:42 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-26 14:30 [PATCH v4 0/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 1/7] dt-bindings: iio: Add " Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 2/7] iio: accel: kionix-kx022a: Remove blank lines Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 3/7] iio: accel: kionix-kx022a: Warn on failed matches and assume compatibility Mehdi Djait
2023-05-26 14:30 ` [PATCH v4 4/7] iio: accel: kionix-kx022a: Add an i2c_device_id table Mehdi Djait
2023-05-26 16:35 ` Matti Vaittinen
2023-05-26 14:30 ` [PATCH v4 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure Mehdi Djait
2023-05-26 17:04 ` Matti Vaittinen
2023-05-28 16:49 ` Jonathan Cameron
2023-05-27 13:39 ` Andy Shevchenko
2023-05-28 16:55 ` Jonathan Cameron
2023-05-26 14:30 ` [PATCH v4 6/7] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer Mehdi Djait
[not found] <cover.1685111274.git.mehdi.djait.k@gmail.com>
2023-05-26 14:30 ` [PATCH v4 7/7] iio: accel: Add support for Kionix/ROHM KX132-1211 accelerometer Mehdi Djait
2023-05-26 14:42 ` Mehdi Djait
2023-05-26 17:13 ` Matti Vaittinen
2023-05-27 8:24 ` Andy Shevchenko
2023-05-28 16:59 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).