[PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Stephan Gerhold]

KX023-1025 [1] is another accelerometer from Kionix that has lots
of additional functionality compared to KXCJK-1013. It combines the
motion interrupt functionality from KXCJK with the tap detection
from KXTF9, plus a lot more other functionality.

This patch set does not add support for any of the extra functionality,
but makes basic functionality work with the existing kxcjk-1013 driver.

At first, the register map for the KX023-1025 seems quite different
from the other accelerometers supported by the kxcjk-1013.
However, it turns out that at least most of the register bits
still mean the same for KX023-1025.

This patch set refactors the kxcjk-1013 driver a little bit
to get the register addresses from a chip-specific struct.
The register bits can be re-used for all the different chips.

The KX023-1025 is used in several smartphones from Huawei.
I tested these changes on a Huawei Ascend G7, someone else reported
they also work fine on the Huawei Honor 5X (codename "kiwi").

[1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf

Stephan Gerhold (3):
  dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025
  iio: accel: kxcjk-1013: Refactor configuration registers into struct
  iio: accel: kxcjk-1013: Add support for KX023-1025

 .../bindings/iio/accel/kionix,kxcjk1013.yaml  |   1 +
 drivers/iio/accel/kxcjk-1013.c                | 217 ++++++++++++++----
 2 files changed, 176 insertions(+), 42 deletions(-)

-- 
2.31.1

[PATCH 1/3] dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025

[Stephan Gerhold]

The KX023-1025 accelerometer uses similar register bits as kxcjk1023,
so it can make use of the same driver. Document the new kionix,kx023-1025
compatible that is also supported by the kxcjk-1013 driver now.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../devicetree/bindings/iio/accel/kionix,kxcjk1013.yaml          | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/kionix,kxcjk1013.yaml b/Documentation/devicetree/bindings/iio/accel/kionix,kxcjk1013.yaml
index fbb714431e3d..52fa0f7c2d0e 100644
--- a/Documentation/devicetree/bindings/iio/accel/kionix,kxcjk1013.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/kionix,kxcjk1013.yaml
@@ -16,6 +16,7 @@ properties:
       - kionix,kxcj91008
       - kionix,kxtj21009
       - kionix,kxtf9
+      - kionix,kx023-1025
 
   reg:
     maxItems: 1
-- 
2.31.1

[PATCH 2/3] iio: accel: kxcjk-1013: Refactor configuration registers into struct

[Stephan Gerhold]

Most Kionix accelerometers seem to use fairly consistent register bits,
but the register addresses are not necessarily the same. This is already
partially the case for the KXTF9 (added in commit 1540d0106bcb
("iio: accel: kxcjk1013: add support for KXTF9")), which has some
registers at different addresses.

However, it's even much worse for the KX023-1025. All register bits
used by the kxcjk-1013 driver seem to be fully compatible with KX023,
but it has most registers at totally different addresses.

In preparation to add support for KX023-1025, move the fixed register
addresses into a struct so we can change them for KX023 more easily.

Cc: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/iio/accel/kxcjk-1013.c | 124 ++++++++++++++++++++++-----------
 1 file changed, 82 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 283e6a3feffc..e630419a11d8 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -51,13 +51,15 @@
 #define KXTF9_REG_TILT_POS_CUR		0x10
 #define KXTF9_REG_TILT_POS_PREV		0x11
 #define KXTF9_REG_INT_SRC1		0x15
-#define KXCJK1013_REG_INT_SRC1		0x16	/* compatible, but called INT_SRC2 in KXTF9 ds */
+#define KXTF9_REG_INT_SRC2		0x16
+#define KXCJK1013_REG_INT_SRC1		0x16
 #define KXCJK1013_REG_INT_SRC2		0x17
 #define KXCJK1013_REG_STATUS_REG	0x18
 #define KXCJK1013_REG_INT_REL		0x1A
 #define KXCJK1013_REG_CTRL1		0x1B
 #define KXTF9_REG_CTRL2			0x1C
-#define KXCJK1013_REG_CTRL2		0x1D	/* mostly compatible, CTRL_REG3 in KTXF9 ds */
+#define KXTF9_REG_CTRL3			0x1D
+#define KXCJK1013_REG_CTRL2		0x1D
 #define KXCJK1013_REG_INT_CTRL1		0x1E
 #define KXCJK1013_REG_INT_CTRL2		0x1F
 #define KXTF9_REG_INT_CTRL3		0x20
@@ -133,6 +135,43 @@ enum kx_acpi_type {
 	ACPI_KIOX010A,
 };
 
+struct kx_chipset_regs {
+	u8 int_src1;
+	u8 int_src2;
+	u8 int_rel;
+	u8 ctrl1;
+	u8 wuf_ctrl;
+	u8 int_ctrl1;
+	u8 data_ctrl;
+	u8 wake_timer;
+	u8 wake_thres;
+};
+
+const struct kx_chipset_regs kxcjk1013_regs = {
+	.int_src1	= KXCJK1013_REG_INT_SRC1,
+	.int_src2	= KXCJK1013_REG_INT_SRC2,
+	.int_rel	= KXCJK1013_REG_INT_REL,
+	.ctrl1		= KXCJK1013_REG_CTRL1,
+	.wuf_ctrl	= KXCJK1013_REG_CTRL2,
+	.int_ctrl1	= KXCJK1013_REG_INT_CTRL1,
+	.data_ctrl	= KXCJK1013_REG_DATA_CTRL,
+	.wake_timer	= KXCJK1013_REG_WAKE_TIMER,
+	.wake_thres	= KXCJK1013_REG_WAKE_THRES,
+};
+
+const struct kx_chipset_regs kxtf9_regs = {
+	/* .int_src1 was moved to INT_SRC2 on KXTF9 */
+	.int_src1	= KXTF9_REG_INT_SRC2,
+	/* .int_src2 is not available */
+	.int_rel	= KXCJK1013_REG_INT_REL,
+	.ctrl1		= KXCJK1013_REG_CTRL1,
+	.wuf_ctrl	= KXTF9_REG_CTRL3,
+	.int_ctrl1	= KXCJK1013_REG_INT_CTRL1,
+	.data_ctrl	= KXCJK1013_REG_DATA_CTRL,
+	.wake_timer	= KXCJK1013_REG_WAKE_TIMER,
+	.wake_thres	= KXTF9_REG_WAKE_THRESH,
+};
+
 struct kxcjk1013_data {
 	struct regulator_bulk_data regulators[2];
 	struct i2c_client *client;
@@ -152,6 +191,7 @@ struct kxcjk1013_data {
 	int64_t timestamp;
 	enum kx_chipset chipset;
 	enum kx_acpi_type acpi_type;
+	const struct kx_chipset_regs *regs;
 };
 
 enum kxcjk1013_axis {
@@ -309,7 +349,7 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 {
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -320,8 +360,7 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 	else
 		ret |= KXCJK1013_REG_CTRL1_BIT_PC1;
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_CTRL1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -335,7 +374,7 @@ static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 {
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -353,7 +392,7 @@ static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 {
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -364,9 +403,7 @@ static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
 	ret |= (KXCJK1013_scale_table[range_index].gsel_0 << 3);
 	ret |= (KXCJK1013_scale_table[range_index].gsel_1 << 4);
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_CTRL1,
-					ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -400,7 +437,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -409,8 +446,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	/* Set 12 bit mode */
 	ret |= KXCJK1013_REG_CTRL1_BIT_RES;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL1,
-					ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl\n");
 		return ret;
@@ -421,7 +457,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_DATA_CTRL);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->data_ctrl);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_data_ctrl\n");
 		return ret;
@@ -430,7 +466,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	data->odr_bits = ret;
 
 	/* Set up INT polarity */
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -441,8 +477,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEA;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
-					ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
@@ -497,10 +532,9 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
-	int waketh_reg, ret;
+	int ret;
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_WAKE_TIMER,
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->wake_timer,
 					data->wake_dur);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
@@ -508,9 +542,7 @@ static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 		return ret;
 	}
 
-	waketh_reg = data->chipset == KXTF9 ?
-		KXTF9_REG_WAKE_THRESH : KXCJK1013_REG_WAKE_THRES;
-	ret = i2c_smbus_write_byte_data(data->client, waketh_reg,
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->wake_thres,
 					data->wake_thres);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_wake_thres\n");
@@ -539,7 +571,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -550,14 +582,13 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
-					ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
 	}
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -568,8 +599,7 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_CTRL1_BIT_WUFE;
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_CTRL1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -599,7 +629,7 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
 		return ret;
@@ -610,14 +640,13 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
-					ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->int_ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
 		return ret;
 	}
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->ctrl1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
 		return ret;
