[PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix

[PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix

[Mitja Spes]

Subject: [PATCH 0/3] ms5611 sensor value bug fix; dt-binding fix

The first patch fixes a bug in ms5611 iio driver where PROM value
compensation table was overwritten by the last initialized sensor.
This in turn produced wrong values when multiple sensors were used.

Second patch removes the hardcoded SPI frequency and uses the setting
from dt-bindings.

Third patch outlines the change from the second patch in the bindings
example.

v2:

[PATCH 1-2]
* no change

[PATCH 3]
* corrected patch subject

Mitja Spes (3):
  iio: pressure: ms5611: fixed value compensation bug
  iio: pressure: ms5611: changed hardcoded SPI speed to value limited
  dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the
    example

 .../bindings/iio/pressure/meas,ms5611.yaml    |  1 +
 drivers/iio/pressure/ms5611.h                 | 12 ++---
 drivers/iio/pressure/ms5611_core.c            | 51 ++++++++++---------
 drivers/iio/pressure/ms5611_spi.c             |  2 +-
 4 files changed, 33 insertions(+), 33 deletions(-)

-- 
2.34.1

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

[Mitja Spes]

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(-)

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 =
-- 
2.34.1

[PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Mitja Spes]

Don't hardcode the ms5611 SPI speed, limit it instead.

Signed-off-by: Mitja Spes <mitja@lxnav.com>
---
 drivers/iio/pressure/ms5611_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
index 432e912096f4..a0a7205c9c3a 100644
--- a/drivers/iio/pressure/ms5611_spi.c
+++ b/drivers/iio/pressure/ms5611_spi.c
@@ -91,7 +91,7 @@ static int ms5611_spi_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, indio_dev);
 
 	spi->mode = SPI_MODE_0;
-	spi->max_speed_hz = 20000000;
+	spi->max_speed_hz = min(spi->max_speed_hz, 20000000U);
 	spi->bits_per_word = 8;
 	ret = spi_setup(spi);
 	if (ret < 0)
-- 
2.34.1

[PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example

[Mitja Spes]

Added max SPI frequency setting to the example. It is now honored by the
driver.

Signed-off-by: Mitja Spes <mitja@lxnav.com>
---
 Documentation/devicetree/bindings/iio/pressure/meas,ms5611.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/pressure/meas,ms5611.yaml b/Documentation/devicetree/bindings/iio/pressure/meas,ms5611.yaml
index 4f06707450bf..08bd06e6dabe 100644
--- a/Documentation/devicetree/bindings/iio/pressure/meas,ms5611.yaml
+++ b/Documentation/devicetree/bindings/iio/pressure/meas,ms5611.yaml
@@ -52,6 +52,7 @@ examples:
             compatible = "meas,ms5611";
             reg = <0>;
             vdd-supply = <&ldo_3v3_gnss>;
+            spi-max-frequency = <20000000>;
         };
     };
 ...
-- 
2.34.1

Re: [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example

[Krzysztof Kozlowski]

On 21/10/2022 09:58, Mitja Spes wrote:
> Added max SPI frequency setting to the example. It is now honored by the
> driver.
> 

This could be there regardless of driver support (because it does not
matter)... anyway:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

Hi Mitja,

On Fri, Oct 21, 2022 at 03:58:21PM +0200, Mitja Spes wrote:
> Don't hardcode the ms5611 SPI speed, limit it instead.
> 
> Signed-off-by: Mitja Spes <mitja@lxnav.com>
> ---
>  drivers/iio/pressure/ms5611_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> index 432e912096f4..a0a7205c9c3a 100644
> --- a/drivers/iio/pressure/ms5611_spi.c
> +++ b/drivers/iio/pressure/ms5611_spi.c
> @@ -91,7 +91,7 @@ static int ms5611_spi_probe(struct spi_device *spi)
>  	spi_set_drvdata(spi, indio_dev);
>  
>  	spi->mode = SPI_MODE_0;
> -	spi->max_speed_hz = 20000000;
> +	spi->max_speed_hz = min(spi->max_speed_hz, 20000000U);

max_speed_hz is a limit, and the max frequency the ms5611 support is
20MHz.

Best regards,
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Jonathan Cameron]

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 =

Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Jonathan Cameron]

On Fri, 21 Oct 2022 17:32:21 +0200
Marcus Folkesson <marcus.folkesson@gmail.com> wrote:

> Hi Mitja,
> 
> On Fri, Oct 21, 2022 at 03:58:21PM +0200, Mitja Spes wrote:
> > Don't hardcode the ms5611 SPI speed, limit it instead.
> > 
> > Signed-off-by: Mitja Spes <mitja@lxnav.com>
Please give a fixes tag for this one as well.
The driver should never have been doing this.

Jonathan

> > ---
> >  drivers/iio/pressure/ms5611_spi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> > index 432e912096f4..a0a7205c9c3a 100644
> > --- a/drivers/iio/pressure/ms5611_spi.c
> > +++ b/drivers/iio/pressure/ms5611_spi.c
> > @@ -91,7 +91,7 @@ static int ms5611_spi_probe(struct spi_device *spi)
> >  	spi_set_drvdata(spi, indio_dev);
> >  
> >  	spi->mode = SPI_MODE_0;
> > -	spi->max_speed_hz = 20000000;
> > +	spi->max_speed_hz = min(spi->max_speed_hz, 20000000U);  
> 
> max_speed_hz is a limit, and the max frequency the ms5611 support is
> 20MHz.
> 
> Best regards,
> Marcus Folkesson

Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Jonathan Cameron]

