[PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Michał Mirosław]

This series adds support for KXTF9 accelerometer. It is mostly compatible
to KXCJK, but replaces motion direction interrupt with tap detection
(not supported in this implementation, yet).

First 5 patches are refactorings with no outside-visible effects.
Patch 6 fixes sysfs attribute name of sampling_frequency_avail
to match sampling_frequency attribute it describes.
Patch 7 finally adds support for KXTF9 accelerometers.

Michał Mirosław (7):
  iio: accel: kxcjk1003: refactor ODR setting
  iio: accel: kxcjk1013: fix INT_CTRL/INT_SRC1 bit names
  iio: accel: kxcjk1013: extract report_motion_event() from interrupt
    handler
  iio: accel: kxcjk1013: rename motion direction bits
  iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic
  iio: accel: kxcjk1013: make sampling_frequency_avail per-type
  iio: accel: kxcjk1013: add support for KXTF9

 drivers/iio/accel/Kconfig      |   4 +-
 drivers/iio/accel/kxcjk-1013.c | 366 ++++++++++++++++++++++++++---------------
 2 files changed, 235 insertions(+), 135 deletions(-)

-- 
2.11.0

[PATCH v2 1/7] iio: accel: kxcjk1003: refactor ODR setting

[Michał Mirosław]

Refactor ODR/WUF setting code in preparation of KXTF9 support.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 102 ++++++++++++++++++-----------------------
 1 file changed, 45 insertions(+), 57 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 3f968c46e667..33e4b98f39fc 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -128,15 +128,27 @@ enum kxcjk1013_range {
 	KXCJK1013_RANGE_8G,
 };
 
-static const struct {
+struct kx_odr_map {
 	int val;
 	int val2;
 	int odr_bits;
-} samp_freq_table[] = { {0, 781000, 0x08}, {1, 563000, 0x09},
-			{3, 125000, 0x0A}, {6, 250000, 0x0B}, {12, 500000, 0},
-			{25, 0, 0x01}, {50, 0, 0x02}, {100, 0, 0x03},
-			{200, 0, 0x04}, {400, 0, 0x05}, {800, 0, 0x06},
-			{1600, 0, 0x07} };
+	int wuf_bits;
+};
+
+static const struct kx_odr_map samp_freq_table[] = {
+	{ 0, 781000, 0x08, 0x00 },
+	{ 1, 563000, 0x09, 0x01 },
+	{ 3, 125000, 0x0A, 0x02 },
+	{ 6, 250000, 0x0B, 0x03 },
+	{ 12, 500000, 0x00, 0x04 },
+	{ 25, 0, 0x01, 0x05 },
+	{ 50, 0, 0x02, 0x06 },
+	{ 100, 0, 0x03, 0x06 },
+	{ 200, 0, 0x04, 0x06 },
+	{ 400, 0, 0x05, 0x06 },
+	{ 800, 0, 0x06, 0x06 },
+	{ 1600, 0, 0x07, 0x06 },
+};
 
 /* Refer to section 4 of the specification */
 static const struct {
@@ -198,23 +210,6 @@ static const struct {
 			      {19163, 1, 0},
 			      {38326, 0, 1} };
 
-static const struct {
-	int val;
-	int val2;
-	int odr_bits;
-} wake_odr_data_rate_table[] = { {0, 781000, 0x00},
-				 {1, 563000, 0x01},
-				 {3, 125000, 0x02},
-				 {6, 250000, 0x03},
-				 {12, 500000, 0x04},
-				 {25, 0, 0x05},
-				 {50, 0, 0x06},
-				 {100, 0, 0x06},
-				 {200, 0, 0x06},
-				 {400, 0, 0x06},
-				 {800, 0, 0x06},
-				 {1600, 0, 0x06} };
-
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
@@ -547,28 +542,30 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	return 0;
 }
 
-static int kxcjk1013_convert_freq_to_bit(int val, int val2)
+static const struct kx_odr_map *kxcjk1013_find_odr_value(
+	const struct kx_odr_map *map, size_t map_size, int val, int val2)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(samp_freq_table); ++i) {
-		if (samp_freq_table[i].val == val &&
-			samp_freq_table[i].val2 == val2) {
-			return samp_freq_table[i].odr_bits;
-		}
+	for (i = 0; i < map_size; ++i) {
+		if (map[i].val == val && map[i].val2 == val2)
+			return &map[i];
 	}
 
-	return -EINVAL;
+	return ERR_PTR(-EINVAL);
 }
 
-static int kxcjk1013_convert_wake_odr_to_bit(int val, int val2)
+static int kxcjk1013_convert_odr_value(const struct kx_odr_map *map,
+				       size_t map_size, int odr_bits,
+				       int *val, int *val2)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(wake_odr_data_rate_table); ++i) {
-		if (wake_odr_data_rate_table[i].val == val &&
-			wake_odr_data_rate_table[i].val2 == val2) {
-			return wake_odr_data_rate_table[i].odr_bits;
+	for (i = 0; i < map_size; ++i) {
+		if (map[i].odr_bits == odr_bits) {
+			*val = map[i].val;
+			*val2 = map[i].val2;
+			return IIO_VAL_INT_PLUS_MICRO;
 		}
 	}
 
@@ -578,16 +575,19 @@ static int kxcjk1013_convert_wake_odr_to_bit(int val, int val2)
 static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 {
 	int ret;
-	int odr_bits;
 	enum kxcjk1013_mode store_mode;
+	const struct kx_odr_map *odr_setting;
 
 	ret = kxcjk1013_get_mode(data, &store_mode);
 	if (ret < 0)
 		return ret;
 
-	odr_bits = kxcjk1013_convert_freq_to_bit(val, val2);
-	if (odr_bits < 0)
-		return odr_bits;
+	odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
+					       ARRAY_SIZE(samp_freq_table),
+					       val, val2);
+
+	if (IS_ERR(odr_setting))
+		return PTR_ERR(odr_setting);
 
 	/* To change ODR, the chip must be set to STANDBY as per spec */
 	ret = kxcjk1013_set_mode(data, STANDBY);
@@ -595,20 +595,16 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 		return ret;
 
 	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_DATA_CTRL,
-					odr_bits);
+					odr_setting->odr_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing data_ctrl\n");
 		return ret;
 	}
 
-	data->odr_bits = odr_bits;
-
-	odr_bits = kxcjk1013_convert_wake_odr_to_bit(val, val2);
-	if (odr_bits < 0)
-		return odr_bits;
+	data->odr_bits = odr_setting->odr_bits;
 
 	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL2,
-					odr_bits);
+					odr_setting->wuf_bits);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
 		return ret;
@@ -625,17 +621,9 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(samp_freq_table); ++i) {
-		if (samp_freq_table[i].odr_bits == data->odr_bits) {
-			*val = samp_freq_table[i].val;
-			*val2 = samp_freq_table[i].val2;
-			return IIO_VAL_INT_PLUS_MICRO;
-		}
-	}
-
-	return -EINVAL;
+	return kxcjk1013_convert_odr_value(samp_freq_table,
+					   ARRAY_SIZE(samp_freq_table),
+					   data->odr_bits, val, val2);
 }
 
 static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
-- 
2.11.0

Re: [PATCH v2 1/7] iio: accel: kxcjk1003: refactor ODR setting

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:30 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Refactor ODR/WUF setting code in preparation of KXTF9 support.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

A comment inline, though I argue myself around to agreeing with how you
have done it so no need to change anything :)

