Re: [PATCH v3] iio: magnetometer: rm3100: Modernize locking and refactor control flow
From: Andy Shevchenko
Date: Wed Apr 29 2026 - 05:14:45 EST
On Tue, Apr 28, 2026 at 05:26:00PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and mutex_unlock() calls in rm3100-core.c with
> the more modern guard(mutex)() family. This will help modernize the
> driver and bring it up-to-date with modern available macros/functions.
>
> While replacing mutex_lock() and mutex_unlock(), the critical sections
> of rm3100_read_mag() and rm3100_get_samp_freq() have been extended to
> include negligible operations for cleaner logic.
>
> Add new helper-wrapper function rm3100_guarded_regmap_bulk_read() to
> help keep rm3100_trigger_handler() switch-cases clean while maintaining
> mutex locking and avoiding re-entrancy risks from potential callbacks.
>
> While at it, remove redundant gotos where applicable, and use direct
> returns instead.
...
> +/**
> + * 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".
Needs to be
* Return:
> + * A value of zero will be returned on success, a negative errno will
> + * be returned in error cases.
* A value of zero on success, a negative errno in error cases.
> + */
> +static int rm3100_guarded_regmap_bulk_read(struct mutex *lock, struct regmap *map,
No, supply *data and not a *lock.
> + 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