| Message ID | 20231125-mcp3911-guard-v1-1-2748d16a3f3f@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: mcp3911: simplify code with guard macro | expand |
On Sat, 25 Nov 2023 19:57:24 +0100 Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > Use the guard(mutex) macro for handle mutex lock/unlocks. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> Hi Marcus, Great to see this being used, but there is a little more you can do here to take advantage fully. > --- > drivers/iio/adc/mcp3911.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > index 974c5bd923a6..85bb13eb6f3b 100644 > --- a/drivers/iio/adc/mcp3911.c > +++ b/drivers/iio/adc/mcp3911.c > @@ -7,6 +7,7 @@ > */ > #include <linux/bitfield.h> > #include <linux/bits.h> > +#include <linux/cleanup.h> > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/err.h> > @@ -168,7 +169,7 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev, > struct mcp3911 *adc = iio_priv(indio_dev); > int ret = -EINVAL; > > - mutex_lock(&adc->lock); > + guard(mutex)(&adc->lock); > switch (mask) { > case IIO_CHAN_INFO_RAW: > ret = mcp3911_read(adc, > @@ -207,7 +208,6 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev, > } > > out: > - mutex_unlock(&adc->lock); > return ret; No point in having the label down here after the change. Just return directly wherever you see an error. Same for other cases. > } > > @@ -218,7 +218,7 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev, > struct mcp3911 *adc = iio_priv(indio_dev); > int ret = -EINVAL; > > - mutex_lock(&adc->lock); > + guard(mutex)(&adc->lock); > switch (mask) { > case IIO_CHAN_INFO_SCALE: > for (int i = 0; i < MCP3911_NUM_SCALES; i++) { > @@ -263,7 +263,6 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev, > } > > out: > - mutex_unlock(&adc->lock); > return ret; > } > > @@ -350,7 +349,7 @@ static irqreturn_t mcp3911_trigger_handler(int irq, void *p) > int i = 0; > int ret; > > - mutex_lock(&adc->lock); > + guard(mutex)(&adc->lock); > adc->tx_buf = MCP3911_REG_READ(MCP3911_CHANNEL(0), adc->dev_addr); > ret = spi_sync_transfer(adc->spi, xfer, ARRAY_SIZE(xfer)); > if (ret < 0) { > @@ -368,7 +367,6 @@ static irqreturn_t mcp3911_trigger_handler(int irq, void *p) > iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, > iio_get_time_ns(indio_dev)); > out: > - mutex_unlock(&adc->lock); > iio_trigger_notify_done(indio_dev->trig); > > return IRQ_HANDLED; > > --- > base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa > change-id: 20231125-mcp3911-guard-866591e2c947 > > Best regards,
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c index 974c5bd923a6..85bb13eb6f3b 100644 --- a/drivers/iio/adc/mcp3911.c +++ b/drivers/iio/adc/mcp3911.c @@ -7,6 +7,7 @@ */ #include <linux/bitfield.h> #include <linux/bits.h> +#include <linux/cleanup.h> #include <linux/clk.h> #include <linux/delay.h> #include <linux/err.h> @@ -168,7 +169,7 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev, struct mcp3911 *adc = iio_priv(indio_dev); int ret = -EINVAL; - mutex_lock(&adc->lock); + guard(mutex)(&adc->lock); switch (mask) { case IIO_CHAN_INFO_RAW: ret = mcp3911_read(adc, @@ -207,7 +208,6 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev, } out: - mutex_unlock(&adc->lock); return ret; } @@ -218,7 +218,7 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev, struct mcp3911 *adc = iio_priv(indio_dev); int ret = -EINVAL; - mutex_lock(&adc->lock); + guard(mutex)(&adc->lock); switch (mask) { case IIO_CHAN_INFO_SCALE: for (int i = 0; i < MCP3911_NUM_SCALES; i++) { @@ -263,7 +263,6 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev, } out: - mutex_unlock(&adc->lock); return ret; } @@ -350,7 +349,7 @@ static irqreturn_t mcp3911_trigger_handler(int irq, void *p) int i = 0; int ret; - mutex_lock(&adc->lock); + guard(mutex)(&adc->lock); adc->tx_buf = MCP3911_REG_READ(MCP3911_CHANNEL(0), adc->dev_addr); ret = spi_sync_transfer(adc->spi, xfer, ARRAY_SIZE(xfer)); if (ret < 0) { @@ -368,7 +367,6 @@ static irqreturn_t mcp3911_trigger_handler(int irq, void *p) iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, iio_get_time_ns(indio_dev)); out: - mutex_unlock(&adc->lock); iio_trigger_notify_done(indio_dev->trig); return IRQ_HANDLED;
Use the guard(mutex) macro for handle mutex lock/unlocks. Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- drivers/iio/adc/mcp3911.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) --- base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa change-id: 20231125-mcp3911-guard-866591e2c947 Best regards,