[v4,2/3] iio: accel: Support Kionix/ROHM KX022A accelerometer

Commit Message

Matti Vaittinen Oct. 21, 2022, 11:22 a.m. UTC

KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
tap/motion detection, wake-up & back-to-sleep events, four acceleration
ranges (2, 4, 8 and 16g), and probably some other cool features.

Add support for the basic accelerometer features such as getting the
acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
using the WMI IRQ).

Important things to be added include the double-tap, motion
detection and wake-up as well as the runtime power management.

Signed-off-by: Matti Vaittinen <[e-mail]>

---
v3 => v4 fixes suggested by Andy:
- styling changes
- use str_on_off()
- drop check for !dev
- drop adding return value to print printed using dev_err_probe()
- use dev_err_probe() in SPI/I2C drivers too
- fix IRQ thread return value

v2 => v3 Mostly fixes suggested by Andy
- styling issues
- spell-checks
- use namespaces for exported symbols
- drop module param
- document the lock
- change value written when clearing fifo
- drop WARN_ON()
- correctly check the return value from fwnode_irq_get_byname()
- do not emphasize config Y over M
- reorder struct kx022a_data from potential optimization
- IIO_DEVICE_ATTR_RO instead of IIO_DEVICE_ATTR where applicable
- directly include bits.h for BIT()
- use sysfs_emit() for sysfs
- use unique name for IRQ
- convert read_raw() values to CPU endianess
- fix HW-fifo size to 258 bytes
- kx022a-spi, Fix kconfig dependency
- disable irq (to protect timestamp / sample amount calculation) when the
  fifo flush is iniriated by user-space

RFCv1 => v2 (mostly based on feedback from Jonathan):
- Fix bunch of typos from the commit message.
- Add missing break; to the kx022a_write_raw()
- Fix SPI driver to use of_match_table
- Fix indentiation in I2C driver
- Drop struct kx022a_trigger
- Drop cross references from Kconfig
- Use /* */ also in file header comments
- Misc minor styling
- Do sensor-reset at probe
- Support both IRQ pins
- Implement read_avail callback
- Use dma aligned buffers for bulk-reads
- Use iio_trigger_poll_chained()
- Use devm consistently
- Drop inclusion of device.h
- Add SPI device ID for module loading
- Add module param for hw fifo / watermark IRQ usage
- Fix io-vdd-supply name to match one in the bindings
---
 drivers/iio/accel/Kconfig             |   21 +
 drivers/iio/accel/Makefile            |    3 +
 drivers/iio/accel/kionix-kx022a-i2c.c |   51 ++
 drivers/iio/accel/kionix-kx022a-spi.c |   58 ++
 drivers/iio/accel/kionix-kx022a.c     | 1145 +++++++++++++++++++++++++
 drivers/iio/accel/kionix-kx022a.h     |   82 ++
 6 files changed, 1360 insertions(+)
 create mode 100644 drivers/iio/accel/kionix-kx022a-i2c.c
 create mode 100644 drivers/iio/accel/kionix-kx022a-spi.c
 create mode 100644 drivers/iio/accel/kionix-kx022a.c
 create mode 100644 drivers/iio/accel/kionix-kx022a.h

Comments

Andy Shevchenko Oct. 21, 2022, 12:41 p.m. UTC | #1

On Fri, Oct 21, 2022 at 02:22:49PM +0300, Matti Vaittinen wrote:
> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
> tap/motion detection, wake-up & back-to-sleep events, four acceleration
> ranges (2, 4, 8 and 16g), and probably some other cool features.
> 
> Add support for the basic accelerometer features such as getting the
> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
> using the WMI IRQ).
> 
> Important things to be added include the double-tap, motion
> detection and wake-up as well as the runtime power management.

While I have some disagreements on some code
pieces, this version is okay to go I think.

Reviewed-by: Andy Shevchenko <[e-mail]>

Below a few nit-picks in case it needs to be a v5.