I'm happy with this patch,

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 102 ++++++++++++++++++-----------------------
>  1 file changed, 45 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 3f968c46e667..33e4b98f39fc 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -128,15 +128,27 @@ enum kxcjk1013_range {
>  	KXCJK1013_RANGE_8G,
>  };
>  
> -static const struct {
> +struct kx_odr_map {
>  	int val;
>  	int val2;
>  	int odr_bits;
> -} samp_freq_table[] = { {0, 781000, 0x08}, {1, 563000, 0x09},
> -			{3, 125000, 0x0A}, {6, 250000, 0x0B}, {12, 500000, 0},
> -			{25, 0, 0x01}, {50, 0, 0x02}, {100, 0, 0x03},
> -			{200, 0, 0x04}, {400, 0, 0x05}, {800, 0, 0x06},
> -			{1600, 0, 0x07} };
> +	int wuf_bits;
> +};
> +
> +static const struct kx_odr_map samp_freq_table[] = {
> +	{ 0, 781000, 0x08, 0x00 },
> +	{ 1, 563000, 0x09, 0x01 },
> +	{ 3, 125000, 0x0A, 0x02 },
> +	{ 6, 250000, 0x0B, 0x03 },
> +	{ 12, 500000, 0x00, 0x04 },
> +	{ 25, 0, 0x01, 0x05 },
> +	{ 50, 0, 0x02, 0x06 },
> +	{ 100, 0, 0x03, 0x06 },
> +	{ 200, 0, 0x04, 0x06 },
> +	{ 400, 0, 0x05, 0x06 },
> +	{ 800, 0, 0x06, 0x06 },
> +	{ 1600, 0, 0x07, 0x06 },
> +};
>  
>  /* Refer to section 4 of the specification */
>  static const struct {
> @@ -198,23 +210,6 @@ static const struct {
>  			      {19163, 1, 0},
>  			      {38326, 0, 1} };
>  
> -static const struct {
> -	int val;
> -	int val2;
> -	int odr_bits;
> -} wake_odr_data_rate_table[] = { {0, 781000, 0x00},
> -				 {1, 563000, 0x01},
> -				 {3, 125000, 0x02},
> -				 {6, 250000, 0x03},
> -				 {12, 500000, 0x04},
> -				 {25, 0, 0x05},
> -				 {50, 0, 0x06},
> -				 {100, 0, 0x06},
> -				 {200, 0, 0x06},
> -				 {400, 0, 0x06},
> -				 {800, 0, 0x06},
> -				 {1600, 0, 0x06} };
> -
>  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
>  			      enum kxcjk1013_mode mode)
>  {
> @@ -547,28 +542,30 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
>  	return 0;
>  }
>  
> -static int kxcjk1013_convert_freq_to_bit(int val, int val2)
> +static const struct kx_odr_map *kxcjk1013_find_odr_value(
> +	const struct kx_odr_map *map, size_t map_size, int val, int val2)
>  {
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(samp_freq_table); ++i) {
> -		if (samp_freq_table[i].val == val &&
> -			samp_freq_table[i].val2 == val2) {
> -			return samp_freq_table[i].odr_bits;
> -		}
> +	for (i = 0; i < map_size; ++i) {
> +		if (map[i].val == val && map[i].val2 == val2)
> +			return &map[i];
>  	}
>  
> -	return -EINVAL;
> +	return ERR_PTR(-EINVAL);
>  }
>  
> -static int kxcjk1013_convert_wake_odr_to_bit(int val, int val2)
> +static int kxcjk1013_convert_odr_value(const struct kx_odr_map *map,
> +				       size_t map_size, int odr_bits,
> +				       int *val, int *val2)
>  {
>  	int i;
>  
> -	for (i = 0; i < ARRAY_SIZE(wake_odr_data_rate_table); ++i) {
> -		if (wake_odr_data_rate_table[i].val == val &&
> -			wake_odr_data_rate_table[i].val2 == val2) {
> -			return wake_odr_data_rate_table[i].odr_bits;
> +	for (i = 0; i < map_size; ++i) {
> +		if (map[i].odr_bits == odr_bits) {
> +			*val = map[i].val;
> +			*val2 = map[i].val2;
> +			return IIO_VAL_INT_PLUS_MICRO;
>  		}
>  	}
>  
> @@ -578,16 +575,19 @@ static int kxcjk1013_convert_wake_odr_to_bit(int val, int val2)
>  static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  {
>  	int ret;
> -	int odr_bits;
>  	enum kxcjk1013_mode store_mode;
> +	const struct kx_odr_map *odr_setting;
>  
>  	ret = kxcjk1013_get_mode(data, &store_mode);
>  	if (ret < 0)
>  		return ret;
>  
> -	odr_bits = kxcjk1013_convert_freq_to_bit(val, val2);
> -	if (odr_bits < 0)
> -		return odr_bits;
> +	odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
> +					       ARRAY_SIZE(samp_freq_table),
> +					       val, val2);
> +
> +	if (IS_ERR(odr_setting))
> +		return PTR_ERR(odr_setting);
>  
>  	/* To change ODR, the chip must be set to STANDBY as per spec */
>  	ret = kxcjk1013_set_mode(data, STANDBY);
> @@ -595,20 +595,16 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  		return ret;
>  
>  	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_DATA_CTRL,
> -					odr_bits);
> +					odr_setting->odr_bits);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "Error writing data_ctrl\n");
>  		return ret;
>  	}
>  
> -	data->odr_bits = odr_bits;
> -
> -	odr_bits = kxcjk1013_convert_wake_odr_to_bit(val, val2);
> -	if (odr_bits < 0)
> -		return odr_bits;
> +	data->odr_bits = odr_setting->odr_bits;

You could change this to store the odr_settings pointer in data thus
avoiding having to look it up again in the read.  Would be slightly
cleaner perhaps, though obviously you'd have to do an initial read to
establish the meaning of the default.

On balance I think how you have it here is probably nicer given this is
a slow path.

>  
>  	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL2,
> -					odr_bits);
> +					odr_setting->wuf_bits);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
>  		return ret;
> @@ -625,17 +621,9 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  
>  static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
>  {
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(samp_freq_table); ++i) {
> -		if (samp_freq_table[i].odr_bits == data->odr_bits) {
> -			*val = samp_freq_table[i].val;
> -			*val2 = samp_freq_table[i].val2;
> -			return IIO_VAL_INT_PLUS_MICRO;
> -		}
> -	}
> -
> -	return -EINVAL;
> +	return kxcjk1013_convert_odr_value(samp_freq_table,
> +					   ARRAY_SIZE(samp_freq_table),
> +					   data->odr_bits, val, val2);
>  }
>  
>  static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)

[PATCH v2 2/7] iio: accel: kxcjk1013: fix INT_CTRL/INT_SRC1 bit names

[Michał Mirosław]

Fix INT_CTRL1 bit names to match register name and add names
for INT_SRC1 bits.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 33e4b98f39fc..7828570d87d2 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -67,14 +67,19 @@
 #define KXCJK1013_REG_CTRL1_BIT_GSEL1	BIT(4)
 #define KXCJK1013_REG_CTRL1_BIT_GSEL0	BIT(3)
 #define KXCJK1013_REG_CTRL1_BIT_WUFE	BIT(1)
-#define KXCJK1013_REG_INT_REG1_BIT_IEA	BIT(4)
-#define KXCJK1013_REG_INT_REG1_BIT_IEN	BIT(5)
+
+#define KXCJK1013_REG_INT_CTRL1_BIT_IEL	BIT(3)
+#define KXCJK1013_REG_INT_CTRL1_BIT_IEA	BIT(4)
+#define KXCJK1013_REG_INT_CTRL1_BIT_IEN	BIT(5)
 
 #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
 #define KXCJK1013_MAX_STARTUP_TIME_US	100000
 
 #define KXCJK1013_SLEEP_DELAY_MS	2000
 
+#define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
+#define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
+
 #define KXCJK1013_REG_INT_SRC2_BIT_ZP	BIT(0)
 #define KXCJK1013_REG_INT_SRC2_BIT_ZN	BIT(1)
 #define KXCJK1013_REG_INT_SRC2_BIT_YP	BIT(2)
@@ -336,9 +341,9 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 	}
 
 	if (data->active_high_intr)
-		ret |= KXCJK1013_REG_INT_REG1_BIT_IEA;
+		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEA;
 	else
-		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEA;
+		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEA;
 
 	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
 					ret);
@@ -444,9 +449,9 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
 	}
 
 	if (status)
-		ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
+		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 	else
-		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
+		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
 	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
 					ret);
@@ -504,9 +509,9 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
 	}
 
 	if (status)
-		ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
+		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 	else
-		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
+		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
 
 	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
 					ret);
@@ -1039,7 +1044,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 		goto ack_intr;
 	}
 
