Re: [PATCH v5] iio: adc: ad7280a: replace mutex_lock() with guard(mutex)

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 08:03:43PM -0300, Matheus Giarola wrote:

First of all, do not top-post!

> Thanks for the review. I noticed that there's another student working on the
> same change, so we decided to join efforts and continue on his patch,
> dropping this one.
> 
> The other patch can be found at:
> https://lore.kernel.org/linux-iio/20260417131029.12568-1-lucas@xxxxxxxxxx/

Make sure that student sees my review that I gave against your contribution.
It will save my time when reviewing that one.

> On Wed, Apr 22, 2026 at 4:46 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> >
> > On Wed, Apr 22, 2026 at 10:44:36AM +0300, Andy Shevchenko wrote:
> > > On Tue, Apr 21, 2026 at 04:08:55PM -0300, Matheus Giarola wrote:
> >
> > ...
> >
> > > > static ssize_t ad7280_show_balance_timer(struct iio_dev *indio_dev,
> > >
> > > >     unsigned int msecs;
> > > >     int ret;
> > > >
> > > > -   mutex_lock(&st->lock);
> > > > -   ret = ad7280_read_reg(st, chan->address >> 8,
> > > > +   scoped_guard(mutex, &st->lock) {
> > > > +           ret = ad7280_read_reg(st, chan->address >> 8,
> > > >                           (chan->address & 0xFF) + AD7280A_CB1_TIMER_REG);
> > > > -   mutex_unlock(&st->lock);
> > > > +   }
> > >
> > > Not sure if we need {} here (as it's one statement body), but the indentation
> > > now got broken of the second line there.
> > >
> > > What about
> > >
> > >       u8 devaddr = chan->address >> 8;
> > >       u8 addr = chan->address;
> >
> > Just checked the second one s/addr/ch/ as per ad7280_store_balance_sw()
> > implementation.
> >
> > >       // ...or use existing names in the driver for the same things
> > >
> > >       scoped_guard(mutex, &st->lock)
> > >               ret = ad7280_read_reg(st, devaddr, addr + AD7280A_CB1_TIMER_REG);
> > >
> > > ?

-- 
With Best Regards,
Andy Shevchenko