> Signed-off-by: Matti Vaittinen <[e-mail]>
> 
> ---
> v3 => v4 fixes suggested by Andy:
> - styling changes
> - use str_on_off()
> - drop check for !dev
> - drop adding return value to print printed using dev_err_probe()
> - use dev_err_probe() in SPI/I2C drivers too
> - fix IRQ thread return value
> 
> v2 => v3 Mostly fixes suggested by Andy
> - styling issues
> - spell-checks
> - use namespaces for exported symbols
> - drop module param
> - document the lock
> - change value written when clearing fifo
> - drop WARN_ON()
> - correctly check the return value from fwnode_irq_get_byname()
> - do not emphasize config Y over M
> - reorder struct kx022a_data from potential optimization
> - IIO_DEVICE_ATTR_RO instead of IIO_DEVICE_ATTR where applicable
> - directly include bits.h for BIT()
> - use sysfs_emit() for sysfs
> - use unique name for IRQ
> - convert read_raw() values to CPU endianess
> - fix HW-fifo size to 258 bytes
> - kx022a-spi, Fix kconfig dependency
> - disable irq (to protect timestamp / sample amount calculation) when the
>   fifo flush is iniriated by user-space
> 
> RFCv1 => v2 (mostly based on feedback from Jonathan):
> - Fix bunch of typos from the commit message.
> - Add missing break; to the kx022a_write_raw()
> - Fix SPI driver to use of_match_table
> - Fix indentiation in I2C driver
> - Drop struct kx022a_trigger
> - Drop cross references from Kconfig
> - Use /* */ also in file header comments
> - Misc minor styling
> - Do sensor-reset at probe
> - Support both IRQ pins
> - Implement read_avail callback
> - Use dma aligned buffers for bulk-reads
> - Use iio_trigger_poll_chained()
> - Use devm consistently
> - Drop inclusion of device.h
> - Add SPI device ID for module loading
> - Add module param for hw fifo / watermark IRQ usage
> - Fix io-vdd-supply name to match one in the bindings
> ---
>  drivers/iio/accel/Kconfig             |   21 +
>  drivers/iio/accel/Makefile            |    3 +
>  drivers/iio/accel/kionix-kx022a-i2c.c |   51 ++
>  drivers/iio/accel/kionix-kx022a-spi.c |   58 ++
>  drivers/iio/accel/kionix-kx022a.c     | 1145 +++++++++++++++++++++++++
>  drivers/iio/accel/kionix-kx022a.h     |   82 ++
>  6 files changed, 1360 insertions(+)
>  create mode 100644 drivers/iio/accel/kionix-kx022a-i2c.c
>  create mode 100644 drivers/iio/accel/kionix-kx022a-spi.c
>  create mode 100644 drivers/iio/accel/kionix-kx022a.c
>  create mode 100644 drivers/iio/accel/kionix-kx022a.h
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index ffac66db7ac9..b7fd054819d2 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -409,6 +409,27 @@ config IIO_ST_ACCEL_SPI_3AXIS
>  	  To compile this driver as a module, choose M here. The module
>  	  will be called st_accel_spi.
>  
> +config IIO_KX022A
> +	tristate
> +
> +config IIO_KX022A_SPI
> +	tristate "Kionix KX022A tri-axis digital accelerometer"
> +	depends on SPI
> +	select IIO_KX022A
> +	select REGMAP_SPI
> +	help
> +	  Enable support for the Kionix KX022A digital tri-axis
> +	  accelerometer connected to I2C interface.
> +
> +config IIO_KX022A_I2C
> +	tristate "Kionix KX022A tri-axis digital accelerometer"
> +	depends on I2C
> +	select IIO_KX022A
> +	select REGMAP_I2C
> +	help
> +	  Enable support for the Kionix KX022A digital tri-axis
> +	  accelerometer connected to I2C interface.
> +
>  config KXSD9
>  	tristate "Kionix KXSD9 Accelerometer Driver"
>  	select IIO_BUFFER
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 5e45b5fa5ab5..311ead9c3ef1 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -40,6 +40,9 @@ obj-$(CONFIG_FXLS8962AF)	+= fxls8962af-core.o
>  obj-$(CONFIG_FXLS8962AF_I2C)	+= fxls8962af-i2c.o
>  obj-$(CONFIG_FXLS8962AF_SPI)	+= fxls8962af-spi.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
> +obj-$(CONFIG_IIO_KX022A)	+= kionix-kx022a.o
> +obj-$(CONFIG_IIO_KX022A_I2C)	+= kionix-kx022a-i2c.o
> +obj-$(CONFIG_IIO_KX022A_SPI)	+= kionix-kx022a-spi.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
>  obj-$(CONFIG_KXSD9_SPI)	+= kxsd9-spi.o
> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> new file mode 100644
> index 000000000000..6510f8d62b85
> --- /dev/null
> +++ b/drivers/iio/accel/kionix-kx022a-i2c.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 ROHM Semiconductors
> + *
> + * ROHM/KIONIX KX022A accelerometer driver
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include "kionix-kx022a.h"
> +
> +static int kx022a_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct device *dev = &i2c->dev;
> +	struct regmap *regmap;
> +
> +	if (!i2c->irq) {
> +		dev_err(dev, "No IRQ configured\n");
> +		return -EINVAL;
> +	}
> +
> +	regmap = devm_regmap_init_i2c(i2c, &kx022a_regmap);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "Failed to initialize Regmap\n");
> +
> +	return kx022a_probe_internal(dev);
> +}
> +
> +static const struct of_device_id kx022a_of_match[] = {
> +	{ .compatible = "kionix,kx022a", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, kx022a_of_match);
> +
> +static struct i2c_driver kx022a_i2c_driver = {
> +	.driver = {
> +		.name  = "kx022a-i2c",
> +		.of_match_table = kx022a_of_match,
> +	  },
> +	.probe_new    = kx022a_i2c_probe,
> +};
> +module_i2c_driver(kx022a_i2c_driver);
> +
> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(KIONIX_ACCEL);
> diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
> new file mode 100644
> index 000000000000..7fe3b0aba1fe
> --- /dev/null
> +++ b/drivers/iio/accel/kionix-kx022a-spi.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 ROHM Semiconductors
> + *
> + * ROHM/KIONIX KX022A accelerometer driver
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +
> +#include "kionix-kx022a.h"
> +
> +static int kx022a_spi_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct regmap *regmap;
> +
> +	if (!spi->irq) {
> +		dev_err(dev, "No IRQ configured\n");
> +		return -EINVAL;
> +	}
> +
> +	regmap = devm_regmap_init_spi(spi, &kx022a_regmap);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "Failed to initialize Regmap\n");
> +
> +	return kx022a_probe_internal(dev);
> +}
> +
> +static const struct spi_device_id kx022a_id[] = {
> +	{ "kx022a" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, kx022a_id);
> +
> +static const struct of_device_id kx022a_of_match[] = {
> +	{ .compatible = "kionix,kx022a", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, kx022a_of_match);
> +
> +static struct spi_driver kx022a_spi_driver = {
> +	.driver = {
> +		.name   = "kx022a-spi",
> +		.of_match_table = kx022a_of_match,
> +	},
> +	.probe = kx022a_spi_probe,
> +	.id_table = kx022a_id,
> +};
> +module_spi_driver(kx022a_spi_driver);
> +
> +MODULE_DESCRIPTION("ROHM/Kionix kx022A accelerometer driver");
> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(KIONIX_ACCEL);
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> new file mode 100644
> index 000000000000..5a8622c8127b
> --- /dev/null
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -0,0 +1,1145 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 ROHM Semiconductors
> + *
> + * ROHM/KIONIX KX022A accelerometer driver
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/string_helpers.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +
> +#include "kionix-kx022a.h"
> +
> +/*
> + * The KX022A has FIFO which can store 43 samples of HiRes data from 2
> + * channels. This equals to 43 (samples) * 3 (channels) * 2 (bytes/sample) to
> + * 258 bytes of sample data. The quirk to know is that the amount of bytes in
> + * the FIFO is advertised via 8 bit register (max value 255). The thing to note
> + * is that full 258 bytes of data is indicated using the max value 255.
> + */
> +#define KX022A_FIFO_LENGTH			43
> +#define KX022A_FIFO_FULL_VALUE			255
> +#define KX022A_SOFT_RESET_WAIT_TIME_US		(5 * USEC_PER_MSEC)
> +#define KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US	(500 * USEC_PER_MSEC)
> +
> +/* 3 axis, 2 bytes of data for each of the axis */
> +#define KX022A_FIFO_SAMPLES_SIZE_BYTES		6
> +#define KX022A_FIFO_MAX_BYTES					\
> +	(KX022A_FIFO_LENGTH * KX022A_FIFO_SAMPLES_SIZE_BYTES)
> +
> +enum {
> +	KX022A_STATE_SAMPLE,
> +	KX022A_STATE_FIFO,
> +};
> +
> +/* Regmap configs */
> +static const struct regmap_range kx022a_volatile_ranges[] = {
> +	{
> +		.range_min = KX022A_REG_XHP_L,
> +		.range_max = KX022A_REG_COTR,
> +	}, {
> +		.range_min = KX022A_REG_TSCP,
> +		.range_max = KX022A_REG_INT_REL,
> +	}, {
> +		/* The reset bit will be cleared by sensor */
> +		.range_min = KX022A_REG_CNTL2,
> +		.range_max = KX022A_REG_CNTL2,
> +	}, {
> +		.range_min = KX022A_REG_BUF_STATUS_1,
> +		.range_max = KX022A_REG_BUF_READ,
> +	},
> +};
> +
> +static const struct regmap_access_table kx022a_volatile_regs = {
> +	.yes_ranges = &kx022a_volatile_ranges[0],
> +	.n_yes_ranges = ARRAY_SIZE(kx022a_volatile_ranges),
> +};
> +
> +static const struct regmap_range kx022a_precious_ranges[] = {
> +	{
> +		.range_min = KX022A_REG_INT_REL,
> +		.range_max = KX022A_REG_INT_REL,
> +	},
> +};
> +
> +static const struct regmap_access_table kx022a_precious_regs = {
> +	.yes_ranges = &kx022a_precious_ranges[0],
> +	.n_yes_ranges = ARRAY_SIZE(kx022a_precious_ranges),
> +};
> +
> +/*
> + * The HW does not set WHO_AM_I reg as read-only but we don't want to write it
> + * so we still include it in the read-only ranges.
> + */
> +static const struct regmap_range kx022a_read_only_ranges[] = {
> +	{
> +		.range_min = KX022A_REG_XHP_L,
> +		.range_max = KX022A_REG_INT_REL,
> +	}, {
> +		.range_min = KX022A_REG_BUF_STATUS_1,
> +		.range_max = KX022A_REG_BUF_STATUS_2,
> +	}, {
> +		.range_min = KX022A_REG_BUF_READ,
> +		.range_max = KX022A_REG_BUF_READ,
> +	},
> +};
> +
> +static const struct regmap_access_table kx022a_ro_regs = {
> +	.no_ranges = &kx022a_read_only_ranges[0],
> +	.n_no_ranges = ARRAY_SIZE(kx022a_read_only_ranges),
> +};
> +
> +static const struct regmap_range kx022a_write_only_ranges[] = {
> +	{
> +		.range_min = KX022A_REG_BTS_WUF_TH,
> +		.range_max = KX022A_REG_BTS_WUF_TH,
> +	}, {
> +		.range_min = KX022A_REG_MAN_WAKE,
> +		.range_max = KX022A_REG_MAN_WAKE,
> +	}, {
> +		.range_min = KX022A_REG_SELF_TEST,
> +		.range_max = KX022A_REG_SELF_TEST,
> +	}, {
> +		.range_min = KX022A_REG_BUF_CLEAR,
> +		.range_max = KX022A_REG_BUF_CLEAR,
> +	},
> +};
> +
> +static const struct regmap_access_table kx022a_wo_regs = {
> +	.no_ranges = &kx022a_write_only_ranges[0],
> +	.n_no_ranges = ARRAY_SIZE(kx022a_write_only_ranges),
> +};
> +
> +static const struct regmap_range kx022a_noinc_read_ranges[] = {
> +	{
> +		.range_min = KX022A_REG_BUF_READ,
> +		.range_max = KX022A_REG_BUF_READ,
> +	},
> +};
> +
> +static const struct regmap_access_table kx022a_nir_regs = {
> +	.yes_ranges = &kx022a_noinc_read_ranges[0],
> +	.n_yes_ranges = ARRAY_SIZE(kx022a_noinc_read_ranges),
> +};
> +
> +const struct regmap_config kx022a_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &kx022a_volatile_regs,
> +	.rd_table = &kx022a_wo_regs,
> +	.wr_table = &kx022a_ro_regs,
> +	.rd_noinc_table = &kx022a_nir_regs,
> +	.precious_table = &kx022a_precious_regs,
> +	.max_register = KX022A_MAX_REGISTER,
> +	.cache_type = REGCACHE_RBTREE,
> +};
> +EXPORT_SYMBOL_NS_GPL(kx022a_regmap, KIONIX_ACCEL);
> +
> +struct kx022a_data {
> +	struct regmap *regmap;
> +	struct iio_trigger *trig;
> +	struct device *dev;
> +	struct iio_mount_matrix orientation;
> +	int64_t timestamp, old_timestamp;
> +
> +	int irq;
> +	int inc_reg;
> +	int ien_reg;
> +
> +	unsigned int g_range;
> +	unsigned int state;
> +	unsigned int odr_ns;
> +
> +	bool trigger_enabled;
> +	/*
> +	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
> +	 * middle of a configuration, or when the fifo is enabled. Also,
> +	 * protect the data stored/retrieved from this structure from
> +	 * concurrent accesses.
> +	 */
> +	struct mutex mutex;
> +	u8 watermark;
> +
> +	/* 3 x 16bit accel data + timestamp */
> +	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
> +	struct {
> +		__le16 channels[3];
> +		s64 ts __aligned(8);
> +	} scan;
> +};
> +
> +static const struct iio_mount_matrix *
> +kx022a_get_mount_matrix(const struct iio_dev *idev,
> +			const struct iio_chan_spec *chan)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +
> +	return &data->orientation;
> +}
> +
> +enum {
> +	AXIS_X,
> +	AXIS_Y,
> +	AXIS_Z,
> +	AXIS_MAX
> +};
> +
> +static const unsigned long kx022a_scan_masks[] = {
> +	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z), 0
> +};
> +
> +static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
> +	IIO_MOUNT_MATRIX(IIO_SHARED_BY_TYPE, kx022a_get_mount_matrix),
> +	{ }
> +};
> +
> +#define KX022A_ACCEL_CHAN(axis, index)						\
> +{								\
> +	.type = IIO_ACCEL,					\
> +	.modified = 1,						\
> +	.channel2 = IIO_MOD_##axis,				\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.info_mask_shared_by_type_available =			\
> +				BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.ext_info = kx022a_ext_info,				\
> +	.address = KX022A_REG_##axis##OUT_L,			\
> +	.scan_index = index,					\
> +	.scan_type = {                                          \
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE,				\
> +	},							\
> +}
> +
> +static const struct iio_chan_spec kx022a_channels[] = {
> +	KX022A_ACCEL_CHAN(X, 0),
> +	KX022A_ACCEL_CHAN(Y, 1),
> +	KX022A_ACCEL_CHAN(Z, 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
> + * not available for higher sample rates. Thus, the driver only supports 200 Hz
> + * and slower ODRs. The slowest is 0.78 Hz.
> + */
> +static const int kx022a_accel_samp_freq_table[][2] = {
> +	{ 0, 780000 },
> +	{ 1, 563000 },
> +	{ 3, 125000 },
> +	{ 6, 250000 },
> +	{ 12, 500000 },
> +	{ 25, 0 },
> +	{ 50, 0 },
> +	{ 100, 0 },
> +	{ 200, 0 },
> +};
> +
> +static const unsigned int kx022a_odrs[] = {
> +	1282051282,
> +	639795266,
> +	320 * MEGA,
> +	160 * MEGA,
> +	80 * MEGA,
> +	40 * MEGA,
> +	20 * MEGA,
> +	10 * MEGA,
> +	5 * MEGA,
> +};
> +
> +/*
> + * range is typically +-2G/4G/8G/16G, distributed over the amount of bits.
> + * The scale table can be calculated using
> + *	(range / 2^bits) * g = (range / 2^bits) * 9.80665 m/s^2
> + *	=> KX022A uses 16 bit (HiRes mode - assume the low 8 bits are zeroed
> + *	in low-power mode(?) )
> + *	=> +/-2G  => 4 / 2^16 * 9,80665 * 10^6 (to scale to micro)
> + *	=> +/-2G  - 598.550415
> + *	   +/-4G  - 1197.10083
> + *	   +/-8G  - 2394.20166
> + *	   +/-16G - 4788.40332
> + */
> +static const int kx022a_scale_table[][2] = {
> +	{ 598, 550415 },
> +	{ 1197, 100830 },
> +	{ 2394, 201660 },
> +	{ 4788, 403320 },
> +};
> +
> +static int kx022a_read_avail(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     const int **vals, int *type, int *length,
> +			     long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = (const int *)kx022a_accel_samp_freq_table;
> +		*length = ARRAY_SIZE(kx022a_accel_samp_freq_table) *
> +			  ARRAY_SIZE(kx022a_accel_samp_freq_table[0]);
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		return IIO_AVAIL_LIST;
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (const int *)kx022a_scale_table;
> +		*length = ARRAY_SIZE(kx022a_scale_table) *
> +			  ARRAY_SIZE(kx022a_scale_table[0]);
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +#define KX022A_DEFAULT_PERIOD_NS (20 * NSEC_PER_MSEC)
> +
> +static void kx022a_reg2freq(unsigned int val,  int *val1, int *val2)
> +{
> +	*val1 = kx022a_accel_samp_freq_table[val & KX022A_MASK_ODR][0];
> +	*val2 = kx022a_accel_samp_freq_table[val & KX022A_MASK_ODR][1];
> +}
> +
> +static void kx022a_reg2scale(unsigned int val, unsigned int *val1,
> +			     unsigned int *val2)
> +{
> +	val &= KX022A_MASK_GSEL;
> +	val >>= KX022A_GSEL_SHIFT;
> +
> +	*val1 = kx022a_scale_table[val][0];
> +	*val2 = kx022a_scale_table[val][1];
> +}
> +
> +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,
> +				      KX022A_MASK_PC1);
> +	else
> +		ret = regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
> +					KX022A_MASK_PC1);
> +	if (ret)
> +		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)
> +{
> +	int ret;
> +
> +	mutex_lock(&data->mutex);
> +	ret = kx022a_turn_on_off_unlocked(data, false);
> +	if (ret)
> +		mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int kx022a_turn_on_unlock(struct kx022a_data *data)
> +{
> +	int ret;
> +
> +	ret = kx022a_turn_on_off_unlocked(data, true);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int kx022a_write_raw(struct iio_dev *idev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +	int ret, n;
> +
> +	/*
> +	 * We should not allow changing scale or frequency when FIFO is running
> +	 * as it will mess the timestamp/scale for samples existing in the
> +	 * buffer. If this turns out to be an issue we can later change logic
> +	 * to internally flush the fifo before reconfiguring so the samples in
> +	 * fifo keep matching the freq/scale settings. (Such setup could cause
> +	 * issues if users trust the watermark to be reached within known
> +	 * time-limit).
> +	 */
> +	ret = iio_device_claim_direct_mode(idev);
> +	if (ret)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		n = ARRAY_SIZE(kx022a_accel_samp_freq_table);
> +
> +		while (n--)
> +			if (val == kx022a_accel_samp_freq_table[n][0] &&
> +			    val2 == kx022a_accel_samp_freq_table[n][1])
> +				break;
> +		if (n < 0) {
> +			ret = -EINVAL;
> +			goto unlock_out;
> +		}
> +		ret = kx022a_turn_off_lock(data);
> +		if (ret)
> +			break;
> +
> +		ret = regmap_update_bits(data->regmap,
> +					 KX022A_REG_ODCNTL,
> +					 KX022A_MASK_ODR, n);
> +		data->odr_ns = kx022a_odrs[n];
> +		kx022a_turn_on_unlock(data);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		n = ARRAY_SIZE(kx022a_scale_table);
> +
> +		while (n-- > 0)
> +			if (val == kx022a_scale_table[n][0] &&
> +			    val2 == kx022a_scale_table[n][1])
> +				break;
> +		if (n < 0) {
> +			ret = -EINVAL;
> +			goto unlock_out;
> +		}
> +
> +		ret = kx022a_turn_off_lock(data);
> +		if (ret)
> +			break;
> +
> +		ret = regmap_update_bits(data->regmap, KX022A_REG_CNTL,
> +					 KX022A_MASK_GSEL,
> +					 n << KX022A_GSEL_SHIFT);
> +		kx022a_turn_on_unlock(data);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +unlock_out:
> +	iio_device_release_direct_mode(idev);
> +
> +	return ret;
> +}
> +
> +static int kx022a_fifo_set_wmi(struct kx022a_data *data)
> +{
> +	u8 threshold;
> +
> +	threshold = data->watermark;
> +
> +	return regmap_update_bits(data->regmap, KX022A_REG_BUF_CNTL1,
> +				  KX022A_MASK_WM_TH, threshold);
> +}
> +
> +static int kx022a_get_axis(struct kx022a_data *data,
> +			   struct iio_chan_spec const *chan,
> +			   int *val)
> +{
> +	int ret;
> +
> +	ret = regmap_bulk_read(data->regmap, chan->address, &data->buffer[0],
> +			       sizeof(__le16));
> +	if (ret)
> +		return ret;
> +
> +	*val = le16_to_cpu(data->buffer[0]);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int kx022a_read_raw(struct iio_dev *idev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +	unsigned int regval;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(idev);
> +		if (ret)
> +			return ret;
> +
> +		mutex_lock(&data->mutex);
> +		ret = kx022a_get_axis(data, chan, val);
> +		mutex_unlock(&data->mutex);
> +
> +		iio_device_release_direct_mode(idev);
> +
> +		return ret;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(data->regmap, KX022A_REG_ODCNTL, &regval);
> +		if (ret)
> +			return ret;
> +
> +		if ((regval & KX022A_MASK_ODR) >
> +		    ARRAY_SIZE(kx022a_accel_samp_freq_table)) {
> +			dev_err(data->dev, "Invalid ODR\n");
> +			return -EINVAL;
> +		}
> +
> +		kx022a_reg2freq(regval, val, val2);
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regmap_read(data->regmap, KX022A_REG_CNTL, &regval);
> +		if (ret < 0)
> +			return ret;
> +
> +		kx022a_reg2scale(regval, val, val2);
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +};
> +
> +static int kx022a_validate_trigger(struct iio_dev *idev,
> +				   struct iio_trigger *trig)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +
> +	if (data->trig != trig)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +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;
> +
> +	mutex_lock(&data->mutex);
> +	data->watermark = val;
> +	mutex_unlock(&data->mutex);
> +
> +	return 0;
> +}
> +
> +static ssize_t hwfifo_enabled_show(struct device *dev,
> +				   struct device_attribute *attr,
> +				   char *buf)
> +{
> +	struct iio_dev *idev = dev_to_iio_dev(dev);
> +	struct kx022a_data *data = iio_priv(idev);
> +	bool state;
> +
> +	mutex_lock(&data->mutex);
> +	state = data->state;
> +	mutex_unlock(&data->mutex);
> +
> +	return sysfs_emit(buf, "%d\n", state);
> +}
> +
> +static ssize_t hwfifo_watermark_show(struct device *dev,
> +				     struct device_attribute *attr,
> +				     char *buf)
> +{
> +	struct iio_dev *idev = dev_to_iio_dev(dev);
> +	struct kx022a_data *data = iio_priv(idev);
> +	int wm;
> +
> +	mutex_lock(&data->mutex);
> +	wm = data->watermark;
> +	mutex_unlock(&data->mutex);
> +
> +	return sysfs_emit(buf, "%d\n", wm);
> +}
> +
> +static IIO_DEVICE_ATTR_RO(hwfifo_enabled, 0);
> +static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
> +
> +static const struct attribute *kx022a_fifo_attributes[] = {
> +	&iio_dev_attr_hwfifo_watermark.dev_attr.attr,
> +	&iio_dev_attr_hwfifo_enabled.dev_attr.attr,
> +	NULL
> +};
> +
> +static int kx022a_drop_fifo_contents(struct kx022a_data *data)
> +{
> +	/*
> +	 * We must clear the old time-stamp to avoid computing the timestamps
> +	 * based on samples acquired when buffer was last enabled.
> +	 *
> +	 * We don't need to protect the timestamp as long as we are only
> +	 * called from fifo-disable where we can guarantee the sensor is not
> +	 * triggering interrupts and where the mutex is locked to prevent the
> +	 * user-space access.
> +	 */
> +	data->timestamp = 0;
> +
> +	return regmap_write(data->regmap, KX022A_REG_BUF_CLEAR, 0x0);
> +}
> +
> +static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
> +			       bool irq)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +	struct device *dev = regmap_get_device(data->regmap);
> +	u16 buffer[KX022A_FIFO_LENGTH * 3];
> +	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;
> +
> +	if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
> +		dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
> +
> +	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
> +	if (!count)
> +		return 0;
> +
> +	/*
> +	 * If we are being called from IRQ handler we know the stored timestamp
> +	 * is fairly accurate for the last stored sample. Otherwise, if we are
> +	 * called as a result of a read operation from userspace and hence
> +	 * before the watermark interrupt was triggered, take a timestamp
> +	 * now. We can fall anywhere in between two samples so the error in this
> +	 * case is at most one sample period.
> +	 */
> +	if (!irq) {
> +		/*
> +		 * We need to have the IRQ disabled or we risk of messing-up
> +		 * the timestamps. If we are ran from IRQ, then the
> +		 * IRQF_ONESHOT has us covered - but if we are ran by the
> +		 * user-space read we need to disable the IRQ to be on a safe
> +		 * side. We do this usng synchronous disable so that if the
> +		 * IRQ thread is being ran on other CPU we wait for it to be
> +		 * finished.
> +		 */
> +		disable_irq(data->irq);
> +		renable = true;
> +
> +		data->old_timestamp = data->timestamp;
> +		data->timestamp = iio_get_time_ns(idev);
> +	}
> +
> +	/*
> +	 * Approximate timestamps for each of the sample based on the sampling
> +	 * frequency, timestamp for last sample and number of samples.
> +	 *
> +	 * We'd better not use the current bandwidth settings to compute the
> +	 * sample period. The real sample rate varies with the device and
> +	 * small variation adds when we store a large number of samples.
> +	 *
> +	 * To avoid this issue we compute the actual sample period ourselves
> +	 * based on the timestamp delta between the last two flush operations.
> +	 */
> +	if (data->old_timestamp) {
> +		sample_period = data->timestamp - data->old_timestamp;
> +		do_div(sample_period, count);
> +	} else {
> +		sample_period = data->odr_ns;
> +	}
> +	tstamp = data->timestamp - (count - 1) * sample_period;
> +
> +	if (samples && count > samples) {
> +		/*
> +		 * Here we leave some old samples to the buffer. We need to
> +		 * adjust the timestamp to match the first sample in the buffer
> +		 * or we will miscalculate the sample_period at next round.
> +		 */
> +		data->timestamp -= (count - samples) * sample_period;
> +		count = samples;
> +	}
> +
> +	fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
> +	ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
> +				buffer, fifo_bytes);
> +	if (ret)
> +		goto renable_out;
> +
> +	for (i = 0; i < count; i++) {
> +		u16 *sam = &buffer[i * 3];
> +		__le16 *chs;
> +		int bit;
> +
> +		chs = &data->scan.channels[0];
> +		for_each_set_bit(bit, idev->active_scan_mask, AXIS_MAX)
> +			memcpy(&chs[bit], &sam[bit], sizeof(*chs));
> +
> +		iio_push_to_buffers_with_timestamp(idev, &data->scan, tstamp);
> +
> +		tstamp += sample_period;
> +	}
> +
> +	ret = count;
> +
> +renable_out:
> +	if (renable)
> +		enable_irq(data->irq);
> +
> +	return ret;
> +}
> +
> +static int kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +	int ret;
> +
> +	mutex_lock(&data->mutex);
> +	ret = __kx022a_fifo_flush(idev, samples, false);
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info kx022a_info = {
> +	.read_raw = &kx022a_read_raw,
> +	.write_raw = &kx022a_write_raw,
> +	.read_avail = &kx022a_read_avail,
> +
> +	.validate_trigger	= kx022a_validate_trigger,
> +	.hwfifo_set_watermark	= kx022a_set_watermark,
> +	.hwfifo_flush_to_buffer	= kx022a_fifo_flush,
> +};
> +
> +static int kx022a_set_drdy_irq(struct kx022a_data *data, bool en)
> +{
> +	if (en)
> +		return regmap_set_bits(data->regmap, KX022A_REG_CNTL,
> +				       KX022A_MASK_DRDY);
> +
> +	return regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
> +				 KX022A_MASK_DRDY);
> +}
> +
> +static int kx022a_prepare_irq_pin(struct kx022a_data *data)
> +{
> +	/* Enable IRQ1 pin. Set polarity to active low */
> +	int mask = KX022A_MASK_IEN | KX022A_MASK_IPOL |
> +		   KX022A_MASK_ITYP;
> +	int val = KX022A_MASK_IEN | KX022A_IPOL_LOW |
> +		  KX022A_ITYP_LEVEL;
> +	int ret;
> +
> +	ret = regmap_update_bits(data->regmap, data->inc_reg, mask, val);
> +	if (ret)
> +		return ret;
> +
> +	/* We enable WMI to IRQ pin only at buffer_enable */
> +	mask = KX022A_MASK_INS2_DRDY;
> +
> +	return regmap_set_bits(data->regmap, data->ien_reg, mask);
> +}
> +
> +static int kx022a_fifo_disable(struct kx022a_data *data)
> +{
> +	int ret = 0;
> +
> +	ret = kx022a_turn_off_lock(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_clear_bits(data->regmap, data->ien_reg, KX022A_MASK_WMI);
> +	if (ret)
> +		goto unlock_out;
> +
> +	ret = regmap_clear_bits(data->regmap, KX022A_REG_BUF_CNTL2,
> +				KX022A_MASK_BUF_EN);
> +	if (ret)
> +		goto unlock_out;

> +	data->state &= (~KX022A_STATE_FIFO);

Too many parentheses.

> +	kx022a_drop_fifo_contents(data);
> +
> +	return kx022a_turn_on_unlock(data);
> +
> +unlock_out:
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int kx022a_buffer_predisable(struct iio_dev *idev)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +
> +	if (iio_device_get_current_mode(idev) == INDIO_BUFFER_TRIGGERED)
> +		return 0;
> +
> +	return kx022a_fifo_disable(data);
> +}
> +
> +static int kx022a_fifo_enable(struct kx022a_data *data)
> +{
> +	int ret;
> +
> +	ret = kx022a_turn_off_lock(data);
> +	if (ret)
> +		return ret;
> +
> +	/* Update watermark to HW */
> +	ret = kx022a_fifo_set_wmi(data);
> +	if (ret)
> +		goto unlock_out;
> +
> +	/* Enable buffer */
> +	ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
> +			      KX022A_MASK_BUF_EN);
> +	if (ret)
> +		goto unlock_out;
> +
> +	data->state |= KX022A_STATE_FIFO;
> +	ret = regmap_set_bits(data->regmap, data->ien_reg,
> +			      KX022A_MASK_WMI);
> +	if (ret)
> +		goto unlock_out;
> +
> +	return kx022a_turn_on_unlock(data);
> +
> +unlock_out:
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int kx022a_buffer_postenable(struct iio_dev *idev)
> +{
> +	struct kx022a_data *data = iio_priv(idev);
> +
> +	/*
> +	 * If we use data-ready trigger, then the IRQ masks should be handled by
> +	 * trigger enable and the hardware buffer is not used but we just update
> +	 * results to the IIO fifo when data-ready triggers.
> +	 */
> +	if (iio_device_get_current_mode(idev) == INDIO_BUFFER_TRIGGERED)
> +		return 0;
> +
> +	return kx022a_fifo_enable(data);
> +}
> +
> +static const struct iio_buffer_setup_ops kx022a_buffer_ops = {
> +	.postenable = kx022a_buffer_postenable,
> +	.predisable = kx022a_buffer_predisable,
> +};
> +
> +static irqreturn_t kx022a_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *idev = pf->indio_dev;
> +	struct kx022a_data *data = iio_priv(idev);
> +	int ret;
> +
> +	ret = regmap_bulk_read(data->regmap, KX022A_REG_XOUT_L, data->buffer,
> +			       KX022A_FIFO_SAMPLES_SIZE_BYTES);
> +	if (ret < 0)
> +		goto err_read;
> +
> +	iio_push_to_buffers_with_timestamp(idev, data->buffer, pf->timestamp);
> +err_read:
> +	iio_trigger_notify_done(idev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/* Get timestamps and wake the thread if we need to read data */
> +static irqreturn_t kx022a_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *idev = private;
> +	struct kx022a_data *data = iio_priv(idev);
> +
> +	data->old_timestamp = data->timestamp;
> +	data->timestamp = iio_get_time_ns(idev);
> +
> +	if (data->state & KX022A_STATE_FIFO || data->trigger_enabled)
> +		return IRQ_WAKE_THREAD;
> +
> +	return IRQ_NONE;
> +}
> +
> +/*
> + * WMI and data-ready IRQs are acked when results are read. If we add
> + * TILT/WAKE or other IRQs - then we may need to implement the acking
> + * (which is racy).
> + */
> +static irqreturn_t kx022a_irq_thread_handler(int irq, void *private)
> +{
> +	struct iio_dev *idev = private;
> +	struct kx022a_data *data = iio_priv(idev);

> +	int ret = IRQ_NONE;

Strictly speaking this should be

	irqreturn_t ret = ...

> +	mutex_lock(&data->mutex);
> +
> +	if (data->trigger_enabled) {
> +		iio_trigger_poll_chained(data->trig);
> +		ret = IRQ_HANDLED;
> +	}
> +
> +	if (data->state & KX022A_STATE_FIFO) {
> +		int ok;
> +
> +		ok = __kx022a_fifo_flush(idev, KX022A_FIFO_LENGTH, true);
> +		if (ok > 0)
> +			ret = IRQ_HANDLED;
> +	}
> +
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static int kx022a_trigger_set_state(struct iio_trigger *trig,
> +				    bool state)
> +{
> +	struct kx022a_data *data = iio_trigger_get_drvdata(trig);
> +	int ret = 0;
> +
> +	mutex_lock(&data->mutex);
> +
> +	if (data->trigger_enabled == state)
> +		goto unlock_out;
> +
> +	if (data->state & KX022A_STATE_FIFO) {
> +		dev_warn(data->dev, "Can't set trigger when FIFO enabled\n");
> +		ret = -EBUSY;
> +		goto unlock_out;
> +	}
> +
> +	ret = kx022a_turn_on_off_unlocked(data, false);
> +	if (ret)
> +		goto unlock_out;
> +
> +	data->trigger_enabled = state;
> +	ret = kx022a_set_drdy_irq(data, state);
> +	if (ret)
> +		goto unlock_out;
> +
> +	ret = kx022a_turn_on_off_unlocked(data, true);
> +
> +unlock_out:
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +
> +static const struct iio_trigger_ops kx022a_trigger_ops = {
> +	.set_trigger_state = kx022a_trigger_set_state,
> +};
> +
> +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);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * I've seen I2C read failures if we poll too fast after the sensor
> +	 * reset. Slight delay gives I2C block the time to recover.
> +	 */
> +	msleep(1);
> +
> +	ret = regmap_read_poll_timeout(data->regmap, KX022A_REG_CNTL2, val,
> +				       !(val & KX022A_MASK_SRST),
> +				       KX022A_SOFT_RESET_WAIT_TIME_US,
> +				       KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US);
> +	if (ret) {
> +		dev_err(data->dev, "Sensor reset %s\n",
> +			val & KX022A_MASK_SRST ? "timeout" : "fail#");
> +		return ret;
> +	}
> +
> +	ret = regmap_reinit_cache(data->regmap, &kx022a_regmap);
> +	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,
> +			      KX022A_MASK_BRES16);
> +	if (ret) {
> +		dev_err(data->dev, "Failed to set data resolution\n");
> +		return ret;
> +	}
> +
> +	return kx022a_prepare_irq_pin(data);
> +}
> +
> +int kx022a_probe_internal(struct device *dev)
> +{
> +	static const char * const regulator_names[] = {"io-vdd", "vdd"};
> +	struct iio_trigger *indio_trig;
> +	struct fwnode_handle *fwnode;
> +	struct kx022a_data *data;
> +	struct regmap *regmap;
> +	unsigned int chip_id;
> +	struct iio_dev *idev;
> +	int ret, irq;
> +	char *name;
> +
> +	regmap = dev_get_regmap(dev, NULL);
> +	if (!regmap) {
> +		dev_err(dev, "no regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	idev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!idev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(idev);
> +
> +	/*
> +	 * VDD is the analog and digital domain voltage supply and
> +	 * IO_VDD is the digital I/O voltage supply.
> +	 */
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
> +					     regulator_names);
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "failed to enable regulator\n");
> +
> +	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
> +	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;
> +	}

> +	fwnode = dev_fwnode(dev);
> +	if (!fwnode)
> +		return -ENODEV;

You can do it before allocating any of the resources, so it will bail out
earlier with less potential issues.

> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq > 0) {
> +		data->inc_reg = KX022A_REG_INC1;
> +		data->ien_reg = KX022A_REG_INC4;
> +
> +		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
> +			dev_warn(dev, "Only one IRQ supported\n");
> +	} 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->regmap = regmap;
> +	data->dev = dev;
> +	data->irq = irq;
> +	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->info = &kx022a_info;
> +	idev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> +	idev->available_scan_masks = kx022a_scan_masks;
> +
> +	/* Read the mounting matrix, if present */
> +	ret = iio_read_mount_matrix(dev, &data->orientation);
> +	if (ret)
> +		return ret;
> +
> +	/* The sensor must be turned off for configuration */
> +	ret = kx022a_turn_off_lock(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = kx022a_chip_init(data);
> +	if (ret) {
> +		mutex_unlock(&data->mutex);
> +		return ret;
> +	}
> +
> +	ret = kx022a_turn_on_unlock(data);
> +	if (ret)
> +		return ret;

Probably a comment that tells why you need an atomic delay.

> +	udelay(100);
> +	ret = devm_iio_triggered_buffer_setup_ext(dev, idev,
> +						  &iio_pollfunc_store_time,
> +						  kx022a_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_IN,
> +						  &kx022a_buffer_ops,
> +						  kx022a_fifo_attributes);
> +
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "iio_triggered_buffer_setup_ext FAIL\n");
> +	indio_trig = devm_iio_trigger_alloc(dev, "%sdata-rdy-dev%d", idev->name,
> +					    iio_device_id(idev));
> +	if (!indio_trig)
> +		return -ENOMEM;
> +
> +	data->trig = indio_trig;
> +
> +	indio_trig->ops = &kx022a_trigger_ops;
> +	iio_trigger_set_drvdata(indio_trig, data);
> +
> +	ret = devm_iio_trigger_register(dev, indio_trig);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "Trigger registration failed\n");
> +
> +	ret = devm_iio_device_register(data->dev, idev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Unable to register iio device\n");
> +
> +	/*
> +	 * No need to check for NULL. request_threadedI_irq() defaults to
> +	 * dev_name() should the alloc fail.
> +	 */
> +	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s-kx022a",
> +			      dev_name(data->dev));
> +
> +	ret = devm_request_threaded_irq(data->dev, irq, kx022a_irq_handler,
> +					&kx022a_irq_thread_handler,
> +					IRQF_ONESHOT, name, idev);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(kx022a_probe_internal, KIONIX_ACCEL);
> +
> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
> new file mode 100644
> index 000000000000..12424649d438
> --- /dev/null
> +++ b/drivers/iio/accel/kionix-kx022a.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2022 ROHM Semiconductors
> + *
> + * ROHM/KIONIX KX022A accelerometer driver
> + */
> +
> +#ifndef _KX022A_H_
> +#define _KX022A_H_
> +
> +#include <linux/bits.h>
> +#include <linux/regmap.h>
> +
> +#define KX022A_REG_WHO		0x0f
> +#define KX022A_ID		0xc8
> +
> +#define KX022A_REG_CNTL2	0x19
> +#define KX022A_MASK_SRST	BIT(7)
> +#define KX022A_REG_CNTL		0x18
> +#define KX022A_MASK_PC1		BIT(7)
> +#define KX022A_MASK_RES		BIT(6)
> +#define KX022A_MASK_DRDY	BIT(5)
> +#define KX022A_MASK_GSEL	GENMASK(4, 3)
> +#define KX022A_GSEL_SHIFT	3
> +#define KX022A_GSEL_2		0x0
> +#define KX022A_GSEL_4		BIT(3)
> +#define KX022A_GSEL_8		BIT(4)
> +#define KX022A_GSEL_16		GENMASK(4, 3)
> +
> +#define KX022A_REG_INS2		0x13
> +#define KX022A_MASK_INS2_DRDY	BIT(4)
> +#define KX122_MASK_INS2_WMI	BIT(5)
> +
> +#define KX022A_REG_XHP_L	0x0
> +#define KX022A_REG_XOUT_L	0x06
> +#define KX022A_REG_YOUT_L	0x08
> +#define KX022A_REG_ZOUT_L	0x0a
> +#define KX022A_REG_COTR		0x0c
> +#define KX022A_REG_TSCP		0x10
> +#define KX022A_REG_INT_REL	0x17
> +
> +#define KX022A_REG_ODCNTL	0x1b
> +
> +#define KX022A_REG_BTS_WUF_TH	0x31
> +#define KX022A_REG_MAN_WAKE	0x2c
> +
> +#define KX022A_REG_BUF_CNTL1	0x3a
> +#define KX022A_MASK_WM_TH	GENMASK(6, 0)
> +#define KX022A_REG_BUF_CNTL2	0x3b
> +#define KX022A_MASK_BUF_EN	BIT(7)
> +#define KX022A_MASK_BRES16	BIT(6)
> +#define KX022A_REG_BUF_STATUS_1	0x3c
> +#define KX022A_REG_BUF_STATUS_2	0x3d
> +#define KX022A_REG_BUF_CLEAR	0x3e
> +#define KX022A_REG_BUF_READ	0x3f
> +#define KX022A_MASK_ODR		GENMASK(3, 0)
> +#define KX022A_ODR_SHIFT	3
> +#define KX022A_FIFO_MAX_WMI_TH	41
> +
> +#define KX022A_REG_INC1		0x1c
> +#define KX022A_REG_INC5		0x20
> +#define KX022A_REG_INC6		0x21
> +#define KX022A_MASK_IEN		BIT(5)
> +#define KX022A_MASK_IPOL	BIT(4)
> +#define KX022A_IPOL_LOW		0
> +#define KX022A_IPOL_HIGH	KX022A_MASK_IPOL1
> +#define KX022A_MASK_ITYP	BIT(3)
> +#define KX022A_ITYP_PULSE	KX022A_MASK_ITYP
> +#define KX022A_ITYP_LEVEL	0
> +
> +#define KX022A_REG_INC4		0x1f
> +#define KX022A_MASK_WMI		BIT(5)
> +
> +#define KX022A_REG_SELF_TEST	0x60
> +#define KX022A_MAX_REGISTER	0x60
> +
> +struct device;
> +
> +int kx022a_probe_internal(struct device *dev);
> +extern const struct regmap_config kx022a_regmap;
> +
> +#endif
> -- 
> 2.37.3

Matti Vaittinen Oct. 21, 2022, 12:47 p.m. UTC | #2

On 10/21/22 15:41, Andy Shevchenko wrote:
> On Fri, Oct 21, 2022 at 02:22:49PM +0300, Matti Vaittinen wrote:
>> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
>> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
>> tap/motion detection, wake-up & back-to-sleep events, four acceleration
>> ranges (2, 4, 8 and 16g), and probably some other cool features.
>>
>> Add support for the basic accelerometer features such as getting the
>> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
>> using the WMI IRQ).
>>
>> Important things to be added include the double-tap, motion
>> detection and wake-up as well as the runtime power management.
> 
> While I have some disagreements on some code
> pieces, this version is okay to go I think.
> 
> Reviewed-by: Andy Shevchenko <[e-mail]>

Thanks for the thorough review Andy.

> Below a few nit-picks in case it needs to be a v5.

Just a note that I do agree with these 'nits'. I'll fix them if I need 
to respin the series.

Jonathan Cameron Oct. 23, 2022, 11:43 a.m. UTC | #3

On Fri, 21 Oct 2022 14:22:49 +0300
Matti Vaittinen <[e-mail]> wrote:

> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
> tap/motion detection, wake-up & back-to-sleep events, four acceleration
> ranges (2, 4, 8 and 16g), and probably some other cool features.
> 
> Add support for the basic accelerometer features such as getting the
> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
> using the WMI IRQ).
> 
> Important things to be added include the double-tap, motion
> detection and wake-up as well as the runtime power management.
> 
> Signed-off-by: Matti Vaittinen <[e-mail]>

Hi Matti,

A few things from me on this read through to add to those last few comments
from Andy.

Thanks,

Jonathan


> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> new file mode 100644
> index 000000000000..6510f8d62b85
> --- /dev/null
> +++ b/drivers/iio/accel/kionix-kx022a-i2c.c
> @@ -0,0 +1,51 @@

> +
> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(KIONIX_ACCEL);
Not sure why I didn't noticed this before, but after various debates we
ended up prefixing namespaces to limit them to IIO. Also, there are other kionix
drivers already and may be more in future. Better to limit this to scope of this
driver, 

IIO_KX022A 

seems like a reaonsable choice.

> diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
> new file mode 100644
> index 000000000000..7fe3b0aba1fe
> --- /dev/null

> +
> +#define KX022A_ACCEL_CHAN(axis, index)						\
Trivial: Not always easy to spot odd alignment of \ in patches but in this case I doubt the
one above is in a sensible place wrt to the others!

> +{								\
> +	.type = IIO_ACCEL,					\
> +	.modified = 1,						\
> +	.channel2 = IIO_MOD_##axis,				\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.info_mask_shared_by_type_available =			\
> +				BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.ext_info = kx022a_ext_info,				\
> +	.address = KX022A_REG_##axis##OUT_L,			\
> +	.scan_index = index,					\
> +	.scan_type = {                                          \
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE,				\
> +	},							\
> +}
>
...

> +
> +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);
> +	u16 buffer[KX022A_FIFO_LENGTH * 3];
> +	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;
> +
> +	if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
> +		dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
> +
> +	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
> +	if (!count)
> +		return 0;
> +
> +	/*
> +	 * If we are being called from IRQ handler we know the stored timestamp
> +	 * is fairly accurate for the last stored sample. Otherwise, if we are
> +	 * called as a result of a read operation from userspace and hence
> +	 * before the watermark interrupt was triggered, take a timestamp
> +	 * now. We can fall anywhere in between two samples so the error in this
> +	 * case is at most one sample period.
> +	 */
> +	if (!irq) {
> +		/*
> +		 * We need to have the IRQ disabled or we risk of messing-up
> +		 * the timestamps. If we are ran from IRQ, then the
> +		 * IRQF_ONESHOT has us covered - but if we are ran by the
> +		 * user-space read we need to disable the IRQ to be on a safe
> +		 * side. We do this usng synchronous disable so that if the
> +		 * IRQ thread is being ran on other CPU we wait for it to be
> +		 * finished.
> +		 */
> +		disable_irq(data->irq);
> +		renable = true;
> +
> +		data->old_timestamp = data->timestamp;
> +		data->timestamp = iio_get_time_ns(idev);
> +	}
> +
> +	/*
> +	 * Approximate timestamps for each of the sample based on the sampling
> +	 * frequency, timestamp for last sample and number of samples.
> +	 *
> +	 * We'd better not use the current bandwidth settings to compute the
> +	 * sample period. The real sample rate varies with the device and
> +	 * small variation adds when we store a large number of samples.
> +	 *
> +	 * To avoid this issue we compute the actual sample period ourselves
> +	 * based on the timestamp delta between the last two flush operations.
> +	 */
> +	if (data->old_timestamp) {
> +		sample_period = data->timestamp - data->old_timestamp;
> +		do_div(sample_period, count);
> +	} else {
> +		sample_period = data->odr_ns;
> +	}
> +	tstamp = data->timestamp - (count - 1) * sample_period;
> +
> +	if (samples && count > samples) {
> +		/*
> +		 * Here we leave some old samples to the buffer. We need to
> +		 * adjust the timestamp to match the first sample in the buffer
> +		 * or we will miscalculate the sample_period at next round.
> +		 */
> +		data->timestamp -= (count - samples) * sample_period;
> +		count = samples;
> +	}
> +
> +	fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
> +	ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
> +				buffer, fifo_bytes);
> +	if (ret)
> +		goto renable_out;
> +
> +	for (i = 0; i < count; i++) {
> +		u16 *sam = &buffer[i * 3];
> +		__le16 *chs;
> +		int bit;
> +
> +		chs = &data->scan.channels[0];
> +		for_each_set_bit(bit, idev->active_scan_mask, AXIS_MAX)
> +			memcpy(&chs[bit], &sam[bit], sizeof(*chs));

Why memcpying from a u16 array to an __le16 array?
buffer should probably also be __le16  and then you can just use
an assignment rather than a memcpy

> +
> +		iio_push_to_buffers_with_timestamp(idev, &data->scan, tstamp);
> +
> +		tstamp += sample_period;
> +	}
> +
> +	ret = count;
> +
> +renable_out:
> +	if (renable)
> +		enable_irq(data->irq);
> +
> +	return ret;
> +}


> +int kx022a_probe_internal(struct device *dev)
> +{
> +	static const char * const regulator_names[] = {"io-vdd", "vdd"};
> +	struct iio_trigger *indio_trig;
> +	struct fwnode_handle *fwnode;
> +	struct kx022a_data *data;
> +	struct regmap *regmap;
> +	unsigned int chip_id;
> +	struct iio_dev *idev;
> +	int ret, irq;
> +	char *name;
> +
> +	regmap = dev_get_regmap(dev, NULL);
> +	if (!regmap) {
> +		dev_err(dev, "no regmap\n");
> +		return -EINVAL;
> +	}
> +
> +	idev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!idev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(idev);
> +
> +	/*
> +	 * VDD is the analog and digital domain voltage supply and
> +	 * IO_VDD is the digital I/O voltage supply.
> +	 */
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
> +					     regulator_names);
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "failed to enable regulator\n");
> +
> +	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
> +	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;
> +	}
> +
> +	fwnode = dev_fwnode(dev);
> +	if (!fwnode)
> +		return -ENODEV;
> +
> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> +	if (irq > 0) {
> +		data->inc_reg = KX022A_REG_INC1;
> +		data->ien_reg = KX022A_REG_INC4;
> +
> +		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
> +			dev_warn(dev, "Only one IRQ supported\n");

Why?  If you get the both, only the first one will be used by the driver.
Not really worth warning about the lack of features... 

> +	} 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->regmap = regmap;
> +	data->dev = dev;
> +	data->irq = irq;
> +	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->info = &kx022a_info;
> +	idev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
> +	idev->available_scan_masks = kx022a_scan_masks;
> +
> +	/* Read the mounting matrix, if present */
> +	ret = iio_read_mount_matrix(dev, &data->orientation);
> +	if (ret)
> +		return ret;
> +
> +	/* The sensor must be turned off for configuration */
> +	ret = kx022a_turn_off_lock(data);
> +	if (ret)
> +		return ret;
> +
> +	ret = kx022a_chip_init(data);
> +	if (ret) {
> +		mutex_unlock(&data->mutex);
> +		return ret;
> +	}
> +
> +	ret = kx022a_turn_on_unlock(data);
> +	if (ret)
> +		return ret;
> +
> +	udelay(100);
> +	ret = devm_iio_triggered_buffer_setup_ext(dev, idev,
> +						  &iio_pollfunc_store_time,
> +						  kx022a_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_IN,
> +						  &kx022a_buffer_ops,
> +						  kx022a_fifo_attributes);
> +
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "iio_triggered_buffer_setup_ext FAIL\n");
> +	indio_trig = devm_iio_trigger_alloc(dev, "%sdata-rdy-dev%d", idev->name,
> +					    iio_device_id(idev));
> +	if (!indio_trig)
> +		return -ENOMEM;
> +
> +	data->trig = indio_trig;
> +
> +	indio_trig->ops = &kx022a_trigger_ops;
> +	iio_trigger_set_drvdata(indio_trig, data);
> +
> +	ret = devm_iio_trigger_register(dev, indio_trig);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "Trigger registration failed\n");
> +
> +	ret = devm_iio_device_register(data->dev, idev);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "Unable to register iio device\n");
> +
> +	/*
> +	 * No need to check for NULL. request_threadedI_irq() defaults to
Why I?

> +	 * dev_name() should the alloc fail.
> +	 */
> +	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s-kx022a",
> +			      dev_name(data->dev));
> +
> +	ret = devm_request_threaded_irq(data->dev, irq, kx022a_irq_handler,
> +					&kx022a_irq_thread_handler,
> +					IRQF_ONESHOT, name, idev);

You are requesting the irq 'after' exposing the userspace interfaces.
Technically that potentially introduces a race as we might in theory successfully
trigger an interrupt before requesting it.

Obviously that would be challenging to pull off given likely short window, but
good to close it anyway!  Rule of thumb is that devm_iio_device_register()
is always the last thing to call in problem.  The very rare exceptions need
a comment to explain why they are there...

Note that the devm_iio_trigger_register() also exposes the userspace part of the
trigger so if you allow that to be used by other drivers you'd need the irq registration
before that as well.

> +	if (ret)
> +		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(kx022a_probe_internal, KIONIX_ACCEL);
> +
> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> +MODULE_LICENSE("GPL");

Vaittinen, Matti Oct. 24, 2022, 5:44 a.m. UTC | #4

Good Morning Jonathan,

On 10/23/22 14:43, Jonathan Cameron wrote:
> On Fri, 21 Oct 2022 14:22:49 +0300
> Matti Vaittinen <[e-mail]> wrote:
> 
>> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
>> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
>> tap/motion detection, wake-up & back-to-sleep events, four acceleration
>> ranges (2, 4, 8 and 16g), and probably some other cool features.
>>
>> Add support for the basic accelerometer features such as getting the
>> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
>> using the WMI IRQ).
>>
>> Important things to be added include the double-tap, motion
>> detection and wake-up as well as the runtime power management.
>>
>> Signed-off-by: Matti Vaittinen <[e-mail]>
> 
> Hi Matti,
> 
> A few things from me on this read through to add to those last few comments
> from Andy.
> 
> Thanks,
> 
> Jonathan
> 
> 
>> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
>> new file mode 100644
>> index 000000000000..6510f8d62b85
>> --- /dev/null
>> +++ b/drivers/iio/accel/kionix-kx022a-i2c.c
>> @@ -0,0 +1,51 @@
> 
>> +
>> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
>> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_IMPORT_NS(KIONIX_ACCEL);
> Not sure why I didn't noticed this before, but after various debates we
> ended up prefixing namespaces to limit them to IIO. Also, there are other kionix
> drivers already and may be more in future. Better to limit this to scope of this
> driver,

