diff mbox series

[v2,1/3] iio: pressure: ms5611: fixed value compensation bug

Message ID 20221021135827.1444793-2-mitja@lxnav.com (mailing list archive)
State Accepted
Headers show
Series ms5611 sensor value bug fix; dt-binding fix | expand

Commit Message

Mitja Špes Oct. 21, 2022, 1:58 p.m. UTC 
When using multiple instances of this driver the compensation PROM was
overwritten by the last initialized sensor. Now each sensor has own PROM
storage.

Signed-off-by: Mitja Spes <mitja@lxnav.com>
---
 drivers/iio/pressure/ms5611.h      | 12 +++----
 drivers/iio/pressure/ms5611_core.c | 51 ++++++++++++++++--------------
 2 files changed, 31 insertions(+), 32 deletions(-)

Comments

Jonathan Cameron Oct. 23, 2022, 11:06 a.m. UTC | #1 
On Fri, 21 Oct 2022 15:58:20 +0200
Mitja Spes <mitja@lxnav.com> wrote:

> When using multiple instances of this driver the compensation PROM was
> overwritten by the last initialized sensor. Now each sensor has own PROM
> storage.
> 
> Signed-off-by: Mitja Spes <mitja@lxnav.com>
Fixes tag?

I think this is one we will want to backport. 

