Skip to content

Commit f9242e3

Browse files
maxskiierjic23
authored andcommitted
iio: imu: kmx61: Use guard(mutex)() over manual locking
Include linux/cleanup.h to take advantage of new macros. Replace manual mutex_lock() and mutex_unlock() calls across the file with guard(mutex)() and scoped_guard() where appropriate to simplify error paths and eliminate manual locking calls. Add new helper function kmx61_read_for_each_active_channel() to mitigate certain style issues and to prevent notifying that the IRQ is finished whilst holding the lock. Update certain returns, and add default case to return -EINVAL in kmx61_read_raw(). Remove now-redundant gotos and ret variables, as the new RAII macros make them unneeded. Signed-off-by: Maxwell Doose <m32285159@gmail.com> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
1 parent f9f9919 commit f9242e3

1 file changed

Lines changed: 67 additions & 62 deletions

File tree

drivers/iio/imu/kmx61.c

Lines changed: 67 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* IIO driver for KMX61 (7-bit I2C slave address 0x0E or 0x0F).
88
*/
99

10+
#include <linux/cleanup.h>
1011
#include <linux/i2c.h>
1112
#include <linux/interrupt.h>
1213
#include <linux/mod_devicetable.h>
@@ -783,7 +784,7 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
783784
struct kmx61_data *data = kmx61_get_data(indio_dev);
784785

785786
switch (mask) {
786-
case IIO_CHAN_INFO_RAW:
787+
case IIO_CHAN_INFO_RAW: {
787788
switch (chan->type) {
788789
case IIO_ACCEL:
789790
base_reg = KMX61_ACC_XOUT_L;
@@ -794,28 +795,24 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
794795
default:
795796
return -EINVAL;
796797
}
797-
mutex_lock(&data->lock);
798+
guard(mutex)(&data->lock);
798799

799800
ret = kmx61_set_power_state(data, true, chan->address);
800-
if (ret) {
801-
mutex_unlock(&data->lock);
801+
if (ret)
802802
return ret;
803-
}
804803

805804
ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
806805
if (ret < 0) {
807806
kmx61_set_power_state(data, false, chan->address);
808-
mutex_unlock(&data->lock);
809807
return ret;
810808
}
811809
*val = sign_extend32(ret >> chan->scan_type.shift,
812810
chan->scan_type.realbits - 1);
813811
ret = kmx61_set_power_state(data, false, chan->address);
814-
815-
mutex_unlock(&data->lock);
816812
if (ret)
817813
return ret;
818814
return IIO_VAL_INT;
815+
}
819816
case IIO_CHAN_INFO_SCALE:
820817
switch (chan->type) {
821818
case IIO_ACCEL:
@@ -830,45 +827,46 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
830827
default:
831828
return -EINVAL;
832829
}
833-
case IIO_CHAN_INFO_SAMP_FREQ:
830+
case IIO_CHAN_INFO_SAMP_FREQ: {
834831
if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
835832
return -EINVAL;
836833

837-
mutex_lock(&data->lock);
834+
guard(mutex)(&data->lock);
835+
838836
ret = kmx61_get_odr(data, val, val2, chan->address);
839-
mutex_unlock(&data->lock);
840837
if (ret)
841838
return -EINVAL;
842839
return IIO_VAL_INT_PLUS_MICRO;
843840
}
844-
return -EINVAL;
841+
default:
842+
return -EINVAL;
843+
}
845844
}
846845