@@ -628,8 +657,7 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	else
 		ret &= ~KXCJK1013_REG_CTRL1_BIT_DRDY;
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_CTRL1, ret);
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->ctrl1, ret);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
 		return ret;
@@ -701,7 +729,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_DATA_CTRL,
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->data_ctrl,
 					odr_setting->odr_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing data_ctrl\n");
@@ -710,7 +738,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 	data->odr_bits = odr_setting->odr_bits;
 
-	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL2,
+	ret = i2c_smbus_write_byte_data(data->client, data->regs->wuf_ctrl,
 					odr_setting->wuf_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
@@ -1113,7 +1141,7 @@ static void kxcjk1013_trig_reen(struct iio_trigger *trig)
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_REL);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_rel);
 	if (ret < 0)
 		dev_err(&data->client->dev, "Error reading reg_int_rel\n");
 }
@@ -1166,8 +1194,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 
-	int ret = i2c_smbus_read_byte_data(data->client,
-					   KXCJK1013_REG_INT_SRC2);
+	int ret = i2c_smbus_read_byte_data(data->client, data->regs->int_src2);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_src2\n");
 		return;
@@ -1234,7 +1261,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_SRC1);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_src1);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_int_src1\n");
 		goto ack_intr;
@@ -1257,7 +1284,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	if (data->dready_trigger_on)
 		return IRQ_HANDLED;
 