> ---
>  drivers/iio/pressure/ms5611.h      | 12 +++----
>  drivers/iio/pressure/ms5611_core.c | 51 ++++++++++++++++--------------
>  2 files changed, 31 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
> index cbc9349c342a..550b75b7186f 100644
> --- a/drivers/iio/pressure/ms5611.h
> +++ b/drivers/iio/pressure/ms5611.h
> @@ -25,13 +25,6 @@ enum {
>  	MS5607,
>  };
>  
> -struct ms5611_chip_info {
> -	u16 prom[MS5611_PROM_WORDS_NB];
> -
> -	int (*temp_and_pressure_compensate)(struct ms5611_chip_info *chip_info,
> -					    s32 *temp, s32 *pressure);
> -};
> -
>  /*
>   * OverSampling Rate descriptor.
>   * Warning: cmd MUST be kept aligned on a word boundary (see
> @@ -50,12 +43,15 @@ struct ms5611_state {
>  	const struct ms5611_osr *pressure_osr;
>  	const struct ms5611_osr *temp_osr;
>  
> +	u16 prom[MS5611_PROM_WORDS_NB];
> +
>  	int (*reset)(struct ms5611_state *st);
>  	int (*read_prom_word)(struct ms5611_state *st, int index, u16 *word);
>  	int (*read_adc_temp_and_pressure)(struct ms5611_state *st,
>  					  s32 *temp, s32 *pressure);
>  
> -	struct ms5611_chip_info *chip_info;
> +	int (*compensate_temp_and_pressure)(struct ms5611_state *st, s32 *temp,
> +					  s32 *pressure);
>  	struct regulator *vdd;
>  };
>  
> diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
> index 717521de66c4..c564a1d6cafe 100644
> --- a/drivers/iio/pressure/ms5611_core.c
> +++ b/drivers/iio/pressure/ms5611_core.c
> @@ -85,7 +85,7 @@ static int ms5611_read_prom(struct iio_dev *indio_dev)
>  	struct ms5611_state *st = iio_priv(indio_dev);
>  
>  	for (i = 0; i < MS5611_PROM_WORDS_NB; i++) {
> -		ret = st->read_prom_word(st, i, &st->chip_info->prom[i]);
> +		ret = st->read_prom_word(st, i, &st->prom[i]);
>  		if (ret < 0) {
>  			dev_err(&indio_dev->dev,
>  				"failed to read prom at %d\n", i);
> @@ -93,7 +93,7 @@ static int ms5611_read_prom(struct iio_dev *indio_dev)
>  		}
>  	}
>  
> -	if (!ms5611_prom_is_valid(st->chip_info->prom, MS5611_PROM_WORDS_NB)) {
> +	if (!ms5611_prom_is_valid(st->prom, MS5611_PROM_WORDS_NB)) {
>  		dev_err(&indio_dev->dev, "PROM integrity check failed\n");
>  		return -ENODEV;
>  	}
> @@ -114,21 +114,20 @@ static int ms5611_read_temp_and_pressure(struct iio_dev *indio_dev,
>  		return ret;
>  	}
>  
> -	return st->chip_info->temp_and_pressure_compensate(st->chip_info,
> -							   temp, pressure);
> +	return st->compensate_temp_and_pressure(st, temp, pressure);
>  }
>  
> -static int ms5611_temp_and_pressure_compensate(struct ms5611_chip_info *chip_info,
> +static int ms5611_temp_and_pressure_compensate(struct ms5611_state *st,
>  					       s32 *temp, s32 *pressure)
>  {
>  	s32 t = *temp, p = *pressure;
>  	s64 off, sens, dt;
>  
> -	dt = t - (chip_info->prom[5] << 8);
> -	off = ((s64)chip_info->prom[2] << 16) + ((chip_info->prom[4] * dt) >> 7);
> -	sens = ((s64)chip_info->prom[1] << 15) + ((chip_info->prom[3] * dt) >> 8);
> +	dt = t - (st->prom[5] << 8);
> +	off = ((s64)st->prom[2] << 16) + ((st->prom[4] * dt) >> 7);
> +	sens = ((s64)st->prom[1] << 15) + ((st->prom[3] * dt) >> 8);
>  
> -	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
> +	t = 2000 + ((st->prom[6] * dt) >> 23);
>  	if (t < 2000) {
>  		s64 off2, sens2, t2;
>  
> @@ -154,17 +153,17 @@ static int ms5611_temp_and_pressure_compensate(struct ms5611_chip_info *chip_inf
>  	return 0;
>  }
>  
> -static int ms5607_temp_and_pressure_compensate(struct ms5611_chip_info *chip_info,
> +static int ms5607_temp_and_pressure_compensate(struct ms5611_state *st,
>  					       s32 *temp, s32 *pressure)
>  {
>  	s32 t = *temp, p = *pressure;
>  	s64 off, sens, dt;
>  
> -	dt = t - (chip_info->prom[5] << 8);
> -	off = ((s64)chip_info->prom[2] << 17) + ((chip_info->prom[4] * dt) >> 6);
> -	sens = ((s64)chip_info->prom[1] << 16) + ((chip_info->prom[3] * dt) >> 7);
> +	dt = t - (st->prom[5] << 8);
> +	off = ((s64)st->prom[2] << 17) + ((st->prom[4] * dt) >> 6);
> +	sens = ((s64)st->prom[1] << 16) + ((st->prom[3] * dt) >> 7);
>  
> -	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
> +	t = 2000 + ((st->prom[6] * dt) >> 23);
>  	if (t < 2000) {
>  		s64 off2, sens2, t2, tmp;
>  
> @@ -342,15 +341,6 @@ static int ms5611_write_raw(struct iio_dev *indio_dev,
>  
>  static const unsigned long ms5611_scan_masks[] = {0x3, 0};
>  
> -static struct ms5611_chip_info chip_info_tbl[] = {
> -	[MS5611] = {
> -		.temp_and_pressure_compensate = ms5611_temp_and_pressure_compensate,
> -	},
> -	[MS5607] = {
> -		.temp_and_pressure_compensate = ms5607_temp_and_pressure_compensate,
> -	}
> -};
> -
>  static const struct iio_chan_spec ms5611_channels[] = {
>  	{
>  		.type = IIO_PRESSURE,
> @@ -433,7 +423,20 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
>  	struct ms5611_state *st = iio_priv(indio_dev);
>  
>  	mutex_init(&st->lock);
> -	st->chip_info = &chip_info_tbl[type];
> +
> +	switch (type) {
> +	case MS5611:
> +		st->compensate_temp_and_pressure =
> +			ms5611_temp_and_pressure_compensate;
> +		break;
> +	case MS5607:
> +		st->compensate_temp_and_pressure =
> +			ms5607_temp_and_pressure_compensate;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>  	st->temp_osr =
>  		&ms5611_avail_temp_osr[ARRAY_SIZE(ms5611_avail_temp_osr) - 1];
>  	st->pressure_osr =
Mitja Špes Oct. 24, 2022, 6:37 a.m. UTC | #2 
On Sun, Oct 23, 2022 at 1:06 PM Jonathan Cameron <jic23@kernel.org> wrote:
> Fixes tag?

I believe this should be:
Fixes: 9690d81a02dc ("iio: pressure: ms5611: add support for MS5607
temperature and pressure sensor")

Kind regards,
Mitja
diff mbox series

Patch

diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h
index cbc9349c342a..550b75b7186f 100644
--- a/drivers/iio/pressure/ms5611.h
+++ b/drivers/iio/pressure/ms5611.h
@@ -25,13 +25,6 @@  enum {
 	MS5607,
 };
 
-struct ms5611_chip_info {
-	u16 prom[MS5611_PROM_WORDS_NB];
-
-	int (*temp_and_pressure_compensate)(struct ms5611_chip_info *chip_info,
-					    s32 *temp, s32 *pressure);
-};
-
 /*
  * OverSampling Rate descriptor.
  * Warning: cmd MUST be kept aligned on a word boundary (see
@@ -50,12 +43,15 @@  struct ms5611_state {
 	const struct ms5611_osr *pressure_osr;
 	const struct ms5611_osr *temp_osr;
 
+	u16 prom[MS5611_PROM_WORDS_NB];
+
 	int (*reset)(struct ms5611_state *st);
 	int (*read_prom_word)(struct ms5611_state *st, int index, u16 *word);
 	int (*read_adc_temp_and_pressure)(struct ms5611_state *st,
 					  s32 *temp, s32 *pressure);
 
-	struct ms5611_chip_info *chip_info;
+	int (*compensate_temp_and_pressure)(struct ms5611_state *st, s32 *temp,
+					  s32 *pressure);
 	struct regulator *vdd;
 };
 
diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c
index 717521de66c4..c564a1d6cafe 100644
--- a/drivers/iio/pressure/ms5611_core.c
+++ b/drivers/iio/pressure/ms5611_core.c
@@ -85,7 +85,7 @@  static int ms5611_read_prom(struct iio_dev *indio_dev)
 	struct ms5611_state *st = iio_priv(indio_dev);
 
 	for (i = 0; i < MS5611_PROM_WORDS_NB; i++) {
-		ret = st->read_prom_word(st, i, &st->chip_info->prom[i]);
+		ret = st->read_prom_word(st, i, &st->prom[i]);
 		if (ret < 0) {
 			dev_err(&indio_dev->dev,
 				"failed to read prom at %d\n", i);
@@ -93,7 +93,7 @@  static int ms5611_read_prom(struct iio_dev *indio_dev)
 		}
 	}
 
-	if (!ms5611_prom_is_valid(st->chip_info->prom, MS5611_PROM_WORDS_NB)) {
+	if (!ms5611_prom_is_valid(st->prom, MS5611_PROM_WORDS_NB)) {
 		dev_err(&indio_dev->dev, "PROM integrity check failed\n");
 		return -ENODEV;
 	}
@@ -114,21 +114,20 @@  static int ms5611_read_temp_and_pressure(struct iio_dev *indio_dev,
 		return ret;
 	}
 
-	return st->chip_info->temp_and_pressure_compensate(st->chip_info,
-							   temp, pressure);
+	return st->compensate_temp_and_pressure(st, temp, pressure);
 }
 
-static int ms5611_temp_and_pressure_compensate(struct ms5611_chip_info *chip_info,
+static int ms5611_temp_and_pressure_compensate(struct ms5611_state *st,
 					       s32 *temp, s32 *pressure)
 {
 	s32 t = *temp, p = *pressure;
 	s64 off, sens, dt;
 
-	dt = t - (chip_info->prom[5] << 8);
-	off = ((s64)chip_info->prom[2] << 16) + ((chip_info->prom[4] * dt) >> 7);
-	sens = ((s64)chip_info->prom[1] << 15) + ((chip_info->prom[3] * dt) >> 8);
+	dt = t - (st->prom[5] << 8);
+	off = ((s64)st->prom[2] << 16) + ((st->prom[4] * dt) >> 7);
+	sens = ((s64)st->prom[1] << 15) + ((st->prom[3] * dt) >> 8);
 
-	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
+	t = 2000 + ((st->prom[6] * dt) >> 23);
 	if (t < 2000) {
 		s64 off2, sens2, t2;
 
@@ -154,17 +153,17 @@  static int ms5611_temp_and_pressure_compensate(struct ms5611_chip_info *chip_inf
 	return 0;
 }
 
-static int ms5607_temp_and_pressure_compensate(struct ms5611_chip_info *chip_info,
+static int ms5607_temp_and_pressure_compensate(struct ms5611_state *st,
 					       s32 *temp, s32 *pressure)
 {
 	s32 t = *temp, p = *pressure;
 	s64 off, sens, dt;
 
-	dt = t - (chip_info->prom[5] << 8);
-	off = ((s64)chip_info->prom[2] << 17) + ((chip_info->prom[4] * dt) >> 6);
-	sens = ((s64)chip_info->prom[1] << 16) + ((chip_info->prom[3] * dt) >> 7);
+	dt = t - (st->prom[5] << 8);
+	off = ((s64)st->prom[2] << 17) + ((st->prom[4] * dt) >> 6);
+	sens = ((s64)st->prom[1] << 16) + ((st->prom[3] * dt) >> 7);
 
-	t = 2000 + ((chip_info->prom[6] * dt) >> 23);
+	t = 2000 + ((st->prom[6] * dt) >> 23);
 	if (t < 2000) {
 		s64 off2, sens2, t2, tmp;
 
@@ -342,15 +341,6 @@  static int ms5611_write_raw(struct iio_dev *indio_dev,
 
 static const unsigned long ms5611_scan_masks[] = {0x3, 0};
 
-static struct ms5611_chip_info chip_info_tbl[] = {
-	[MS5611] = {
-		.temp_and_pressure_compensate = ms5611_temp_and_pressure_compensate,
-	},
-	[MS5607] = {
-		.temp_and_pressure_compensate = ms5607_temp_and_pressure_compensate,
-	}
-};
-
 static const struct iio_chan_spec ms5611_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -433,7 +423,20 @@  int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 	struct ms5611_state *st = iio_priv(indio_dev);
 
 	mutex_init(&st->lock);
-	st->chip_info = &chip_info_tbl[type];
+
+	switch (type) {
+	case MS5611:
+		st->compensate_temp_and_pressure =
+			ms5611_temp_and_pressure_compensate;
+		break;
+	case MS5607:
+		st->compensate_temp_and_pressure =
+			ms5607_temp_and_pressure_compensate;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	st->temp_osr =
 		&ms5611_avail_temp_osr[ARRAY_SIZE(ms5611_avail_temp_osr) - 1];
 	st->pressure_osr =