On Fri, 21 Oct 2022 17:32:21 +0200
Marcus Folkesson <marcus.folkesson@gmail.com> wrote:

> Hi Mitja,
> 
> On Fri, Oct 21, 2022 at 03:58:21PM +0200, Mitja Spes wrote:
> > Don't hardcode the ms5611 SPI speed, limit it instead.
> > 
> > Signed-off-by: Mitja Spes <mitja@lxnav.com>
> > ---
> >  drivers/iio/pressure/ms5611_spi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> > index 432e912096f4..a0a7205c9c3a 100644
> > --- a/drivers/iio/pressure/ms5611_spi.c
> > +++ b/drivers/iio/pressure/ms5611_spi.c
> > @@ -91,7 +91,7 @@ static int ms5611_spi_probe(struct spi_device *spi)
> >  	spi_set_drvdata(spi, indio_dev);
> >  
> >  	spi->mode = SPI_MODE_0;
> > -	spi->max_speed_hz = 20000000;
> > +	spi->max_speed_hz = min(spi->max_speed_hz, 20000000U);  
> 
> max_speed_hz is a limit, and the max frequency the ms5611 support is
> 20MHz.
Hi Marcus,

I'm not following what you are commenting on.. Perhaps give more
detail?

Thanks,

Jonathan

> 
> Best regards,
> Marcus Folkesson

Re: [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example

[Jonathan Cameron]

On Fri, 21 Oct 2022 10:08:42 -0400
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 21/10/2022 09:58, Mitja Spes wrote:
> > Added max SPI frequency setting to the example. It is now honored by the
> > driver.
> >   
> 
> This could be there regardless of driver support (because it does not
> matter)... anyway:
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Having the max frequency specified is certainly a valid addition, so I've
applied this patch to the togreg branch (targetting next merge window).
The other two are fixes so will go in quicker.

Jonathan

> 
> Best regards,
> Krzysztof
> 

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

[Mitja Špes]

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

Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Mitja Špes]

On Sun, Oct 23, 2022 at 1:09 PM Jonathan Cameron <jic23@kernel.org> wrote:
> Please give a fixes tag for this one as well.
> The driver should never have been doing this.

Fixes: c0644160a8b5 ("iio: pressure: add support for MS5611 pressure
and temperature sensor")

Kind regards,
Mitja

Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

Hi Jonathan,

On Sun, Oct 23, 2022 at 12:12:22PM +0100, Jonathan Cameron wrote:
> On Fri, 21 Oct 2022 17:32:21 +0200
> Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> 
> > Hi Mitja,
> > 
> > On Fri, Oct 21, 2022 at 03:58:21PM +0200, Mitja Spes wrote:
> > > Don't hardcode the ms5611 SPI speed, limit it instead.
> > > 
> > > Signed-off-by: Mitja Spes <mitja@lxnav.com>
> > > ---
> > >  drivers/iio/pressure/ms5611_spi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c
> > > index 432e912096f4..a0a7205c9c3a 100644
> > > --- a/drivers/iio/pressure/ms5611_spi.c
> > > +++ b/drivers/iio/pressure/ms5611_spi.c
> > > @@ -91,7 +91,7 @@ static int ms5611_spi_probe(struct spi_device *spi)
> > >  	spi_set_drvdata(spi, indio_dev);
> > >  
> > >  	spi->mode = SPI_MODE_0;
> > > -	spi->max_speed_hz = 20000000;
> > > +	spi->max_speed_hz = min(spi->max_speed_hz, 20000000U);  
> > 
> > max_speed_hz is a limit, and the max frequency the ms5611 support is
> > 20MHz.
> Hi Marcus,
> 
> I'm not following what you are commenting on.. Perhaps give more
> detail?

Sorry for being unclear.
However, I must have thought wrong, this is good as it takes the speedlimit set
in e.g. devicetree into account.


> 
> Thanks,
> 
> Jonathan
> 
> > 
> > Best regards,
> > Marcus Folkesson
> 

Best regards,
Marcus Folkesson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix

[Jonathan Cameron]

On Fri, 21 Oct 2022 15:58:19 +0200
Mitja Spes <mitja@lxnav.com> wrote:

> Subject: [PATCH 0/3] ms5611 sensor value bug fix; dt-binding fix
> 
> The first patch fixes a bug in ms5611 iio driver where PROM value
> compensation table was overwritten by the last initialized sensor.
> This in turn produced wrong values when multiple sensors were used.
> 
> Second patch removes the hardcoded SPI frequency and uses the setting
> from dt-bindings.
> 
> Third patch outlines the change from the second patch in the bindings
> example.

1+2 applied to the fixes-togreg branch of iio.git and marked for
stable.

Thanks,

Jonathan

> 
> v2:
> 
> [PATCH 1-2]
> * no change
> 
> [PATCH 3]
> * corrected patch subject
> 
> Mitja Spes (3):
>   iio: pressure: ms5611: fixed value compensation bug
>   iio: pressure: ms5611: changed hardcoded SPI speed to value limited
>   dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the
>     example
> 
>  .../bindings/iio/pressure/meas,ms5611.yaml    |  1 +
>  drivers/iio/pressure/ms5611.h                 | 12 ++---
>  drivers/iio/pressure/ms5611_core.c            | 51 ++++++++++---------
>  drivers/iio/pressure/ms5611_spi.c             |  2 +-
>  4 files changed, 33 insertions(+), 33 deletions(-)
>