Re: [PATCH v4] iio: magnetometer: rm3100: Modernize locking and refactor control flow

[Maxwell Doose]

On Thu, Apr 30, 2026 at 1:31 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxx> wrote:
>
> Almost there, a couple of minor issues and feel free to add
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>

Alright sounds good, I'll make sure to add the recommended changes
below, but right now it's incredibly late over here, so I'll get back
with that patch later.

> (Side note:
>  compare this change to ACPI PMIC one, you see what I have told about
>  in that thread.)

I probably should stop wasting my time over there, I'm tripping over
my own commit details there and that driver seems to be at the point
of diminishing returns.

>
> ...
>
> > +/**
> > + * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex
>
> Always validate kernel-doc
>
>         scripts/kernel-doc -v -none -Wall -- $FILE

Thanks for informing me about that, I didn't realize there was a
script for that.

>
>
> You forgot to update function name here.

Whoops, nice catch.

Anyways, I think I need to sincerely thank you for your help on this,
it's been great.

Best Regards,
Maxwell Doose




>
> > + * @data: Data structure containing regmap and mutex
> > + * @reg: First register to be read from, passed to regmap_bulk_read()
> > + * @val: Pointer to store read value, in native register size for device,
> > + * passed to regmap_bulk_read()
> > + * @val_count: Number of registers to read, passed to regmap_bulk_read()
> > + *
> > + * Intended for use only in rm3100_trigger_handler().
> > + *
> > + * Return:
> > + * A value of zero on success, a negative errno in error cases.
> > + */
>
> ...
>
> >               for_each_set_bit(bit, &scan_mask, mask_len) {
> > -                     ret = regmap_bulk_read(regmap, RM3100_REG_MX2 + 3 * bit,
> > -                                            data->buffer, 3);
> > -                     if (ret < 0) {
> > -                             mutex_unlock(&data->lock);
> > +                     ret = rm3100_regmap_bulk_read_locked(data, RM3100_REG_MX2 + 3 * bit,
> > +                                                          data->buffer, 3);
>
>                         ret = rm3100_regmap_bulk_read_locked(data,
>                                                              RM3100_REG_MX2 + 3 * bit,
>                                                              data->buffer, 3);
>
> > +                     if (ret < 0)
> >                               goto done;
> > -                     }
> >               }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>