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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250511162618.34883bcb@jic23-huawei>
Date: Sun, 11 May 2025 16:26:18 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Brajesh Patil <brajeshpatil11@...il.com>, lars@...afoo.de,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 marcelo.schmitt1@...il.com
Subject: Re: [PATCH v2 3/5] iio: magnetometer: qmc5883l: Add initial driver
 support

On Thu, 8 May 2025 11:03:51 -0500
David Lechner <dlechner@...libre.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@...il.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.