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

[Andy Shevchenko]

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@xxxxxxxxx>

(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