| 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
| ||
|
Date: Mon, 01 Aug 2022 09:49:27 +0200
From: Angel Iglesias <ang.iglesiasg@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: linux-iio <linux-iio@...r.kernel.org>,
Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Nikita Yushchenko <nikita.yoush@...entembedded.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Ulf Hansson <ulf.hansson@...aro.org>,
Paul Cercueil <paul@...pouillou.net>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 4/5] iio: pressure: bmp280: Add support for BMP380
sensor family
On lun, 2022-07-25 at 23:15 +0200, Andy Shevchenko wrote:
> On Sat, Jul 23, 2022 at 7:40 PM Angel Iglesias <ang.iglesiasg@...il.com>
> wrote:
> >
> > Adds compatibility with the new generation of this sensor, the BMP380
> >
> > Includes basic sensor initialization to do pressure and temp
> > measurements and allows tuning oversampling settings for each channel.
> >
> > The compensation algorithms are adapted from the device datasheet and
> > the repository https://github.com/BoschSensortec/BMP3-Sensor-API
>
> ...
>
> > + /* wait for 2ms for command to be proccessed */
>
> processed
>
> > + dev_dbg(data->dev, "Command 0x%X proccessed successfully\n", cmd);
>
> Ditto. Can you run a spell checker? Kernel has a little one called codespell.
Once again, my typos strike back! I didn't know of this tool, I only checked
format issues using the script check_patch.pl
> > +
> > + return 0;
> > +}
>
> ...
>
> > +static s32 bmp380_compensate_temp(struct bmp280_data *data, u32 adc_temp)
> > +{
> > + s64 var1, var2, var3, var4, var5, var6, comp_temp;
> > + struct bmp380_calib *calib = &data->calib.bmp380;
> > +
> > + var1 = ((s64) adc_temp) - (((s64) calib->T1) << 8);
> > + var2 = var1 * ((s64) calib->T2);
> > + var3 = var1 * var1;
> > + var4 = var3 * ((s64) calib->T3);
> > + var5 = (var2 << 18) + var4;
> > + var6 = var5 >> 32;
> > + data->t_fine = (s32) var6;
> > + comp_temp = (var6 * 25) >> 14;
> > +
> > + comp_temp = clamp_val(comp_temp, BMP380_MIN_TEMP, BMP380_MAX_TEMP);
> > + return (s32) comp_temp;
> > +}
>
> ...
>
> > + s64 var1, var2, var3, var4, var5, var6, offset, sensitivity;
> > + u64 comp_press;
> > + struct bmp380_calib *calib = &data->calib.bmp380;
> > +
> > + var1 = ((s64)data->t_fine) * ((s64)data->t_fine);
> > + var2 = var1 >> 6;
> > + var3 = (var2 * ((s64) data->t_fine)) >> 8;
> > + var4 = (((s64)calib->P8) * var3) >> 5;
> > + var5 = (((s64) calib->P7) * var1) << 4;
> > + var6 = (((s64) calib->P6) * ((s64)data->t_fine)) << 22;
> > + offset = (((s64)calib->P5) << 47) + var4 + var5 + var6;
> > + var2 = (((s64)calib->P4) * var3) >> 5;
> > + var4 = (((s64) calib->P3) * var1) << 2;
> > + var5 = (((s64) calib->P2) - ((s64) 1<<14)) *
> > + (((s64)data->t_fine) << 21);
> > + sensitivity = ((((s64) calib->P1) - ((s64) 1 << 14)) << 46) +
> > + var2 + var4 + var5;
> > + var1 = (sensitivity >> 24) * ((s64)adc_press);
> > + var2 = ((s64)calib->P10) * ((s64) data->t_fine);
> > + var3 = var2 + (((s64) calib->P9) << 16);
> > + var4 = (var3 * ((s64)adc_press)) >> 13;
> > +
> > + /*
> > + * Dividing by 10 followed by multiplying by 10 to avoid
> > + * possible overflow caused by (uncomp_data->pressure *
> > partial_data4)
> > + */
> > + var5 = (((s64)adc_press) * (var4 / 10)) >> 9;
> > + var5 *= 10;
> > + var6 = ((s64)adc_press) * ((s64)adc_press);
> > + var2 = (((s64)calib->P11) * var6) >> 16;
> > + var3 = (var2 * ((s64)adc_press)) >> 7;
> > + var4 = (offset >> 2) + var1 + var5 + var3;
> > + comp_press = ((u64)var4 * 25) >> 40;
>
>
> Kbuild bot is right, you forgot to (compile-)test for a 32-bit machine.
Oh I see. Thanks for heads up, I only tested arm64 and x86_64 archs.
> ...
>
> > + ret = regmap_bulk_read(data->regmap, BMP380_REG_TEMP_XLSB, data-
> > >buf, 3);
>
> sizeof() ?
>
> ...
>
> > + /* Read and compensate temperature so we get a reading of t_fine. */
>
> for temperature
>
> ...
>
> > + ret = regmap_bulk_read(data->regmap, BMP380_REG_PRESS_XLSB, data-
> > >buf, 3);
>
> sizeof() ?
>
> ...
>
> > + .oversampling_temp_default = ilog2(1),
>
> > + .oversampling_press_default = ilog2(4),
>
> BIT()
I preserved previous committer use of ilog2() to recover the index value as it
is done on bmp280_write_oversampling_* functions, but you're right, is better to
initialize using BIT() macros.
> ...
> ´
> > +#define BMP380_REG_CMD 0x7E
> > +#define BMP380_REG_CONFIG 0x1F
> > +#define BMP380_REG_ODR 0X1D
> > +#define BMP380_REG_OSR 0X1C
> > +#define BMP380_REG_POWER_CONTROL 0X1B
> > +#define BMP380_REG_IF_CONFIG 0X1A
> > +#define BMP380_REG_INT_CONTROL 0X19
> > +#define BMP380_REG_INT_STATUS 0X11
> > +#define BMP380_REG_EVENT 0X10
> > +#define BMP380_REG_STATUS 0X03
> > +#define BMP380_REG_ERROR 0X02
> > +#define BMP380_REG_ID 0X00
> > +
> > +#define BMP380_REG_FIFO_CONFIG_1 0X18
> > +#define BMP380_REG_FIFO_CONFIG_2 0X17
> > +#define BMP380_REG_FIFO_WATERMARK_MSB 0X16
> > +#define BMP380_REG_FIFO_WATERMARK_LSB 0X15
> > +#define BMP380_REG_FIFO_DATA 0X14
> > +#define BMP380_REG_FIFO_LENGTH_MSB 0X13
> > +#define BMP380_REG_FIFO_LENGTH_LSB 0X12
> > +
> > +#define BMP380_REG_SENSOR_TIME_MSB 0X0E
> > +#define BMP380_REG_SENSOR_TIME_LSB 0X0D
> > +#define BMP380_REG_SENSOR_TIME_XLSB 0X0C
> > +
> > +#define BMP380_REG_TEMP_MSB 0X09
> > +#define BMP380_REG_TEMP_LSB 0X08
> > +#define BMP380_REG_TEMP_XLSB 0X07
> > +
> > +#define BMP380_REG_PRESS_MSB 0X06
> > +#define BMP380_REG_PRESS_LSB 0X05
> > +#define BMP380_REG_PRESS_XLSB 0X04
> > +
> > +#define BMP380_REG_CALIB_TEMP_START 0x31
>
> Be consistent x vs X (we prefer x).
>
Sorry about that
As always, thank you for your comments,
Kind regards,
Angel
Powered by blists - more mailing lists