* [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9
@ 2017-08-17 14:21 Michał Mirosław
2017-08-17 14:21 ` [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting Michał Mirosław
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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 4 patches are refactorings with no outside-visible effects.
Patch 5 changes sysfs attribute name of sampling_frequency_avail
to match sampling_frequency attribute it describes.
Michał Mirosław (6):
iio: accel: kxcjk1003: refactor ODR setting
iio: accel: kxcjk1013: extract report_motion_event() from interrupt
handler
iio: accel: kxcjk1013: rename registers for KXTF9 compatibility
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 | 448 +++++++++++++++++++++++++----------------
2 files changed, 276 insertions(+), 176 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 10:02 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility Michał Mirosław
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
2017-08-17 14:21 ` [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 9:52 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler Michał Mirosław
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
Rename some registers that are shared between KXTF9 and KXCJK.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
drivers/iio/accel/kxcjk-1013.c | 123 ++++++++++++++++++++++-------------------
1 file changed, 65 insertions(+), 58 deletions(-)
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 80a6508d6370..27147b687471 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -48,39 +48,48 @@
#define KXCJK1013_REG_DCST_RESP 0x0C
#define KXCJK1013_REG_WHO_AM_I 0x0F
-#define KXCJK1013_REG_INT_SRC1 0x16
+#define KXREG_INT_SRC_DATA 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 KXCJK1013_REG_CTRL2 0x1D
-#define KXCJK1013_REG_INT_CTRL1 0x1E
-#define KXCJK1013_REG_INT_CTRL2 0x1F
+#define KXREG_CTRL1 0x1B
+#define KXREG_CTRL3 0x1D
+#define KXREG_INT_CTRL1 0x1E
+#define KXREG_INT_CTRL2 0x1F
#define KXCJK1013_REG_DATA_CTRL 0x21
#define KXCJK1013_REG_WAKE_TIMER 0x29
#define KXCJK1013_REG_SELF_TEST 0x3A
#define KXCJK1013_REG_WAKE_THRES 0x6A
-#define KXCJK1013_REG_CTRL1_BIT_PC1 BIT(7)
-#define KXCJK1013_REG_CTRL1_BIT_RES BIT(6)
-#define KXCJK1013_REG_CTRL1_BIT_DRDY BIT(5)
-#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 KXREG_CTRL1_BIT_PC1 BIT(7)
+#define KXREG_CTRL1_BIT_RES BIT(6)
+#define KXREG_CTRL1_BIT_DRDY BIT(5)
+#define KXREG_CTRL1_BIT_GSEL1 BIT(4)
+#define KXREG_CTRL1_BIT_GSEL0 BIT(3)
+#define KXREG_CTRL1_BIT_TDTE BIT(2)
+#define KXREG_CTRL1_BIT_WUFE BIT(1)
+#define KXREG_CTRL1_BIT_TPE BIT(0)
+
+#define KXREG_INT_CTRL1_BIT_IEL BIT(3)
+#define KXREG_INT_CTRL1_BIT_IEA BIT(4)
+#define KXREG_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_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)
+/* KXCJK: INT_SOURCE1 */
+#define KXREG_INT_BIT_WUFS BIT(1)
+#define KXREG_INT_BIT_DRDY BIT(4)
+
+/* KXCJK: INT_SOURCE2: motion detect */
+#define KXREG_MOTION_INT_BIT_ZP BIT(0)
+#define KXREG_MOTION_INT_BIT_ZN BIT(1)
+#define KXREG_MOTION_INT_BIT_YP BIT(2)
+#define KXREG_MOTION_INT_BIT_YN BIT(3)
+#define KXREG_MOTION_INT_BIT_XP BIT(4)
+#define KXREG_MOTION_INT_BIT_XN BIT(5)
#define KXCJK1013_DEFAULT_WAKE_THRES 1
@@ -215,19 +224,19 @@ 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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
if (mode == STANDBY)
- ret &= ~KXCJK1013_REG_CTRL1_BIT_PC1;
+ ret &= ~KXREG_CTRL1_BIT_PC1;
else
- ret |= KXCJK1013_REG_CTRL1_BIT_PC1;
+ ret |= KXREG_CTRL1_BIT_PC1;
ret = i2c_smbus_write_byte_data(data->client,
- KXCJK1013_REG_CTRL1, ret);
+ KXREG_CTRL1, ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
return ret;
@@ -241,13 +250,13 @@ 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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
- if (ret & KXCJK1013_REG_CTRL1_BIT_PC1)
+ if (ret & KXREG_CTRL1_BIT_PC1)
*mode = OPERATION;
else
*mode = STANDBY;
@@ -259,19 +268,19 @@ 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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
- ret &= ~(KXCJK1013_REG_CTRL1_BIT_GSEL0 |
- KXCJK1013_REG_CTRL1_BIT_GSEL1);
+ ret &= ~(KXREG_CTRL1_BIT_GSEL0 |
+ KXREG_CTRL1_BIT_GSEL1);
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,
+ KXREG_CTRL1,
ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
@@ -299,16 +308,16 @@ 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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
/* Set 12 bit mode */
- ret |= KXCJK1013_REG_CTRL1_BIT_RES;
+ ret |= KXREG_CTRL1_BIT_RES;
- ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL1,
+ ret = i2c_smbus_write_byte_data(data->client, KXREG_CTRL1,
ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl\n");
@@ -329,18 +338,18 @@ 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, KXREG_INT_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
return ret;
}
if (data->active_high_intr)
- ret |= KXCJK1013_REG_INT_REG1_BIT_IEA;
+ ret |= KXREG_INT_CTRL1_BIT_IEA;
else
- ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEA;
+ ret &= ~KXREG_INT_CTRL1_BIT_IEA;
- ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
+ ret = i2c_smbus_write_byte_data(data->client, KXREG_INT_CTRL1,
ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
@@ -437,37 +446,37 @@ 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, KXREG_INT_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
return ret;
}
if (status)
- ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
+ ret |= KXREG_INT_CTRL1_BIT_IEN;
else
- ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
+ ret &= ~KXREG_INT_CTRL1_BIT_IEN;
- ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
+ ret = i2c_smbus_write_byte_data(data->client, KXREG_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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
if (status)
- ret |= KXCJK1013_REG_CTRL1_BIT_WUFE;
+ ret |= KXREG_CTRL1_BIT_WUFE;
else
- ret &= ~KXCJK1013_REG_CTRL1_BIT_WUFE;
+ ret &= ~KXREG_CTRL1_BIT_WUFE;
ret = i2c_smbus_write_byte_data(data->client,
- KXCJK1013_REG_CTRL1, ret);
+ KXREG_CTRL1, ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
return ret;
@@ -497,37 +506,35 @@ 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, KXREG_INT_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
return ret;
}
if (status)
- ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
+ ret |= KXREG_INT_CTRL1_BIT_IEN;
else
- ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
+ ret &= ~KXREG_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, KXREG_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, KXREG_CTRL1);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
return ret;
}
if (status)
- ret |= KXCJK1013_REG_CTRL1_BIT_DRDY;
+ ret |= KXREG_CTRL1_BIT_DRDY;
else
- ret &= ~KXCJK1013_REG_CTRL1_BIT_DRDY;
+ ret &= ~KXREG_CTRL1_BIT_DRDY;
- ret = i2c_smbus_write_byte_data(data->client,
- KXCJK1013_REG_CTRL1, ret);
+ ret = i2c_smbus_write_byte_data(data->client, KXREG_CTRL1, ret);
if (ret < 0) {
dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
return ret;
@@ -603,10 +610,10 @@ 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, KXREG_CTRL3,
odr_setting->wuf_bits);
if (ret < 0) {
- dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
+ dev_err(&data->client->dev, "Error writing reg_ctrl3\n");
return ret;
}
@@ -1097,13 +1104,13 @@ 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, KXREG_INT_SRC_DATA);
if (ret < 0) {
dev_err(&data->client->dev, "Error reading reg_int_src1\n");
goto ack_intr;
}
- if (ret & 0x02) {
+ if (ret & KXREG_INT_BIT_WUFS) {
kxcjk1013_report_motion_event(indio_dev);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
2017-08-17 14:21 ` [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting Michał Mirosław
2017-08-17 14:21 ` [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 10:01 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic Michał Mirosław
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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 | 124 +++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 59 deletions(-)
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 33e4b98f39fc..80a6508d6370 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1027,6 +1027,70 @@ 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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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;
@@ -1040,65 +1104,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
}
if (ret & 0x02) {
- 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
` (4 preceding siblings ...)
2017-08-17 14:21 ` [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9 Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 9:57 ` Jonathan Cameron
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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 50d9eefa745f..8bde6bead073 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -891,12 +891,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
` (3 preceding siblings ...)
2017-08-17 14:21 ` [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 10:00 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type Michał Mirosław
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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 | 119 +++++++++++++++++++++++++++++++++++------
2 files changed, 106 insertions(+), 17 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 8bde6bead073..a3f4fe1813d1 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,19 +55,43 @@
#define KXCJK1013_REG_DCST_RESP 0x0C
#define KXCJK1013_REG_WHO_AM_I 0x0F
-#define KXREG_INT_SRC_DATA 0x16
+#define KXTF9_REG_TILT_POS_CUR 0x10
+#define KXTF9_REG_TILT_POS_PREV 0x11
+#define KXTF9_REG_INT_SRC1 0x15
+#define KXREG_INT_SRC_DATA 0x16 /* compatible; KXTF9: INT_SRC2, KXCJK: INT_SRC1 */
#define KXCJK1013_REG_INT_SRC2 0x17
#define KXCJK1013_REG_STATUS_REG 0x18
#define KXCJK1013_REG_INT_REL 0x1A
#define KXREG_CTRL1 0x1B
-#define KXREG_CTRL3 0x1D
+#define KXREG_CTRL2 0x1C /* KXTF9: CTRL_REG2 */
+#define KXREG_CTRL3 0x1D /* mostly compatible; KXTF9: CTRL_REG3, KXCJK: CTRL_REG2 */
#define KXREG_INT_CTRL1 0x1E
#define KXREG_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
+/* TILT_POS bits */
+#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 KXREG_CTRL1_BIT_PC1 BIT(7)
#define KXREG_CTRL1_BIT_RES BIT(6)
#define KXREG_CTRL1_BIT_DRDY BIT(5)
@@ -70,6 +101,7 @@
#define KXREG_CTRL1_BIT_WUFE BIT(1)
#define KXREG_CTRL1_BIT_TPE BIT(0)
+#define KXREG_INT_CTRL1_BIT_IEU BIT(2)
#define KXREG_INT_CTRL1_BIT_IEL BIT(3)
#define KXREG_INT_CTRL1_BIT_IEA BIT(4)
#define KXREG_INT_CTRL1_BIT_IEN BIT(5)
@@ -79,11 +111,16 @@
#define KXCJK1013_SLEEP_DELAY_MS 2000
-/* KXCJK: INT_SOURCE1 */
+/* KXCJK: INT_SOURCE1, KXTF9: INT_SRC_REG2 */
+#define KXREG_INT_BIT_TPS BIT(0)
#define KXREG_INT_BIT_WUFS BIT(1)
+#define KXREG_INT_MASK_TDTS (BIT(2) | BIT(3))
+#define KXREG_INT_TAP_NONE 0
+#define KXREG_INT_TAP_SINGLE BIT(2)
+#define KXREG_INT_TAP_DOUBLE BIT(3)
#define KXREG_INT_BIT_DRDY BIT(4)
-/* KXCJK: INT_SOURCE2: motion detect */
+/* KXCJK: INT_SOURCE2: motion detect, KXTF9: INT_SRC_REG1: tap detect */
#define KXREG_MOTION_INT_BIT_ZP BIT(0)
#define KXREG_MOTION_INT_BIT_ZN BIT(1)
#define KXREG_MOTION_INT_BIT_YP BIT(2)
@@ -97,6 +134,7 @@ enum kx_chipset {
KXCJK1013,
KXCJ91008,
KXTJ21009,
+ KXTF9,
KX_MAX_CHIPS /* this must be last */
};
@@ -162,6 +200,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;
@@ -212,6 +262,15 @@ static const struct {
{0x06, 3000},
{0x07, 2000},
},
+ /* KXTF9 */
+ {
+ {0x01, 81000},
+ {0x02, 41000},
+ {0x03, 21000},
+ {0x04, 11000},
+ {0x05, 5100},
+ {0x06, 2700},
+ },
};
static const struct {
@@ -408,7 +467,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,
@@ -419,8 +478,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");
@@ -592,9 +652,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);
@@ -631,9 +696,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)
@@ -888,7 +958,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 & KXREG_INT_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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
` (2 preceding siblings ...)
2017-08-17 14:21 ` [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler Michał Mirosław
@ 2017-08-17 14:21 ` Michał Mirosław
2017-08-20 9:53 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9 Michał Mirosław
2017-08-17 14:21 ` [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type Michał Mirosław
5 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-17 14:21 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Cc: linux-iio
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 27147b687471..50d9eefa745f 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -159,6 +159,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;
@@ -881,13 +884,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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility
2017-08-17 14:21 ` [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility Michał Mirosław
@ 2017-08-20 9:52 ` Jonathan Cameron
2017-08-21 21:45 ` Michał Mirosław
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 9:52 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:36 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> Rename some registers that are shared between KXTF9 and KXCJK.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Hi Michał,
I'm not keen on this change. Going for generic names is always
fragile as all it takes is another part coming along which is almost
but not quite the same as you shared register set and we end up with
a mess.
General convention for both register values and driver naming is they
should be named after one supported part rather than trying to find
a generic name that covers all supported parts.
So please revert this change and resend the series.
Sorry to be a pain, but this has gone wrong quite a lot of times in
the past!
Jonathan
> ---
> drivers/iio/accel/kxcjk-1013.c | 123 ++++++++++++++++++++++-------------------
> 1 file changed, 65 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 80a6508d6370..27147b687471 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -48,39 +48,48 @@
>
> #define KXCJK1013_REG_DCST_RESP 0x0C
> #define KXCJK1013_REG_WHO_AM_I 0x0F
> -#define KXCJK1013_REG_INT_SRC1 0x16
> +#define KXREG_INT_SRC_DATA 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 KXCJK1013_REG_CTRL2 0x1D
> -#define KXCJK1013_REG_INT_CTRL1 0x1E
> -#define KXCJK1013_REG_INT_CTRL2 0x1F
> +#define KXREG_CTRL1 0x1B
> +#define KXREG_CTRL3 0x1D
> +#define KXREG_INT_CTRL1 0x1E
> +#define KXREG_INT_CTRL2 0x1F
> #define KXCJK1013_REG_DATA_CTRL 0x21
> #define KXCJK1013_REG_WAKE_TIMER 0x29
> #define KXCJK1013_REG_SELF_TEST 0x3A
> #define KXCJK1013_REG_WAKE_THRES 0x6A
>
> -#define KXCJK1013_REG_CTRL1_BIT_PC1 BIT(7)
> -#define KXCJK1013_REG_CTRL1_BIT_RES BIT(6)
> -#define KXCJK1013_REG_CTRL1_BIT_DRDY BIT(5)
> -#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 KXREG_CTRL1_BIT_PC1 BIT(7)
> +#define KXREG_CTRL1_BIT_RES BIT(6)
> +#define KXREG_CTRL1_BIT_DRDY BIT(5)
> +#define KXREG_CTRL1_BIT_GSEL1 BIT(4)
> +#define KXREG_CTRL1_BIT_GSEL0 BIT(3)
> +#define KXREG_CTRL1_BIT_TDTE BIT(2)
> +#define KXREG_CTRL1_BIT_WUFE BIT(1)
> +#define KXREG_CTRL1_BIT_TPE BIT(0)
> +
> +#define KXREG_INT_CTRL1_BIT_IEL BIT(3)
> +#define KXREG_INT_CTRL1_BIT_IEA BIT(4)
> +#define KXREG_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_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)
> +/* KXCJK: INT_SOURCE1 */
> +#define KXREG_INT_BIT_WUFS BIT(1)
> +#define KXREG_INT_BIT_DRDY BIT(4)
> +
> +/* KXCJK: INT_SOURCE2: motion detect */
> +#define KXREG_MOTION_INT_BIT_ZP BIT(0)
> +#define KXREG_MOTION_INT_BIT_ZN BIT(1)
> +#define KXREG_MOTION_INT_BIT_YP BIT(2)
> +#define KXREG_MOTION_INT_BIT_YN BIT(3)
> +#define KXREG_MOTION_INT_BIT_XP BIT(4)
> +#define KXREG_MOTION_INT_BIT_XN BIT(5)
>
> #define KXCJK1013_DEFAULT_WAKE_THRES 1
>
> @@ -215,19 +224,19 @@ 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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> if (mode == STANDBY)
> - ret &= ~KXCJK1013_REG_CTRL1_BIT_PC1;
> + ret &= ~KXREG_CTRL1_BIT_PC1;
> else
> - ret |= KXCJK1013_REG_CTRL1_BIT_PC1;
> + ret |= KXREG_CTRL1_BIT_PC1;
>
> ret = i2c_smbus_write_byte_data(data->client,
> - KXCJK1013_REG_CTRL1, ret);
> + KXREG_CTRL1, ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> return ret;
> @@ -241,13 +250,13 @@ 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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> - if (ret & KXCJK1013_REG_CTRL1_BIT_PC1)
> + if (ret & KXREG_CTRL1_BIT_PC1)
> *mode = OPERATION;
> else
> *mode = STANDBY;
> @@ -259,19 +268,19 @@ 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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> - ret &= ~(KXCJK1013_REG_CTRL1_BIT_GSEL0 |
> - KXCJK1013_REG_CTRL1_BIT_GSEL1);
> + ret &= ~(KXREG_CTRL1_BIT_GSEL0 |
> + KXREG_CTRL1_BIT_GSEL1);
> 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,
> + KXREG_CTRL1,
> ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> @@ -299,16 +308,16 @@ 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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> /* Set 12 bit mode */
> - ret |= KXCJK1013_REG_CTRL1_BIT_RES;
> + ret |= KXREG_CTRL1_BIT_RES;
>
> - ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_CTRL1,
> + ret = i2c_smbus_write_byte_data(data->client, KXREG_CTRL1,
> ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl\n");
> @@ -329,18 +338,18 @@ 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, KXREG_INT_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
> return ret;
> }
>
> if (data->active_high_intr)
> - ret |= KXCJK1013_REG_INT_REG1_BIT_IEA;
> + ret |= KXREG_INT_CTRL1_BIT_IEA;
> else
> - ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEA;
> + ret &= ~KXREG_INT_CTRL1_BIT_IEA;
>
> - ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
> + ret = i2c_smbus_write_byte_data(data->client, KXREG_INT_CTRL1,
> ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_int_ctrl1\n");
> @@ -437,37 +446,37 @@ 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, KXREG_INT_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
> return ret;
> }
>
> if (status)
> - ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
> + ret |= KXREG_INT_CTRL1_BIT_IEN;
> else
> - ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
> + ret &= ~KXREG_INT_CTRL1_BIT_IEN;
>
> - ret = i2c_smbus_write_byte_data(data->client, KXCJK1013_REG_INT_CTRL1,
> + ret = i2c_smbus_write_byte_data(data->client, KXREG_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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> if (status)
> - ret |= KXCJK1013_REG_CTRL1_BIT_WUFE;
> + ret |= KXREG_CTRL1_BIT_WUFE;
> else
> - ret &= ~KXCJK1013_REG_CTRL1_BIT_WUFE;
> + ret &= ~KXREG_CTRL1_BIT_WUFE;
>
> ret = i2c_smbus_write_byte_data(data->client,
> - KXCJK1013_REG_CTRL1, ret);
> + KXREG_CTRL1, ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> return ret;
> @@ -497,37 +506,35 @@ 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, KXREG_INT_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_int_ctrl1\n");
> return ret;
> }
>
> if (status)
> - ret |= KXCJK1013_REG_INT_REG1_BIT_IEN;
> + ret |= KXREG_INT_CTRL1_BIT_IEN;
> else
> - ret &= ~KXCJK1013_REG_INT_REG1_BIT_IEN;
> + ret &= ~KXREG_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, KXREG_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, KXREG_CTRL1);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> return ret;
> }
>
> if (status)
> - ret |= KXCJK1013_REG_CTRL1_BIT_DRDY;
> + ret |= KXREG_CTRL1_BIT_DRDY;
> else
> - ret &= ~KXCJK1013_REG_CTRL1_BIT_DRDY;
> + ret &= ~KXREG_CTRL1_BIT_DRDY;
>
> - ret = i2c_smbus_write_byte_data(data->client,
> - KXCJK1013_REG_CTRL1, ret);
> + ret = i2c_smbus_write_byte_data(data->client, KXREG_CTRL1, ret);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> return ret;
> @@ -603,10 +610,10 @@ 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, KXREG_CTRL3,
> odr_setting->wuf_bits);
> if (ret < 0) {
> - dev_err(&data->client->dev, "Error writing reg_ctrl2\n");
> + dev_err(&data->client->dev, "Error writing reg_ctrl3\n");
> return ret;
> }
>
> @@ -1097,13 +1104,13 @@ 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, KXREG_INT_SRC_DATA);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error reading reg_int_src1\n");
> goto ack_intr;
> }
>
> - if (ret & 0x02) {
> + if (ret & KXREG_INT_BIT_WUFS) {
> kxcjk1013_report_motion_event(indio_dev);
> }
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic
2017-08-17 14:21 ` [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic Michał Mirosław
@ 2017-08-20 9:53 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 9:53 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:37 +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 is fine. I'll pick it up in V2.
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 27147b687471..50d9eefa745f 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -159,6 +159,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;
> @@ -881,13 +884,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,
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type
2017-08-17 14:21 ` [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type Michał Mirosław
@ 2017-08-20 9:57 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 9:57 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:37 +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 an ABI change, but as the interface is clearly rather
odd lets take the risk and hope no one notices. Which they shouldn't
if they are using the ABI correctly.
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 50d9eefa745f..8bde6bead073 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -891,12 +891,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,
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9
2017-08-17 14:21 ` [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9 Michał Mirosław
@ 2017-08-20 10:00 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:00 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:37 +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>
I'd have been tempted to introduce a full chip_info type structure
to represent the differences rather than the slightly messier
approach used here. However, perhaps not worth it whilst we are only
supporting 2 parts (effectively anyway).
This is fine, and I'll be happy to pick up these last few patches
once the changes to register naming are backed out.
Thanks,
Jonathan
> ---
> drivers/iio/accel/Kconfig | 4 +-
> drivers/iio/accel/kxcjk-1013.c | 119 +++++++++++++++++++++++++++++++++++------
> 2 files changed, 106 insertions(+), 17 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 8bde6bead073..a3f4fe1813d1 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,19 +55,43 @@
>
> #define KXCJK1013_REG_DCST_RESP 0x0C
> #define KXCJK1013_REG_WHO_AM_I 0x0F
> -#define KXREG_INT_SRC_DATA 0x16
> +#define KXTF9_REG_TILT_POS_CUR 0x10
> +#define KXTF9_REG_TILT_POS_PREV 0x11
> +#define KXTF9_REG_INT_SRC1 0x15
> +#define KXREG_INT_SRC_DATA 0x16 /* compatible; KXTF9: INT_SRC2, KXCJK: INT_SRC1 */
> #define KXCJK1013_REG_INT_SRC2 0x17
> #define KXCJK1013_REG_STATUS_REG 0x18
> #define KXCJK1013_REG_INT_REL 0x1A
> #define KXREG_CTRL1 0x1B
> -#define KXREG_CTRL3 0x1D
> +#define KXREG_CTRL2 0x1C /* KXTF9: CTRL_REG2 */
> +#define KXREG_CTRL3 0x1D /* mostly compatible; KXTF9: CTRL_REG3, KXCJK: CTRL_REG2 */
> #define KXREG_INT_CTRL1 0x1E
> #define KXREG_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
>
> +/* TILT_POS bits */
> +#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 KXREG_CTRL1_BIT_PC1 BIT(7)
> #define KXREG_CTRL1_BIT_RES BIT(6)
> #define KXREG_CTRL1_BIT_DRDY BIT(5)
> @@ -70,6 +101,7 @@
> #define KXREG_CTRL1_BIT_WUFE BIT(1)
> #define KXREG_CTRL1_BIT_TPE BIT(0)
>
> +#define KXREG_INT_CTRL1_BIT_IEU BIT(2)
> #define KXREG_INT_CTRL1_BIT_IEL BIT(3)
> #define KXREG_INT_CTRL1_BIT_IEA BIT(4)
> #define KXREG_INT_CTRL1_BIT_IEN BIT(5)
> @@ -79,11 +111,16 @@
>
> #define KXCJK1013_SLEEP_DELAY_MS 2000
>
> -/* KXCJK: INT_SOURCE1 */
> +/* KXCJK: INT_SOURCE1, KXTF9: INT_SRC_REG2 */
> +#define KXREG_INT_BIT_TPS BIT(0)
> #define KXREG_INT_BIT_WUFS BIT(1)
> +#define KXREG_INT_MASK_TDTS (BIT(2) | BIT(3))
> +#define KXREG_INT_TAP_NONE 0
> +#define KXREG_INT_TAP_SINGLE BIT(2)
> +#define KXREG_INT_TAP_DOUBLE BIT(3)
> #define KXREG_INT_BIT_DRDY BIT(4)
>
> -/* KXCJK: INT_SOURCE2: motion detect */
> +/* KXCJK: INT_SOURCE2: motion detect, KXTF9: INT_SRC_REG1: tap detect */
> #define KXREG_MOTION_INT_BIT_ZP BIT(0)
> #define KXREG_MOTION_INT_BIT_ZN BIT(1)
> #define KXREG_MOTION_INT_BIT_YP BIT(2)
> @@ -97,6 +134,7 @@ enum kx_chipset {
> KXCJK1013,
> KXCJ91008,
> KXTJ21009,
> + KXTF9,
> KX_MAX_CHIPS /* this must be last */
> };
>
> @@ -162,6 +200,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;
> @@ -212,6 +262,15 @@ static const struct {
> {0x06, 3000},
> {0x07, 2000},
> },
> + /* KXTF9 */
> + {
> + {0x01, 81000},
> + {0x02, 41000},
> + {0x03, 21000},
> + {0x04, 11000},
> + {0x05, 5100},
> + {0x06, 2700},
> + },
> };
>
> static const struct {
> @@ -408,7 +467,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,
> @@ -419,8 +478,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");
> @@ -592,9 +652,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);
> @@ -631,9 +696,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)
> @@ -888,7 +958,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 & KXREG_INT_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},
> {}
> };
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler
2017-08-17 14:21 ` [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler Michał Mirosław
@ 2017-08-20 10:01 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:01 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:36 +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>
This uses register naming changes that aren't introduced until patch 3.
I've asked you to drop those anyway so this will get cleaned up when
you do that.
Jonathan
> ---
> drivers/iio/accel/kxcjk-1013.c | 124 +++++++++++++++++++++--------------------
> 1 file changed, 65 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 33e4b98f39fc..80a6508d6370 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -1027,6 +1027,70 @@ 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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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 & KXREG_MOTION_INT_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;
> @@ -1040,65 +1104,7 @@ static irqreturn_t kxcjk1013_event_handler(int irq, void *private)
> }
>
> if (ret & 0x02) {
> - 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:
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting
2017-08-17 14:21 ` [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting Michał Mirosław
@ 2017-08-20 10:02 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-20 10:02 UTC (permalink / raw)
To: Michał Mirosław
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Thu, 17 Aug 2017 16:21:35 +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>
Looks good. However I haven't applied it as it will just
cause confusion as I want you to rework the rest of the
series to remove the register renames.
Thanks,
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;
>
> 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)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility
2017-08-20 9:52 ` Jonathan Cameron
@ 2017-08-21 21:45 ` Michał Mirosław
2017-08-22 12:47 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Michał Mirosław @ 2017-08-21 21:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio
On Sun, Aug 20, 2017 at 10:52:22AM +0100, Jonathan Cameron wrote:
> On Thu, 17 Aug 2017 16:21:36 +0200
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > Rename some registers that are shared between KXTF9 and KXCJK.
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>
> Hi Michał,
>
> I'm not keen on this change. Going for generic names is always
> fragile as all it takes is another part coming along which is almost
> but not quite the same as you shared register set and we end up with
> a mess.
>
> General convention for both register values and driver naming is they
> should be named after one supported part rather than trying to find
> a generic name that covers all supported parts.
>
> So please revert this change and resend the series.
>
> Sorry to be a pain, but this has gone wrong quite a lot of times in
> the past!
I removed the renaming, but left KXTF9-specific registers with KXTF9_
prefix. This is hard to avoid as some registers are named differently
in KXTF9 and KXCJK datasheets, but are otherwise compatible. There is
also a register that is the same but has different address.
I'll send v2 shortly.
BTW, I browsed through "sell sheets" of Kionix's accelerometers, and
I would guess that for eg. KXCJK-1013, "KXCJK" is a model, and "1013"
is variation. KXTF9 has two such variations differing only in supply
voltages it is calibrated to run with. Based on that I would rename
the driver to "kxcjk" to save on typing if there are more similar
parts to add later. There is KXSD9 driver (different register set), so
this would match clearly.
Best Regards,
Michał Mirosław
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility
2017-08-21 21:45 ` Michał Mirosław
@ 2017-08-22 12:47 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-08-22 12:47 UTC (permalink / raw)
To: Michał Mirosław
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, linux-iio
On Mon, 21 Aug 2017 23:45:10 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> On Sun, Aug 20, 2017 at 10:52:22AM +0100, Jonathan Cameron wrote:
> > On Thu, 17 Aug 2017 16:21:36 +0200
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > Rename some registers that are shared between KXTF9 and KXCJK.
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Hi Michał,
> >
> > I'm not keen on this change. Going for generic names is always
> > fragile as all it takes is another part coming along which is almost
> > but not quite the same as you shared register set and we end up with
> > a mess.
> >
> > General convention for both register values and driver naming is they
> > should be named after one supported part rather than trying to find
> > a generic name that covers all supported parts.
> >
> > So please revert this change and resend the series.
> >
> > Sorry to be a pain, but this has gone wrong quite a lot of times in
> > the past!
>
> I removed the renaming, but left KXTF9-specific registers with KXTF9_
> prefix. This is hard to avoid as some registers are named differently
> in KXTF9 and KXCJK datasheets, but are otherwise compatible. There is
> also a register that is the same but has different address.
That's what I expected. Thanks.
>
> I'll send v2 shortly.
>
> BTW, I browsed through "sell sheets" of Kionix's accelerometers, and
> I would guess that for eg. KXCJK-1013, "KXCJK" is a model, and "1013"
> is variation. KXTF9 has two such variations differing only in supply
> voltages it is calibrated to run with. Based on that I would rename
> the driver to "kxcjk" to save on typing if there are more similar
> parts to add later. There is KXSD9 driver (different register set), so
> this would match clearly.
While that makes sense, be careful not to change anything that is
reflected in userspace ABI and that includes module names. Tends
to break things in odd ways.
Jonathan
>
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-08-22 12:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17 14:21 [PATCH 0/6] iio: accel: kxcjk1003: support Kionix KXTF9 Michał Mirosław
2017-08-17 14:21 ` [PATCH 1/6] iio: accel: kxcjk1003: refactor ODR setting Michał Mirosław
2017-08-20 10:02 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 3/6] iio: accel: kxcjk1013: rename registers for KXTF9 compatibility Michał Mirosław
2017-08-20 9:52 ` Jonathan Cameron
2017-08-21 21:45 ` Michał Mirosław
2017-08-22 12:47 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 2/6] iio: accel: kxcjk1013: extract report_motion_event() from interrupt handler Michał Mirosław
2017-08-20 10:01 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 4/6] iio: accel: kxcjk1013: make sysfs/sampling_frequency_avail dynamic Michał Mirosław
2017-08-20 9:53 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 6/6] iio: accel: kxcjk1013: add support for KXTF9 Michał Mirosław
2017-08-20 10:00 ` Jonathan Cameron
2017-08-17 14:21 ` [PATCH 5/6] iio: accel: kxcjk1013: make sampling_frequency_avail per-type Michał Mirosław
2017-08-20 9:57 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.