Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor

[Jonathan Cameron]

> 
> > +	{
> > +		/* 50ms headroom over the slowest ODR (10Hz) */
> > +		ret = regmap_read_poll_timeout(data->regmap,
> > +					       QMC5883L_REG_STATUS1, status,
> > +					       (status & QMC5883L_STATUS_DRDY),
> > +					       2 * USEC_PER_MSEC,
> > +					       150 * USEC_PER_MSEC);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (status & QMC5883L_STATUS_OVL)
> > +			return -ERANGE;  
> 
> Sashiko has a remark:
> 
> If we return -ERANGE here when the overflow flag (OVL) is set, does the
> sensor get permanently stuck in an overflow state?
> In typical I2C magnetometers, the Data Ready (DRDY) and Overflow (OVL)
> status bits are only cleared by reading the data registers. By returning
> early without reading the data registers via regmap_bulk_read(), the DRDY
> and OVL flags might remain set indefinitely. 
> On subsequent measurement attempts, regmap_read_poll_timeout() will return
> immediately and this check will instantly fail again, potentially locking up
> the sensor until a reset.
> 

I was curious so checked the datasheet.
https://www.qstcorp.com/upload/pdf/202512/13-52-04%20QMC5883L%20Datasheet%20Rev.%20B.pdf

Sashiko looses this time as overflow resets if the next value is in range.

I'm with Sashiko for explicit access being needed for data ready (or a write 1 to clear)
but overflow is more of an intermittent thing so both styles exist for devices
(with and without need for specific action to clear).

Jonathan
> > +
> > +		ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
> > +				       sizeof(buf));
> > +		if (ret)
> > +			return ret;
> > +
> > +		*val = (s16)le16_to_cpu(buf[index]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +