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

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

[Rupesh Majhi]

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: stable@vger.kernel.org
Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
---
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
-- 
2.43.0

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

[David Lechner]

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: stable@vger.kernel.org
> Signed-off-by: Rupesh Majhi <zoone.rupert@gmail.com>
> ---
> 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

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

[Rupesh Majhi]

On 7/27/26 4:27 PM, David Lechner wrote:
> 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.

Makes sense, I will do that. I have a cleanup series queued from Andy's
review of the ACPI fix, so the sort will go in as the first patch of that,
after the buffer work lands.

One thing worth flagging about the block you pasted: it keeps FIFO_EN at
BIT(5) and SPI_EN at BIT(6), which is what the driver has today, but the
datasheet puts FIFO_EN at bit 1 and SPI_MODE at bit 0. Datasheet V1.1
(2019-07-11), section 8.6 "Interrupt and FIFO configuration (CFG_REG)", 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. So bits 5 and 6 are INT_TMP and
INT_FIFO.

Neither define is used right now, so nothing is broken today, but the FIFO
enable is needed by the FIFO patches. I have that as a separate one-liner
ahead of them, since it changes values rather than moving lines around.

That leaves CFG_REG looking like this once both go in:

#define DPS310_CFG_REG		0x09
#define  DPS310_INT_HL		BIT(7)
#define  DPS310_TMP_SHIFT_EN	BIT(3)
#define  DPS310_PRS_SHIFT_EN	BIT(2)
#define  DPS310_FIFO_EN		BIT(1)
#define  DPS310_SPI_EN		BIT(0)

I left the three interrupt enables at bits 6 to 4 out, since nothing uses
them and the driver has no interrupt path. Say if you would rather see them
defined for completeness.

The rest of your ordering I will take as is.

Thanks for looking at this.

Rupesh

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

[Jonathan Cameron]

On Mon, 27 Jul 2026 18:09:34 +0300
Rupesh Majhi <zoone.rupert@gmail.com> wrote:

> On 7/27/26 4:27 PM, David Lechner wrote:
> > 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.  
> 
> Makes sense, I will do that. I have a cleanup series queued from Andy's
> review of the ACPI fix, so the sort will go in as the first patch of that,
> after the buffer work lands.
> 
> One thing worth flagging about the block you pasted: it keeps FIFO_EN at
> BIT(5) and SPI_EN at BIT(6), which is what the driver has today, but the
> datasheet puts FIFO_EN at bit 1 and SPI_MODE at bit 0. Datasheet V1.1
> (2019-07-11), section 8.6 "Interrupt and FIFO configuration (CFG_REG)", 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. So bits 5 and 6 are INT_TMP and
> INT_FIFO.
> 
> Neither define is used right now, so nothing is broken today, but the FIFO
> enable is needed by the FIFO patches. I have that as a separate one-liner
> ahead of them, since it changes values rather than moving lines around.
> 
> That leaves CFG_REG looking like this once both go in:
> 
> #define DPS310_CFG_REG		0x09
> #define  DPS310_INT_HL		BIT(7)
> #define  DPS310_TMP_SHIFT_EN	BIT(3)
> #define  DPS310_PRS_SHIFT_EN	BIT(2)
> #define  DPS310_FIFO_EN		BIT(1)
> #define  DPS310_SPI_EN		BIT(0)
> 
> I left the three interrupt enables at bits 6 to 4 out, since nothing uses
> them and the driver has no interrupt path. Say if you would rather see them
> defined for completeness.

If there are more bits to correct (even if not used) let use doing
them all in one patch. Given where we are in the cycle I doubt I'll be
trying to get this in before the merge window.  As such feel free to
just put the fix as first patch in your series making other changes.

Thanks,

Jonathan
> 
> The rest of your ordering I will take as is.
> 
> Thanks for looking at this.
> 
> Rupesh