* [PATCH 00/10] iio: KMX61 fixes
@ 2014-12-23 13:22 Daniel Baluta
2014-12-23 13:22 ` [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use Daniel Baluta
` (10 more replies)
0 siblings, 11 replies; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
This is a series of cleanups after Hartmut's review:
* http://marc.info/?l=linux-iio&m=141859700810393&w=2
* http://marc.info/?l=linux-iio&m=141859774010580&w=2
* http://marc.info/?l=linux-iio&m=141859828910703&w=2
* http://marc.info/?l=linux-kernel&m=141868557909385&w=2
Daniel Baluta (10):
iio: imu: kmx61: Save odr_bits for later use
iio: imu: kmx61: Don't ignore kmx61_set_power_state errors
iio: imu: kmx61: Enhance error handling
iio: imu: kmx61: Fixup parameters alignment
iio: imu: kmx61: Drop unused device parameter
iio: imu: kmx61: Use false instead of 0 for ev_enable_state
iio: imu: kmx61: Fix device initialization when setting trigger state
iio: imu: kmx61: Remove unnecessary REG_INS1 read
iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
iio: imu: kmx61: Use correct base when reading data
drivers/iio/imu/kmx61.c | 216 ++++++++++++++++++++++--------------------------
1 file changed, 101 insertions(+), 115 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:45 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors Daniel Baluta
` (9 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 972424b..fe0cee7 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -465,6 +465,8 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
if (ret < 0)
return ret;
+ data->odr_bits = odr_bits;
+
if (device & KMX61_ACC) {
ret = kmx61_set_wake_up_odr(data, val, val2);
if (ret)
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
2014-12-23 13:22 ` [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:45 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 03/10] iio: imu: kmx61: Enhance error handling Daniel Baluta
` (8 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
..except while in an error handler, where there is nothing
to be done anyway.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index fe0cee7..e9cbd91 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -830,7 +830,12 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
}
mutex_lock(&data->lock);
- kmx61_set_power_state(data, true, chan->address);
+ ret = kmx61_set_power_state(data, true, chan->address);
+ if (ret) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
if (ret < 0) {
kmx61_set_power_state(data, false, chan->address);
@@ -839,9 +844,11 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
}
*val = sign_extend32(ret >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
- kmx61_set_power_state(data, false, chan->address);
+ ret = kmx61_set_power_state(data, false, chan->address);
mutex_unlock(&data->lock);
+ if (ret)
+ return ret;
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 03/10] iio: imu: kmx61: Enhance error handling
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
2014-12-23 13:22 ` [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use Daniel Baluta
2014-12-23 13:22 ` [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:47 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment Daniel Baluta
` (7 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
This fixes parts of kmx61 error handling to make code easier to read and to be
more consistent with IIO coding conventions:
* prefer as single point for error handling instead of duplicating code
for each function
* directly return a value from a case branch instead of breaking
* fix error message for writing REG_CTRL1
Also, add separate error paths for kmx61_trigger_setup/iio_triggered_buffer_setup
calls.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 100 +++++++++++++++++++++---------------------------
1 file changed, 43 insertions(+), 57 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index e9cbd91..4fc4f63 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -656,11 +656,7 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
return ret;
}
- ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
- if (ret)
- return ret;
-
- return 0;
+ return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
}
static int kmx61_chip_update_thresholds(struct kmx61_data *data)
@@ -678,12 +674,10 @@ static int kmx61_chip_update_thresholds(struct kmx61_data *data)
ret = i2c_smbus_write_byte_data(data->client,
KMX61_REG_WUF_THRESH,
data->wake_thresh);
- if (ret < 0) {
+ if (ret < 0)
dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
- return ret;
- }
- return 0;
+ return ret;
}
static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
@@ -737,11 +731,7 @@ static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
return ret;
}
mode |= KMX61_ACT_STBY_BIT;
- ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
- if (ret)
- return ret;
-
- return 0;
+ return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
}
/**
@@ -924,10 +914,10 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
switch (info) {
case IIO_EV_INFO_VALUE:
*val = data->wake_thresh;
- break;
+ return IIO_VAL_INT;
case IIO_EV_INFO_PERIOD:
*val = data->wake_duration;
- break;
+ return IIO_VAL_INT;
default:
return -EINVAL;
}
@@ -950,10 +940,10 @@ static int kmx61_write_event(struct iio_dev *indio_dev,
switch (info) {
case IIO_EV_INFO_VALUE:
data->wake_thresh = val;
- break;
+ return IIO_VAL_INT;
case IIO_EV_INFO_PERIOD:
data->wake_duration = val;
- break;
+ return IIO_VAL_INT;
default:
return -EINVAL;
}
@@ -978,7 +968,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
int state)
{
struct kmx61_data *data = kmx61_get_data(indio_dev);
- int ret;
+ int ret = 0;
if (state && data->ev_enable_state)
return 0;
@@ -987,27 +977,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
if (!state && data->motion_trig_on) {
data->ev_enable_state = 0;
- mutex_unlock(&data->lock);
- return 0;
+ goto err_unlock;
}
ret = kmx61_set_power_state(data, state, KMX61_ACC);
- if (ret < 0) {
- mutex_unlock(&data->lock);
- return ret;
- }
+ if (ret < 0)
+ goto err_unlock;
ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
if (ret < 0) {
kmx61_set_power_state(data, false, KMX61_ACC);
- mutex_unlock(&data->lock);
- return ret;
+ goto err_unlock;
}
data->ev_enable_state = state;
+
+err_unlock:
mutex_unlock(&data->lock);
- return 0;
+ return ret;
}
static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
@@ -1066,8 +1054,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
if (!state && data->ev_enable_state && data->motion_trig_on) {
data->motion_trig_on = false;
- mutex_unlock(&data->lock);
- return 0;
+ goto err_unlock;
}
@@ -1077,10 +1064,8 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
device = KMX61_MAG;
ret = kmx61_set_power_state(data, state, device);
- if (ret < 0) {
- mutex_unlock(&data->lock);
- return ret;
- }
+ if (ret < 0)
+ goto err_unlock;
if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
ret = kmx61_setup_new_data_interrupt(data, state, device);
@@ -1088,8 +1073,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
if (ret < 0) {
kmx61_set_power_state(data, false, device);
- mutex_unlock(&data->lock);
- return ret;
+ goto err_unlock;
}
if (data->acc_dready_trig == trig)
@@ -1098,10 +1082,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
data->mag_dready_trig_on = state;
else
data->motion_trig_on = state;
-
+err_unlock:
mutex_unlock(&data->lock);
- return 0;
+ return ret;
}
static int kmx61_trig_try_reenable(struct iio_trigger *trig)
@@ -1207,7 +1191,7 @@ ack_intr:
ret |= KMX61_REG_CTRL1_BIT_RES;
ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
if (ret < 0)
- dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+ dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
if (ret < 0)
@@ -1409,15 +1393,17 @@ static int kmx61_probe(struct i2c_client *client,
data->acc_dready_trig =
kmx61_trigger_setup(data, data->acc_indio_dev,
"dready");
- if (IS_ERR(data->acc_dready_trig))
- return PTR_ERR(data->acc_dready_trig);
+ if (IS_ERR(data->acc_dready_trig)) {
+ ret = PTR_ERR(data->acc_dready_trig);
+ goto err_chip_uninit;
+ }
data->mag_dready_trig =
kmx61_trigger_setup(data, data->mag_indio_dev,
"dready");
if (IS_ERR(data->mag_dready_trig)) {
ret = PTR_ERR(data->mag_dready_trig);
- goto err_trigger_unregister;
+ goto err_trigger_unregister_acc_dready;
}
data->motion_trig =
@@ -1425,7 +1411,7 @@ static int kmx61_probe(struct i2c_client *client,
"any-motion");
if (IS_ERR(data->motion_trig)) {
ret = PTR_ERR(data->motion_trig);
- goto err_trigger_unregister;
+ goto err_trigger_unregister_mag_dready;
}
ret = iio_triggered_buffer_setup(data->acc_indio_dev,
@@ -1435,7 +1421,7 @@ static int kmx61_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&data->client->dev,
"Failed to setup acc triggered buffer\n");
- goto err_trigger_unregister;
+ goto err_trigger_unregister_motion;
}
ret = iio_triggered_buffer_setup(data->mag_indio_dev,
@@ -1445,14 +1431,14 @@ static int kmx61_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&data->client->dev,
"Failed to setup mag triggered buffer\n");
- goto err_trigger_unregister;
+ goto err_buffer_cleanup_acc;
}
}
ret = iio_device_register(data->acc_indio_dev);
if (ret < 0) {
dev_err(&client->dev, "Failed to register acc iio device\n");
- goto err_buffer_cleanup;
+ goto err_buffer_cleanup_mag;
}
ret = iio_device_register(data->mag_indio_dev);
@@ -1475,18 +1461,18 @@ err_iio_unregister_mag:
iio_device_unregister(data->mag_indio_dev);
err_iio_unregister_acc:
iio_device_unregister(data->acc_indio_dev);
-err_buffer_cleanup:
- if (client->irq >= 0) {
- iio_triggered_buffer_cleanup(data->acc_indio_dev);
+err_buffer_cleanup_mag:
+ if (client->irq >= 0)
iio_triggered_buffer_cleanup(data->mag_indio_dev);
- }
-err_trigger_unregister:
- if (data->acc_dready_trig)
- iio_trigger_unregister(data->acc_dready_trig);
- if (data->mag_dready_trig)
- iio_trigger_unregister(data->mag_dready_trig);
- if (data->motion_trig)
- iio_trigger_unregister(data->motion_trig);
+err_buffer_cleanup_acc:
+ if (client->irq >= 0)
+ iio_triggered_buffer_cleanup(data->acc_indio_dev);
+err_trigger_unregister_motion:
+ iio_trigger_unregister(data->motion_trig);
+err_trigger_unregister_mag_dready:
+ iio_trigger_unregister(data->mag_dready_trig);
+err_trigger_unregister_acc_dready:
+ iio_trigger_unregister(data->acc_dready_trig);
err_chip_uninit:
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (2 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 03/10] iio: imu: kmx61: Enhance error handling Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:47 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter Daniel Baluta
` (6 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 4fc4f63..2652179 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -926,11 +926,11 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
}
static int kmx61_write_event(struct iio_dev *indio_dev,
- const struct iio_chan_spec *chan,
- enum iio_event_type type,
- enum iio_event_direction dir,
- enum iio_event_info info,
- int val, int val2)
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
{
struct kmx61_data *data = kmx61_get_data(indio_dev);
@@ -962,10 +962,10 @@ static int kmx61_read_event_config(struct iio_dev *indio_dev,
}
static int kmx61_write_event_config(struct iio_dev *indio_dev,
- const struct iio_chan_spec *chan,
- enum iio_event_type type,
- enum iio_event_direction dir,
- int state)
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ int state)
{
struct kmx61_data *data = kmx61_get_data(indio_dev);
int ret = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (3 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:49 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state Daniel Baluta
` (5 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 2652179..332ee67 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -681,7 +681,7 @@ static int kmx61_chip_update_thresholds(struct kmx61_data *data)
}
static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
- bool status, u8 device)
+ bool status)
{
u8 mode;
int ret;
@@ -984,7 +984,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
if (ret < 0)
goto err_unlock;
- ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
+ ret = kmx61_setup_any_motion_interrupt(data, state);
if (ret < 0) {
kmx61_set_power_state(data, false, KMX61_ACC);
goto err_unlock;
@@ -1070,7 +1070,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
ret = kmx61_setup_new_data_interrupt(data, state, device);
else
- ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
+ ret = kmx61_setup_any_motion_interrupt(data, state);
if (ret < 0) {
kmx61_set_power_state(data, false, device);
goto err_unlock;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (4 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:50 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state Daniel Baluta
` (4 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 332ee67..30825cf 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -976,7 +976,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
mutex_lock(&data->lock);
if (!state && data->motion_trig_on) {
- data->ev_enable_state = 0;
+ data->ev_enable_state = false;
goto err_unlock;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (5 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:51 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read Daniel Baluta
` (3 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 30825cf..5b5be2b 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -1057,8 +1057,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
goto err_unlock;
}
-
- if (data->acc_dready_trig == trig || data->motion_trig)
+ if (data->acc_dready_trig == trig || data->motion_trig == trig)
device = KMX61_ACC;
else
device = KMX61_MAG;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (6 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:51 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table Daniel Baluta
` (2 subsequent siblings)
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
Useful in the debugging phase, not needed now.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 5b5be2b..eb3900e 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -1196,8 +1196,6 @@ ack_intr:
if (ret < 0)
dev_err(&data->client->dev, "Error reading reg_inl\n");
- ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
-
return IRQ_HANDLED;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (7 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:53 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data Daniel Baluta
2014-12-26 9:57 ` [PATCH 00/10] iio: KMX61 fixes Jonathan Cameron
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
odr_bits values are between 0 and 11, so we can use the index
in kmx61_samp_freq_table instead of odr_bits structure member.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 64 ++++++++++++++++++++-----------------------------
1 file changed, 26 insertions(+), 38 deletions(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index eb3900e..98369eb 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
static const struct {
int val;
int val2;
- u8 odr_bits;
-} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
- {25, 0, 0x01},
- {50, 0, 0x02},
- {100, 0, 0x03},
- {200, 0, 0x04},
- {400, 0, 0x05},
- {800, 0, 0x06},
- {1600, 0, 0x07},
- {0, 781000, 0x08},
- {1, 563000, 0x09},
- {3, 125000, 0x0A},
- {6, 250000, 0x0B} };
+} kmx61_samp_freq_table[] = { {12, 500000},
+ {25, 0},
+ {50, 0},
+ {100, 0},
+ {200, 0},
+ {400, 0},
+ {800, 0},
+ {1600, 0},
+ {0, 781000},
+ {1, 563000},
+ {3, 125000},
+ {6, 250000} };
static const struct {
int val;
@@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
if (val == kmx61_samp_freq_table[i].val &&
val2 == kmx61_samp_freq_table[i].val2)
- return kmx61_samp_freq_table[i].odr_bits;
- return -EINVAL;
-}
-
-static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
- if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
- *val = kmx61_samp_freq_table[i].val;
- *val2 = kmx61_samp_freq_table[i].val2;
- return 0;
- }
+ return i;
return -EINVAL;
}
-
static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
{
int i;
@@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
u8 device)
-{ int i;
+{
u8 lodr_bits;
if (device & KMX61_ACC)
@@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
else
return -EINVAL;
- for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
- if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
- *val = kmx61_samp_freq_table[i].val;
- *val2 = kmx61_samp_freq_table[i].val2;
- return 0;
- }
- return -EINVAL;
+ if (lodr_bits > 0xB)
+ return -EINVAL;
+
+ *val = kmx61_samp_freq_table[lodr_bits].val;
+ *val2 = kmx61_samp_freq_table[lodr_bits].val2;
+
+ return 0;
}
static int kmx61_set_range(struct kmx61_data *data, u8 range)
@@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
}
data->odr_bits = ret;
- /* set output data rate for wake up (motion detection) function */
- ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
+ /*
+ * set output data rate for wake up (motion detection) function
+ * to match data rate for accelerometer sampling
+ */
+ ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
if (ret < 0)
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (8 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table Daniel Baluta
@ 2014-12-23 13:22 ` Daniel Baluta
2015-01-01 13:54 ` Hartmut Knaack
2014-12-26 9:57 ` [PATCH 00/10] iio: KMX61 fixes Jonathan Cameron
10 siblings, 1 reply; 32+ messages in thread
From: Daniel Baluta @ 2014-12-23 13:22 UTC (permalink / raw)
To: jic23, knaack.h
Cc: lars, pmeerw, daniel.baluta, linux-iio, linux-kernel,
srinivas.pandruvada
We have two IIO devices and we need to adjust the base
when reading data.
Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
---
drivers/iio/imu/kmx61.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index 98369eb..6178cea 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -1210,12 +1210,18 @@ static irqreturn_t kmx61_trigger_handler(int irq, void *p)
struct iio_dev *indio_dev = pf->indio_dev;
struct kmx61_data *data = kmx61_get_data(indio_dev);
int bit, ret, i = 0;
+ u8 base;
s16 buffer[8];
+ if (indio_dev == data->acc_indio_dev)
+ base = KMX61_ACC_XOUT_L;
+ else
+ base = KMX61_MAG_XOUT_L;
+
mutex_lock(&data->lock);
for_each_set_bit(bit, indio_dev->buffer->scan_mask,
indio_dev->masklength) {
- ret = kmx61_read_measurement(data, KMX61_ACC_XOUT_L, bit);
+ ret = kmx61_read_measurement(data, base, bit);
if (ret < 0) {
mutex_unlock(&data->lock);
goto err;
--
1.9.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 00/10] iio: KMX61 fixes
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
` (9 preceding siblings ...)
2014-12-23 13:22 ` [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data Daniel Baluta
@ 2014-12-26 9:57 ` Jonathan Cameron
10 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2014-12-26 9:57 UTC (permalink / raw)
To: Daniel Baluta, knaack.h
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 23/12/14 13:22, Daniel Baluta wrote:
> This is a series of cleanups after Hartmut's review:
>
> * http://marc.info/?l=linux-iio&m=141859700810393&w=2
> * http://marc.info/?l=linux-iio&m=141859774010580&w=2
> * http://marc.info/?l=linux-iio&m=141859828910703&w=2
> * http://marc.info/?l=linux-kernel&m=141868557909385&w=2
>
> Daniel Baluta (10):
> iio: imu: kmx61: Save odr_bits for later use
> iio: imu: kmx61: Don't ignore kmx61_set_power_state errors
> iio: imu: kmx61: Enhance error handling
> iio: imu: kmx61: Fixup parameters alignment
> iio: imu: kmx61: Drop unused device parameter
> iio: imu: kmx61: Use false instead of 0 for ev_enable_state
> iio: imu: kmx61: Fix device initialization when setting trigger state
> iio: imu: kmx61: Remove unnecessary REG_INS1 read
> iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
> iio: imu: kmx61: Use correct base when reading data
>
> drivers/iio/imu/kmx61.c | 216 ++++++++++++++++++++++--------------------------
> 1 file changed, 101 insertions(+), 115 deletions(-)
>
Ouch - I need to concentrate more when reviewing!
Anyhow, these all look fine to me, but I'd like to leave them for a few days
for Hartmut to take a look.
Jonathan
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use
2014-12-23 13:22 ` [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use Daniel Baluta
@ 2015-01-01 13:45 ` Hartmut Knaack
2015-01-04 10:41 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:45 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 972424b..fe0cee7 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -465,6 +465,8 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
> if (ret < 0)
> return ret;
>
> + data->odr_bits = odr_bits;
> +
> if (device & KMX61_ACC) {
> ret = kmx61_set_wake_up_odr(data, val, val2);
> if (ret)
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors
2014-12-23 13:22 ` [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors Daniel Baluta
@ 2015-01-01 13:45 ` Hartmut Knaack
2015-01-04 10:42 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:45 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> ..except while in an error handler, where there is nothing
> to be done anyway.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index fe0cee7..e9cbd91 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -830,7 +830,12 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> }
> mutex_lock(&data->lock);
>
> - kmx61_set_power_state(data, true, chan->address);
> + ret = kmx61_set_power_state(data, true, chan->address);
> + if (ret) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> +
> ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
> if (ret < 0) {
> kmx61_set_power_state(data, false, chan->address);
> @@ -839,9 +844,11 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
> }
> *val = sign_extend32(ret >> chan->scan_type.shift,
> chan->scan_type.realbits - 1);
> - kmx61_set_power_state(data, false, chan->address);
> + ret = kmx61_set_power_state(data, false, chan->address);
>
> mutex_unlock(&data->lock);
> + if (ret)
> + return ret;
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 03/10] iio: imu: kmx61: Enhance error handling
2014-12-23 13:22 ` [PATCH 03/10] iio: imu: kmx61: Enhance error handling Daniel Baluta
@ 2015-01-01 13:47 ` Hartmut Knaack
2015-01-04 10:45 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:47 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> This fixes parts of kmx61 error handling to make code easier to read and to be
> more consistent with IIO coding conventions:
> * prefer as single point for error handling instead of duplicating code
> for each function
> * directly return a value from a case branch instead of breaking
> * fix error message for writing REG_CTRL1
>
> Also, add separate error paths for kmx61_trigger_setup/iio_triggered_buffer_setup
> calls.
Some issues remain in this one, please see inline. Otherwise looking good.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/kmx61.c | 100 +++++++++++++++++++++---------------------------
> 1 file changed, 43 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index e9cbd91..4fc4f63 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -656,11 +656,7 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
> return ret;
> }
>
> - ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> - if (ret)
> - return ret;
> -
> - return 0;
> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> }
>
> static int kmx61_chip_update_thresholds(struct kmx61_data *data)
> @@ -678,12 +674,10 @@ static int kmx61_chip_update_thresholds(struct kmx61_data *data)
> ret = i2c_smbus_write_byte_data(data->client,
> KMX61_REG_WUF_THRESH,
> data->wake_thresh);
> - if (ret < 0) {
> + if (ret < 0)
> dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
> - return ret;
> - }
>
> - return 0;
> + return ret;
> }
>
> static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
> @@ -737,11 +731,7 @@ static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
> return ret;
> }
> mode |= KMX61_ACT_STBY_BIT;
> - ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> - if (ret)
> - return ret;
> -
> - return 0;
> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
> }
>
> /**
> @@ -924,10 +914,10 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
> switch (info) {
> case IIO_EV_INFO_VALUE:
> *val = data->wake_thresh;
> - break;
> + return IIO_VAL_INT;
> case IIO_EV_INFO_PERIOD:
> *val = data->wake_duration;
> - break;
> + return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
Down below this line is still a return IIO_VAL_INT, which is now obsolete.
> @@ -950,10 +940,10 @@ static int kmx61_write_event(struct iio_dev *indio_dev,
> switch (info) {
> case IIO_EV_INFO_VALUE:
> data->wake_thresh = val;
> - break;
> + return IIO_VAL_INT;
> case IIO_EV_INFO_PERIOD:
> data->wake_duration = val;
> - break;
> + return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
Same obsolete return exists below this line.
> @@ -978,7 +968,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> int state)
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
> - int ret;
> + int ret = 0;
>
> if (state && data->ev_enable_state)
> return 0;
> @@ -987,27 +977,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>
> if (!state && data->motion_trig_on) {
> data->ev_enable_state = 0;
> - mutex_unlock(&data->lock);
> - return 0;
> + goto err_unlock;
> }
>
> ret = kmx61_set_power_state(data, state, KMX61_ACC);
> - if (ret < 0) {
> - mutex_unlock(&data->lock);
> - return ret;
> - }
> + if (ret < 0)
> + goto err_unlock;
>
> ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> if (ret < 0) {
> kmx61_set_power_state(data, false, KMX61_ACC);
> - mutex_unlock(&data->lock);
> - return ret;
> + goto err_unlock;
> }
>
> data->ev_enable_state = state;
> +
> +err_unlock:
> mutex_unlock(&data->lock);
>
> - return 0;
> + return ret;
> }
>
> static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
> @@ -1066,8 +1054,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>
> if (!state && data->ev_enable_state && data->motion_trig_on) {
> data->motion_trig_on = false;
> - mutex_unlock(&data->lock);
> - return 0;
> + goto err_unlock;
> }
>
>
> @@ -1077,10 +1064,8 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> device = KMX61_MAG;
>
> ret = kmx61_set_power_state(data, state, device);
> - if (ret < 0) {
> - mutex_unlock(&data->lock);
> - return ret;
> - }
> + if (ret < 0)
> + goto err_unlock;
>
> if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
> ret = kmx61_setup_new_data_interrupt(data, state, device);
> @@ -1088,8 +1073,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> if (ret < 0) {
> kmx61_set_power_state(data, false, device);
> - mutex_unlock(&data->lock);
> - return ret;
> + goto err_unlock;
> }
>
> if (data->acc_dready_trig == trig)
> @@ -1098,10 +1082,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> data->mag_dready_trig_on = state;
> else
> data->motion_trig_on = state;
> -
> +err_unlock:
> mutex_unlock(&data->lock);
>
> - return 0;
> + return ret;
> }
>
> static int kmx61_trig_try_reenable(struct iio_trigger *trig)
> @@ -1207,7 +1191,7 @@ ack_intr:
> ret |= KMX61_REG_CTRL1_BIT_RES;
> ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
> if (ret < 0)
> - dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
>
> ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
> if (ret < 0)
> @@ -1409,15 +1393,17 @@ static int kmx61_probe(struct i2c_client *client,
> data->acc_dready_trig =
> kmx61_trigger_setup(data, data->acc_indio_dev,
> "dready");
> - if (IS_ERR(data->acc_dready_trig))
> - return PTR_ERR(data->acc_dready_trig);
> + if (IS_ERR(data->acc_dready_trig)) {
> + ret = PTR_ERR(data->acc_dready_trig);
Double whitespace.
> + goto err_chip_uninit;
> + }
>
> data->mag_dready_trig =
> kmx61_trigger_setup(data, data->mag_indio_dev,
> "dready");
> if (IS_ERR(data->mag_dready_trig)) {
> ret = PTR_ERR(data->mag_dready_trig);
> - goto err_trigger_unregister;
> + goto err_trigger_unregister_acc_dready;
> }
>
> data->motion_trig =
> @@ -1425,7 +1411,7 @@ static int kmx61_probe(struct i2c_client *client,
> "any-motion");
> if (IS_ERR(data->motion_trig)) {
> ret = PTR_ERR(data->motion_trig);
> - goto err_trigger_unregister;
> + goto err_trigger_unregister_mag_dready;
> }
>
> ret = iio_triggered_buffer_setup(data->acc_indio_dev,
> @@ -1435,7 +1421,7 @@ static int kmx61_probe(struct i2c_client *client,
> if (ret < 0) {
> dev_err(&data->client->dev,
> "Failed to setup acc triggered buffer\n");
> - goto err_trigger_unregister;
> + goto err_trigger_unregister_motion;
> }
>
> ret = iio_triggered_buffer_setup(data->mag_indio_dev,
> @@ -1445,14 +1431,14 @@ static int kmx61_probe(struct i2c_client *client,
> if (ret < 0) {
> dev_err(&data->client->dev,
> "Failed to setup mag triggered buffer\n");
> - goto err_trigger_unregister;
> + goto err_buffer_cleanup_acc;
> }
> }
>
> ret = iio_device_register(data->acc_indio_dev);
> if (ret < 0) {
> dev_err(&client->dev, "Failed to register acc iio device\n");
> - goto err_buffer_cleanup;
> + goto err_buffer_cleanup_mag;
> }
>
> ret = iio_device_register(data->mag_indio_dev);
> @@ -1475,18 +1461,18 @@ err_iio_unregister_mag:
> iio_device_unregister(data->mag_indio_dev);
> err_iio_unregister_acc:
> iio_device_unregister(data->acc_indio_dev);
> -err_buffer_cleanup:
> - if (client->irq >= 0) {
> - iio_triggered_buffer_cleanup(data->acc_indio_dev);
> +err_buffer_cleanup_mag:
> + if (client->irq >= 0)
> iio_triggered_buffer_cleanup(data->mag_indio_dev);
> - }
> -err_trigger_unregister:
> - if (data->acc_dready_trig)
> - iio_trigger_unregister(data->acc_dready_trig);
> - if (data->mag_dready_trig)
> - iio_trigger_unregister(data->mag_dready_trig);
> - if (data->motion_trig)
> - iio_trigger_unregister(data->motion_trig);
> +err_buffer_cleanup_acc:
> + if (client->irq >= 0)
> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
> +err_trigger_unregister_motion:
> + iio_trigger_unregister(data->motion_trig);
> +err_trigger_unregister_mag_dready:
> + iio_trigger_unregister(data->mag_dready_trig);
> +err_trigger_unregister_acc_dready:
> + iio_trigger_unregister(data->acc_dready_trig);
> err_chip_uninit:
> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
> return ret;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment
2014-12-23 13:22 ` [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment Daniel Baluta
@ 2015-01-01 13:47 ` Hartmut Knaack
2015-01-04 10:47 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:47 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Acked-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 4fc4f63..2652179 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -926,11 +926,11 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
> }
>
> static int kmx61_write_event(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - enum iio_event_type type,
> - enum iio_event_direction dir,
> - enum iio_event_info info,
> - int val, int val2)
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
>
> @@ -962,10 +962,10 @@ static int kmx61_read_event_config(struct iio_dev *indio_dev,
> }
>
> static int kmx61_write_event_config(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan,
> - enum iio_event_type type,
> - enum iio_event_direction dir,
> - int state)
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + int state)
> {
> struct kmx61_data *data = kmx61_get_data(indio_dev);
> int ret = 0;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter
2014-12-23 13:22 ` [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter Daniel Baluta
@ 2015-01-01 13:49 ` Hartmut Knaack
0 siblings, 0 replies; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:49 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 2652179..332ee67 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -681,7 +681,7 @@ static int kmx61_chip_update_thresholds(struct kmx61_data *data)
> }
>
> static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
> - bool status, u8 device)
> + bool status)
> {
> u8 mode;
> int ret;
> @@ -984,7 +984,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> if (ret < 0)
> goto err_unlock;
>
> - ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> + ret = kmx61_setup_any_motion_interrupt(data, state);
> if (ret < 0) {
> kmx61_set_power_state(data, false, KMX61_ACC);
> goto err_unlock;
> @@ -1070,7 +1070,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
> ret = kmx61_setup_new_data_interrupt(data, state, device);
> else
> - ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
> + ret = kmx61_setup_any_motion_interrupt(data, state);
> if (ret < 0) {
> kmx61_set_power_state(data, false, device);
> goto err_unlock;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state
2014-12-23 13:22 ` [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state Daniel Baluta
@ 2015-01-01 13:50 ` Hartmut Knaack
2015-01-04 10:49 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:50 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Acked-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 332ee67..30825cf 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -976,7 +976,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
> mutex_lock(&data->lock);
>
> if (!state && data->motion_trig_on) {
> - data->ev_enable_state = 0;
> + data->ev_enable_state = false;
> goto err_unlock;
> }
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state
2014-12-23 13:22 ` [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state Daniel Baluta
@ 2015-01-01 13:51 ` Hartmut Knaack
2015-01-04 10:51 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:51 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Acked-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 30825cf..5b5be2b 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -1057,8 +1057,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
> goto err_unlock;
> }
>
> -
> - if (data->acc_dready_trig == trig || data->motion_trig)
> + if (data->acc_dready_trig == trig || data->motion_trig == trig)
> device = KMX61_ACC;
> else
> device = KMX61_MAG;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read
2014-12-23 13:22 ` [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read Daniel Baluta
@ 2015-01-01 13:51 ` Hartmut Knaack
2015-01-04 10:54 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:51 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> Useful in the debugging phase, not needed now.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Acked-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 5b5be2b..eb3900e 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -1196,8 +1196,6 @@ ack_intr:
> if (ret < 0)
> dev_err(&data->client->dev, "Error reading reg_inl\n");
>
> - ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
> -
> return IRQ_HANDLED;
> }
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
2014-12-23 13:22 ` [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table Daniel Baluta
@ 2015-01-01 13:53 ` Hartmut Knaack
2015-01-04 10:55 ` Jonathan Cameron
0 siblings, 1 reply; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:53 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> odr_bits values are between 0 and 11, so we can use the index
> in kmx61_samp_freq_table instead of odr_bits structure member.
Basically looking good, but I would feel more comfortable to check
against the boundaries of the table. Please see inline.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> ---
> drivers/iio/imu/kmx61.c | 64 ++++++++++++++++++++-----------------------------
> 1 file changed, 26 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index eb3900e..98369eb 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
> static const struct {
> int val;
> int val2;
> - u8 odr_bits;
> -} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
> - {25, 0, 0x01},
> - {50, 0, 0x02},
> - {100, 0, 0x03},
> - {200, 0, 0x04},
> - {400, 0, 0x05},
> - {800, 0, 0x06},
> - {1600, 0, 0x07},
> - {0, 781000, 0x08},
> - {1, 563000, 0x09},
> - {3, 125000, 0x0A},
> - {6, 250000, 0x0B} };
> +} kmx61_samp_freq_table[] = { {12, 500000},
> + {25, 0},
> + {50, 0},
> + {100, 0},
> + {200, 0},
> + {400, 0},
> + {800, 0},
> + {1600, 0},
> + {0, 781000},
> + {1, 563000},
> + {3, 125000},
> + {6, 250000} };
>
> static const struct {
> int val;
> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
> for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> if (val == kmx61_samp_freq_table[i].val &&
> val2 == kmx61_samp_freq_table[i].val2)
> - return kmx61_samp_freq_table[i].odr_bits;
> - return -EINVAL;
> -}
> -
> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> - if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
> - *val = kmx61_samp_freq_table[i].val;
> - *val2 = kmx61_samp_freq_table[i].val2;
> - return 0;
> - }
> + return i;
> return -EINVAL;
> }
>
> -
> static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
> {
> int i;
> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>
> static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
> u8 device)
> -{ int i;
> +{
> u8 lodr_bits;
>
> if (device & KMX61_ACC)
> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
> else
> return -EINVAL;
>
> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> - if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
> - *val = kmx61_samp_freq_table[i].val;
> - *val2 = kmx61_samp_freq_table[i].val2;
> - return 0;
> - }
> - return -EINVAL;
> + if (lodr_bits > 0xB)
Since we use it as an index, I regard it safer to check against
ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.
> + return -EINVAL;
> +
> + *val = kmx61_samp_freq_table[lodr_bits].val;
> + *val2 = kmx61_samp_freq_table[lodr_bits].val2;
> +
> + return 0;
> }
>
> static int kmx61_set_range(struct kmx61_data *data, u8 range)
> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
> }
> data->odr_bits = ret;
>
> - /* set output data rate for wake up (motion detection) function */
> - ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
> + /*
> + * set output data rate for wake up (motion detection) function
> + * to match data rate for accelerometer sampling
> + */
> + ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
> if (ret < 0)
> return ret;
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data
2014-12-23 13:22 ` [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data Daniel Baluta
@ 2015-01-01 13:54 ` Hartmut Knaack
0 siblings, 0 replies; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-01 13:54 UTC (permalink / raw)
To: Daniel Baluta, jic23
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> We have two IIO devices and we need to adjust the base
> when reading data.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> drivers/iio/imu/kmx61.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index 98369eb..6178cea 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -1210,12 +1210,18 @@ static irqreturn_t kmx61_trigger_handler(int irq, void *p)
> struct iio_dev *indio_dev = pf->indio_dev;
> struct kmx61_data *data = kmx61_get_data(indio_dev);
> int bit, ret, i = 0;
> + u8 base;
> s16 buffer[8];
>
> + if (indio_dev == data->acc_indio_dev)
> + base = KMX61_ACC_XOUT_L;
> + else
> + base = KMX61_MAG_XOUT_L;
> +
> mutex_lock(&data->lock);
> for_each_set_bit(bit, indio_dev->buffer->scan_mask,
> indio_dev->masklength) {
> - ret = kmx61_read_measurement(data, KMX61_ACC_XOUT_L, bit);
> + ret = kmx61_read_measurement(data, base, bit);
> if (ret < 0) {
> mutex_unlock(&data->lock);
> goto err;
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use
2015-01-01 13:45 ` Hartmut Knaack
@ 2015-01-04 10:41 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:41 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:45, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play.
>> ---
>> drivers/iio/imu/kmx61.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 972424b..fe0cee7 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -465,6 +465,8 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>> if (ret < 0)
>> return ret;
>>
>> + data->odr_bits = odr_bits;
>> +
>> if (device & KMX61_ACC) {
>> ret = kmx61_set_wake_up_odr(data, val, val2);
>> if (ret)
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors
2015-01-01 13:45 ` Hartmut Knaack
@ 2015-01-04 10:42 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:42 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:45, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> ..except while in an error handler, where there is nothing
>> to be done anyway.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
Applied
>> ---
>> drivers/iio/imu/kmx61.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index fe0cee7..e9cbd91 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -830,7 +830,12 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
>> }
>> mutex_lock(&data->lock);
>>
>> - kmx61_set_power_state(data, true, chan->address);
>> + ret = kmx61_set_power_state(data, true, chan->address);
>> + if (ret) {
>> + mutex_unlock(&data->lock);
>> + return ret;
>> + }
>> +
>> ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
>> if (ret < 0) {
>> kmx61_set_power_state(data, false, chan->address);
>> @@ -839,9 +844,11 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
>> }
>> *val = sign_extend32(ret >> chan->scan_type.shift,
>> chan->scan_type.realbits - 1);
>> - kmx61_set_power_state(data, false, chan->address);
>> + ret = kmx61_set_power_state(data, false, chan->address);
>>
>> mutex_unlock(&data->lock);
>> + if (ret)
>> + return ret;
>> return IIO_VAL_INT;
>> case IIO_CHAN_INFO_SCALE:
>> switch (chan->type) {
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 03/10] iio: imu: kmx61: Enhance error handling
2015-01-01 13:47 ` Hartmut Knaack
@ 2015-01-04 10:45 ` Jonathan Cameron
2015-01-05 8:57 ` Daniel Baluta
0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:45 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:47, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> This fixes parts of kmx61 error handling to make code easier to read and to be
>> more consistent with IIO coding conventions:
>> * prefer as single point for error handling instead of duplicating code
>> for each function
>> * directly return a value from a case branch instead of breaking
>> * fix error message for writing REG_CTRL1
>>
>> Also, add separate error paths for kmx61_trigger_setup/iio_triggered_buffer_setup
>> calls.
> Some issues remain in this one, please see inline. Otherwise looking good.
>
Fixed up and applied.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> ---
>> drivers/iio/imu/kmx61.c | 100 +++++++++++++++++++++---------------------------
>> 1 file changed, 43 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index e9cbd91..4fc4f63 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -656,11 +656,7 @@ static int kmx61_setup_new_data_interrupt(struct kmx61_data *data,
>> return ret;
>> }
>>
>> - ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> - if (ret)
>> - return ret;
>> -
>> - return 0;
>> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> }
>>
>> static int kmx61_chip_update_thresholds(struct kmx61_data *data)
>> @@ -678,12 +674,10 @@ static int kmx61_chip_update_thresholds(struct kmx61_data *data)
>> ret = i2c_smbus_write_byte_data(data->client,
>> KMX61_REG_WUF_THRESH,
>> data->wake_thresh);
>> - if (ret < 0) {
>> + if (ret < 0)
>> dev_err(&data->client->dev, "Error writing reg_wuf_thresh\n");
>> - return ret;
>> - }
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
>> @@ -737,11 +731,7 @@ static int kmx61_setup_any_motion_interrupt(struct kmx61_data *data,
>> return ret;
>> }
>> mode |= KMX61_ACT_STBY_BIT;
>> - ret = kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> - if (ret)
>> - return ret;
>> -
>> - return 0;
>> + return kmx61_set_mode(data, mode, KMX61_ACC | KMX61_MAG, true);
>> }
>>
>> /**
>> @@ -924,10 +914,10 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
>> switch (info) {
>> case IIO_EV_INFO_VALUE:
>> *val = data->wake_thresh;
>> - break;
>> + return IIO_VAL_INT;
>> case IIO_EV_INFO_PERIOD:
>> *val = data->wake_duration;
>> - break;
>> + return IIO_VAL_INT;
>> default:
>> return -EINVAL;
>> }
> Down below this line is still a return IIO_VAL_INT, which is now obsolete.
>> @@ -950,10 +940,10 @@ static int kmx61_write_event(struct iio_dev *indio_dev,
>> switch (info) {
>> case IIO_EV_INFO_VALUE:
>> data->wake_thresh = val;
>> - break;
>> + return IIO_VAL_INT;
>> case IIO_EV_INFO_PERIOD:
>> data->wake_duration = val;
>> - break;
>> + return IIO_VAL_INT;
>> default:
>> return -EINVAL;
>> }
> Same obsolete return exists below this line.
>> @@ -978,7 +968,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>> int state)
>> {
>> struct kmx61_data *data = kmx61_get_data(indio_dev);
>> - int ret;
>> + int ret = 0;
>>
>> if (state && data->ev_enable_state)
>> return 0;
>> @@ -987,27 +977,25 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>>
>> if (!state && data->motion_trig_on) {
>> data->ev_enable_state = 0;
>> - mutex_unlock(&data->lock);
>> - return 0;
>> + goto err_unlock;
>> }
>>
>> ret = kmx61_set_power_state(data, state, KMX61_ACC);
>> - if (ret < 0) {
>> - mutex_unlock(&data->lock);
>> - return ret;
>> - }
>> + if (ret < 0)
>> + goto err_unlock;
>>
>> ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
>> if (ret < 0) {
>> kmx61_set_power_state(data, false, KMX61_ACC);
>> - mutex_unlock(&data->lock);
>> - return ret;
>> + goto err_unlock;
>> }
>>
>> data->ev_enable_state = state;
>> +
>> +err_unlock:
>> mutex_unlock(&data->lock);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
>> @@ -1066,8 +1054,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>
>> if (!state && data->ev_enable_state && data->motion_trig_on) {
>> data->motion_trig_on = false;
>> - mutex_unlock(&data->lock);
>> - return 0;
>> + goto err_unlock;
>> }
>>
>>
>> @@ -1077,10 +1064,8 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> device = KMX61_MAG;
>>
>> ret = kmx61_set_power_state(data, state, device);
>> - if (ret < 0) {
>> - mutex_unlock(&data->lock);
>> - return ret;
>> - }
>> + if (ret < 0)
>> + goto err_unlock;
>>
>> if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
>> ret = kmx61_setup_new_data_interrupt(data, state, device);
>> @@ -1088,8 +1073,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> ret = kmx61_setup_any_motion_interrupt(data, state, KMX61_ACC);
>> if (ret < 0) {
>> kmx61_set_power_state(data, false, device);
>> - mutex_unlock(&data->lock);
>> - return ret;
>> + goto err_unlock;
>> }
>>
>> if (data->acc_dready_trig == trig)
>> @@ -1098,10 +1082,10 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> data->mag_dready_trig_on = state;
>> else
>> data->motion_trig_on = state;
>> -
>> +err_unlock:
>> mutex_unlock(&data->lock);
>>
>> - return 0;
>> + return ret;
>> }
>>
>> static int kmx61_trig_try_reenable(struct iio_trigger *trig)
>> @@ -1207,7 +1191,7 @@ ack_intr:
>> ret |= KMX61_REG_CTRL1_BIT_RES;
>> ret = i2c_smbus_write_byte_data(data->client, KMX61_REG_CTRL1, ret);
>> if (ret < 0)
>> - dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
>> + dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
>>
>> ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INL);
>> if (ret < 0)
>> @@ -1409,15 +1393,17 @@ static int kmx61_probe(struct i2c_client *client,
>> data->acc_dready_trig =
>> kmx61_trigger_setup(data, data->acc_indio_dev,
>> "dready");
>> - if (IS_ERR(data->acc_dready_trig))
>> - return PTR_ERR(data->acc_dready_trig);
>> + if (IS_ERR(data->acc_dready_trig)) {
>> + ret = PTR_ERR(data->acc_dready_trig);
> Double whitespace.
>> + goto err_chip_uninit;
>> + }
>>
>> data->mag_dready_trig =
>> kmx61_trigger_setup(data, data->mag_indio_dev,
>> "dready");
>> if (IS_ERR(data->mag_dready_trig)) {
>> ret = PTR_ERR(data->mag_dready_trig);
>> - goto err_trigger_unregister;
>> + goto err_trigger_unregister_acc_dready;
>> }
>>
>> data->motion_trig =
>> @@ -1425,7 +1411,7 @@ static int kmx61_probe(struct i2c_client *client,
>> "any-motion");
>> if (IS_ERR(data->motion_trig)) {
>> ret = PTR_ERR(data->motion_trig);
>> - goto err_trigger_unregister;
>> + goto err_trigger_unregister_mag_dready;
>> }
>>
>> ret = iio_triggered_buffer_setup(data->acc_indio_dev,
>> @@ -1435,7 +1421,7 @@ static int kmx61_probe(struct i2c_client *client,
>> if (ret < 0) {
>> dev_err(&data->client->dev,
>> "Failed to setup acc triggered buffer\n");
>> - goto err_trigger_unregister;
>> + goto err_trigger_unregister_motion;
>> }
>>
>> ret = iio_triggered_buffer_setup(data->mag_indio_dev,
>> @@ -1445,14 +1431,14 @@ static int kmx61_probe(struct i2c_client *client,
>> if (ret < 0) {
>> dev_err(&data->client->dev,
>> "Failed to setup mag triggered buffer\n");
>> - goto err_trigger_unregister;
>> + goto err_buffer_cleanup_acc;
>> }
>> }
>>
>> ret = iio_device_register(data->acc_indio_dev);
>> if (ret < 0) {
>> dev_err(&client->dev, "Failed to register acc iio device\n");
>> - goto err_buffer_cleanup;
>> + goto err_buffer_cleanup_mag;
>> }
>>
>> ret = iio_device_register(data->mag_indio_dev);
>> @@ -1475,18 +1461,18 @@ err_iio_unregister_mag:
>> iio_device_unregister(data->mag_indio_dev);
>> err_iio_unregister_acc:
>> iio_device_unregister(data->acc_indio_dev);
>> -err_buffer_cleanup:
>> - if (client->irq >= 0) {
>> - iio_triggered_buffer_cleanup(data->acc_indio_dev);
>> +err_buffer_cleanup_mag:
>> + if (client->irq >= 0)
>> iio_triggered_buffer_cleanup(data->mag_indio_dev);
>> - }
>> -err_trigger_unregister:
>> - if (data->acc_dready_trig)
>> - iio_trigger_unregister(data->acc_dready_trig);
>> - if (data->mag_dready_trig)
>> - iio_trigger_unregister(data->mag_dready_trig);
>> - if (data->motion_trig)
>> - iio_trigger_unregister(data->motion_trig);
>> +err_buffer_cleanup_acc:
>> + if (client->irq >= 0)
>> + iio_triggered_buffer_cleanup(data->acc_indio_dev);
>> +err_trigger_unregister_motion:
>> + iio_trigger_unregister(data->motion_trig);
>> +err_trigger_unregister_mag_dready:
>> + iio_trigger_unregister(data->mag_dready_trig);
>> +err_trigger_unregister_acc_dready:
>> + iio_trigger_unregister(data->acc_dready_trig);
>> err_chip_uninit:
>> kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
>> return ret;
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment
2015-01-01 13:47 ` Hartmut Knaack
@ 2015-01-04 10:47 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:47 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:47, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Acked-by: Hartmut Knaack <knaack.h@gmx.de>
Applied.
>> ---
>> drivers/iio/imu/kmx61.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 4fc4f63..2652179 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -926,11 +926,11 @@ static int kmx61_read_event(struct iio_dev *indio_dev,
>> }
>>
>> static int kmx61_write_event(struct iio_dev *indio_dev,
>> - const struct iio_chan_spec *chan,
>> - enum iio_event_type type,
>> - enum iio_event_direction dir,
>> - enum iio_event_info info,
>> - int val, int val2)
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + enum iio_event_info info,
>> + int val, int val2)
>> {
>> struct kmx61_data *data = kmx61_get_data(indio_dev);
>>
>> @@ -962,10 +962,10 @@ static int kmx61_read_event_config(struct iio_dev *indio_dev,
>> }
>>
>> static int kmx61_write_event_config(struct iio_dev *indio_dev,
>> - const struct iio_chan_spec *chan,
>> - enum iio_event_type type,
>> - enum iio_event_direction dir,
>> - int state)
>> + const struct iio_chan_spec *chan,
>> + enum iio_event_type type,
>> + enum iio_event_direction dir,
>> + int state)
>> {
>> struct kmx61_data *data = kmx61_get_data(indio_dev);
>> int ret = 0;
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state
2015-01-01 13:50 ` Hartmut Knaack
@ 2015-01-04 10:49 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:49 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:50, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Acked-by: Hartmut Knaack <knaack.h@gmx.de>
Applied
>> ---
>> drivers/iio/imu/kmx61.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 332ee67..30825cf 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -976,7 +976,7 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
>> mutex_lock(&data->lock);
>>
>> if (!state && data->motion_trig_on) {
>> - data->ev_enable_state = 0;
>> + data->ev_enable_state = false;
>> goto err_unlock;
>> }
>>
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state
2015-01-01 13:51 ` Hartmut Knaack
@ 2015-01-04 10:51 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:51 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:51, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Acked-by: Hartmut Knaack <knaack.h@gmx.de>
Applied
>> ---
>> drivers/iio/imu/kmx61.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 30825cf..5b5be2b 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -1057,8 +1057,7 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
>> goto err_unlock;
>> }
>>
>> -
>> - if (data->acc_dready_trig == trig || data->motion_trig)
>> + if (data->acc_dready_trig == trig || data->motion_trig == trig)
>> device = KMX61_ACC;
>> else
>> device = KMX61_MAG;
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read
2015-01-01 13:51 ` Hartmut Knaack
@ 2015-01-04 10:54 ` Jonathan Cameron
0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:54 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:51, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> Useful in the debugging phase, not needed now.
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
> Acked-by: Hartmut Knaack <knaack.h@gmx.de>
Applied.
>> ---
>> drivers/iio/imu/kmx61.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index 5b5be2b..eb3900e 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -1196,8 +1196,6 @@ ack_intr:
>> if (ret < 0)
>> dev_err(&data->client->dev, "Error reading reg_inl\n");
>>
>> - ret = i2c_smbus_read_byte_data(data->client, KMX61_REG_INS1);
>> -
>> return IRQ_HANDLED;
>> }
>>
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
2015-01-01 13:53 ` Hartmut Knaack
@ 2015-01-04 10:55 ` Jonathan Cameron
2015-01-04 11:19 ` Hartmut Knaack
0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Cameron @ 2015-01-04 10:55 UTC (permalink / raw)
To: Hartmut Knaack, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/01/15 13:53, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> odr_bits values are between 0 and 11, so we can use the index
>> in kmx61_samp_freq_table instead of odr_bits structure member.
> Basically looking good, but I would feel more comfortable to check
> against the boundaries of the table. Please see inline.
>
>>
>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>> ---
>> drivers/iio/imu/kmx61.c | 64 ++++++++++++++++++++-----------------------------
>> 1 file changed, 26 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index eb3900e..98369eb 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
>> static const struct {
>> int val;
>> int val2;
>> - u8 odr_bits;
>> -} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
>> - {25, 0, 0x01},
>> - {50, 0, 0x02},
>> - {100, 0, 0x03},
>> - {200, 0, 0x04},
>> - {400, 0, 0x05},
>> - {800, 0, 0x06},
>> - {1600, 0, 0x07},
>> - {0, 781000, 0x08},
>> - {1, 563000, 0x09},
>> - {3, 125000, 0x0A},
>> - {6, 250000, 0x0B} };
>> +} kmx61_samp_freq_table[] = { {12, 500000},
>> + {25, 0},
>> + {50, 0},
>> + {100, 0},
>> + {200, 0},
>> + {400, 0},
>> + {800, 0},
>> + {1600, 0},
>> + {0, 781000},
>> + {1, 563000},
>> + {3, 125000},
>> + {6, 250000} };
>>
>> static const struct {
>> int val;
>> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
>> for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> if (val == kmx61_samp_freq_table[i].val &&
>> val2 == kmx61_samp_freq_table[i].val2)
>> - return kmx61_samp_freq_table[i].odr_bits;
>> - return -EINVAL;
>> -}
>> -
>> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
>> -{
>> - int i;
>> -
>> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> - if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
>> - *val = kmx61_samp_freq_table[i].val;
>> - *val2 = kmx61_samp_freq_table[i].val2;
>> - return 0;
>> - }
>> + return i;
>> return -EINVAL;
>> }
>>
>> -
>> static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>> {
>> int i;
>> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>>
>> static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>> u8 device)
>> -{ int i;
>> +{
>> u8 lodr_bits;
>>
>> if (device & KMX61_ACC)
>> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>> else
>> return -EINVAL;
>>
>> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> - if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
>> - *val = kmx61_samp_freq_table[i].val;
>> - *val2 = kmx61_samp_freq_table[i].val2;
>> - return 0;
>> - }
>> - return -EINVAL;
>> + if (lodr_bits > 0xB)
> Since we use it as an index, I regard it safer to check against
> ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.
Makes sense to me as well - though obviously ARRAY_SIZE(kmx61_samp_freq_table) is
0xC (12) rather than 0xB (11) so you'll need a minus 1
I'll leave this one for a new spin.
>
>> + return -EINVAL;
>> +
>> + *val = kmx61_samp_freq_table[lodr_bits].val;
>> + *val2 = kmx61_samp_freq_table[lodr_bits].val2;
>> +
>> + return 0;
>> }
>>
>> static int kmx61_set_range(struct kmx61_data *data, u8 range)
>> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
>> }
>> data->odr_bits = ret;
>>
>> - /* set output data rate for wake up (motion detection) function */
>> - ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
>> + /*
>> + * set output data rate for wake up (motion detection) function
>> + * to match data rate for accelerometer sampling
>> + */
>> + ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
>> if (ret < 0)
>> return ret;
>>
>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table
2015-01-04 10:55 ` Jonathan Cameron
@ 2015-01-04 11:19 ` Hartmut Knaack
0 siblings, 0 replies; 32+ messages in thread
From: Hartmut Knaack @ 2015-01-04 11:19 UTC (permalink / raw)
To: Jonathan Cameron, Daniel Baluta
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
Jonathan Cameron schrieb am 04.01.2015 um 11:55:
> On 01/01/15 13:53, Hartmut Knaack wrote:
>> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>>> odr_bits values are between 0 and 11, so we can use the index
>>> in kmx61_samp_freq_table instead of odr_bits structure member.
>> Basically looking good, but I would feel more comfortable to check
>> against the boundaries of the table. Please see inline.
>>
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta@intel.com>
>>> ---
>>> drivers/iio/imu/kmx61.c | 64 ++++++++++++++++++++-----------------------------
>>> 1 file changed, 26 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>>> index eb3900e..98369eb 100644
>>> --- a/drivers/iio/imu/kmx61.c
>>> +++ b/drivers/iio/imu/kmx61.c
>>> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
>>> static const struct {
>>> int val;
>>> int val2;
>>> - u8 odr_bits;
>>> -} kmx61_samp_freq_table[] = { {12, 500000, 0x00},
>>> - {25, 0, 0x01},
>>> - {50, 0, 0x02},
>>> - {100, 0, 0x03},
>>> - {200, 0, 0x04},
>>> - {400, 0, 0x05},
>>> - {800, 0, 0x06},
>>> - {1600, 0, 0x07},
>>> - {0, 781000, 0x08},
>>> - {1, 563000, 0x09},
>>> - {3, 125000, 0x0A},
>>> - {6, 250000, 0x0B} };
>>> +} kmx61_samp_freq_table[] = { {12, 500000},
>>> + {25, 0},
>>> + {50, 0},
>>> + {100, 0},
>>> + {200, 0},
>>> + {400, 0},
>>> + {800, 0},
>>> + {1600, 0},
>>> + {0, 781000},
>>> + {1, 563000},
>>> + {3, 125000},
>>> + {6, 250000} };
>>>
>>> static const struct {
>>> int val;
>>> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
>>> for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> if (val == kmx61_samp_freq_table[i].val &&
>>> val2 == kmx61_samp_freq_table[i].val2)
>>> - return kmx61_samp_freq_table[i].odr_bits;
>>> - return -EINVAL;
>>> -}
>>> -
>>> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
>>> -{
>>> - int i;
>>> -
>>> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> - if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
>>> - *val = kmx61_samp_freq_table[i].val;
>>> - *val2 = kmx61_samp_freq_table[i].val2;
>>> - return 0;
>>> - }
>>> + return i;
>>> return -EINVAL;
>>> }
>>>
>>> -
>>> static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>>> {
>>> int i;
>>> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, int val2, u8 device)
>>>
>>> static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>>> u8 device)
>>> -{ int i;
>>> +{
>>> u8 lodr_bits;
>>>
>>> if (device & KMX61_ACC)
>>> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>>> else
>>> return -EINVAL;
>>>
>>> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> - if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
>>> - *val = kmx61_samp_freq_table[i].val;
>>> - *val2 = kmx61_samp_freq_table[i].val2;
>>> - return 0;
>>> - }
>>> - return -EINVAL;
>>> + if (lodr_bits > 0xB)
>> Since we use it as an index, I regard it safer to check against
>> ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.
> Makes sense to me as well - though obviously ARRAY_SIZE(kmx61_samp_freq_table) is
> 0xC (12) rather than 0xB (11) so you'll need a minus 1
>
Instead of subtracting, I would favor this one:
if !(lodr_bits < ARRAY_SIZE(...))
Or this one:
if (lodr_bits >= ARRAY_SIZE(...))
> I'll leave this one for a new spin.
>>
>>> + return -EINVAL;
>>> +
>>> + *val = kmx61_samp_freq_table[lodr_bits].val;
>>> + *val2 = kmx61_samp_freq_table[lodr_bits].val2;
>>> +
>>> + return 0;
>>> }
>>>
>>> static int kmx61_set_range(struct kmx61_data *data, u8 range)
>>> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
>>> }
>>> data->odr_bits = ret;
>>>
>>> - /* set output data rate for wake up (motion detection) function */
>>> - ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
>>> + /*
>>> + * set output data rate for wake up (motion detection) function
>>> + * to match data rate for accelerometer sampling
>>> + */
>>> + ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
>>> if (ret < 0)
>>> return ret;
>>>
>>>
>>
>
> --
> 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] 32+ messages in thread
* Re: [PATCH 03/10] iio: imu: kmx61: Enhance error handling
2015-01-04 10:45 ` Jonathan Cameron
@ 2015-01-05 8:57 ` Daniel Baluta
0 siblings, 0 replies; 32+ messages in thread
From: Daniel Baluta @ 2015-01-05 8:57 UTC (permalink / raw)
To: Jonathan Cameron, Hartmut Knaack
Cc: lars, pmeerw, linux-iio, linux-kernel, srinivas.pandruvada
On 01/04/2015 12:45 PM, Jonathan Cameron wrote:
> On 01/01/15 13:47, Hartmut Knaack wrote:
>> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>>> This fixes parts of kmx61 error handling to make code easier to read and to be
>>> more consistent with IIO coding conventions:
>>> * prefer as single point for error handling instead of duplicating code
>>> for each function
>>> * directly return a value from a case branch instead of breaking
>>> * fix error message for writing REG_CTRL1
>>>
>>> Also, add separate error paths for kmx61_trigger_setup/iio_triggered_buffer_setup
>>> calls.
>> Some issues remain in this one, please see inline. Otherwise looking good.
>>
> Fixed up and applied.
Thanks for doing this Jonathan.
Daniel.
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2015-01-05 8:55 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-23 13:22 [PATCH 00/10] iio: KMX61 fixes Daniel Baluta
2014-12-23 13:22 ` [PATCH 01/10] iio: imu: kmx61: Save odr_bits for later use Daniel Baluta
2015-01-01 13:45 ` Hartmut Knaack
2015-01-04 10:41 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 02/10] iio: imu: kmx61: Don't ignore kmx61_set_power_state errors Daniel Baluta
2015-01-01 13:45 ` Hartmut Knaack
2015-01-04 10:42 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 03/10] iio: imu: kmx61: Enhance error handling Daniel Baluta
2015-01-01 13:47 ` Hartmut Knaack
2015-01-04 10:45 ` Jonathan Cameron
2015-01-05 8:57 ` Daniel Baluta
2014-12-23 13:22 ` [PATCH 04/10] iio: imu: kmx61: Fixup parameters alignment Daniel Baluta
2015-01-01 13:47 ` Hartmut Knaack
2015-01-04 10:47 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 05/10] iio: imu: kmx61: Drop unused device parameter Daniel Baluta
2015-01-01 13:49 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 06/10] iio: imu: kmx61: Use false instead of 0 for ev_enable_state Daniel Baluta
2015-01-01 13:50 ` Hartmut Knaack
2015-01-04 10:49 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 07/10] iio: imu: kmx61: Fix device initialization when setting trigger state Daniel Baluta
2015-01-01 13:51 ` Hartmut Knaack
2015-01-04 10:51 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 08/10] iio: imu: kmx61: Remove unnecessary REG_INS1 read Daniel Baluta
2015-01-01 13:51 ` Hartmut Knaack
2015-01-04 10:54 ` Jonathan Cameron
2014-12-23 13:22 ` [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table Daniel Baluta
2015-01-01 13:53 ` Hartmut Knaack
2015-01-04 10:55 ` Jonathan Cameron
2015-01-04 11:19 ` Hartmut Knaack
2014-12-23 13:22 ` [PATCH 10/10] iio: imu: kmx61: Use correct base when reading data Daniel Baluta
2015-01-01 13:54 ` Hartmut Knaack
2014-12-26 9:57 ` [PATCH 00/10] iio: KMX61 fixes Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).