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

Maxwell Doose
2026-04-29T14:27:05Z

Hi Andy,

On Wed, Apr 29, 2026 at 4:12 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> ...
>
> > +/**
> > + * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex
> > + *
> > + * @lock: Mutex to lock while performing operations
> > + * @map: Register map to read from, passed to regmap_bulk_read()
> > + * @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().
> > + *
>
> I don't mind having kernel-doc for a static function, but if you do that,
> validate. Here is "Return section is missing".
>

I'll make sure to add that in the v4, I probably should've been
explicit in that description.
>
> > + */
> > +static int rm3100_guarded_regmap_bulk_read(struct mutex *lock, struct regmap *map,
>
> No, supply *data and not a *lock.
>

Understood, I'll also change the function signature for that in the v4.

best regards,
maxwell

> > + unsigned int reg, void *val, size_t val_count)
>
> static int rm3100_guarded_regmap_bulk_read(struct rm3100_data *data, unsigned int reg,
> void *val, size_t val_count)
> > +{
> > + guard(mutex)(lock);
> > + return regmap_bulk_read(map, reg, val, val_count);
>
> guard(mutex)(&data->lock);
> return regmap_bulk_read(data->regmap, reg, val, val_count);
>
> > +}
>
> --
> With Best Regards,
> Andy Shevchenko
>
>