* [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix
@ 2022-10-21 13:58 Mitja Spes
2022-10-21 13:58 ` [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug Mitja Spes
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Mitja Spes @ 2022-10-21 13:58 UTC (permalink / raw)
Cc: Mitja Spes, Jonathan Cameron, Lars-Peter Clausen, Rob Herring,
Krzysztof Kozlowski, Andy Shevchenko, Marc Kleine-Budde,
Lee Jones, Uwe Kleine-König, Tomasz Duszynski, linux-iio,
devicetree, linux-kernel
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
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug 2022-10-21 13:58 [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Mitja Spes @ 2022-10-21 13:58 ` Mitja Spes 2022-10-23 11:06 ` Jonathan Cameron 2022-10-21 13:58 ` [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited Mitja Spes ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Mitja Spes @ 2022-10-21 13:58 UTC (permalink / raw) Cc: Mitja Spes, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Jérôme Pouiller, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug 2022-10-21 13:58 ` [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug Mitja Spes @ 2022-10-23 11:06 ` Jonathan Cameron 2022-10-24 6:37 ` Mitja Špes 0 siblings, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2022-10-23 11:06 UTC (permalink / raw) To: Mitja Spes Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Jérôme Pouiller, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 = ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug 2022-10-23 11:06 ` Jonathan Cameron @ 2022-10-24 6:37 ` Mitja Špes 0 siblings, 0 replies; 14+ messages in thread From: Mitja Špes @ 2022-10-24 6:37 UTC (permalink / raw) To: Jonathan Cameron Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Jérôme Pouiller, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-21 13:58 [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Mitja Spes 2022-10-21 13:58 ` [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug Mitja Spes @ 2022-10-21 13:58 ` Mitja Spes 2022-10-21 15:32 ` Marcus Folkesson 2022-10-21 13:58 ` [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example Mitja Spes 2022-10-29 11:40 ` [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Jonathan Cameron 3 siblings, 1 reply; 14+ messages in thread From: Mitja Spes @ 2022-10-21 13:58 UTC (permalink / raw) Cc: Mitja Spes, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Marcus Folkesson, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-21 13:58 ` [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited Mitja Spes @ 2022-10-21 15:32 ` Marcus Folkesson 2022-10-23 11:10 ` Jonathan Cameron 2022-10-23 11:12 ` Jonathan Cameron 0 siblings, 2 replies; 14+ messages in thread From: Marcus Folkesson @ 2022-10-21 15:32 UTC (permalink / raw) To: Mitja Spes Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-21 15:32 ` Marcus Folkesson @ 2022-10-23 11:10 ` Jonathan Cameron 2022-10-24 6:46 ` Mitja Špes 2022-10-23 11:12 ` Jonathan Cameron 1 sibling, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2022-10-23 11:10 UTC (permalink / raw) To: Marcus Folkesson Cc: Mitja Spes, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-23 11:10 ` Jonathan Cameron @ 2022-10-24 6:46 ` Mitja Špes 0 siblings, 0 replies; 14+ messages in thread From: Mitja Špes @ 2022-10-24 6:46 UTC (permalink / raw) To: Jonathan Cameron Cc: Marcus Folkesson, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-21 15:32 ` Marcus Folkesson 2022-10-23 11:10 ` Jonathan Cameron @ 2022-10-23 11:12 ` Jonathan Cameron 2022-10-24 9:05 ` Marcus Folkesson 1 sibling, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2022-10-23 11:12 UTC (permalink / raw) To: Marcus Folkesson Cc: Mitja Spes, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited 2022-10-23 11:12 ` Jonathan Cameron @ 2022-10-24 9:05 ` Marcus Folkesson 0 siblings, 0 replies; 14+ messages in thread From: Marcus Folkesson @ 2022-10-24 9:05 UTC (permalink / raw) To: Jonathan Cameron Cc: Mitja Spes, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Lee Jones, Mark Brown, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example 2022-10-21 13:58 [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Mitja Spes 2022-10-21 13:58 ` [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug Mitja Spes 2022-10-21 13:58 ` [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited Mitja Spes @ 2022-10-21 13:58 ` Mitja Spes 2022-10-21 14:08 ` Krzysztof Kozlowski 2022-10-29 11:40 ` [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Jonathan Cameron 3 siblings, 1 reply; 14+ messages in thread From: Mitja Spes @ 2022-10-21 13:58 UTC (permalink / raw) Cc: Mitja Spes, Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Geert Uytterhoeven, Lee Jones, Alexandre Belloni, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example 2022-10-21 13:58 ` [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example Mitja Spes @ 2022-10-21 14:08 ` Krzysztof Kozlowski 2022-10-23 11:14 ` Jonathan Cameron 0 siblings, 1 reply; 14+ messages in thread From: Krzysztof Kozlowski @ 2022-10-21 14:08 UTC (permalink / raw) To: Mitja Spes Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Geert Uytterhoeven, Lee Jones, Alexandre Belloni, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example 2022-10-21 14:08 ` Krzysztof Kozlowski @ 2022-10-23 11:14 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2022-10-23 11:14 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mitja Spes, Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Geert Uytterhoeven, Lee Jones, Alexandre Belloni, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix 2022-10-21 13:58 [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Mitja Spes ` (2 preceding siblings ...) 2022-10-21 13:58 ` [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example Mitja Spes @ 2022-10-29 11:40 ` Jonathan Cameron 3 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2022-10-29 11:40 UTC (permalink / raw) To: Mitja Spes Cc: Lars-Peter Clausen, Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Marc Kleine-Budde, Lee Jones, Uwe Kleine-König, Tomasz Duszynski, linux-iio, devicetree, linux-kernel 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(-) > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-10-29 11:28 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-21 13:58 [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Mitja Spes 2022-10-21 13:58 ` [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation bug Mitja Spes 2022-10-23 11:06 ` Jonathan Cameron 2022-10-24 6:37 ` Mitja Špes 2022-10-21 13:58 ` [PATCH v2 2/3] iio: pressure: ms5611: changed hardcoded SPI speed to value limited Mitja Spes 2022-10-21 15:32 ` Marcus Folkesson 2022-10-23 11:10 ` Jonathan Cameron 2022-10-24 6:46 ` Mitja Špes 2022-10-23 11:12 ` Jonathan Cameron 2022-10-24 9:05 ` Marcus Folkesson 2022-10-21 13:58 ` [PATCH v2 3/3] dt-bindings: iio: pressure: meas,ms5611: add max SPI frequency to the example Mitja Spes 2022-10-21 14:08 ` Krzysztof Kozlowski 2022-10-23 11:14 ` Jonathan Cameron 2022-10-29 11:40 ` [PATCH v2 0/3] ms5611 sensor value bug fix; dt-binding fix Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.