Prefixing with IIO_ is all fine. And I was thinking of using the KX022A 
but ended up with KIONIX as I am pretty sure this same driver will be 
extended to cover the old 022 (without A) and probably also 122 (at 
least)...

...After that being said - I have been doing this same naming by 
part-name with ROHM PMIC drivers. As an example, first IC I was working 
with after jumping from Nokia to ROHM was BD71837. After the driver was 
written I encountered BD71847 - which was almost identical but just 
omitted few BUCKs. So, I renamed driver to BD718x7 and added the support.

As you can probably guess, this was not the end of the story. Next came 
BD71850 - which is pretty much the BD71847 with different OTP. I think I 
used some defines with BD718XX_...

...and later I did drivers for BD71815, BD71827, BD71828 ... - which all 
were completely different from the ones supported by bd718x7 driver.

What I learned was - either use the exact part name in file/defines 
(even when supporting many ICs) or use completely abstract name with no 
part name. It seems to be impossible to do correct wildcards unless you 
can dictate the IC naming :)

> 
> IIO_KX022A >
> seems like a reaonsable choice.
> 

Long babbling just to say: "Yes".
We can use IIO_KX022A even if we support many ICs. I don't plan to 
change the file/function/define naming when adding support to new ICs :)


>> +int kx022a_probe_internal(struct device *dev)
>> +{
>> +	static const char * const regulator_names[] = {"io-vdd", "vdd"};
>> +	struct iio_trigger *indio_trig;
>> +	struct fwnode_handle *fwnode;
>> +	struct kx022a_data *data;
>> +	struct regmap *regmap;
>> +	unsigned int chip_id;
>> +	struct iio_dev *idev;
>> +	int ret, irq;
>> +	char *name;
>> +
>> +	regmap = dev_get_regmap(dev, NULL);
>> +	if (!regmap) {
>> +		dev_err(dev, "no regmap\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	idev = devm_iio_device_alloc(dev, sizeof(*data));
>> +	if (!idev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(idev);
>> +
>> +	/*
>> +	 * VDD is the analog and digital domain voltage supply and
>> +	 * IO_VDD is the digital I/O voltage supply.
>> +	 */
>> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
>> +					     regulator_names);
>> +	if (ret && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret, "failed to enable regulator\n");
>> +
>> +	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
>> +	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;
>> +	}
>> +
>> +	fwnode = dev_fwnode(dev);
>> +	if (!fwnode)
>> +		return -ENODEV;
>> +
>> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
>> +	if (irq > 0) {
>> +		data->inc_reg = KX022A_REG_INC1;
>> +		data->ien_reg = KX022A_REG_INC4;
>> +
>> +		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
>> +			dev_warn(dev, "Only one IRQ supported\n");
> 
> Why?  If you get the both, only the first one will be used by the driver.
> Not really worth warning about the lack of features...

My thinking regarding developing new device went along the lines:

Precondition: The HW (and data-sheet) explain how there is two INT pins.
1. Board designer reads the data-sheet and uses both INT pins.
2. SW engineer finds the driver and reads the DT-binding description.
3. SW engineer writes the DT-description and hopes everything "just 
works". (Amount of hope is probably inversely proportional to the amount 
of experience XD).
4) SW engineer gives a first go to the sensor SW and notices everything 
does not just magically work.

I think there is important difference between following options:
4a) Log shows print "Only one IRQ supported" from the kx022a driver
4b) Log shows no errors.

The amount of work potentially avoided by the check here is probably 
again inversely proportional to the SW engineer's experience - but I 
think it may be significant. OTOH, the amount of problems caused by this 
check is (in my opinion) negligible. Well, the additional print string 
does eat up few bytes of space - and I know people/systems that do not 
like this. I am still tempted to say that those systems can opt to 
disable the config for this driver though.

Well, this is not a point I would like to start fighting over though. If 
this email does not convince the audience about usefulness of the print 
- then I'll just drop it from the v5 :)

>> +	ret = devm_iio_device_register(data->dev, idev);
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret,
>> +				     "Unable to register iio device\n");
>> +
>> +	/*
>> +	 * No need to check for NULL. request_threadedI_irq() defaults to
> Why I?
> 

NoI_idea(). I'll fix this, thanks! :)

>> +	 * dev_name() should the alloc fail.
>> +	 */
>> +	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s-kx022a",
>> +			      dev_name(data->dev));
>> +
>> +	ret = devm_request_threaded_irq(data->dev, irq, kx022a_irq_handler,
>> +					&kx022a_irq_thread_handler,
>> +					IRQF_ONESHOT, name, idev);
> 
> You are requesting the irq 'after' exposing the userspace interfaces.
> Technically that potentially introduces a race as we might in theory successfully
> trigger an interrupt before requesting it.
> 
> Obviously that would be challenging to pull off given likely short window, but
> good to close it anyway!  Rule of thumb is that devm_iio_device_register()
> is always the last thing to call in problem.  The very rare exceptions need
> a comment to explain why they are there...
> 
> Note that the devm_iio_trigger_register() also exposes the userspace part of the
> trigger so if you allow that to be used by other drivers you'd need the irq registration
> before that as well.
> 

Thanks! I did overlook this. In a few cases handling the IRQs use 
subsystem facilities established during driver registration. In those 
cases the dependencies and the race goes the other way around. Thus it 
is almost a habit for me to register IRQs as last thing and purge them 
as first thing.

I'll cook-up the v5 soon-ish :)

Yours
	-- Matti Vaittinen

Christophe JAILLET Oct. 24, 2022, 1:34 p.m. UTC | #5

Le 21/10/2022 à 13:22, Matti Vaittinen a écrit :
> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
> tap/motion detection, wake-up & back-to-sleep events, four acceleration
> ranges (2, 4, 8 and 16g), and probably some other cool features.
> 
> Add support for the basic accelerometer features such as getting the
> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
> using the WMI IRQ).
> 
> Important things to be added include the double-tap, motion
> detection and wake-up as well as the runtime power management.
> 
> Signed-off-by: Matti Vaittinen <[e-mail]>
> 
> ---

Hi, should there be a v5:

> +/*
> + * The KX022A has FIFO which can store 43 samples of HiRes data from 2

Nit: 2 here, but 3 the lines just before.

> + * channels. This equals to 43 (samples) * 3 (channels) * 2 (bytes/sample) to
> + * 258 bytes of sample data. The quirk to know is that the amount of bytes in
> + * the FIFO is advertised via 8 bit register (max value 255). The thing to note
> + * is that full 258 bytes of data is indicated using the max value 255.
> + */

Matti Vaittinen Oct. 24, 2022, 2:16 p.m. UTC | #6

Thanks for the review Christophe,

On 10/24/22 16:34, Christophe JAILLET wrote:
> Le 21/10/2022 à 13:22, Matti Vaittinen a écrit :
>> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
>> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
>> tap/motion detection, wake-up & back-to-sleep events, four acceleration
>> ranges (2, 4, 8 and 16g), and probably some other cool features.
>>
>> Add support for the basic accelerometer features such as getting the
>> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
>> using the WMI IRQ).
>>
>> Important things to be added include the double-tap, motion
>> detection and wake-up as well as the runtime power management.
>>
>> Signed-off-by: Matti Vaittinen 
>> <[e-mail]>
>>
>> ---
> 
> Hi, should there be a v5:

I did already send out the v5 shortly before your review. If I need to 
send v6 I'll apply your suggestions.


Yours
	-- Matti

Jonathan Cameron Oct. 29, 2022, 11:35 a.m. UTC | #7

On Mon, 24 Oct 2022 05:44:21 +0000
"Vaittinen, Matti" <[e-mail]> wrote:

> Good Morning Jonathan,
> 
> On 10/23/22 14:43, Jonathan Cameron wrote:
> > On Fri, 21 Oct 2022 14:22:49 +0300
> > Matti Vaittinen <[e-mail]> wrote:
> >   
> >> KX022A is a 3-axis accelerometer from ROHM/Kionix. The sensor features
> >> include variable ODRs, I2C and SPI control, FIFO/LIFO with watermark IRQ,
> >> tap/motion detection, wake-up & back-to-sleep events, four acceleration
> >> ranges (2, 4, 8 and 16g), and probably some other cool features.
> >>
> >> Add support for the basic accelerometer features such as getting the
> >> acceleration data via IIO. (raw reads, triggered buffer [data-ready] or
> >> using the WMI IRQ).
> >>
> >> Important things to be added include the double-tap, motion
> >> detection and wake-up as well as the runtime power management.
> >>
> >> Signed-off-by: Matti Vaittinen <[e-mail]>  
> > 
> > Hi Matti,
> > 
> > A few things from me on this read through to add to those last few comments
> > from Andy.
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> >> diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
> >> new file mode 100644
> >> index 000000000000..6510f8d62b85
> >> --- /dev/null
> >> +++ b/drivers/iio/accel/kionix-kx022a-i2c.c
> >> @@ -0,0 +1,51 @@  
> >   
> >> +
> >> +MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
> >> +MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_IMPORT_NS(KIONIX_ACCEL);  
> > Not sure why I didn't noticed this before, but after various debates we
> > ended up prefixing namespaces to limit them to IIO. Also, there are other kionix
> > drivers already and may be more in future. Better to limit this to scope of this
> > driver,  
> 
> Prefixing with IIO_ is all fine. And I was thinking of using the KX022A 
> but ended up with KIONIX as I am pretty sure this same driver will be 
> extended to cover the old 022 (without A) and probably also 122 (at 
> least)...
> 
> ...After that being said - I have been doing this same naming by 
> part-name with ROHM PMIC drivers. As an example, first IC I was working 
> with after jumping from Nokia to ROHM was BD71837. After the driver was 
> written I encountered BD71847 - which was almost identical but just 
> omitted few BUCKs. So, I renamed driver to BD718x7 and added the support.
> 
> As you can probably guess, this was not the end of the story. Next came 
> BD71850 - which is pretty much the BD71847 with different OTP. I think I 
> used some defines with BD718XX_...
> 
> ...and later I did drivers for BD71815, BD71827, BD71828 ... - which all 
> were completely different from the ones supported by bd718x7 driver.
> 
> What I learned was - either use the exact part name in file/defines 
> (even when supporting many ICs) or use completely abstract name with no 
> part name. It seems to be impossible to do correct wildcards unless you 
> can dictate the IC naming :)
Yup.  I'd stick to exact as event really generic ends up messy as it can
imply coverage of devices that are entirely separate.

> 
> > 
> > IIO_KX022A >
> > seems like a reaonsable choice.
> >   
> 
> Long babbling just to say: "Yes".
> We can use IIO_KX022A even if we support many ICs. I don't plan to 
> change the file/function/define naming when adding support to new ICs :)
> 

Absolutely.  We have lots of drivers like this where naming is after one
part but they support many different parts (in some cases even from different
manufacturers though that is rarer.)

> 
> >> +int kx022a_probe_internal(struct device *dev)
> >> +{
> >> +	static const char * const regulator_names[] = {"io-vdd", "vdd"};
> >> +	struct iio_trigger *indio_trig;
> >> +	struct fwnode_handle *fwnode;
> >> +	struct kx022a_data *data;
> >> +	struct regmap *regmap;
> >> +	unsigned int chip_id;
> >> +	struct iio_dev *idev;
> >> +	int ret, irq;
> >> +	char *name;
> >> +
> >> +	regmap = dev_get_regmap(dev, NULL);
> >> +	if (!regmap) {
> >> +		dev_err(dev, "no regmap\n");
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	idev = devm_iio_device_alloc(dev, sizeof(*data));
> >> +	if (!idev)
> >> +		return -ENOMEM;
> >> +
> >> +	data = iio_priv(idev);
> >> +
> >> +	/*
> >> +	 * VDD is the analog and digital domain voltage supply and
> >> +	 * IO_VDD is the digital I/O voltage supply.
> >> +	 */
> >> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
> >> +					     regulator_names);
> >> +	if (ret && ret != -ENODEV)
> >> +		return dev_err_probe(dev, ret, "failed to enable regulator\n");
> >> +
> >> +	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
> >> +	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;
> >> +	}
> >> +
> >> +	fwnode = dev_fwnode(dev);
> >> +	if (!fwnode)
> >> +		return -ENODEV;
> >> +
> >> +	irq = fwnode_irq_get_byname(fwnode, "INT1");
> >> +	if (irq > 0) {
> >> +		data->inc_reg = KX022A_REG_INC1;
> >> +		data->ien_reg = KX022A_REG_INC4;
> >> +
> >> +		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
> >> +			dev_warn(dev, "Only one IRQ supported\n");  
> > 
> > Why?  If you get the both, only the first one will be used by the driver.
> > Not really worth warning about the lack of features...  
> 
> My thinking regarding developing new device went along the lines:
> 
> Precondition: The HW (and data-sheet) explain how there is two INT pins.
> 1. Board designer reads the data-sheet and uses both INT pins.
> 2. SW engineer finds the driver and reads the DT-binding description.
> 3. SW engineer writes the DT-description and hopes everything "just 
> works". (Amount of hope is probably inversely proportional to the amount 
> of experience XD).
> 4) SW engineer gives a first go to the sensor SW and notices everything 
> does not just magically work.

Ah but it does "work". We simply don't use one of the IRQs.  That's not
normally a problem as there are lots of other features we don't fully
support.  Not using something is not normally considered a problem.

> 
> I think there is important difference between following options:
> 4a) Log shows print "Only one IRQ supported" from the kx022a driver
> 4b) Log shows no errors.
> 
> The amount of work potentially avoided by the check here is probably 
> again inversely proportional to the SW engineer's experience - but I 
> think it may be significant. OTOH, the amount of problems caused by this 
> check is (in my opinion) negligible. Well, the additional print string 
> does eat up few bytes of space - and I know people/systems that do not 
> like this. I am still tempted to say that those systems can opt to 
> disable the config for this driver though.
> 
> Well, this is not a point I would like to start fighting over though. If 
> this email does not convince the audience about usefulness of the print 
> - then I'll just drop it from the v5 :)

So far I'm not seeing anything that doesn't work because we only support
one IRQ.  There may be combinations of interrupts that are tricky to handle
on one IRQ line (or may not be supported at the same time on a single line)
but so far you don't support those anyway..  Adding a more informative warning
when adding those features would be reasonable

"Feature X not supported as only a single IRQ line available".

