Re: [PATCH v2 2/2] iio: magnetometer: add support for QST QMC6308

[Siratul Islam <siratul.islam@linux.dev>]

On Tue, 2026-07-21 at 15:48 +0200, Jorijn van der Graaf wrote:
> The QST QMC6308 is a 3-axis anisotropic magnetoresistive (AMR)
> magnetometer on I2C, found e.g. in the Fairphone 6. Its register map
> is not compatible with the QMC5883L's, so add a separate driver rather
> than extending the QMC5883L driver.
> 
...
> - Switch the license to GPL-2.0-only: the dual license had been
>   carried over from qmc5883l.c, which this driver is modeled on,
>   without a reason of our own (Jonathan).
I wrote the QMC5883L driver. My reason to use dual-license was I wanted 
people to be able to port the driver to Non-GPL OSes like Zephyr, NuttX, BSDs (specially RTOSes, 
none of them are GPL, and these sensors are widely used with MCUs) with least friction.
There are stuff that can't be ported (like regmap etc.) but the rest of it could.
Also, I read the "Linux Device Drivers 3" book where the samples use Dual BSD/GPL.
> 
...
> +	/*
> +	 * Reading the status register clears DRDY, which is why the poll
> +	 * and the data read stay under one mutex hold. A runtime resume
> +	 * clears DRDY too, so a sample converted before the last suspend
> +	 * is never returned here.
> +	 *
> +	 * The timeout is 50ms of headroom over the slowest ODR (10Hz).
> +	 */
> +	ret = regmap_read_poll_timeout(data->regmap, QMC6308_REG_STATUS,
> +				       status,
> +				       (status & QMC6308_STATUS_DRDY),
You can split by logic. i.e., put "status, (status & QMC6308_STATUS_DRDY)" on the same line.

> +				       2 * USEC_PER_MSEC,
> +				       150 * USEC_PER_MSEC);
> +	if (ret)
> +		return ret;
> +
> 
...
> +
> +static int qmc6308_write_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     int val, int val2, long mask)
> +{
> 
...
> +		if (ret)
> +			dev_warn(regmap_get_device(data->regmap),
> +				 "Failed to discard stale sample (%d)\n",
> +				 ret);
Move "ret" to the line above.
> +
> +		return 0;
> +	}
> +
...
> +static int qmc6308_probe(struct i2c_client *client)
> +{
...
> +
> +	ret = devm_add_action_or_reset(dev, qmc6308_power_down_action, data);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_autosuspend_delay(dev, QMC6308_AUTOSUSPEND_DELAY_MS);
Add a space here.
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int qmc6308_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc6308_data *data = iio_priv(indio_dev);
> +	unsigned int status;
> +	int ret;
> +
> +	ret = qmc6308_set_mode(data, QMC6308_MODE_NORMAL);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * DRDY may still be set for a sample converted before the last
> +	 * suspend; reading the status register clears it so the next
> +	 * measurement waits for fresh data.
> +	 */
> +	ret = regmap_read(data->regmap, QMC6308_REG_STATUS, &status);
> +	if (ret) {
> +		/* Best effort to leave the chip in a consistent state */
> +		qmc6308_set_mode(data, QMC6308_MODE_SUSPEND);
> +	}
It's single line so the brackets { } are not needed.
> +
> +	return ret;
> +}
> +
...
> +
> +MODULE_DESCRIPTION("QST QMC6308 3-Axis Magnetic Sensor driver");
> +MODULE_AUTHOR("Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>");
> +MODULE_LICENSE("GPL");

--
Best regards,
Sirat