| Message ID | df331cd681feb756d06df4173f67f75ec655bfb2.1737985435.git.Jonathan.Santos@analog.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add features, improvements, and fixes | expand |
On 1/27/25 9:13 AM, Jonathan Santos wrote: > The current locking is only preventing a triggered buffer Transfer and a > debugfs register access from happening at the same time. If a register > access happens during a buffered read, the action is doomed to fail anyway, > since we need to write a magic value to exit continuous read mode. > > Remove locking from the trigger handler and use > iio_device_claim_direct_mode() instead in the register access function. > > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> > --- > v2 Changes: > * New patch in v2. It replaces the guard(mutex) patch. > --- > drivers/iio/adc/ad7768-1.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c > index 17a49bf74637..5e2093be9b92 100644 > --- a/drivers/iio/adc/ad7768-1.c > +++ b/drivers/iio/adc/ad7768-1.c > @@ -271,16 +271,16 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, > struct ad7768_state *st = iio_priv(indio_dev); > int ret; > > - mutex_lock(&st->lock); > - if (readval) { > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + if (readval) > ret = regmap_read(st->regmap, reg, readval); > - if (ret) > - goto err_unlock; > - } else { > + else > ret = regmap_write(st->regmap, reg, writeval); > - } > -err_unlock: > - mutex_unlock(&st->lock); > + > + iio_device_release_direct_mode(indio_dev); > > return ret; > } > @@ -495,18 +495,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) > struct ad7768_state *st = iio_priv(indio_dev); > int ret; > > - mutex_lock(&st->lock); > - > ret = spi_read(st->spi, &st->data.scan.chan, 3); > if (ret < 0) > - goto err_unlock; > + goto out; > > iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan, > iio_get_time_ns(indio_dev)); > > -err_unlock: > +out: > iio_trigger_notify_done(indio_dev->trig); > - mutex_unlock(&st->lock); > > return IRQ_HANDLED; > } I think the lock can be fully removed from struct ad7768_state if it isn't being used anymore (and remove mutex_init() in probe()).
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c index 17a49bf74637..5e2093be9b92 100644 --- a/drivers/iio/adc/ad7768-1.c +++ b/drivers/iio/adc/ad7768-1.c @@ -271,16 +271,16 @@ static int ad7768_reg_access(struct iio_dev *indio_dev, struct ad7768_state *st = iio_priv(indio_dev); int ret; - mutex_lock(&st->lock); - if (readval) { + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + if (readval) ret = regmap_read(st->regmap, reg, readval); - if (ret) - goto err_unlock; - } else { + else ret = regmap_write(st->regmap, reg, writeval); - } -err_unlock: - mutex_unlock(&st->lock); + + iio_device_release_direct_mode(indio_dev); return ret; } @@ -495,18 +495,15 @@ static irqreturn_t ad7768_trigger_handler(int irq, void *p) struct ad7768_state *st = iio_priv(indio_dev); int ret; - mutex_lock(&st->lock); - ret = spi_read(st->spi, &st->data.scan.chan, 3); if (ret < 0) - goto err_unlock; + goto out; iio_push_to_buffers_with_timestamp(indio_dev, &st->data.scan, iio_get_time_ns(indio_dev)); -err_unlock: +out: iio_trigger_notify_done(indio_dev->trig); - mutex_unlock(&st->lock); return IRQ_HANDLED; }
The current locking is only preventing a triggered buffer Transfer and a debugfs register access from happening at the same time. If a register access happens during a buffered read, the action is doomed to fail anyway, since we need to write a magic value to exit continuous read mode. Remove locking from the trigger handler and use iio_device_claim_direct_mode() instead in the register access function. Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com> --- v2 Changes: * New patch in v2. It replaces the guard(mutex) patch. --- drivers/iio/adc/ad7768-1.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)