lkml.org 
[lkml]   [2015]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 03/10] iio: imu: kmx61: Enhance error handling
    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
    >



    \
     
     \ /
      Last update: 2015-01-04 12:21    [W:3.128 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site