* [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 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.