> 
> >> +	ret = devm_iio_device_register(data->dev, idev);
> >> +	if (ret < 0)
> >> +		return dev_err_probe(dev, ret,
> >> +				     "Unable to register iio device\n");
> >> +
> >> +	/*
> >> +	 * No need to check for NULL. request_threadedI_irq() defaults to  
> > Why I?
> >   
> 
> NoI_idea(). I'll fix this, thanks! :)
> 
> >> +	 * dev_name() should the alloc fail.
> >> +	 */
> >> +	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s-kx022a",
> >> +			      dev_name(data->dev));
> >> +
> >> +	ret = devm_request_threaded_irq(data->dev, irq, kx022a_irq_handler,
> >> +					&kx022a_irq_thread_handler,
> >> +					IRQF_ONESHOT, name, idev);  
> > 
> > You are requesting the irq 'after' exposing the userspace interfaces.
> > Technically that potentially introduces a race as we might in theory successfully
> > trigger an interrupt before requesting it.
> > 
> > Obviously that would be challenging to pull off given likely short window, but
> > good to close it anyway!  Rule of thumb is that devm_iio_device_register()
> > is always the last thing to call in problem.  The very rare exceptions need
> > a comment to explain why they are there...
> > 
> > Note that the devm_iio_trigger_register() also exposes the userspace part of the
> > trigger so if you allow that to be used by other drivers you'd need the irq registration
> > before that as well.
> >   
> 
> Thanks! I did overlook this. In a few cases handling the IRQs use 
> subsystem facilities established during driver registration. In those 
> cases the dependencies and the race goes the other way around. Thus it 
> is almost a habit for me to register IRQs as last thing and purge them 
> as first thing.

Hmm. If that's at the subsystem level and results in anything other than
an error that needs handling (not ready yet) then those are probably race
conditions that need fixing - unless the subsystem has some way of ensuring
the userspace interfaces aren't used until after the interrupts are registered
which would be unusual.

Ah well, care needed either way around.

Jonathan
> 
> I'll cook-up the v5 soon-ish :)
> 
> Yours
> 	-- Matti Vaittinen
>

Vaittinen, Matti Oct. 31, 2022, 6:25 a.m. UTC | #8

On 10/29/22 14:35, Jonathan Cameron wrote:
> On Mon, 24 Oct 2022 05:44:21 +0000
> "Vaittinen, Matti" <[e-mail]> wrote:
> 
>>>> +
>>>> +		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
>>>> +			dev_warn(dev, "Only one IRQ supported\n");
>>>
>>> Why?  If you get the both, only the first one will be used by the driver.
>>> Not really worth warning about the lack of features...
>>
>> My thinking regarding developing new device went along the lines:
>>
>> Precondition: The HW (and data-sheet) explain how there is two INT pins.
>> 1. Board designer reads the data-sheet and uses both INT pins.
>> 2. SW engineer finds the driver and reads the DT-binding description.
>> 3. SW engineer writes the DT-description and hopes everything "just
>> works". (Amount of hope is probably inversely proportional to the amount
>> of experience XD).
>> 4) SW engineer gives a first go to the sensor SW and notices everything
>> does not just magically work.
> 
> Ah but it does "work". We simply don't use one of the IRQs.  That's not
> normally a problem as there are lots of other features we don't fully
> support.  Not using something is not normally considered a problem.

I guess you're correct...

> So far I'm not seeing anything that doesn't work because we only support
> one IRQ.  There may be combinations of interrupts that are tricky to handle
> on one IRQ line (or may not be supported at the same time on a single line)
> but so far you don't support those anyway..  Adding a more informative warning
> when adding those features would be reasonable
> 
> "Feature X not supported as only a single IRQ line available".

... and I also fully agree with this. My thinking was stuck with each 
IRQ line having a purpose fixed by HW - which is not the case with this 
sensor.

Yours
	-- Matti

Patch

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index ffac66db7ac9..b7fd054819d2 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -409,6 +409,27 @@  config IIO_ST_ACCEL_SPI_3AXIS
 	  To compile this driver as a module, choose M here. The module
 	  will be called st_accel_spi.
 
+config IIO_KX022A
+	tristate
+
+config IIO_KX022A_SPI
+	tristate "Kionix KX022A tri-axis digital accelerometer"
+	depends on SPI
+	select IIO_KX022A
+	select REGMAP_SPI
+	help
+	  Enable support for the Kionix KX022A digital tri-axis
+	  accelerometer connected to I2C interface.
+
+config IIO_KX022A_I2C
+	tristate "Kionix KX022A tri-axis digital accelerometer"
+	depends on I2C
+	select IIO_KX022A
+	select REGMAP_I2C
+	help
+	  Enable support for the Kionix KX022A digital tri-axis
+	  accelerometer connected to I2C interface.
+
 config KXSD9
 	tristate "Kionix KXSD9 Accelerometer Driver"
 	select IIO_BUFFER
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 5e45b5fa5ab5..311ead9c3ef1 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -40,6 +40,9 @@  obj-$(CONFIG_FXLS8962AF)	+= fxls8962af-core.o
 obj-$(CONFIG_FXLS8962AF_I2C)	+= fxls8962af-i2c.o
 obj-$(CONFIG_FXLS8962AF_SPI)	+= fxls8962af-spi.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
+obj-$(CONFIG_IIO_KX022A)	+= kionix-kx022a.o
+obj-$(CONFIG_IIO_KX022A_I2C)	+= kionix-kx022a-i2c.o
+obj-$(CONFIG_IIO_KX022A_SPI)	+= kionix-kx022a-spi.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
 obj-$(CONFIG_KXSD9_SPI)	+= kxsd9-spi.o