-	if (ret & 0x02) {
+	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
 		ret = i2c_smbus_read_byte_data(data->client,
 					       KXCJK1013_REG_INT_SRC2);
 		if (ret < 0) {
-- 
2.11.0

Re: [PATCH v2 2/7] iio: accel: kxcjk1013: fix INT_CTRL/INT_SRC1 bit names

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:30 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Fix INT_CTRL1 bit names to match register name and add names
> for INT_SRC1 bits.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

If I was being really picky I'd argue this is two changes that aren't all that
connected so should have been two patches.  I don't really care though so
don't bother updating!

Change looks fine to me.

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 33e4b98f39fc..7828570d87d2 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -67,14 +67,19 @@
>  #define KXCJK1013_REG_CTRL1_BIT_GSEL1	BIT(4)
>  #define KXCJK1013_REG_CTRL1_BIT_GSEL0	BIT(3)
>  #define KXCJK1013_REG_CTRL1_BIT_WUFE	BIT(1)
> -#define KXCJK1013_REG_INT_REG1_BIT_IEA	BIT(4)
> -#define KXCJK1013_REG_INT_REG1_BIT_IEN	BIT(5)
> +
> +#define KXCJK1013_REG_INT_CTRL1_BIT_IEL	BIT(3)
> +#define KXCJK1013_REG_INT_CTRL1_BIT_IEA	BIT(4)
> +#define KXCJK1013_REG_INT_CTRL1_BIT_IEN	BIT(5)
>  
>  #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
>  #define KXCJK1013_MAX_STARTUP_TIME_US	100000
>  
>  #define KXCJK1013_SLEEP_DELAY_MS	2000
>  
> +#define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
> +#define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
> +
>  #define KXCJK1013_REG_INT_SRC2_BIT_ZP	BIT(0)
>  #define KXCJK1013_REG_INT_SRC2_BIT_ZN	BIT(1)
>  #define KXCJK1013_REG_INT_SRC2_BIT_YP	BIT(2)
> @@ -336,9 +341,9 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  	}
>  
>  	if (data->active_high_intr)
> -		ret |= KXCJK1013_REG_INT_REG1_BIT_IEA;
> +		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEA;
>  	else
> -		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEA;
> +		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEA;
>  
>  	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
>  					ret);
> @@ -444,9 +449,9 @@ static int kxcjk1013_setup_any_motion_interrupt(struct kxcjk1013_data *data,
>  	}
>  
>  	if (status)
> -		ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
> +		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEN;
>  	else
> -		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
> +		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
>  
>  	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
>  					ret);
> @@ -504,9 +509,9 @@ static int kxcjk1013_setup_new_data_interrupt(struct kxcjk1013_data *data,
>  	}
>  
>  	if (status)
> -		ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
> +		ret |= KXCJK1013_REG_INT_CTRL1_BIT_IEN;
>  	else
> -		ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
> +		ret &= ~KXCJK1013_REG_INT_CTRL1_BIT_IEN;
>  
>  	ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
>  					ret);
> @@ -1039,7 +1044,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
>  		goto ack_intr;
>  	}
>  
> -	if (ret & 0x02) {
> +	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
>  		ret = i2c_smbus_read_byte_data(data->client,
>  					       KXCJK1013_REG_INT_SRC2);
>  		if (ret < 0) {

[PATCH v2 4/7] iio: accel: kxcjk1013: rename motion direction bits

[Michał Mirosław]

Rename INT_SRC2 bits - detected motion direction. This will avoid
duplication when KXTF9's tap direction is implemented.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 3022bc951dc2..252004640403 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -80,12 +80,12 @@
 #define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
 #define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
 
-#define KXCJK1013_REG_INT_SRC2_BIT_ZP	BIT(0)
-#define KXCJK1013_REG_INT_SRC2_BIT_ZN	BIT(1)
-#define KXCJK1013_REG_INT_SRC2_BIT_YP	BIT(2)
-#define KXCJK1013_REG_INT_SRC2_BIT_YN	BIT(3)
-#define KXCJK1013_REG_INT_SRC2_BIT_XP	BIT(4)
-#define KXCJK1013_REG_INT_SRC2_BIT_XN	BIT(5)
+#define KXCJK1013_DIRECTION_BIT_ZP	BIT(0)
+#define KXCJK1013_DIRECTION_BIT_ZN	BIT(1)
+#define KXCJK1013_DIRECTION_BIT_YP	BIT(2)
+#define KXCJK1013_DIRECTION_BIT_YN	BIT(3)
+#define KXCJK1013_DIRECTION_BIT_XP	BIT(4)
+#define KXCJK1013_DIRECTION_BIT_XN	BIT(5)
 
 #define KXCJK1013_DEFAULT_WAKE_THRES	1
 
@@ -1043,7 +1043,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 		return;
 	}
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
+	if (ret & KXCJK1013_DIRECTION_BIT_XN)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
@@ -1052,7 +1052,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 						  IIO_EV_DIR_FALLING),
 			       data->timestamp);
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
+	if (ret & KXCJK1013_DIRECTION_BIT_XP)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
@@ -1061,7 +1061,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 						  IIO_EV_DIR_RISING),
 			       data->timestamp);
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
+	if (ret & KXCJK1013_DIRECTION_BIT_YN)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
@@ -1070,7 +1070,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 						  IIO_EV_DIR_FALLING),
 			       data->timestamp);
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
+	if (ret & KXCJK1013_DIRECTION_BIT_YP)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
@@ -1079,7 +1079,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 						  IIO_EV_DIR_RISING),
 			       data->timestamp);
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
+	if (ret & KXCJK1013_DIRECTION_BIT_ZN)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
@@ -1088,7 +1088,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
 						  IIO_EV_DIR_FALLING),
 			       data->timestamp);
 
-	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
+	if (ret & KXCJK1013_DIRECTION_BIT_ZP)
 		iio_push_event(indio_dev,
 			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
 						  0,
-- 
2.11.0

Re: [PATCH v2 4/7] iio: accel: kxcjk1013: rename motion direction bits

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:31 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Rename INT_SRC2 bits - detected motion direction. This will avoid
> duplication when KXTF9's tap direction is implemented.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

I would have preferred this being left until the tap direction
stuff is ready to go.   It's a premature change at the moment.

It is not that important though so lets leave it here to avoid a
respin if there are no issues in later patches.

If you are respining for some reason I'd like this dropped for now.

Jonathan


> ---
>  drivers/iio/accel/kxcjk-1013.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 3022bc951dc2..252004640403 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -80,12 +80,12 @@
>  #define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
>  #define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
>  
> -#define KXCJK1013_REG_INT_SRC2_BIT_ZP	BIT(0)
> -#define KXCJK1013_REG_INT_SRC2_BIT_ZN	BIT(1)
> -#define KXCJK1013_REG_INT_SRC2_BIT_YP	BIT(2)
> -#define KXCJK1013_REG_INT_SRC2_BIT_YN	BIT(3)
> -#define KXCJK1013_REG_INT_SRC2_BIT_XP	BIT(4)
> -#define KXCJK1013_REG_INT_SRC2_BIT_XN	BIT(5)
> +#define KXCJK1013_DIRECTION_BIT_ZP	BIT(0)
> +#define KXCJK1013_DIRECTION_BIT_ZN	BIT(1)
> +#define KXCJK1013_DIRECTION_BIT_YP	BIT(2)
> +#define KXCJK1013_DIRECTION_BIT_YN	BIT(3)
> +#define KXCJK1013_DIRECTION_BIT_XP	BIT(4)
> +#define KXCJK1013_DIRECTION_BIT_XN	BIT(5)
>  
>  #define KXCJK1013_DEFAULT_WAKE_THRES	1
>  
> @@ -1043,7 +1043,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  		return;
>  	}
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
> +	if (ret & KXCJK1013_DIRECTION_BIT_XN)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1052,7 +1052,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  						  IIO_EV_DIR_FALLING),
>  			       data->timestamp);
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
> +	if (ret & KXCJK1013_DIRECTION_BIT_XP)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1061,7 +1061,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  						  IIO_EV_DIR_RISING),
>  			       data->timestamp);
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
> +	if (ret & KXCJK1013_DIRECTION_BIT_YN)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1070,7 +1070,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  						  IIO_EV_DIR_FALLING),
>  			       data->timestamp);
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
> +	if (ret & KXCJK1013_DIRECTION_BIT_YP)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1079,7 +1079,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  						  IIO_EV_DIR_RISING),
>  			       data->timestamp);
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
> +	if (ret & KXCJK1013_DIRECTION_BIT_ZN)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,
> @@ -1088,7 +1088,7 @@ static void kxcjk1013_report_motion_event(struct iio_dev *indio_dev)
>  						  IIO_EV_DIR_FALLING),
>  			       data->timestamp);
>  
> -	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
> +	if (ret & KXCJK1013_DIRECTION_BIT_ZP)
>  		iio_push_event(indio_dev,
>  			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
>  						  0,

[PATCH v2 3/7] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler

[Michał Mirosław]

Extract reporting of motion event direction from interrupt handler,
as it is not supported by KXTF9.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 126 ++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 59 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 7828570d87d2..3022bc951dc2 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1032,6 +1032,72 @@ static const struct iio_trigger_ops kxcjk1013_trigger_ops = {
 	.owner = THIS_MODULE,
 };
 
+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);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "Error reading reg_int_src2\n");
+		return;
+	}
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_X,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_FALLING),
+			       data->timestamp);
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_X,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_RISING),
+			       data->timestamp);
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Y,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_FALLING),
+			       data->timestamp);
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Y,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_RISING),
+			       data->timestamp);
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_FALLING),
+			       data->timestamp);
+
+	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
+		iio_push_event(indio_dev,
+			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+						  0,
+						  IIO_MOD_Z,
+						  IIO_EV_TYPE_THRESH,
+						  IIO_EV_DIR_RISING),
+			       data->timestamp);
+}
+
 static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 {
 	struct iio_dev *indio_dev = private;
@@ -1045,65 +1111,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	}
 
 	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
