| Message ID | 245cce4d379d225ab6794fc3326d95f88d2abf1a.1736201898.git.Jonathan.Santos@analog.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iio: adc: ad7768-1: Add features, improvements, and fixes | expand |
On 1/7/25 9:25 AM, Jonathan Santos wrote: > Use guard(mutex) from cleanup.h to remove most of the gotos and to make > the code simpler and less likely to fail due to forgetting to unlock > the resources. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- ... > @@ -484,7 +477,7 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) > struct ad7768_state *st = iio_priv(indio_dev); > int ret; > > - mutex_lock(&st->lock); > + guard(mutex)(&st->lock); > > ret = spi_read(st->spi, &st->data.scan.chan, 3); > if (ret < 0) > @@ -495,7 +488,6 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) > > err_unlock: nit: also rename the label since it is no longer unlocking > iio_trigger_notify_done(indio_dev->trig); > - mutex_unlock(&st->lock); > > return IRQ_HANDLED; > } I'm also wondering if we should just drop this lock. It is only protecting a triggered buffer SPI xfer and debugfs register access from happening at the same time. Since we have to write a magic value to exit conversion mode, reading registers during a buffered read is going to cause problems anyway. So we could just remove the mutex lock and use iio_device_claim_direct_mode() instead in ad7768_reg_access().
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index f73b9aec8b0f..cd1b08053105 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -5,6 +5,7 @@ * Copyright 2017 Analog Devices Inc. */ #include <linux/bitfield.h> +#include <linux/cleanup.h> #include <linux/clk.h> #include <linux/delay.h> #include <linux/device.h> @@ -257,20 +258,12 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, unsigned int *readval) { struct ad7768_state *st = iio_priv(indio_dev); - int ret; - mutex_lock(&st->lock); - if (readval) { - ret = ad7768_spi_reg_read(st, reg, readval, 1); - if (ret < 0) - goto err_unlock; - } else { - ret = ad7768_spi_reg_write(st, reg, writeval); - } -err_unlock: - mutex_unlock(&st->lock); + guard(mutex)(&st->lock); + if (readval) + return ad7768_spi_reg_read(st, reg, readval, 1); - return ret; + return ad7768_spi_reg_write(st, reg, writeval); } static int ad7768_set_dig_fil(struct ad7768_state *st, @@ -484,7 +477,7 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) struct ad7768_state *st = iio_priv(indio_dev); int ret; - mutex_lock(&st->lock); + guard(mutex)(&st->lock); ret = spi_read(st->spi, &st->data.scan.chan, 3); if (ret < 0) @@ -495,7 +488,6 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) err_unlock: iio_trigger_notify_done(indio_dev->trig); - mutex_unlock(&st->lock); return IRQ_HANDLED; }
Use guard(mutex) from cleanup.h to remove most of the gotos and to make the code simpler and less likely to fail due to forgetting to unlock the resources. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- drivers/iio/adc/ad7768-1.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-)