847846
static int kmx61_write_raw(struct iio_dev *indio_dev,
848847
struct iio_chan_spec const *chan, int val,
849848
int val2, long mask)
850849
{
851-
int ret;
852850
struct kmx61_data *data = kmx61_get_data(indio_dev);
853851

854852
switch (mask) {
855-
case IIO_CHAN_INFO_SAMP_FREQ:
853+
case IIO_CHAN_INFO_SAMP_FREQ: {
856854
if (chan->type != IIO_ACCEL && chan->type != IIO_MAGN)
857855
return -EINVAL;
858856

859-
mutex_lock(&data->lock);
860-
ret = kmx61_set_odr(data, val, val2, chan->address);
861-
mutex_unlock(&data->lock);
862-
return ret;
857+
guard(mutex)(&data->lock);
858+
859+
return kmx61_set_odr(data, val, val2, chan->address);
860+
}
863861
case IIO_CHAN_INFO_SCALE:
864862
switch (chan->type) {
865-
case IIO_ACCEL:
863+
case IIO_ACCEL: {
866864
if (val != 0)
867865
return -EINVAL;
868-
mutex_lock(&data->lock);
869-
ret = kmx61_set_scale(data, val2);
870-
mutex_unlock(&data->lock);
871-
return ret;
866+
guard(mutex)(&data->lock);
867+
868+
return kmx61_set_scale(data, val2);
869+
}
872870
default:
873871
return -EINVAL;
874872
}
@@ -945,29 +943,26 @@ static int kmx61_write_event_config(struct iio_dev *indio_dev,
945943
if (state && data->ev_enable_state)
946944
return 0;
947945

948-
mutex_lock(&data->lock);
946+
guard(mutex)(&data->lock);
949947

950948
if (!state && data->motion_trig_on) {
951949
data->ev_enable_state = false;
952-
goto err_unlock;
950+
return ret;
953951
}
954952

955953
ret = kmx61_set_power_state(data, state, KMX61_ACC);
956954
if (ret < 0)
957-
goto err_unlock;
955+
return ret;
958956

959957
ret = kmx61_setup_any_motion_interrupt(data, state);
960958
if (ret < 0) {
961959
kmx61_set_power_state(data, false, KMX61_ACC);
962-
goto err_unlock;
960+
return ret;
963961
}
964962

965963
data->ev_enable_state = state;
966964

967-
err_unlock:
968-
mutex_unlock(&data->lock);
969-
970-
return ret;
965+
return 0;
971966
}
972967

973968
static int kmx61_acc_validate_trigger(struct iio_dev *indio_dev,
@@ -1020,11 +1015,11 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
10201015
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
10211016
struct kmx61_data *data = kmx61_get_data(indio_dev);
10221017

1023-
mutex_lock(&data->lock);
1018+
guard(mutex)(&data->lock);
10241019

10251020
if (!state && data->ev_enable_state && data->motion_trig_on) {
10261021
data->motion_trig_on = false;
1027-
goto err_unlock;
1022+
return ret;
10281023
}
10291024

10301025
if (data->acc_dready_trig == trig || data->motion_trig == trig)
@@ -1034,15 +1029,15 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
10341029

10351030
ret = kmx61_set_power_state(data, state, device);
10361031
if (ret < 0)
1037-
goto err_unlock;
1032+
return ret;
10381033

10391034
if (data->acc_dready_trig == trig || data->mag_dready_trig == trig)
10401035
ret = kmx61_setup_new_data_interrupt(data, state, device);
10411036
else
10421037
ret = kmx61_setup_any_motion_interrupt(data, state);
10431038
if (ret < 0) {
10441039
kmx61_set_power_state(data, false, device);
1045-
goto err_unlock;
1040+
return ret;
10461041
}
10471042

10481043
if (data->acc_dready_trig == trig)
@@ -1051,10 +1046,8 @@ static int kmx61_data_rdy_trigger_set_state(struct iio_trigger *trig,
10511046
data->mag_dready_trig_on = state;
10521047
else
10531048
data->motion_trig_on = state;
1054-
err_unlock:
1055-
mutex_unlock(&data->lock);
10561049

1057-
return ret;
1050+
return 0;
10581051
}
10591052

10601053
static void kmx61_trig_reenable(struct iio_trigger *trig)
@@ -1181,30 +1174,49 @@ static irqreturn_t kmx61_data_rdy_trig_poll(int irq, void *private)
11811174
return IRQ_HANDLED;
11821175
}
11831176

1184-
static irqreturn_t kmx61_trigger_handler(int irq, void *p)
1177+
/**
1178+
* kmx61_read_for_each_active_channel() - Read each active channel into a buffer
1179+
*
1180+
* @indio_dev: IIO Device struct to read from
1181+
* @buffer: Destination buffer to write to, the array must be of at least size 8
1182+
*
1183+
* Return:
1184+
* 0 on success, negative errno on failure.
1185+
*/
1186+
static int kmx61_read_for_each_active_channel(struct iio_dev *indio_dev, s16 *buffer)
11851187
{
1186-
struct iio_poll_func *pf = p;
1187-
struct iio_dev *indio_dev = pf->indio_dev;
11881188
struct kmx61_data *data = kmx61_get_data(indio_dev);
1189-
int bit, ret, i = 0;
11901189
u8 base;
1191-
s16 buffer[8] = { };
1190+
int ret, bit;
1191+
int i = 0;
11921192

11931193
if (indio_dev == data->acc_indio_dev)
11941194
base = KMX61_ACC_XOUT_L;
11951195
else
11961196
base = KMX61_MAG_XOUT_L;
11971197

1198-
mutex_lock(&data->lock);
1198+
guard(mutex)(&data->lock);
1199+
11991200
iio_for_each_active_channel(indio_dev, bit) {
12001201
ret = kmx61_read_measurement(data, base, bit);
1201-
if (ret < 0) {
1202-
mutex_unlock(&data->lock);
1203-
goto err;
1204-
}
1202+
if (ret < 0)
1203+
return ret;
12051204
buffer[i++] = ret;
12061205
}
1207-
mutex_unlock(&data->lock);
1206+
1207+
return 0;
1208+
}
1209+
1210+
static irqreturn_t kmx61_trigger_handler(int irq, void *p)
1211+
{
1212+
struct iio_poll_func *pf = p;
1213+
struct iio_dev *indio_dev = pf->indio_dev;
1214+
int ret;
1215+
s16 buffer[8] = { };
1216+
1217+
ret = kmx61_read_for_each_active_channel(indio_dev, buffer);
1218+
if (ret < 0)
1219+
goto err;
12081220

12091221
iio_push_to_buffers(indio_dev, buffer);
12101222
err:
@@ -1419,22 +1431,18 @@ static void kmx61_remove(struct i2c_client *client)
14191431
iio_trigger_unregister(data->motion_trig);
14201432
}
14211433

1422-
mutex_lock(&data->lock);
1434+
guard(mutex)(&data->lock);
1435+
14231436
kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
1424-
mutex_unlock(&data->lock);
14251437
}
14261438

14271439
static int kmx61_suspend(struct device *dev)
14281440
{
1429-
int ret;
14301441
struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
14311442

1432-
mutex_lock(&data->lock);
1433-
ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG,
1434-
false);
1435-
mutex_unlock(&data->lock);
1443+
guard(mutex)(&data->lock);
14361444

1437-
return ret;
1445+
return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, false);
14381446
}
14391447

14401448
static int kmx61_resume(struct device *dev)
@@ -1453,13 +1461,10 @@ static int kmx61_resume(struct device *dev)
14531461
static int kmx61_runtime_suspend(struct device *dev)
14541462
{
14551463
struct kmx61_data *data = i2c_get_clientdata(to_i2c_client(dev));
1456-
int ret;
14571464

1458-
mutex_lock(&data->lock);
1459-
ret = kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
1460-
mutex_unlock(&data->lock);
1465+
guard(mutex)(&data->lock);
14611466

1462-
return ret;
1467+
return kmx61_set_mode(data, KMX61_ALL_STBY, KMX61_ACC | KMX61_MAG, true);
14631468
}
14641469

14651470
static int kmx61_runtime_resume(struct device *dev)

0 commit comments

Comments
 (0)