-		ret = i2c_smbus_read_byte_data(data->client,
-					       KXCJK1013_REG_INT_SRC2);
-		if (ret < 0) {
-			dev_err(&data->client->dev,
-				"Error reading reg_int_src2\n");
-			goto ack_intr;
-		}
-
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_X,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_FALLING),
-				       data->timestamp);
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_X,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_RISING),
-				       data->timestamp);
-
-
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_Y,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_FALLING),
-				       data->timestamp);
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_Y,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_RISING),
-				       data->timestamp);
-
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_Z,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_FALLING),
-				       data->timestamp);
-		if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
-			iio_push_event(indio_dev,
-				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
-				       0,
-				       IIO_MOD_Z,
-				       IIO_EV_TYPE_THRESH,
-				       IIO_EV_DIR_RISING),
-				       data->timestamp);
+		kxcjk1013_report_motion_event(indio_dev);
 	}
 
 ack_intr:
-- 
2.11.0

Re: [PATCH v2 3/7] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:31 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Extract reporting of motion event direction from interrupt handler,
> as it is not supported by KXTF9.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Looks fine to me.  Will pick up once we hear from Srinivas.

Jonathan
> ---
>  drivers/iio/accel/kxcjk-1013.c | 126 ++++++++++++++++++++++-------------------
>  1 file changed, 67 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 7828570d87d2..3022bc951dc2 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1032,6 +1032,72 @@ static const struct iio_trigger_ops kxcjk1013_trigger_ops = {
>  	.owner = THIS_MODULE,
>  };
>  
> +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);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "Error reading reg_int_src2\n");
> +		return;
> +	}
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_X,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_FALLING),
> +			       data->timestamp);
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_X,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_RISING),
> +			       data->timestamp);
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_Y,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_FALLING),
> +			       data->timestamp);
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_Y,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_RISING),
> +			       data->timestamp);
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_FALLING),
> +			       data->timestamp);
> +
> +	if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
> +		iio_push_event(indio_dev,
> +			       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +						  0,
> +						  IIO_MOD_Z,
> +						  IIO_EV_TYPE_THRESH,
> +						  IIO_EV_DIR_RISING),
> +			       data->timestamp);
> +}
> +
>  static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> @@ -1045,65 +1111,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
>  	}
>  
>  	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
> -		ret = i2c_smbus_read_byte_data(data->client,
> -					       KXCJK1013_REG_INT_SRC2);
> -		if (ret < 0) {
> -			dev_err(&data->client->dev,
> -				"Error reading reg_int_src2\n");
> -			goto ack_intr;
> -		}
> -
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_XN)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_X,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_FALLING),
> -				       data->timestamp);
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_XP)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_X,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_RISING),
> -				       data->timestamp);
> -
> -
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_YN)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_Y,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_FALLING),
> -				       data->timestamp);
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_YP)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_Y,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_RISING),
> -				       data->timestamp);
> -
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZN)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_Z,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_FALLING),
> -				       data->timestamp);
> -		if (ret & KXCJK1013_REG_INT_SRC2_BIT_ZP)
> -			iio_push_event(indio_dev,
> -				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> -				       0,
> -				       IIO_MOD_Z,
> -				       IIO_EV_TYPE_THRESH,
> -				       IIO_EV_DIR_RISING),
> -				       data->timestamp);
> +		kxcjk1013_report_motion_event(indio_dev);
>  	}
>  
>  ack_intr:

[PATCH v2 7/7] iio: accel: kxcjk1013: add support for KXTF9

[Michał Mirosław]

KXTF9 has mostly compatible register layout to KXCJK accelerometer.
There is no motion direction interrupt support, but there is tap
direction detection instead (not implemented in this patch).

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/Kconfig      |   4 +-
 drivers/iio/accel/kxcjk-1013.c | 115 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 104 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 15de262015df..0be352a7b6f4 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -219,8 +219,8 @@ config KXCJK1013
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here if you want to build a driver for the Kionix KXCJK-1013
-	  triaxial acceleration sensor. This driver also supports KXCJ9-1008
-	  and KXTJ2-1009.
+	  triaxial acceleration sensor. This driver also supports KXCJ9-1008,
+	  KXTJ2-1009 and KXTF9.
 
 	  To compile this driver as a module, choose M here: the module will
 	  be called kxcjk-1013.
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 48b9a97e8a58..583393f454fa 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -34,6 +34,13 @@
 #define KXCJK1013_DRV_NAME "kxcjk1013"
 #define KXCJK1013_IRQ_NAME "kxcjk1013_event"
 
+#define KXTF9_REG_HP_XOUT_L		0x00
+#define KXTF9_REG_HP_XOUT_H		0x01
+#define KXTF9_REG_HP_YOUT_L		0x02
+#define KXTF9_REG_HP_YOUT_H		0x03
+#define KXTF9_REG_HP_ZOUT_L		0x04
+#define KXTF9_REG_HP_ZOUT_H		0x05
+
 #define KXCJK1013_REG_XOUT_L		0x06
 /*
  * From low byte X axis register, all the other addresses of Y and Z can be
@@ -48,17 +55,33 @@
 
 #define KXCJK1013_REG_DCST_RESP		0x0C
 #define KXCJK1013_REG_WHO_AM_I		0x0F
-#define KXCJK1013_REG_INT_SRC1		0x16
+#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 KXCJK1013_REG_INT_SRC2		0x17
 #define KXCJK1013_REG_STATUS_REG	0x18
 #define KXCJK1013_REG_INT_REL		0x1A
 #define KXCJK1013_REG_CTRL1		0x1B
-#define KXCJK1013_REG_CTRL2		0x1D
+#define KXTF9_REG_CTRL2			0x1C
+#define KXCJK1013_REG_CTRL2		0x1D	/* mostly compatible, CTRL_REG3 in KTXF9 ds */
 #define KXCJK1013_REG_INT_CTRL1		0x1E
 #define KXCJK1013_REG_INT_CTRL2		0x1F
+#define KXTF9_REG_INT_CTRL3		0x20
 #define KXCJK1013_REG_DATA_CTRL		0x21
+#define KXTF9_REG_TILT_TIMER		0x28
 #define KXCJK1013_REG_WAKE_TIMER	0x29
+#define KXTF9_REG_TDT_TIMER		0x2B
+#define KXTF9_REG_TDT_THRESH_H		0x2C
+#define KXTF9_REG_TDT_THRESH_L		0x2D
+#define KXTF9_REG_TDT_TAP_TIMER		0x2E
+#define KXTF9_REG_TDT_TOTAL_TIMER	0x2F
+#define KXTF9_REG_TDT_LATENCY_TIMER	0x30
+#define KXTF9_REG_TDT_WINDOW_TIMER	0x31
 #define KXCJK1013_REG_SELF_TEST		0x3A