-	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_INT_REL);
+	ret = i2c_smbus_read_byte_data(data->client, data->regs->int_rel);
 	if (ret < 0)
 		dev_err(&data->client->dev, "Error reading reg_int_rel\n");
 
@@ -1378,6 +1405,19 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	} else
 		return -ENODEV;
 
+	switch (data->chipset) {
+	case KXCJK1013:
+	case KXCJ91008:
+	case KXTJ21009:
+		data->regs = &kxcjk1013_regs;
+		break;
+	case KXTF9:
+		data->regs = &kxtf9_regs;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	ret = kxcjk1013_chip_init(data);
 	if (ret < 0)
 		return ret;
-- 
2.31.1

Re: [PATCH 2/3] iio: accel: kxcjk-1013: Refactor configuration registers into struct

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

Hi Stephan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on robh/for-next v5.13-rc1 next-20210511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Stephan-Gerhold/iio-accel-kxcjk-1013-Add-support-for-KX023-1025/20210511-175848
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-s002-20210511 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/8fe54fbb0ce5d3b78a61f74a50e77a771bb77c82
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stephan-Gerhold/iio-accel-kxcjk-1013-Add-support-for-KX023-1025/20210511-175848
        git checkout 8fe54fbb0ce5d3b78a61f74a50e77a771bb77c82
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/iio/accel/kxcjk-1013.c:150:30: sparse: sparse: symbol 'kxcjk1013_regs' was not declared. Should it be static?
>> drivers/iio/accel/kxcjk-1013.c:162:30: sparse: sparse: symbol 'kxtf9_regs' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35379 bytes --]

[RFC PATCH] iio: accel: kxcjk-1013: kxcjk1013_regs can be static

[kernel test robot]

drivers/iio/accel/kxcjk-1013.c:150:30: warning: symbol 'kxcjk1013_regs' was not declared. Should it be static?
drivers/iio/accel/kxcjk-1013.c:162:30: warning: symbol 'kxtf9_regs' was not declared. Should it be static?

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 kxcjk-1013.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 8800ebf47b5a7..492f576f97348 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -147,7 +147,7 @@ struct kx_chipset_regs {
 	u8 wake_thres;
 };
 
