Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, Jun 19, 2026 at 02:36:00AM +0530, Nikhil Gautam wrote:
> On 19-06-2026 12:56 am, Andy Shevchenko wrote:
> > On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> 
> Thank you very much for taking the time to do such a thorough review of this
> patch series.
> 
> Your detailed comments and suggestions are very helpful.
> I'll address the issues you've pointed out, update the cover letter to
> better explain the design decisions,
> and incorporate the requested coding style and API changes in the next
> revision.
> 
> I appreciate your review and feedback.

You're welcome!

...

> > > +config MLX90393
> > > +	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
> > > +	depends on I2C
> > Why not a regmap?
> The MLX90393 uses both register-based and command-based transactions.
> Since regmap does not naturally model the command-based interface,
> using it would require workarounds such as virtual registers or bypass
> paths.
> A custom transport abstraction is therefore simpler and better suited for
> this device.
> 
> I already discussed this on thread, Link :
> https://lore.kernel.org/linux-iio/20260423121834.4244-1-nikhilgtr@gmail.com/

Right, but please, put a summary in the commit message as it's  important
detail of implementation choice.

...

> > > +	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
> > > +		if (mlx90393_osr_avail[i] == val) {
> > > +			*osr = i;
> > > +			return 0;
> > > +		}
> > Missing {}.
> Agreed, removed intentionally as single statement, will add as per
> guidelines all the places
> where needed

It's also better to have them when it might be ambiguous. And also when even
a single statement takes a few lines. TL;DR: Also apply a common sense.

-- 
With Best Regards,
Andy Shevchenko