+#define KXTF9_REG_WAKE_THRESH		0x5A
+#define KXTF9_REG_TILT_ANGLE		0x5C
+#define KXTF9_REG_HYST_SET		0x5F
 #define KXCJK1013_REG_WAKE_THRES	0x6A
 
 #define KXCJK1013_REG_CTRL1_BIT_PC1	BIT(7)
@@ -68,18 +91,32 @@
 #define KXCJK1013_REG_CTRL1_BIT_GSEL0	BIT(3)
 #define KXCJK1013_REG_CTRL1_BIT_WUFE	BIT(1)
 
+#define KXCJK1013_REG_INT_CTRL1_BIT_IEU	BIT(2)	/* KXTF9 */
 #define KXCJK1013_REG_INT_CTRL1_BIT_IEL	BIT(3)
 #define KXCJK1013_REG_INT_CTRL1_BIT_IEA	BIT(4)
 #define KXCJK1013_REG_INT_CTRL1_BIT_IEN	BIT(5)
 
+#define KXTF9_REG_TILT_BIT_LEFT_EDGE	BIT(5)
+#define KXTF9_REG_TILT_BIT_RIGHT_EDGE	BIT(4)
+#define KXTF9_REG_TILT_BIT_LOWER_EDGE	BIT(3)
+#define KXTF9_REG_TILT_BIT_UPPER_EDGE	BIT(2)
+#define KXTF9_REG_TILT_BIT_FACE_DOWN	BIT(1)
+#define KXTF9_REG_TILT_BIT_FACE_UP	BIT(0)
+
 #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
 #define KXCJK1013_MAX_STARTUP_TIME_US	100000
 
 #define KXCJK1013_SLEEP_DELAY_MS	2000
 
+#define KXCJK1013_REG_INT_SRC1_BIT_TPS	BIT(0)	/* KXTF9 */
 #define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
+#define KXCJK1013_REG_INT_SRC1_MASK_TDTS	(BIT(2) | BIT(3))	/* KXTF9 */
+#define KXCJK1013_REG_INT_SRC1_TAP_NONE		0
+#define KXCJK1013_REG_INT_SRC1_TAP_SINGLE		BIT(2)
+#define KXCJK1013_REG_INT_SRC1_TAP_DOUBLE		BIT(3)
 #define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
 
+/* KXCJK: INT_SOURCE2: motion detect, KXTF9: INT_SRC_REG1: tap detect */
 #define KXCJK1013_DIRECTION_BIT_ZP	BIT(0)
 #define KXCJK1013_DIRECTION_BIT_ZN	BIT(1)
 #define KXCJK1013_DIRECTION_BIT_YP	BIT(2)
@@ -93,6 +130,7 @@ enum kx_chipset {
 	KXCJK1013,
 	KXCJ91008,
 	KXTJ21009,
+	KXTF9,
 	KX_MAX_CHIPS /* this must be last */
 };
 
@@ -158,6 +196,18 @@ static const struct kx_odr_map samp_freq_table[] = {
 static const char *const samp_freq_avail =
 	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600";
 
+static const struct kx_odr_map kxtf9_samp_freq_table[] = {
+	{ 25, 0, 0x01, 0x00 },
+	{ 50, 0, 0x02, 0x01 },
+	{ 100, 0, 0x03, 0x01 },
+	{ 200, 0, 0x04, 0x01 },
+	{ 400, 0, 0x05, 0x01 },
+	{ 800, 0, 0x06, 0x01 },
+};
+
+static const char *const kxtf9_samp_freq_avail =
+	"25 50 100 200 400 800";
+
 /* Refer to section 4 of the specification */
 static const struct {
 	int odr_bits;
@@ -208,6 +258,15 @@ static const struct {
 		{0x06, 3000},
 		{0x07, 2000},
 	},
+	/* KXTF9 */
+	{
+		{0x01, 81000},
+		{0x02, 41000},
+		{0x03, 21000},
+		{0x04, 11000},
+		{0x05, 5100},
+		{0x06, 2700},
+	},
 };
 
 static const struct {
@@ -404,7 +463,7 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
 
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
-	int ret;
+	int waketh_reg, ret;
 
 	ret = i2c_smbus_write_byte_data(data->client,
 					KXCJK1013_REG_WAKE_TIMER,
@@ -415,8 +474,9 @@ static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 		return ret;
 	}
 
-	ret = i2c_smbus_write_byte_data(data->client,
-					KXCJK1013_REG_WAKE_THRES,
+	waketh_reg = data->chipset == KXTF9 ?
+		KXTF9_REG_WAKE_THRESH : KXCJK1013_REG_WAKE_THRES;
+	ret = i2c_smbus_write_byte_data(data->client, waketh_reg,
 					data->wake_thres);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error writing reg_wake_thres\n");
@@ -590,9 +650,14 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 	if (ret < 0)
 		return ret;
 
-	odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
-					       ARRAY_SIZE(samp_freq_table),
-					       val, val2);
+	if (data->chipset == KXTF9)
+		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
+						       ARRAY_SIZE(kxtf9_samp_freq_table),
+						       val, val2);
+	else
+		odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
+						       ARRAY_SIZE(samp_freq_table),
+						       val, val2);
 
 	if (IS_ERR(odr_setting))
 		return PTR_ERR(odr_setting);
@@ -629,9 +694,14 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
 {
-	return kxcjk1013_convert_odr_value(samp_freq_table,
-					   ARRAY_SIZE(samp_freq_table),
-					   data->odr_bits, val, val2);
+	if (data->chipset == KXTF9)
+		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
+						   ARRAY_SIZE(kxtf9_samp_freq_table),
+						   data->odr_bits, val, val2);
+	else
+		return kxcjk1013_convert_odr_value(samp_freq_table,
+						   ARRAY_SIZE(samp_freq_table),
+						   data->odr_bits, val, val2);
 }
 
 static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
@@ -886,7 +956,16 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
 					     struct device_attribute *attr,
 					     char *buf)
 {
-	return sprintf(buf, "%s\n", samp_freq_avail);
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	const char *str;
+
+	if (data->chipset == KXTF9)
+		str = kxtf9_samp_freq_avail;
+	else
+		str = samp_freq_avail;
+
+	return sprintf(buf, "%s\n", str);
 }
 
 static IIO_DEVICE_ATTR(in_accel_sampling_frequency_available, S_IRUGO,
@@ -1121,7 +1200,16 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
 	}
 
 	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
-		kxcjk1013_report_motion_event(indio_dev);
+		if (data->chipset == KXTF9)
+			iio_push_event(indio_dev,
+				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
+				       0,
+				       IIO_MOD_X_AND_Y_AND_Z,
+				       IIO_EV_TYPE_THRESH,
+				       IIO_EV_DIR_RISING),
+				       data->timestamp);
+		else
+			kxcjk1013_report_motion_event(indio_dev);
 	}
 
 ack_intr:
@@ -1414,6 +1502,7 @@ static const struct i2c_device_id kxcjk1013_id[] = {
 	{"kxcjk1013", KXCJK1013},
 	{"kxcj91008", KXCJ91008},
 	{"kxtj21009", KXTJ21009},
+	{"kxtf9",     KXTF9},
 	{"SMO8500",   KXCJ91008},
 	{}
 };
-- 
2.11.0

Re: [PATCH v2 7/7] iio: accel: kxcjk1013: add support for KXTF9

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:32 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> KXTF9 has mostly compatible register layout to KXCJK accelerometer.
> There is no motion direction interrupt support, but there is tap
> direction detection instead (not implemented in this patch).
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Just a follow up on the lack of prefix on the samp frequency list,
otherwise looks good to me.

We have now missed the coming merge window anyway so this is 4.15 material
and we have lots of time.  If we might have 'snuck' it in I might have
pinged Srinivas to get him to check it out once it was applied, but given
the timing lets do things the ideal way!

Jonathan