-const struct kx_chipset_regs kxcjk1013_regs = {
+static const struct kx_chipset_regs kxcjk1013_regs = {
 	.int_src1	= KXCJK1013_REG_INT_SRC1,
 	.int_src2	= KXCJK1013_REG_INT_SRC2,
 	.int_rel	= KXCJK1013_REG_INT_REL,
@@ -159,7 +159,7 @@ const struct kx_chipset_regs kxcjk1013_regs = {
 	.wake_thres	= KXCJK1013_REG_WAKE_THRES,
 };
 
-const struct kx_chipset_regs kxtf9_regs = {
+static const struct kx_chipset_regs kxtf9_regs = {
 	/* .int_src1 was moved to INT_SRC2 on KXTF9 */
 	.int_src1	= KXTF9_REG_INT_SRC2,
 	/* .int_src2 is not available */

[PATCH 3/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Stephan Gerhold]

The KX023-1025 accelerometer [1] seems to be some mixture of
KXCJK and KXTF9. It has the motion interrupt functionality from KXCJK
but also the tap detection from KXTF9, and a lot more functionality.

The configuration register map seems fairly different at first,
but actually all register bits used by the kxcjk-1013 driver are
available at the same bit positions on KX023-1025. It's just quite
misleading because:

  1. The registers have entirely different names and are at different
     addresses, but the bits are mostly named the same (and mean the same).
  2. There are many more registers and bits used that are reserved on KXCJK
     to enable additional functionality.

Ignoring all additionally available functionality for now, the KX023
works just fine after setting up the struct with the correct register
addresses. The only difference that needs to be handled additionally
is that the KX023 supports two configurable interrupt lines (INT1/2).

For now only INT1 is supported so we route all interrupts used by
the driver there.

[1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf

Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 drivers/iio/accel/kxcjk-1013.c | 93 ++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index e630419a11d8..20f3634d225d 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -79,6 +79,45 @@
 #define KXTF9_REG_HYST_SET		0x5F
 #define KXCJK1013_REG_WAKE_THRES	0x6A
 
+/* Everything up to 0x11 is equal to KXCJK1013/KXTF9 above */
+#define KX023_REG_INS1			0x12
+#define KX023_REG_INS2			0x13
+#define KX023_REG_INS3			0x14
+#define KX023_REG_STAT			0x15
+#define KX023_REG_INT_REL		0x17
+#define KX023_REG_CNTL1			0x18
+#define KX023_REG_CNTL2			0x19
+#define KX023_REG_CNTL3			0x1A
+#define KX023_REG_ODCNTL		0x1B
+#define KX023_REG_INC1			0x1C
+#define KX023_REG_INC2			0x1D
+#define KX023_REG_INC3			0x1E
+#define KX023_REG_INC4			0x1F
+#define KX023_REG_INC5			0x20
+#define KX023_REG_INC6			0x21
+#define KX023_REG_TILT_TIMER		0x22
+#define KX023_REG_WUFC			0x23
+#define KX023_REG_TDTRC			0x24
+#define KX023_REG_TDTC			0x25
+#define KX023_REG_TTH			0x26
+#define KX023_REG_TTL			0x27
+#define KX023_REG_FTD			0x28
+#define KX023_REG_STD			0x29
+#define KX023_REG_TLT			0x2A
+#define KX023_REG_TWS			0x2B
+#define KX023_REG_ATH			0x30
+#define KX023_REG_TILT_ANGLE_LL		0x32
+#define KX023_REG_TILT_ANGLE_HL		0x33
+#define KX023_REG_HYST_SET		0x34
+#define KX023_REG_LP_CNTL		0x35
+#define KX023_REG_BUF_CNTL1		0x3A
+#define KX023_REG_BUF_CNTL2		0x3B
+#define KX023_REG_BUF_STATUS_1		0x3C
+#define KX023_REG_BUF_STATUS_2		0x3D
+#define KX023_REG_BUF_CLEAR		0x3E
+#define KX023_REG_BUF_READ		0x3F
+#define KX023_REG_SELF_TEST		0x60
+
 #define KXCJK1013_REG_CTRL1_BIT_PC1	BIT(7)
 #define KXCJK1013_REG_CTRL1_BIT_RES	BIT(6)
 #define KXCJK1013_REG_CTRL1_BIT_DRDY	BIT(5)
@@ -119,6 +158,14 @@
 #define KXCJK1013_REG_INT_SRC2_BIT_XP	BIT(4)
 #define KXCJK1013_REG_INT_SRC2_BIT_XN	BIT(5)
 
+/* KX023 interrupt routing to INT1. INT2 can be configured with INC6 */
+#define KX023_REG_INC4_BFI1		BIT(6)
+#define KX023_REG_INC4_WMI1		BIT(5)
+#define KX023_REG_INC4_DRDY1		BIT(4)
+#define KX023_REG_INC4_TDTI1		BIT(2)
+#define KX023_REG_INC4_WUFI1		BIT(1)
+#define KX023_REG_INC4_TPI1		BIT(0)
+
 #define KXCJK1013_DEFAULT_WAKE_THRES	1
 
 enum kx_chipset {
@@ -126,6 +173,7 @@ enum kx_chipset {
 	KXCJ91008,
 	KXTJ21009,
 	KXTF9,
+	KX0231025,
 	KX_MAX_CHIPS /* this must be last */
 };
 
@@ -172,6 +220,19 @@ const struct kx_chipset_regs kxtf9_regs = {
 	.wake_thres	= KXTF9_REG_WAKE_THRESH,
 };
 
+/* The registers have totally different names but the bits are compatible */
+const struct kx_chipset_regs kx0231025_regs = {
+	.int_src1	= KX023_REG_INS2,
+	.int_src2	= KX023_REG_INS3,
+	.int_rel	= KX023_REG_INT_REL,
+	.ctrl1		= KX023_REG_CNTL1,
+	.wuf_ctrl	= KX023_REG_CNTL3,
+	.int_ctrl1	= KX023_REG_INC1,
+	.data_ctrl	= KX023_REG_ODCNTL,
+	.wake_timer	= KX023_REG_WUFC,
+	.wake_thres	= KX023_REG_ATH,
+};
+
 struct kxcjk1013_data {
 	struct regulator_bulk_data regulators[2];
 	struct i2c_client *client;
@@ -308,6 +369,22 @@ static const struct {
 		{0x05, 5100},
 		{0x06, 2700},
 	},
+	/* KX023-1025 */
+	{
+		/* First 4 are not in datasheet, taken from KXCTJ2-1009 */
+		{0x08, 1240000},
+		{0x09, 621000},
+		{0x0A, 309000},
+		{0x0B, 151000},
+		{0, 81000},
+		{0x01, 40000},
+		{0x02, 22000},
+		{0x03, 12000},
+		{0x04, 7000},
+		{0x05, 4400},
+		{0x06, 3000},
+		{0x07, 3000},
+	},
 };
 
 static const struct {
@@ -483,6 +560,17 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
+	/* On KX023, route all used interrupts to INT1 for now */
+	if (data->chipset == KX0231025 && data->client->irq > 0) {
+		ret = i2c_smbus_write_byte_data(data->client, KX023_REG_INC4,
+						KX023_REG_INC4_DRDY1 |
+						KX023_REG_INC4_WUFI1);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "Error writing reg_inc4\n");
+			return ret;
+		}
+	}
+
 	ret = kxcjk1013_set_mode(data, OPERATION);
 	if (ret < 0)
 		return ret;
@@ -1414,6 +1502,9 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	case KXTF9:
 		data->regs = &kxtf9_regs;
 		break;
+	case KX0231025:
+		data->regs = &kx0231025_regs;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -1633,6 +1724,7 @@ static const struct i2c_device_id kxcjk1013_id[] = {
 	{"kxcj91008", KXCJ91008},
 	{"kxtj21009", KXTJ21009},
 	{"kxtf9",     KXTF9},
+	{"kx023-1025", KX0231025},
 	{"SMO8500",   KXCJ91008},
 	{}
 };
@@ -1644,6 +1736,7 @@ static const struct of_device_id kxcjk1013_of_match[] = {
 	{ .compatible = "kionix,kxcj91008", },
 	{ .compatible = "kionix,kxtj21009", },
 	{ .compatible = "kionix,kxtf9", },
+	{ .compatible = "kionix,kx023-1025", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, kxcjk1013_of_match);
-- 
2.31.1

Re: [PATCH 3/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1926 bytes --]

Hi Stephan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on iio/togreg]
[also build test WARNING on robh/for-next v5.13-rc1 next-20210511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Stephan-Gerhold/iio-accel-kxcjk-1013-Add-support-for-KX023-1025/20210511-175848
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: i386-randconfig-s002-20210511 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/2554f3e50319ec1ea070a8c55b18e0e4c7aadf11
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Stephan-Gerhold/iio-accel-kxcjk-1013-Add-support-for-KX023-1025/20210511-175848
        git checkout 2554f3e50319ec1ea070a8c55b18e0e4c7aadf11
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   drivers/iio/accel/kxcjk-1013.c:198:30: sparse: sparse: symbol 'kxcjk1013_regs' was not declared. Should it be static?
   drivers/iio/accel/kxcjk-1013.c:210:30: sparse: sparse: symbol 'kxtf9_regs' was not declared. Should it be static?
>> drivers/iio/accel/kxcjk-1013.c:224:30: sparse: sparse: symbol 'kx0231025_regs' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35379 bytes --]

[RFC PATCH] iio: accel: kxcjk-1013: kx0231025_regs can be static

[kernel test robot]

drivers/iio/accel/kxcjk-1013.c:224:30: warning: symbol 'kx0231025_regs' was not declared. Should it be static?

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---
 kxcjk-1013.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 5aa2a1cb2309d..1e5e241f2bcdf 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -221,7 +221,7 @@ const struct kx_chipset_regs kxtf9_regs = {
 };
 
 /* The registers have totally different names but the bits are compatible */
-const struct kx_chipset_regs kx0231025_regs = {
+static const struct kx_chipset_regs kx0231025_regs = {
 	.int_src1	= KX023_REG_INS2,
 	.int_src2	= KX023_REG_INS3,
 	.int_rel	= KX023_REG_INT_REL,

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Hans de Goede]

Hi,

On 5/11/21 11:54 AM, Stephan Gerhold wrote:
> KX023-1025 [1] is another accelerometer from Kionix that has lots
> of additional functionality compared to KXCJK-1013. It combines the
> motion interrupt functionality from KXCJK with the tap detection
> from KXTF9, plus a lot more other functionality.
> 
> This patch set does not add support for any of the extra functionality,
> but makes basic functionality work with the existing kxcjk-1013 driver.
> 
> At first, the register map for the KX023-1025 seems quite different
> from the other accelerometers supported by the kxcjk-1013.
> However, it turns out that at least most of the register bits
> still mean the same for KX023-1025.
> 
> This patch set refactors the kxcjk-1013 driver a little bit
> to get the register addresses from a chip-specific struct.
> The register bits can be re-used for all the different chips.
> 
> The KX023-1025 is used in several smartphones from Huawei.
> I tested these changes on a Huawei Ascend G7, someone else reported
> they also work fine on the Huawei Honor 5X (codename "kiwi").
> 
> [1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf
> 
> Stephan Gerhold (3):
>   dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025
>   iio: accel: kxcjk-1013: Refactor configuration registers into struct
>   iio: accel: kxcjk-1013: Add support for KX023-1025

Thanks, the entire series looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the series.

Regards,

Hans

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Jonathan Cameron]

On Tue, 11 May 2021 12:44:23 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 5/11/21 11:54 AM, Stephan Gerhold wrote:
> > KX023-1025 [1] is another accelerometer from Kionix that has lots
> > of additional functionality compared to KXCJK-1013. It combines the
> > motion interrupt functionality from KXCJK with the tap detection
> > from KXTF9, plus a lot more other functionality.
> > 
> > This patch set does not add support for any of the extra functionality,
> > but makes basic functionality work with the existing kxcjk-1013 driver.
> > 
> > At first, the register map for the KX023-1025 seems quite different
> > from the other accelerometers supported by the kxcjk-1013.
> > However, it turns out that at least most of the register bits
> > still mean the same for KX023-1025.
> > 
> > This patch set refactors the kxcjk-1013 driver a little bit
> > to get the register addresses from a chip-specific struct.
> > The register bits can be re-used for all the different chips.
> > 
> > The KX023-1025 is used in several smartphones from Huawei.
> > I tested these changes on a Huawei Ascend G7, someone else reported
> > they also work fine on the Huawei Honor 5X (codename "kiwi").
> > 
> > [1]: https://kionixfs.azureedge.net/en/datasheet/KX023-1025%20Specifications%20Rev%2012.0.pdf
> > 
> > Stephan Gerhold (3):
> >   dt-bindings: iio: kionix,kxcjk1013: Document kionix,kx023-1025
> >   iio: accel: kxcjk-1013: Refactor configuration registers into struct
> >   iio: accel: kxcjk-1013: Add support for KX023-1025  
> 
> Thanks, the entire series looks good to me:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 

Other than those things 0-day found which would be good to tidy up for a v2
looks good to me.

Thanks,

Jonathan

> for the series.
> 
> Regards,
> 
> Hans
> 

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Michał Mirosław]

On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote:
> KX023-1025 [1] is another accelerometer from Kionix that has lots
> of additional functionality compared to KXCJK-1013. It combines the
> motion interrupt functionality from KXCJK with the tap detection
> from KXTF9, plus a lot more other functionality.

When I researched KXTF9 support it occurred to me that the -10xx part is
duplicating the information in 'KXyyy' - it seems to be a project number
or something. I would suggest to use just 'kx023' prefix for the code
and DT but leave the full identification in the comments/description.

Best Regards
Michał Mirosław

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Stephan Gerhold]

Hi Michał,

On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote:
> On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote:
> > KX023-1025 [1] is another accelerometer from Kionix that has lots
> > of additional functionality compared to KXCJK-1013. It combines the
> > motion interrupt functionality from KXCJK with the tap detection
> > from KXTF9, plus a lot more other functionality.
> 
> When I researched KXTF9 support it occurred to me that the -10xx part is
> duplicating the information in 'KXyyy' - it seems to be a project number
> or something. I would suggest to use just 'kx023' prefix for the code
> and DT but leave the full identification in the comments/description.
> 

There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1]
and a KXTF9-2050 [2] with separate datasheets. Have you checked if there
is a meaningful difference between them?

In any case, I think for KX023 there is only KX023-1025,
so I suppose I can omit it. I used KX023-1025 as name mostly for
consistency, although I did change the convention a bit already since
"kionix,kx0231025" was terribly readable.

So both the current "kionix,kx023-1025" and "kionix,kx023" would be fine
with me, any other opinions?

Thanks!
Stephan

[1]: https://kionixfs.azureedge.net/en/datasheet/KXTF9-4100%20Specifications%20Rev%206.pdf
[2]: https://kionixfs.azureedge.net/en/datasheet/KXTF9-2050%20Specifications%20Rev%207.pdf

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Michał Mirosław]

On Tue, May 11, 2021 at 04:38:06PM +0200, Stephan Gerhold wrote:
> Hi Michał,
> 
> On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote:
> > On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote:
> > > KX023-1025 [1] is another accelerometer from Kionix that has lots
> > > of additional functionality compared to KXCJK-1013. It combines the
> > > motion interrupt functionality from KXCJK with the tap detection
> > > from KXTF9, plus a lot more other functionality.
> > When I researched KXTF9 support it occurred to me that the -10xx part is
> > duplicating the information in 'KXyyy' - it seems to be a project number
> > or something. I would suggest to use just 'kx023' prefix for the code
> > and DT but leave the full identification in the comments/description.
> There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1]
> and a KXTF9-2050 [2] with separate datasheets. Have you checked if there
> is a meaningful difference between them?

I haven't compared them thoroughly, but the versions seem to differ only
in power consumption (maybe a different manufacturing process?). The
register sets seem the same.

Best Regards
Michał Mirosław

Re: [PATCH 0/3] iio: accel: kxcjk-1013: Add support for KX023-1025

[Jonathan Cameron]

On Tue, 11 May 2021 16:50:51 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Tue, May 11, 2021 at 04:38:06PM +0200, Stephan Gerhold wrote:
> > Hi Michał,
> > 
> > On Tue, May 11, 2021 at 04:28:47PM +0200, Michał Mirosław wrote:  
> > > On Tue, May 11, 2021 at 11:54:06AM +0200, Stephan Gerhold wrote:  
> > > > KX023-1025 [1] is another accelerometer from Kionix that has lots
> > > > of additional functionality compared to KXCJK-1013. It combines the
> > > > motion interrupt functionality from KXCJK with the tap detection
> > > > from KXTF9, plus a lot more other functionality.  
> > > When I researched KXTF9 support it occurred to me that the -10xx part is
> > > duplicating the information in 'KXyyy' - it seems to be a project number
> > > or something. I would suggest to use just 'kx023' prefix for the code
> > > and DT but leave the full identification in the comments/description.  
> > There do seem to be two different KXTF9 from Kionix, a KXTF9-4100 [1]
> > and a KXTF9-2050 [2] with separate datasheets. Have you checked if there
> > is a meaningful difference between them?  
> 
> I haven't compared them thoroughly, but the versions seem to differ only
> in power consumption (maybe a different manufacturing process?). The
> register sets seem the same.

Differ in expected Vdd supply voltage. 3.3 vs 1.8V .  Looks like this has
knock on effects on things like self test values.  So I'd argue it's worth keeping
the distinction for device tree. 

We could do a double compatible

compatible = kionix,kx023-1024, konix,kx023;

but may be too late to do that now.

Jonathan


> 
> Best Regards
> Michał Mirosław