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

Andy Shevchenko
2026-04-30T06:31:12Z

On Wed, Apr 29, 2026 at 09:14:39PM -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. In addition, remove regmap variable in
> rm3100_trigger_handler() as its references have been replaced with
> variable data.

Almost there, a couple of minor issues and feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

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

...

> +/**
> + * rm3100_guarded_regmap_bulk_read() - Wrapper around regmap_bulk_read() with a mutex

Always validate kernel-doc

scripts/kernel-doc -v -none -Wall -- $FILE

You forgot to update function name here.

> + * @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