| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20221023120634.0dda4b5f@jic23-huawei>
Date: Sun, 23 Oct 2022 12:06:34 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Mitja Spes <mitja@...av.com>
Cc: Lars-Peter Clausen <lars@...afoo.de>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Andy Shevchenko <andy.shevchenko@...il.com>,
Lee Jones <lee.jones@...aro.org>,
Jérôme Pouiller <jerome.pouiller@...abs.com>,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>,
Tomasz Duszynski <tduszyns@...il.com>,
linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] iio: pressure: ms5611: fixed value compensation
bug
On Fri, 21 Oct 2022 15:58:20 +0200
Mitja Spes <mitja@...av.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@...av.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 =
Powered by blists - more mailing lists