Re: [PATCH] iio: pressure: dps310: fix pressure result shift bit definition

On 7/27/26 2:05 AM, Rupesh Majhi wrote:
> DPS310_PRS_SHIFT_EN is defined as BIT(4), but P_SHIFT is bit 2 of
> CFG_REG. Bit 4 is INT_PRS, which enables the pressure measurement ready
> interrupt on the SDO pin.
> 
> The datasheet requires the pressure result bit-shift to be enabled when
> the oversampling rate is higher than 8 times, so
> dps310_set_pres_precision() sets it for oversampling ratios of 16 and
> above. With the wrong definition it leaves P_SHIFT clear and toggles the
> pressure ready interrupt instead, so the result register is never shifted
> and the pressure values read at those oversampling ratios are wrong.
> 
> Define the bit at its documented position. Temperature is not affected,
> T_SHIFT is bit 3 and DPS310_TMP_SHIFT_EN already matches it.
> 
> Fixes: d711a3c7dc82 ("iio: dps310: Add pressure sensing capability")
> Cc: [email protected]
> Signed-off-by: Rupesh Majhi <[email protected]>
> ---
> Found by inspection while working on FIFO support, and checked against the
> DPS310 datasheet V1.1 (2019-07-11), section 8.6 "Interrupt and FIFO
> configuration (CFG_REG)", where the bit table reads INT_HL, INT_FIFO,
> INT_TMP, INT_PRS, T_SHIFT, P_SHIFT, FIFO_EN, SPI_MODE for bits 7 down to 0.
> 
> Not tested on hardware yet: the driver probes and reads correctly under
> qemu-system-arm -M rainier-bmc, but QEMU's DPS310 model does not implement
> the shift bits, so it cannot show the difference. I have a DPS310 breakout
> on order and can confirm the raw pressure values at oversampling >= 16 once
> it arrives, if you would rather wait for that.
> 
>  drivers/iio/pressure/dps310.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 45bdb8c7670f..473973dd0694 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -50,7 +50,7 @@
>  #define DPS310_CFG_REG		0x09
>  #define  DPS310_INT_HL		BIT(7)
>  #define  DPS310_TMP_SHIFT_EN	BIT(3)
> -#define  DPS310_PRS_SHIFT_EN	BIT(4)
> +#define  DPS310_PRS_SHIFT_EN	BIT(2)
>  #define  DPS310_FIFO_EN		BIT(5)
>  #define  DPS310_SPI_EN		BIT(6)
>  #define DPS310_RESET		0x0c

In your followup work, it would be nice to start with a patch to
sort these in a logical order.

Right now, there is a mix of GENMASK being sorted from high to low
while bits are low to high (with DPS310_INT_HL being out of order).
Normally, we go from low to high on everything because that is how
datasheets usually list things.

Ideally, would end up something like:

#define DPS310_PRS_B0		0x00
#define DPS310_PRS_B1		0x01
#define DPS310_PRS_B2		0x02
#define DPS310_TMP_B0		0x03
#define DPS310_TMP_B1		0x04
#define DPS310_TMP_B2		0x05
#define DPS310_PRS_CFG		0x06
#define  DPS310_PRS_RATE_BITS	GENMASK(6, 4)
#define  DPS310_PRS_PRC_BITS	GENMASK(3, 0)
#define DPS310_TMP_CFG		0x07
#define  DPS310_TMP_EXT		BIT(7)
#define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
#define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
#define DPS310_MEAS_CFG		0x08
#define  DPS310_COEF_RDY	BIT(7)
#define  DPS310_SENSOR_RDY	BIT(6)
#define  DPS310_TMP_RDY		BIT(5)
#define  DPS310_PRS_RDY		BIT(4)
#define  DPS310_MEAS_CTRL_BITS	GENMASK(2, 0)
#define   DPS310_BACKGROUND	 BIT(2)
#define   DPS310_TEMP_EN	 BIT(1)
#define   DPS310_PRS_EN		 BIT(0)
#define DPS310_CFG_REG		0x09
#define  DPS310_INT_HL		BIT(7)
#define  DPS310_SPI_EN		BIT(6)
#define  DPS310_FIFO_EN		BIT(5)
#define  DPS310_TMP_SHIFT_EN	BIT(3)
#define  DPS310_PRS_SHIFT_EN	BIT(2)
#define DPS310_RESET		0x0c
#define  DPS310_RESET_MAGIC	0x09
#define DPS310_COEF_BASE	0x10