> ---
>  drivers/iio/accel/Kconfig      |   4 +-
>  drivers/iio/accel/kxcjk-1013.c | 115 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 104 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 15de262015df..0be352a7b6f4 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -219,8 +219,8 @@ config KXCJK1013
>  	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say Y here if you want to build a driver for the Kionix KXCJK-1013
> -	  triaxial acceleration sensor. This driver also supports KXCJ9-1008
> -	  and KXTJ2-1009.
> +	  triaxial acceleration sensor. This driver also supports KXCJ9-1008,
> +	  KXTJ2-1009 and KXTF9.
>  
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called kxcjk-1013.
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 48b9a97e8a58..583393f454fa 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -34,6 +34,13 @@
>  #define KXCJK1013_DRV_NAME "kxcjk1013"
>  #define KXCJK1013_IRQ_NAME "kxcjk1013_event"
>  
> +#define KXTF9_REG_HP_XOUT_L		0x00
> +#define KXTF9_REG_HP_XOUT_H		0x01
> +#define KXTF9_REG_HP_YOUT_L		0x02
> +#define KXTF9_REG_HP_YOUT_H		0x03
> +#define KXTF9_REG_HP_ZOUT_L		0x04
> +#define KXTF9_REG_HP_ZOUT_H		0x05
> +
>  #define KXCJK1013_REG_XOUT_L		0x06
>  /*
>   * From low byte X axis register, all the other addresses of Y and Z can be
> @@ -48,17 +55,33 @@
>  
>  #define KXCJK1013_REG_DCST_RESP		0x0C
>  #define KXCJK1013_REG_WHO_AM_I		0x0F
> -#define KXCJK1013_REG_INT_SRC1		0x16
> +#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 KXCJK1013_REG_INT_SRC2		0x17
>  #define KXCJK1013_REG_STATUS_REG	0x18
>  #define KXCJK1013_REG_INT_REL		0x1A
>  #define KXCJK1013_REG_CTRL1		0x1B
> -#define KXCJK1013_REG_CTRL2		0x1D
> +#define KXTF9_REG_CTRL2			0x1C
> +#define KXCJK1013_REG_CTRL2		0x1D	/* mostly compatible, CTRL_REG3 in KTXF9 ds */
>  #define KXCJK1013_REG_INT_CTRL1		0x1E
>  #define KXCJK1013_REG_INT_CTRL2		0x1F
> +#define KXTF9_REG_INT_CTRL3		0x20
>  #define KXCJK1013_REG_DATA_CTRL		0x21
> +#define KXTF9_REG_TILT_TIMER		0x28
>  #define KXCJK1013_REG_WAKE_TIMER	0x29
> +#define KXTF9_REG_TDT_TIMER		0x2B
> +#define KXTF9_REG_TDT_THRESH_H		0x2C
> +#define KXTF9_REG_TDT_THRESH_L		0x2D
> +#define KXTF9_REG_TDT_TAP_TIMER		0x2E
> +#define KXTF9_REG_TDT_TOTAL_TIMER	0x2F
> +#define KXTF9_REG_TDT_LATENCY_TIMER	0x30
> +#define KXTF9_REG_TDT_WINDOW_TIMER	0x31
>  #define KXCJK1013_REG_SELF_TEST		0x3A
> +#define KXTF9_REG_WAKE_THRESH		0x5A
> +#define KXTF9_REG_TILT_ANGLE		0x5C
> +#define KXTF9_REG_HYST_SET		0x5F
>  #define KXCJK1013_REG_WAKE_THRES	0x6A
>  
>  #define KXCJK1013_REG_CTRL1_BIT_PC1	BIT(7)
> @@ -68,18 +91,32 @@
>  #define KXCJK1013_REG_CTRL1_BIT_GSEL0	BIT(3)
>  #define KXCJK1013_REG_CTRL1_BIT_WUFE	BIT(1)
>  
> +#define KXCJK1013_REG_INT_CTRL1_BIT_IEU	BIT(2)	/* KXTF9 */
>  #define KXCJK1013_REG_INT_CTRL1_BIT_IEL	BIT(3)
>  #define KXCJK1013_REG_INT_CTRL1_BIT_IEA	BIT(4)
>  #define KXCJK1013_REG_INT_CTRL1_BIT_IEN	BIT(5)
>  
> +#define KXTF9_REG_TILT_BIT_LEFT_EDGE	BIT(5)
> +#define KXTF9_REG_TILT_BIT_RIGHT_EDGE	BIT(4)
> +#define KXTF9_REG_TILT_BIT_LOWER_EDGE	BIT(3)
> +#define KXTF9_REG_TILT_BIT_UPPER_EDGE	BIT(2)
> +#define KXTF9_REG_TILT_BIT_FACE_DOWN	BIT(1)
> +#define KXTF9_REG_TILT_BIT_FACE_UP	BIT(0)
> +
>  #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
>  #define KXCJK1013_MAX_STARTUP_TIME_US	100000
>  
>  #define KXCJK1013_SLEEP_DELAY_MS	2000
>  
> +#define KXCJK1013_REG_INT_SRC1_BIT_TPS	BIT(0)	/* KXTF9 */
>  #define KXCJK1013_REG_INT_SRC1_BIT_WUFS	BIT(1)
> +#define KXCJK1013_REG_INT_SRC1_MASK_TDTS	(BIT(2) | BIT(3))	/* KXTF9 */
> +#define KXCJK1013_REG_INT_SRC1_TAP_NONE		0
> +#define KXCJK1013_REG_INT_SRC1_TAP_SINGLE		BIT(2)
> +#define KXCJK1013_REG_INT_SRC1_TAP_DOUBLE		BIT(3)
>  #define KXCJK1013_REG_INT_SRC1_BIT_DRDY	BIT(4)
>  
> +/* KXCJK: INT_SOURCE2: motion detect, KXTF9: INT_SRC_REG1: tap detect */
>  #define KXCJK1013_DIRECTION_BIT_ZP	BIT(0)
>  #define KXCJK1013_DIRECTION_BIT_ZN	BIT(1)
>  #define KXCJK1013_DIRECTION_BIT_YP	BIT(2)
> @@ -93,6 +130,7 @@ enum kx_chipset {
>  	KXCJK1013,
>  	KXCJ91008,
>  	KXTJ21009,
> +	KXTF9,
>  	KX_MAX_CHIPS /* this must be last */
>  };
>  
> @@ -158,6 +196,18 @@ static const struct kx_odr_map samp_freq_table[] = {
>  static const char *const samp_freq_avail =
>  	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600";
>  
> +static const struct kx_odr_map kxtf9_samp_freq_table[] = {
> +	{ 25, 0, 0x01, 0x00 },
> +	{ 50, 0, 0x02, 0x01 },
> +	{ 100, 0, 0x03, 0x01 },
> +	{ 200, 0, 0x04, 0x01 },
> +	{ 400, 0, 0x05, 0x01 },
> +	{ 800, 0, 0x06, 0x01 },
> +};
> +
> +static const char *const kxtf9_samp_freq_avail =
> +	"25 50 100 200 400 800";
> +
>  /* Refer to section 4 of the specification */
>  static const struct {
>  	int odr_bits;
> @@ -208,6 +258,15 @@ static const struct {
>  		{0x06, 3000},
>  		{0x07, 2000},
>  	},
> +	/* KXTF9 */
> +	{
> +		{0x01, 81000},
> +		{0x02, 41000},
> +		{0x03, 21000},
> +		{0x04, 11000},
> +		{0x05, 5100},
> +		{0x06, 2700},
> +	},
>  };
>  
>  static const struct {
> @@ -404,7 +463,7 @@ static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
>  
>  static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
>  {
> -	int ret;
> +	int waketh_reg, ret;
>  
>  	ret = i2c_smbus_write_byte_data(data->client,
>  					KXCJK1013_REG_WAKE_TIMER,
> @@ -415,8 +474,9 @@ static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>  
> -	ret = i2c_smbus_write_byte_data(data->client,
> -					KXCJK1013_REG_WAKE_THRES,
> +	waketh_reg = data->chipset == KXTF9 ?
> +		KXTF9_REG_WAKE_THRESH : KXCJK1013_REG_WAKE_THRES;
> +	ret = i2c_smbus_write_byte_data(data->client, waketh_reg,
>  					data->wake_thres);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "Error writing reg_wake_thres\n");
> @@ -590,9 +650,14 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  	if (ret < 0)
>  		return ret;
>  
> -	odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
> -					       ARRAY_SIZE(samp_freq_table),
> -					       val, val2);
> +	if (data->chipset == KXTF9)
> +		odr_setting = kxcjk1013_find_odr_value(kxtf9_samp_freq_table,
> +						       ARRAY_SIZE(kxtf9_samp_freq_table),
> +						       val, val2);
> +	else
> +		odr_setting = kxcjk1013_find_odr_value(samp_freq_table,
> +						       ARRAY_SIZE(samp_freq_table),
> +						       val, val2);
>  
>  	if (IS_ERR(odr_setting))
>  		return PTR_ERR(odr_setting);
> @@ -629,9 +694,14 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  
>  static int kxcjk1013_get_odr(struct kxcjk1013_data *data, int *val, int *val2)
>  {
> -	return kxcjk1013_convert_odr_value(samp_freq_table,
> -					   ARRAY_SIZE(samp_freq_table),
> -					   data->odr_bits, val, val2);
> +	if (data->chipset == KXTF9)
> +		return kxcjk1013_convert_odr_value(kxtf9_samp_freq_table,
> +						   ARRAY_SIZE(kxtf9_samp_freq_table),
> +						   data->odr_bits, val, val2);
> +	else
> +		return kxcjk1013_convert_odr_value(samp_freq_table,
> +						   ARRAY_SIZE(samp_freq_table),
> +						   data->odr_bits, val, val2);
>  }
>  
>  static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
> @@ -886,7 +956,16 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
>  					     struct device_attribute *attr,
>  					     char *buf)
>  {
> -	return sprintf(buf, "%s\n", samp_freq_avail);
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct kxcjk1013_data *data = iio_priv(indio_dev);
> +	const char *str;
> +
> +	if (data->chipset == KXTF9)
> +		str = kxtf9_samp_freq_avail;
> +	else
> +		str = samp_freq_avail;

Ah, I'd missed the lack of prefix when this was introduced.  Result is this
looks a bit odd here and there is a potential for a name clash in future.
Better to add a suitable prefix kxcjk_samp_freq_avail perhaps?

> +
> +	return sprintf(buf, "%s\n", str);
>  }
>  
>  static IIO_DEVICE_ATTR(in_accel_sampling_frequency_available, S_IRUGO,
> @@ -1121,7 +1200,16 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
>  	}
>  
>  	if (ret & KXCJK1013_REG_INT_SRC1_BIT_WUFS) {
> -		kxcjk1013_report_motion_event(indio_dev);
> +		if (data->chipset == KXTF9)
> +			iio_push_event(indio_dev,
> +				       IIO_MOD_EVENT_CODE(IIO_ACCEL,
> +				       0,
> +				       IIO_MOD_X_AND_Y_AND_Z,
> +				       IIO_EV_TYPE_THRESH,
> +				       IIO_EV_DIR_RISING),
> +				       data->timestamp);
> +		else
> +			kxcjk1013_report_motion_event(indio_dev);
>  	}
>  
>  ack_intr:
> @@ -1414,6 +1502,7 @@ static const struct i2c_device_id kxcjk1013_id[] = {
>  	{"kxcjk1013", KXCJK1013},
>  	{"kxcj91008", KXCJ91008},
>  	{"kxtj21009", KXTJ21009},
> +	{"kxtf9",     KXTF9},
>  	{"SMO8500",   KXCJ91008},
>  	{}
>  };

[PATCH v2 5/7] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic

[Michał Mirosław]

In preparation for KXTF9 support, make sampling_frequency_avail
attribute dynamic.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 252004640403..11bf9afbd248 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -155,6 +155,9 @@ static const struct kx_odr_map samp_freq_table[] = {
 	{ 1600, 0, 0x07, 0x06 },
 };
 
+static const char *const samp_freq_avail =
+	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600";
+
 /* Refer to section 4 of the specification */
 static const struct {
 	int odr_bits;
@@ -879,13 +882,19 @@ static int kxcjk1013_buffer_postdisable(struct iio_dev *indio_dev)
 	return kxcjk1013_set_power_state(data, false);
 }
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
-	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600");
+static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	return sprintf(buf, "%s\n", samp_freq_avail);
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(kxcjk1013_get_samp_freq_avail);
 
 static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
 
 static struct attribute *kxcjk1013_attributes[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
 	NULL,
 };
-- 
2.11.0

Re: [PATCH v2 5/7] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:32 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> In preparation for KXTF9 support, make sampling_frequency_avail
> attribute dynamic.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

This looks fine,

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 252004640403..11bf9afbd248 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -155,6 +155,9 @@ static const struct kx_odr_map samp_freq_table[] = {
>  	{ 1600, 0, 0x07, 0x06 },
>  };
>  
> +static const char *const samp_freq_avail =
> +	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600";
> +
>  /* Refer to section 4 of the specification */
>  static const struct {
>  	int odr_bits;
> @@ -879,13 +882,19 @@ static int kxcjk1013_buffer_postdisable(struct iio_dev *indio_dev)
>  	return kxcjk1013_set_power_state(data, false);
>  }
>  
> -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> -	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600");
> +static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
> +					     struct device_attribute *attr,
> +					     char *buf)
> +{
> +	return sprintf(buf, "%s\n", samp_freq_avail);
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(kxcjk1013_get_samp_freq_avail);
>  
>  static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
>  
>  static struct attribute *kxcjk1013_attributes[] = {
> -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
>  	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>  	NULL,
>  };

Re: [PATCH v2 5/7] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic

[Jonathan Cameron]

On Sun, 3 Sep 2017 12:58:28 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 22 Aug 2017 00:03:32 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> 
> > In preparation for KXTF9 support, make sampling_frequency_avail
> > attribute dynamic.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>  
> 
> This looks fine,
> 

Actually I would like one really small change...

> Jonathan
> 
> > ---
> >  drivers/iio/accel/kxcjk-1013.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > index 252004640403..11bf9afbd248 100644
> > --- a/drivers/iio/accel/kxcjk-1013.c
> > +++ b/drivers/iio/accel/kxcjk-1013.c
> > @@ -155,6 +155,9 @@ static const struct kx_odr_map samp_freq_table[] = {
> >  	{ 1600, 0, 0x07, 0x06 },
> >  };
> >  
> > +static const char *const samp_freq_avail =
> > +	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600";
> > +

I'd like this prefixed with one of the supported parts to avoid a potential name
clash down the line (and because it looks weird in patch 7 ;)

Jonathan

> >  /* Refer to section 4 of the specification */
> >  static const struct {
> >  	int odr_bits;
> > @@ -879,13 +882,19 @@ static int kxcjk1013_buffer_postdisable(struct iio_dev *indio_dev)
> >  	return kxcjk1013_set_power_state(data, false);
> >  }
> >  
> > -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
> > -	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600");
> > +static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
> > +					     struct device_attribute *attr,
> > +					     char *buf)
> > +{
> > +	return sprintf(buf, "%s\n", samp_freq_avail);
> > +}
> > +
> > +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(kxcjk1013_get_samp_freq_avail);
> >  
> >  static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
> >  
> >  static struct attribute *kxcjk1013_attributes[] = {
> > -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> > +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> >  	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
> >  	NULL,
> >  };  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/7] iio: accel: kxcjk1013: make sampling_frequency_avail per-type

[Michał Mirosław]

Make sampling_frequency_avail per-type - like sampling_frequency is.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/iio/accel/kxcjk-1013.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 11bf9afbd248..48b9a97e8a58 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -889,12 +889,13 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
 	return sprintf(buf, "%s\n", samp_freq_avail);
 }
 
-static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(kxcjk1013_get_samp_freq_avail);
+static IIO_DEVICE_ATTR(in_accel_sampling_frequency_available, S_IRUGO,
+		       kxcjk1013_get_samp_freq_avail, NULL, 0);
 
 static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
 
 static struct attribute *kxcjk1013_attributes[] = {
-	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_sampling_frequency_available.dev_attr.attr,
 	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
 	NULL,
 };
-- 
2.11.0

Re: [PATCH v2 6/7] iio: accel: kxcjk1013: make sampling_frequency_avail per-type

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:32 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Make sampling_frequency_avail per-type - like sampling_frequency is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Hmm. This is obviously ABI change.  However, it's pretty obscure so I very
much doubt anyone will notice.  I'd like this 'fixed' anyway so lets
go with it and deal with the fall out should there be any problems.

Jonathan

> ---
>  drivers/iio/accel/kxcjk-1013.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 11bf9afbd248..48b9a97e8a58 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -889,12 +889,13 @@ static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
>  	return sprintf(buf, "%s\n", samp_freq_avail);
>  }
>  
> -static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(kxcjk1013_get_samp_freq_avail);
> +static IIO_DEVICE_ATTR(in_accel_sampling_frequency_available, S_IRUGO,
> +		       kxcjk1013_get_samp_freq_avail, NULL, 0);
>  
>  static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
>  
>  static struct attribute *kxcjk1013_attributes[] = {
> -	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_in_accel_sampling_frequency_available.dev_attr.attr,
>  	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>  	NULL,
>  };