diff --git a/drivers/iio/accel/kionix-kx022a-i2c.c b/drivers/iio/accel/kionix-kx022a-i2c.c
new file mode 100644
index 000000000000..6510f8d62b85
--- /dev/null
+++ b/drivers/iio/accel/kionix-kx022a-i2c.c
@@ -0,0 +1,51 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 ROHM Semiconductors
+ *
+ * ROHM/KIONIX KX022A accelerometer driver
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "kionix-kx022a.h"
+
+static int kx022a_i2c_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct regmap *regmap;
+
+	if (!i2c->irq) {
+		dev_err(dev, "No IRQ configured\n");
+		return -EINVAL;
+	}
+
+	regmap = devm_regmap_init_i2c(i2c, &kx022a_regmap);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				     "Failed to initialize Regmap\n");
+
+	return kx022a_probe_internal(dev);
+}
+
+static const struct of_device_id kx022a_of_match[] = {
+	{ .compatible = "kionix,kx022a", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, kx022a_of_match);
+
+static struct i2c_driver kx022a_i2c_driver = {
+	.driver = {
+		.name  = "kx022a-i2c",
+		.of_match_table = kx022a_of_match,
+	  },
+	.probe_new    = kx022a_i2c_probe,
+};
+module_i2c_driver(kx022a_i2c_driver);
+
+MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
+MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(KIONIX_ACCEL);
diff --git a/drivers/iio/accel/kionix-kx022a-spi.c b/drivers/iio/accel/kionix-kx022a-spi.c
new file mode 100644
index 000000000000..7fe3b0aba1fe
--- /dev/null
+++ b/drivers/iio/accel/kionix-kx022a-spi.c
@@ -0,0 +1,58 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 ROHM Semiconductors
+ *
+ * ROHM/KIONIX KX022A accelerometer driver
+ */
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "kionix-kx022a.h"
+
+static int kx022a_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct regmap *regmap;
+
+	if (!spi->irq) {
+		dev_err(dev, "No IRQ configured\n");
+		return -EINVAL;
+	}
+
+	regmap = devm_regmap_init_spi(spi, &kx022a_regmap);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				     "Failed to initialize Regmap\n");
+
+	return kx022a_probe_internal(dev);
+}
+
+static const struct spi_device_id kx022a_id[] = {
+	{ "kx022a" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, kx022a_id);
+
+static const struct of_device_id kx022a_of_match[] = {
+	{ .compatible = "kionix,kx022a", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, kx022a_of_match);
+
+static struct spi_driver kx022a_spi_driver = {
+	.driver = {
+		.name   = "kx022a-spi",
+		.of_match_table = kx022a_of_match,
+	},
+	.probe = kx022a_spi_probe,
+	.id_table = kx022a_id,
+};
+module_spi_driver(kx022a_spi_driver);
+
+MODULE_DESCRIPTION("ROHM/Kionix kx022A accelerometer driver");
+MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(KIONIX_ACCEL);
diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
new file mode 100644
index 000000000000..5a8622c8127b
--- /dev/null
+++ b/drivers/iio/accel/kionix-kx022a.c
@@ -0,0 +1,1145 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 ROHM Semiconductors
+ *
+ * ROHM/KIONIX KX022A accelerometer driver
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/string_helpers.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+#include "kionix-kx022a.h"
+
+/*
+ * The KX022A has FIFO which can store 43 samples of HiRes data from 2
+ * channels. This equals to 43 (samples) * 3 (channels) * 2 (bytes/sample) to
+ * 258 bytes of sample data. The quirk to know is that the amount of bytes in
+ * the FIFO is advertised via 8 bit register (max value 255). The thing to note
+ * is that full 258 bytes of data is indicated using the max value 255.
+ */
+#define KX022A_FIFO_LENGTH			43
+#define KX022A_FIFO_FULL_VALUE			255
+#define KX022A_SOFT_RESET_WAIT_TIME_US		(5 * USEC_PER_MSEC)
+#define KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US	(500 * USEC_PER_MSEC)
+
+/* 3 axis, 2 bytes of data for each of the axis */
+#define KX022A_FIFO_SAMPLES_SIZE_BYTES		6
+#define KX022A_FIFO_MAX_BYTES					\
+	(KX022A_FIFO_LENGTH * KX022A_FIFO_SAMPLES_SIZE_BYTES)
+
+enum {
+	KX022A_STATE_SAMPLE,
+	KX022A_STATE_FIFO,
+};
+
+/* Regmap configs */
+static const struct regmap_range kx022a_volatile_ranges[] = {
+	{
+		.range_min = KX022A_REG_XHP_L,
+		.range_max = KX022A_REG_COTR,
+	}, {
+		.range_min = KX022A_REG_TSCP,
+		.range_max = KX022A_REG_INT_REL,
+	}, {
+		/* The reset bit will be cleared by sensor */
+		.range_min = KX022A_REG_CNTL2,
+		.range_max = KX022A_REG_CNTL2,
+	}, {
+		.range_min = KX022A_REG_BUF_STATUS_1,
+		.range_max = KX022A_REG_BUF_READ,
+	},
+};
+
+static const struct regmap_access_table kx022a_volatile_regs = {
+	.yes_ranges = &kx022a_volatile_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(kx022a_volatile_ranges),
+};
+
+static const struct regmap_range kx022a_precious_ranges[] = {
+	{
+		.range_min = KX022A_REG_INT_REL,
+		.range_max = KX022A_REG_INT_REL,
+	},
+};
+
+static const struct regmap_access_table kx022a_precious_regs = {
+	.yes_ranges = &kx022a_precious_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(kx022a_precious_ranges),
+};
+
+/*
+ * The HW does not set WHO_AM_I reg as read-only but we don't want to write it
+ * so we still include it in the read-only ranges.
+ */
+static const struct regmap_range kx022a_read_only_ranges[] = {
+	{
+		.range_min = KX022A_REG_XHP_L,
+		.range_max = KX022A_REG_INT_REL,
+	}, {
+		.range_min = KX022A_REG_BUF_STATUS_1,
+		.range_max = KX022A_REG_BUF_STATUS_2,
+	}, {
+		.range_min = KX022A_REG_BUF_READ,
+		.range_max = KX022A_REG_BUF_READ,
+	},
+};
+
+static const struct regmap_access_table kx022a_ro_regs = {
+	.no_ranges = &kx022a_read_only_ranges[0],
+	.n_no_ranges = ARRAY_SIZE(kx022a_read_only_ranges),
+};
+
+static const struct regmap_range kx022a_write_only_ranges[] = {
+	{
+		.range_min = KX022A_REG_BTS_WUF_TH,
+		.range_max = KX022A_REG_BTS_WUF_TH,
+	}, {
+		.range_min = KX022A_REG_MAN_WAKE,
+		.range_max = KX022A_REG_MAN_WAKE,
+	}, {
+		.range_min = KX022A_REG_SELF_TEST,
+		.range_max = KX022A_REG_SELF_TEST,
+	}, {
+		.range_min = KX022A_REG_BUF_CLEAR,
+		.range_max = KX022A_REG_BUF_CLEAR,
+	},
+};
+
+static const struct regmap_access_table kx022a_wo_regs = {
+	.no_ranges = &kx022a_write_only_ranges[0],
+	.n_no_ranges = ARRAY_SIZE(kx022a_write_only_ranges),
+};
+
+static const struct regmap_range kx022a_noinc_read_ranges[] = {
+	{
+		.range_min = KX022A_REG_BUF_READ,
+		.range_max = KX022A_REG_BUF_READ,
+	},
+};
+
+static const struct regmap_access_table kx022a_nir_regs = {
+	.yes_ranges = &kx022a_noinc_read_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(kx022a_noinc_read_ranges),
+};
+
+const struct regmap_config kx022a_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &kx022a_volatile_regs,
+	.rd_table = &kx022a_wo_regs,
+	.wr_table = &kx022a_ro_regs,
+	.rd_noinc_table = &kx022a_nir_regs,
+	.precious_table = &kx022a_precious_regs,
+	.max_register = KX022A_MAX_REGISTER,
+	.cache_type = REGCACHE_RBTREE,
+};
+EXPORT_SYMBOL_NS_GPL(kx022a_regmap, KIONIX_ACCEL);
+
+struct kx022a_data {
+	struct regmap *regmap;
+	struct iio_trigger *trig;
+	struct device *dev;
+	struct iio_mount_matrix orientation;
+	int64_t timestamp, old_timestamp;
+
+	int irq;
+	int inc_reg;
+	int ien_reg;
+
+	unsigned int g_range;
+	unsigned int state;
+	unsigned int odr_ns;
+
+	bool trigger_enabled;
+	/*
+	 * Prevent toggling the sensor stby/active state (PC1 bit) in the
+	 * middle of a configuration, or when the fifo is enabled. Also,
+	 * protect the data stored/retrieved from this structure from
+	 * concurrent accesses.
+	 */
+	struct mutex mutex;
+	u8 watermark;
+
+	/* 3 x 16bit accel data + timestamp */
+	__le16 buffer[8] __aligned(IIO_DMA_MINALIGN);
+	struct {
+		__le16 channels[3];
+		s64 ts __aligned(8);
+	} scan;
+};
+
+static const struct iio_mount_matrix *
+kx022a_get_mount_matrix(const struct iio_dev *idev,
+			const struct iio_chan_spec *chan)
+{
+	struct kx022a_data *data = iio_priv(idev);
+
+	return &data->orientation;
+}
+
+enum {
+	AXIS_X,
+	AXIS_Y,
+	AXIS_Z,
+	AXIS_MAX
+};
+
+static const unsigned long kx022a_scan_masks[] = {
+	BIT(AXIS_X) | BIT(AXIS_Y) | BIT(AXIS_Z), 0
+};
+
+static const struct iio_chan_spec_ext_info kx022a_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_TYPE, kx022a_get_mount_matrix),
+	{ }
+};
+
+#define KX022A_ACCEL_CHAN(axis, index)						\
+{								\
+	.type = IIO_ACCEL,					\
+	.modified = 1,						\
+	.channel2 = IIO_MOD_##axis,				\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.info_mask_shared_by_type_available =			\
+				BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.ext_info = kx022a_ext_info,				\
+	.address = KX022A_REG_##axis##OUT_L,			\
+	.scan_index = index,					\
+	.scan_type = {                                          \
+		.sign = 's',					\
+		.realbits = 16,					\
+		.storagebits = 16,				\
+		.endianness = IIO_LE,				\
+	},							\
+}
+
+static const struct iio_chan_spec kx022a_channels[] = {
+	KX022A_ACCEL_CHAN(X, 0),
+	KX022A_ACCEL_CHAN(Y, 1),
+	KX022A_ACCEL_CHAN(Z, 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
+ * not available for higher sample rates. Thus, the driver only supports 200 Hz
+ * and slower ODRs. The slowest is 0.78 Hz.
+ */
+static const int kx022a_accel_samp_freq_table[][2] = {
+	{ 0, 780000 },
+	{ 1, 563000 },
+	{ 3, 125000 },
+	{ 6, 250000 },
+	{ 12, 500000 },
+	{ 25, 0 },
+	{ 50, 0 },
+	{ 100, 0 },
+	{ 200, 0 },
+};
+
+static const unsigned int kx022a_odrs[] = {
+	1282051282,
+	639795266,
+	320 * MEGA,
+	160 * MEGA,
+	80 * MEGA,
+	40 * MEGA,
+	20 * MEGA,
+	10 * MEGA,
+	5 * MEGA,
+};
+
+/*
+ * range is typically +-2G/4G/8G/16G, distributed over the amount of bits.
+ * The scale table can be calculated using
+ *	(range / 2^bits) * g = (range / 2^bits) * 9.80665 m/s^2
+ *	=> KX022A uses 16 bit (HiRes mode - assume the low 8 bits are zeroed
+ *	in low-power mode(?) )
+ *	=> +/-2G  => 4 / 2^16 * 9,80665 * 10^6 (to scale to micro)
+ *	=> +/-2G  - 598.550415
+ *	   +/-4G  - 1197.10083
+ *	   +/-8G  - 2394.20166
+ *	   +/-16G - 4788.40332
+ */
+static const int kx022a_scale_table[][2] = {
+	{ 598, 550415 },
+	{ 1197, 100830 },
+	{ 2394, 201660 },
+	{ 4788, 403320 },
+};
+
+static int kx022a_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = (const int *)kx022a_accel_samp_freq_table;
+		*length = ARRAY_SIZE(kx022a_accel_samp_freq_table) *
+			  ARRAY_SIZE(kx022a_accel_samp_freq_table[0]);
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)kx022a_scale_table;
+		*length = ARRAY_SIZE(kx022a_scale_table) *
+			  ARRAY_SIZE(kx022a_scale_table[0]);
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+#define KX022A_DEFAULT_PERIOD_NS (20 * NSEC_PER_MSEC)
+
+static void kx022a_reg2freq(unsigned int val,  int *val1, int *val2)
+{
+	*val1 = kx022a_accel_samp_freq_table[val & KX022A_MASK_ODR][0];
+	*val2 = kx022a_accel_samp_freq_table[val & KX022A_MASK_ODR][1];
+}
+
+static void kx022a_reg2scale(unsigned int val, unsigned int *val1,
+			     unsigned int *val2)
+{
+	val &= KX022A_MASK_GSEL;
+	val >>= KX022A_GSEL_SHIFT;
+
+	*val1 = kx022a_scale_table[val][0];
+	*val2 = kx022a_scale_table[val][1];
+}
+
+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,
+				      KX022A_MASK_PC1);
+	else
+		ret = regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
+					KX022A_MASK_PC1);
+	if (ret)
+		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)
+{
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = kx022a_turn_on_off_unlocked(data, false);
+	if (ret)
+		mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int kx022a_turn_on_unlock(struct kx022a_data *data)
+{
+	int ret;
+
+	ret = kx022a_turn_on_off_unlocked(data, true);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int kx022a_write_raw(struct iio_dev *idev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct kx022a_data *data = iio_priv(idev);
+	int ret, n;
+
+	/*
+	 * We should not allow changing scale or frequency when FIFO is running
+	 * as it will mess the timestamp/scale for samples existing in the
+	 * buffer. If this turns out to be an issue we can later change logic
+	 * to internally flush the fifo before reconfiguring so the samples in
+	 * fifo keep matching the freq/scale settings. (Such setup could cause
+	 * issues if users trust the watermark to be reached within known
+	 * time-limit).
+	 */
+	ret = iio_device_claim_direct_mode(idev);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		n = ARRAY_SIZE(kx022a_accel_samp_freq_table);
+
+		while (n--)
+			if (val == kx022a_accel_samp_freq_table[n][0] &&
+			    val2 == kx022a_accel_samp_freq_table[n][1])
+				break;
+		if (n < 0) {
+			ret = -EINVAL;
+			goto unlock_out;
+		}
+		ret = kx022a_turn_off_lock(data);
+		if (ret)
+			break;
+
+		ret = regmap_update_bits(data->regmap,
+					 KX022A_REG_ODCNTL,
+					 KX022A_MASK_ODR, n);
+		data->odr_ns = kx022a_odrs[n];
+		kx022a_turn_on_unlock(data);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		n = ARRAY_SIZE(kx022a_scale_table);
+
+		while (n-- > 0)
+			if (val == kx022a_scale_table[n][0] &&
+			    val2 == kx022a_scale_table[n][1])
+				break;
+		if (n < 0) {
+			ret = -EINVAL;
+			goto unlock_out;
+		}
+
+		ret = kx022a_turn_off_lock(data);
+		if (ret)
+			break;
+
+		ret = regmap_update_bits(data->regmap, KX022A_REG_CNTL,
+					 KX022A_MASK_GSEL,
+					 n << KX022A_GSEL_SHIFT);
+		kx022a_turn_on_unlock(data);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+unlock_out:
+	iio_device_release_direct_mode(idev);
+
+	return ret;
+}
+
+static int kx022a_fifo_set_wmi(struct kx022a_data *data)
+{
+	u8 threshold;
+
+	threshold = data->watermark;
+
+	return regmap_update_bits(data->regmap, KX022A_REG_BUF_CNTL1,
+				  KX022A_MASK_WM_TH, threshold);
+}
+
+static int kx022a_get_axis(struct kx022a_data *data,
+			   struct iio_chan_spec const *chan,
+			   int *val)
+{
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, chan->address, &data->buffer[0],
+			       sizeof(__le16));
+	if (ret)
+		return ret;
+
+	*val = le16_to_cpu(data->buffer[0]);
+
+	return IIO_VAL_INT;
+}
+
+static int kx022a_read_raw(struct iio_dev *idev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct kx022a_data *data = iio_priv(idev);
+	unsigned int regval;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(idev);
+		if (ret)
+			return ret;
+
+		mutex_lock(&data->mutex);
+		ret = kx022a_get_axis(data, chan, val);
+		mutex_unlock(&data->mutex);
+
+		iio_device_release_direct_mode(idev);
+
+		return ret;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(data->regmap, KX022A_REG_ODCNTL, &regval);
+		if (ret)
+			return ret;
+
+		if ((regval & KX022A_MASK_ODR) >
+		    ARRAY_SIZE(kx022a_accel_samp_freq_table)) {
+			dev_err(data->dev, "Invalid ODR\n");
+			return -EINVAL;
+		}
+
+		kx022a_reg2freq(regval, val, val2);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regmap_read(data->regmap, KX022A_REG_CNTL, &regval);
+		if (ret < 0)
+			return ret;
+
+		kx022a_reg2scale(regval, val, val2);
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	return -EINVAL;
+};
+
+static int kx022a_validate_trigger(struct iio_dev *idev,
+				   struct iio_trigger *trig)
+{
+	struct kx022a_data *data = iio_priv(idev);
+
+	if (data->trig != trig)
+		return -EINVAL;
+
+	return 0;
+}
+
+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;
+
+	mutex_lock(&data->mutex);
+	data->watermark = val;
+	mutex_unlock(&data->mutex);
+
+	return 0;
+}
+
+static ssize_t hwfifo_enabled_show(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct iio_dev *idev = dev_to_iio_dev(dev);
+	struct kx022a_data *data = iio_priv(idev);
+	bool state;
+
+	mutex_lock(&data->mutex);
+	state = data->state;
+	mutex_unlock(&data->mutex);
+
+	return sysfs_emit(buf, "%d\n", state);
+}
+
+static ssize_t hwfifo_watermark_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	struct iio_dev *idev = dev_to_iio_dev(dev);
+	struct kx022a_data *data = iio_priv(idev);
+	int wm;
+
+	mutex_lock(&data->mutex);
+	wm = data->watermark;
+	mutex_unlock(&data->mutex);
+
+	return sysfs_emit(buf, "%d\n", wm);
+}
+
+static IIO_DEVICE_ATTR_RO(hwfifo_enabled, 0);
+static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
+
+static const struct attribute *kx022a_fifo_attributes[] = {
+	&iio_dev_attr_hwfifo_watermark.dev_attr.attr,
+	&iio_dev_attr_hwfifo_enabled.dev_attr.attr,
+	NULL
+};
+
+static int kx022a_drop_fifo_contents(struct kx022a_data *data)
+{
+	/*
+	 * We must clear the old time-stamp to avoid computing the timestamps
+	 * based on samples acquired when buffer was last enabled.
+	 *
+	 * We don't need to protect the timestamp as long as we are only
+	 * called from fifo-disable where we can guarantee the sensor is not
+	 * triggering interrupts and where the mutex is locked to prevent the
+	 * user-space access.
+	 */
+	data->timestamp = 0;
+
+	return regmap_write(data->regmap, KX022A_REG_BUF_CLEAR, 0x0);
+}
+
+static int __kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples,
+			       bool irq)
+{
+	struct kx022a_data *data = iio_priv(idev);
+	struct device *dev = regmap_get_device(data->regmap);
+	u16 buffer[KX022A_FIFO_LENGTH * 3];
+	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;
+
+	if (fifo_bytes % KX022A_FIFO_SAMPLES_SIZE_BYTES)
+		dev_warn(data->dev, "Bad FIFO alignment. Data may be corrupt\n");
+
+	count = fifo_bytes / KX022A_FIFO_SAMPLES_SIZE_BYTES;
+	if (!count)
+		return 0;
+
+	/*
+	 * If we are being called from IRQ handler we know the stored timestamp
+	 * is fairly accurate for the last stored sample. Otherwise, if we are
+	 * called as a result of a read operation from userspace and hence
+	 * before the watermark interrupt was triggered, take a timestamp
+	 * now. We can fall anywhere in between two samples so the error in this
+	 * case is at most one sample period.
+	 */
+	if (!irq) {
+		/*
+		 * We need to have the IRQ disabled or we risk of messing-up
+		 * the timestamps. If we are ran from IRQ, then the
+		 * IRQF_ONESHOT has us covered - but if we are ran by the
+		 * user-space read we need to disable the IRQ to be on a safe
+		 * side. We do this usng synchronous disable so that if the
+		 * IRQ thread is being ran on other CPU we wait for it to be
+		 * finished.
+		 */
+		disable_irq(data->irq);
+		renable = true;
+
+		data->old_timestamp = data->timestamp;
+		data->timestamp = iio_get_time_ns(idev);
+	}
+
+	/*
+	 * Approximate timestamps for each of the sample based on the sampling
+	 * frequency, timestamp for last sample and number of samples.
+	 *
+	 * We'd better not use the current bandwidth settings to compute the
+	 * sample period. The real sample rate varies with the device and
+	 * small variation adds when we store a large number of samples.
+	 *
+	 * To avoid this issue we compute the actual sample period ourselves
+	 * based on the timestamp delta between the last two flush operations.
+	 */
+	if (data->old_timestamp) {
+		sample_period = data->timestamp - data->old_timestamp;
+		do_div(sample_period, count);
+	} else {
+		sample_period = data->odr_ns;
+	}
+	tstamp = data->timestamp - (count - 1) * sample_period;
+
+	if (samples && count > samples) {
+		/*
+		 * Here we leave some old samples to the buffer. We need to
+		 * adjust the timestamp to match the first sample in the buffer
+		 * or we will miscalculate the sample_period at next round.
+		 */
+		data->timestamp -= (count - samples) * sample_period;
+		count = samples;
+	}
+
+	fifo_bytes = count * KX022A_FIFO_SAMPLES_SIZE_BYTES;
+	ret = regmap_noinc_read(data->regmap, KX022A_REG_BUF_READ,
+				buffer, fifo_bytes);
+	if (ret)
+		goto renable_out;
+
+	for (i = 0; i < count; i++) {
+		u16 *sam = &buffer[i * 3];
+		__le16 *chs;
+		int bit;
+
+		chs = &data->scan.channels[0];
+		for_each_set_bit(bit, idev->active_scan_mask, AXIS_MAX)
+			memcpy(&chs[bit], &sam[bit], sizeof(*chs));
+
+		iio_push_to_buffers_with_timestamp(idev, &data->scan, tstamp);
+
+		tstamp += sample_period;
+	}
+
+	ret = count;
+
+renable_out:
+	if (renable)
+		enable_irq(data->irq);
+
+	return ret;
+}
+
+static int kx022a_fifo_flush(struct iio_dev *idev, unsigned int samples)
+{
+	struct kx022a_data *data = iio_priv(idev);
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = __kx022a_fifo_flush(idev, samples, false);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static const struct iio_info kx022a_info = {
+	.read_raw = &kx022a_read_raw,
+	.write_raw = &kx022a_write_raw,
+	.read_avail = &kx022a_read_avail,
+
+	.validate_trigger	= kx022a_validate_trigger,
+	.hwfifo_set_watermark	= kx022a_set_watermark,
+	.hwfifo_flush_to_buffer	= kx022a_fifo_flush,
+};
+
+static int kx022a_set_drdy_irq(struct kx022a_data *data, bool en)
+{
+	if (en)
+		return regmap_set_bits(data->regmap, KX022A_REG_CNTL,
+				       KX022A_MASK_DRDY);
+
+	return regmap_clear_bits(data->regmap, KX022A_REG_CNTL,
+				 KX022A_MASK_DRDY);
+}
+
+static int kx022a_prepare_irq_pin(struct kx022a_data *data)
+{
+	/* Enable IRQ1 pin. Set polarity to active low */
+	int mask = KX022A_MASK_IEN | KX022A_MASK_IPOL |
+		   KX022A_MASK_ITYP;
+	int val = KX022A_MASK_IEN | KX022A_IPOL_LOW |
+		  KX022A_ITYP_LEVEL;
+	int ret;
+
+	ret = regmap_update_bits(data->regmap, data->inc_reg, mask, val);
+	if (ret)
+		return ret;
+
+	/* We enable WMI to IRQ pin only at buffer_enable */
+	mask = KX022A_MASK_INS2_DRDY;
+
+	return regmap_set_bits(data->regmap, data->ien_reg, mask);
+}
+
+static int kx022a_fifo_disable(struct kx022a_data *data)
+{
+	int ret = 0;
+
+	ret = kx022a_turn_off_lock(data);
+	if (ret)
+		return ret;
+
+	ret = regmap_clear_bits(data->regmap, data->ien_reg, KX022A_MASK_WMI);
+	if (ret)
+		goto unlock_out;
+
+	ret = regmap_clear_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+				KX022A_MASK_BUF_EN);
+	if (ret)
+		goto unlock_out;
+
+	data->state &= (~KX022A_STATE_FIFO);
+
+	kx022a_drop_fifo_contents(data);
+
+	return kx022a_turn_on_unlock(data);
+
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int kx022a_buffer_predisable(struct iio_dev *idev)
+{
+	struct kx022a_data *data = iio_priv(idev);
+
+	if (iio_device_get_current_mode(idev) == INDIO_BUFFER_TRIGGERED)
+		return 0;
+
+	return kx022a_fifo_disable(data);
+}
+
+static int kx022a_fifo_enable(struct kx022a_data *data)
+{
+	int ret;
+
+	ret = kx022a_turn_off_lock(data);
+	if (ret)
+		return ret;
+
+	/* Update watermark to HW */
+	ret = kx022a_fifo_set_wmi(data);
+	if (ret)
+		goto unlock_out;
+
+	/* Enable buffer */
+	ret = regmap_set_bits(data->regmap, KX022A_REG_BUF_CNTL2,
+			      KX022A_MASK_BUF_EN);
+	if (ret)
+		goto unlock_out;
+
+	data->state |= KX022A_STATE_FIFO;
+	ret = regmap_set_bits(data->regmap, data->ien_reg,
+			      KX022A_MASK_WMI);
+	if (ret)
+		goto unlock_out;
+
+	return kx022a_turn_on_unlock(data);
+
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int kx022a_buffer_postenable(struct iio_dev *idev)
+{
+	struct kx022a_data *data = iio_priv(idev);
+
+	/*
+	 * If we use data-ready trigger, then the IRQ masks should be handled by
+	 * trigger enable and the hardware buffer is not used but we just update
+	 * results to the IIO fifo when data-ready triggers.
+	 */
+	if (iio_device_get_current_mode(idev) == INDIO_BUFFER_TRIGGERED)
+		return 0;
+
+	return kx022a_fifo_enable(data);
+}
+
+static const struct iio_buffer_setup_ops kx022a_buffer_ops = {
+	.postenable = kx022a_buffer_postenable,
+	.predisable = kx022a_buffer_predisable,
+};
+
+static irqreturn_t kx022a_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *idev = pf->indio_dev;
+	struct kx022a_data *data = iio_priv(idev);
+	int ret;
+
+	ret = regmap_bulk_read(data->regmap, KX022A_REG_XOUT_L, data->buffer,
+			       KX022A_FIFO_SAMPLES_SIZE_BYTES);
+	if (ret < 0)
+		goto err_read;
+
+	iio_push_to_buffers_with_timestamp(idev, data->buffer, pf->timestamp);
+err_read:
+	iio_trigger_notify_done(idev->trig);
+
+	return IRQ_HANDLED;
+}
+
+/* Get timestamps and wake the thread if we need to read data */
+static irqreturn_t kx022a_irq_handler(int irq, void *private)
+{
+	struct iio_dev *idev = private;
+	struct kx022a_data *data = iio_priv(idev);
+
+	data->old_timestamp = data->timestamp;
+	data->timestamp = iio_get_time_ns(idev);
+
+	if (data->state & KX022A_STATE_FIFO || data->trigger_enabled)
+		return IRQ_WAKE_THREAD;
+
+	return IRQ_NONE;
+}
+
+/*
+ * WMI and data-ready IRQs are acked when results are read. If we add
+ * TILT/WAKE or other IRQs - then we may need to implement the acking
+ * (which is racy).
+ */
+static irqreturn_t kx022a_irq_thread_handler(int irq, void *private)
+{
+	struct iio_dev *idev = private;
+	struct kx022a_data *data = iio_priv(idev);
+	int ret = IRQ_NONE;
+
+	mutex_lock(&data->mutex);
+
+	if (data->trigger_enabled) {
+		iio_trigger_poll_chained(data->trig);
+		ret = IRQ_HANDLED;
+	}
+
+	if (data->state & KX022A_STATE_FIFO) {
+		int ok;
+
+		ok = __kx022a_fifo_flush(idev, KX022A_FIFO_LENGTH, true);
+		if (ok > 0)
+			ret = IRQ_HANDLED;
+	}
+
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int kx022a_trigger_set_state(struct iio_trigger *trig,
+				    bool state)
+{
+	struct kx022a_data *data = iio_trigger_get_drvdata(trig);
+	int ret = 0;
+
+	mutex_lock(&data->mutex);
+
+	if (data->trigger_enabled == state)
+		goto unlock_out;
+
+	if (data->state & KX022A_STATE_FIFO) {
+		dev_warn(data->dev, "Can't set trigger when FIFO enabled\n");
+		ret = -EBUSY;
+		goto unlock_out;
+	}
+
+	ret = kx022a_turn_on_off_unlocked(data, false);
+	if (ret)
+		goto unlock_out;
+
+	data->trigger_enabled = state;
+	ret = kx022a_set_drdy_irq(data, state);
+	if (ret)
+		goto unlock_out;
+
+	ret = kx022a_turn_on_off_unlocked(data, true);
+
+unlock_out:
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static const struct iio_trigger_ops kx022a_trigger_ops = {
+	.set_trigger_state = kx022a_trigger_set_state,
+};
+
+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);
+	if (ret)
+		return ret;
+
+	/*
+	 * I've seen I2C read failures if we poll too fast after the sensor
+	 * reset. Slight delay gives I2C block the time to recover.
+	 */
+	msleep(1);
+
+	ret = regmap_read_poll_timeout(data->regmap, KX022A_REG_CNTL2, val,
+				       !(val & KX022A_MASK_SRST),
+				       KX022A_SOFT_RESET_WAIT_TIME_US,
+				       KX022A_SOFT_RESET_TOTAL_WAIT_TIME_US);
+	if (ret) {
+		dev_err(data->dev, "Sensor reset %s\n",
+			val & KX022A_MASK_SRST ? "timeout" : "fail#");
+		return ret;
+	}
+
+	ret = regmap_reinit_cache(data->regmap, &kx022a_regmap);
+	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,
+			      KX022A_MASK_BRES16);
+	if (ret) {
+		dev_err(data->dev, "Failed to set data resolution\n");
+		return ret;
+	}
+
+	return kx022a_prepare_irq_pin(data);
+}
+
+int kx022a_probe_internal(struct device *dev)
+{
+	static const char * const regulator_names[] = {"io-vdd", "vdd"};
+	struct iio_trigger *indio_trig;
+	struct fwnode_handle *fwnode;
+	struct kx022a_data *data;
+	struct regmap *regmap;
+	unsigned int chip_id;
+	struct iio_dev *idev;
+	int ret, irq;
+	char *name;
+
+	regmap = dev_get_regmap(dev, NULL);
+	if (!regmap) {
+		dev_err(dev, "no regmap\n");
+		return -EINVAL;
+	}
+
+	idev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!idev)
+		return -ENOMEM;
+
+	data = iio_priv(idev);
+
+	/*
+	 * VDD is the analog and digital domain voltage supply and
+	 * IO_VDD is the digital I/O voltage supply.
+	 */
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulator_names),
+					     regulator_names);
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "failed to enable regulator\n");
+
+	ret = regmap_read(regmap, KX022A_REG_WHO, &chip_id);
+	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;
+	}
+
+	fwnode = dev_fwnode(dev);
+	if (!fwnode)
+		return -ENODEV;
+
+	irq = fwnode_irq_get_byname(fwnode, "INT1");
+	if (irq > 0) {
+		data->inc_reg = KX022A_REG_INC1;
+		data->ien_reg = KX022A_REG_INC4;
+
+		if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
+			dev_warn(dev, "Only one IRQ supported\n");
+	} 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->regmap = regmap;
+	data->dev = dev;
+	data->irq = irq;
+	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->info = &kx022a_info;
+	idev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+	idev->available_scan_masks = kx022a_scan_masks;
+
+	/* Read the mounting matrix, if present */
+	ret = iio_read_mount_matrix(dev, &data->orientation);
+	if (ret)
+		return ret;
+
+	/* The sensor must be turned off for configuration */
+	ret = kx022a_turn_off_lock(data);
+	if (ret)
+		return ret;
+
+	ret = kx022a_chip_init(data);
+	if (ret) {
+		mutex_unlock(&data->mutex);
+		return ret;
+	}
+
+	ret = kx022a_turn_on_unlock(data);
+	if (ret)
+		return ret;
+
+	udelay(100);
+	ret = devm_iio_triggered_buffer_setup_ext(dev, idev,
+						  &iio_pollfunc_store_time,
+						  kx022a_trigger_handler,
+						  IIO_BUFFER_DIRECTION_IN,
+						  &kx022a_buffer_ops,
+						  kx022a_fifo_attributes);
+
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "iio_triggered_buffer_setup_ext FAIL\n");
+	indio_trig = devm_iio_trigger_alloc(dev, "%sdata-rdy-dev%d", idev->name,
+					    iio_device_id(idev));
+	if (!indio_trig)
+		return -ENOMEM;
+
+	data->trig = indio_trig;
+
+	indio_trig->ops = &kx022a_trigger_ops;
+	iio_trigger_set_drvdata(indio_trig, data);
+
+	ret = devm_iio_trigger_register(dev, indio_trig);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "Trigger registration failed\n");
+
+	ret = devm_iio_device_register(data->dev, idev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "Unable to register iio device\n");
+
+	/*
+	 * No need to check for NULL. request_threadedI_irq() defaults to
+	 * dev_name() should the alloc fail.
+	 */
+	name = devm_kasprintf(data->dev, GFP_KERNEL, "%s-kx022a",
+			      dev_name(data->dev));
+
+	ret = devm_request_threaded_irq(data->dev, irq, kx022a_irq_handler,
+					&kx022a_irq_thread_handler,
+					IRQF_ONESHOT, name, idev);
+	if (ret)
+		return dev_err_probe(data->dev, ret, "Could not request IRQ\n");
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(kx022a_probe_internal, KIONIX_ACCEL);
+
+MODULE_DESCRIPTION("ROHM/Kionix KX022A accelerometer driver");
+MODULE_AUTHOR("Matti Vaittinen <[e-mail]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/accel/kionix-kx022a.h b/drivers/iio/accel/kionix-kx022a.h
new file mode 100644
index 000000000000..12424649d438
--- /dev/null
+++ b/drivers/iio/accel/kionix-kx022a.h
@@ -0,0 +1,82 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2022 ROHM Semiconductors
+ *
+ * ROHM/KIONIX KX022A accelerometer driver
+ */
+
+#ifndef _KX022A_H_
+#define _KX022A_H_
+
+#include <linux/bits.h>
+#include <linux/regmap.h>
+
+#define KX022A_REG_WHO		0x0f
+#define KX022A_ID		0xc8
+
+#define KX022A_REG_CNTL2	0x19
+#define KX022A_MASK_SRST	BIT(7)
+#define KX022A_REG_CNTL		0x18
+#define KX022A_MASK_PC1		BIT(7)
+#define KX022A_MASK_RES		BIT(6)
+#define KX022A_MASK_DRDY	BIT(5)
+#define KX022A_MASK_GSEL	GENMASK(4, 3)
+#define KX022A_GSEL_SHIFT	3
+#define KX022A_GSEL_2		0x0
+#define KX022A_GSEL_4		BIT(3)
+#define KX022A_GSEL_8		BIT(4)
+#define KX022A_GSEL_16		GENMASK(4, 3)
+
+#define KX022A_REG_INS2		0x13
+#define KX022A_MASK_INS2_DRDY	BIT(4)
+#define KX122_MASK_INS2_WMI	BIT(5)
+
+#define KX022A_REG_XHP_L	0x0
+#define KX022A_REG_XOUT_L	0x06
+#define KX022A_REG_YOUT_L	0x08
+#define KX022A_REG_ZOUT_L	0x0a
+#define KX022A_REG_COTR		0x0c
+#define KX022A_REG_TSCP		0x10
+#define KX022A_REG_INT_REL	0x17
+
+#define KX022A_REG_ODCNTL	0x1b
+
+#define KX022A_REG_BTS_WUF_TH	0x31
+#define KX022A_REG_MAN_WAKE	0x2c
+
+#define KX022A_REG_BUF_CNTL1	0x3a
+#define KX022A_MASK_WM_TH	GENMASK(6, 0)
+#define KX022A_REG_BUF_CNTL2	0x3b
+#define KX022A_MASK_BUF_EN	BIT(7)
+#define KX022A_MASK_BRES16	BIT(6)
+#define KX022A_REG_BUF_STATUS_1	0x3c
+#define KX022A_REG_BUF_STATUS_2	0x3d
+#define KX022A_REG_BUF_CLEAR	0x3e
+#define KX022A_REG_BUF_READ	0x3f
+#define KX022A_MASK_ODR		GENMASK(3, 0)
+#define KX022A_ODR_SHIFT	3
+#define KX022A_FIFO_MAX_WMI_TH	41
+
+#define KX022A_REG_INC1		0x1c
+#define KX022A_REG_INC5		0x20
+#define KX022A_REG_INC6		0x21
+#define KX022A_MASK_IEN		BIT(5)
+#define KX022A_MASK_IPOL	BIT(4)
+#define KX022A_IPOL_LOW		0
+#define KX022A_IPOL_HIGH	KX022A_MASK_IPOL1
+#define KX022A_MASK_ITYP	BIT(3)
+#define KX022A_ITYP_PULSE	KX022A_MASK_ITYP
+#define KX022A_ITYP_LEVEL	0
+
+#define KX022A_REG_INC4		0x1f
+#define KX022A_MASK_WMI		BIT(5)
+
+#define KX022A_REG_SELF_TEST	0x60
+#define KX022A_MAX_REGISTER	0x60
+
+struct device;
+
+int kx022a_probe_internal(struct device *dev);
+extern const struct regmap_config kx022a_regmap;
+
+#endif