Re: [PATCH v2 3/5] iio: magnetometer: qmc5883l: Add initial driver support

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 8 May 2025 11:03:51 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 5/8/25 7:08 AM, Brajesh Patil wrote:
> 
> This needs a description that explains why we would want to add this to the
> kernel.
> 
> > Signed-off-by: Brajesh Patil <brajeshpatil11@gmail.com>
A jumped on top of David's review (note I cropped lots of good feedback so do
look at his reply!) and added a few things.

J

> > diff --git a/drivers/iio/magnetometer/qmc5883l.c b/drivers/iio/magnetometer/qmc5883l.c
> > new file mode 100644
> > index 000000000000..68597cdd0ca8
> > --- /dev/null
> > +++ b/drivers/iio/magnetometer/qmc5883l.c
> > @@ -0,0 +1,471 @@

> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		ret = regmap_read(data->regmap, QMC5883L_CONTROL_REG_1, &rval);
> > +		if (ret < 0)
> > +			return ret;
> > +
> > +		rval = (rval & QMC5883L_ODR_MASK) >> QMC5883L_ODR_SHIFT;
> > +
> > +		if (rval >= ARRAY_SIZE(qmc5883l_odr_map) || !qmc5883l_odr_map[rval])
> > +			return -EINVAL;
> > +
> > +		*val = qmc5883l_odr_map[rval];
> > +		*val2 = 0;
> > +		return IIO_VAL_INT;
> > +	}
> > +	return -EINVAL;

Probably can't get here - which is good, but if you can't drop this line.

> > +}

> > +static const struct iio_info qmc5883l_info = {
> > +	.read_raw = &qmc5883l_read_raw,
> > +};
> > +
> > +static const unsigned long qmc5883l_scan_masks[] = {0x7, 0};

static const unsigned long qmc5883l_scan_masks[] = { 0x7, 0 };

> > +
> > +static int qmc5883l_probe(struct i2c_client *client)
> > +{

...

> > +
> > +	data = iio_priv(indio_dev);
> > +	data->client = client;
> > +	data->regmap = regmap;
> > +	mutex_init(&data->lock);
> > +
	ret = devm_mutex_init(&data->lock);
	if (ret)
		return ret;

Cleaning up mutexes only does stuff in debug modes, so traditionally we didn't
bother but now we have devm_ handling it is a nice to have thing.