Re: [PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Michał Mirosław]

On Tue, Aug 22, 2017 at 12:03:29AM +0200, Michał Mirosław wrote:
> This series adds support for KXTF9 accelerometer. It is mostly compatible
> to KXCJK, but replaces motion direction interrupt with tap detection
> (not supported in this implementation, yet).
> 
> First 5 patches are refactorings with no outside-visible effects.
> Patch 6 fixes sysfs attribute name of sampling_frequency_avail
> to match sampling_frequency attribute it describes.
> Patch 7 finally adds support for KXTF9 accelerometers.

I forgot to add changelog. Here it is:

v2:
	- removed register renaming
	- split report_motion_event() patch

Re: [PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Jonathan Cameron]

On Tue, 22 Aug 2017 00:03:29 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> This series adds support for KXTF9 accelerometer. It is mostly compatible
> to KXCJK, but replaces motion direction interrupt with tap detection
> (not supported in this implementation, yet).
> 
> First 5 patches are refactorings with no outside-visible effects.
> Patch 6 fixes sysfs attribute name of sampling_frequency_avail
> to match sampling_frequency attribute it describes.
> Patch 7 finally adds support for KXTF9 accelerometers.

Cc'd Srinivas (original author of this driver - should really have been cc'd
from the start).

Michał: I'm assuming you don't have the parts already supported by the driver?

Srinivas: Are you likely to get time to take a quick look at this series any
time in the next few weeks?

Even better if you still have test hardware to check nothing broke on the
original supported parts (not to worry if not!)

We are past the point where these are going to make the coming merge window
anyway so have plenty of time.

Thanks,

Jonathan

p.s. If anyone else has the parts already supported by this driver and
can give the changes a quick check that would be great!
> 
> Michał Mirosław (7):
>   iio: accel: kxcjk1003: refactor ODR setting
>   iio: accel: kxcjk1013: fix INT_CTRL/INT_SRC1 bit names
>   iio: accel: kxcjk1013: extract report_motion_event() from interrupt
>     handler
>   iio: accel: kxcjk1013: rename motion direction bits
>   iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic
>   iio: accel: kxcjk1013: make sampling_frequency_avail per-type
>   iio: accel: kxcjk1013: add support for KXTF9
> 
>  drivers/iio/accel/Kconfig      |   4 +-
>  drivers/iio/accel/kxcjk-1013.c | 366 ++++++++++++++++++++++++++---------------
>  2 files changed, 235 insertions(+), 135 deletions(-)
> 

Re: [PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Michał Mirosław]

On Sun, Sep 03, 2017 at 12:43:35PM +0100, Jonathan Cameron wrote:
> On Tue, 22 Aug 2017 00:03:29 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > This series adds support for KXTF9 accelerometer. It is mostly compatible
> > to KXCJK, but replaces motion direction interrupt with tap detection
> > (not supported in this implementation, yet).
> > 
> > First 5 patches are refactorings with no outside-visible effects.
> > Patch 6 fixes sysfs attribute name of sampling_frequency_avail
> > to match sampling_frequency attribute it describes.
> > Patch 7 finally adds support for KXTF9 accelerometers.
> Cc'd Srinivas (original author of this driver - should really have been cc'd
> from the start).
> 
> Michał: I'm assuming you don't have the parts already supported by the driver?

No. The only one I have is inside my Asus TF300T I'm trying to
get working fully under Debian.

> We are past the point where these are going to make the coming merge window
> anyway so have plenty of time.

I'll rebase the series after v4.14-rc1 comes out, then. I can add
the global renaming cleanup on top (removing '1013' from driver and
register names) if you're willing to accept it.

Thanks for reviewing!

Best Regards,
Michał Mirosław

Re: [PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Jonathan Cameron]

On Mon, 4 Sep 2017 23:04:01 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Sun, Sep 03, 2017 at 12:43:35PM +0100, Jonathan Cameron wrote:
> > On Tue, 22 Aug 2017 00:03:29 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:  
> > > This series adds support for KXTF9 accelerometer. It is mostly compatible
> > > to KXCJK, but replaces motion direction interrupt with tap detection
> > > (not supported in this implementation, yet).
> > > 
> > > First 5 patches are refactorings with no outside-visible effects.
> > > Patch 6 fixes sysfs attribute name of sampling_frequency_avail
> > > to match sampling_frequency attribute it describes.
> > > Patch 7 finally adds support for KXTF9 accelerometers.  
> > Cc'd Srinivas (original author of this driver - should really have been cc'd
> > from the start).
> > 
> > Michał: I'm assuming you don't have the parts already supported by the driver?  
> 
> No. The only one I have is inside my Asus TF300T I'm trying to
> get working fully under Debian.
> 
> > We are past the point where these are going to make the coming merge window
> > anyway so have plenty of time.  
> 
> I'll rebase the series after v4.14-rc1 comes out, then. I can add
> the global renaming cleanup on top (removing '1013' from driver and
> register names) if you're willing to accept it.

That would be great.

Jonathan

> 
> Thanks for reviewing!
> 
> Best Regards,
> Michał Mirosław
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/7] iio: accel: kxcjk1003: support Kionix KXTF9

[Srinivas Pandruvada]

On Sun, 2017-09-03 at 12:43 +0100, Jonathan Cameron wrote:
> On Tue, 22 Aug 2017 00:03:29 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> 
> > 
> > This series adds support for KXTF9 accelerometer. It is mostly
> > compatible
> > to KXCJK, but replaces motion direction interrupt with tap
> > detection
> > (not supported in this implementation, yet).
> > 
> > First 5 patches are refactorings with no outside-visible effects.
> > Patch 6 fixes sysfs attribute name of sampling_frequency_avail
> > to match sampling_frequency attribute it describes.
> > Patch 7 finally adds support for KXTF9 accelerometers.
> 
> Cc'd Srinivas (original author of this driver - should really have
> been cc'd
> from the start).
Michal, if you update series, please copy me.

> 
> Michał: I'm assuming you don't have the parts already supported by
> the driver?
> 
> Srinivas: Are you likely to get time to take a quick look at this
> series any
> time in the next few weeks?
I will look either this week or next week.

> 
> Even better if you still have test hardware to check nothing broke on
> the
> original supported parts (not to worry if not!)
I don't, but will check around.

Thanks,
Srinivas




> We are past the point where these are going to make the coming merge
> window
> anyway so have plenty of time.
> 
> Thanks,
> 
> Jonathan
> 
> p.s. If anyone else has the parts already supported by this driver
> and
> can give the changes a quick check that would be great!
> > 
> > 
> > Michał Mirosław (7):
> >   iio: accel: kxcjk1003: refactor ODR setting
> >   iio: accel: kxcjk1013: fix INT_CTRL/INT_SRC1 bit names
> >   iio: accel: kxcjk1013: extract report_motion_event() from
> > interrupt
> >     handler
> >   iio: accel: kxcjk1013: rename motion direction bits
> >   iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail
> > dynamic
> >   iio: accel: kxcjk1013: make sampling_frequency_avail per-type
> >   iio: accel: kxcjk1013: add support for KXTF9
> > 
> >  drivers/iio/accel/Kconfig      |   4 +-
> >  drivers/iio/accel/kxcjk-1013.c | 366 ++++++++++++++++++++++++++---
> > ------------
> >  2 files changed, 235 insertions(+), 135 deletions(-)
> > 
>