On Sun, Aug 7, 2022 at 1:56 PM Angel Iglesias <ang.iglesiasg@xxxxxxxxx> 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 Missed period. ... > * https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMP180-DS000-121.pdf > * https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BMP280-DS001-12.pdf > * https://ae-bst.resource.bosch.com/media/_tech/media/datasheets/BST-BME280_DS001-11.pdf Seems like the above links are unresponsive now? Perhaps you may fix them as well in a separate patch? > + * https://www.bosch-sensortec.com/media/boschsensortec/downloads/datasheets/bst-bmp388-ds001.pdf ... > +/* See datasheet Section 3.11.1. */ Again, a bit of consistency in (one-line) comments, please. > +struct bmp380_calib { > + u16 T1; > + u16 T2; > + s8 T3; > + s16 P1; > + s16 P2; > + s8 P3; > + s8 P4; > + u16 P5; > + u16 P6; > + s8 P7; > + s8 P8; > + s16 P9; > + s8 P10; > + s8 P11; > +}; __packed ? ... > +/* Send a command to BMP3XX sensors */ > + /* check if device is ready to process a command */ > + /* send command to process */ > + /* wait for 2ms for command to be processed */ > + /* check for command processing error */ Consistency, please! ... > + dev_dbg(data->dev, "Command 0x%X processed successfully\n", cmd); Useless. ... > +/* > + * Returns temperature in DegC, resolution is 0.01 DegC. Output value of DegC?! Perhaps "Celsius degrees"? > + * "5123" equals 51.23 DegC. t_fine carries fine temperature as global > + * value. > + * > + * Taken from datasheet, Section Appendix 9, "Compensation formula" and repo > + * https://github.com/BoschSensortec/BMP3-Sensor-API Missed period. > + */ ... > + return (s32) comp_temp; Do you need casting? ... > +/* > + * Returns pressure in Pa as unsigned 32 bit integer in fractional Pascal. an unsigned 32-bit > + * Output value of "9528709" represents 9528709/100 = 95287.09 Pa = 952.8709 hPa > + * > + * Taken from datasheet, Section 9.3. "Pressure compensation" and repository > + * https://github.com/BoschSensortec/BMP3-Sensor-API Missed period. > + */ ... > + var1 = ((s64)data->t_fine) * ((s64)data->t_fine); Too many parentheses? ... > + /* > + * Dividing by 10 followed by multiplying by 10 to avoid > + * possible overflow caused by (uncomp_data->pressure * partial_data4) Missed period. > + */ ... > + return (u32)comp_press; Do you need casting? ... > + /* reading was skipped */ The useless comment. > + dev_err(data->dev, "reading pressure skipped\n"); ... > + /* Compensated pressure is in cPa (centipascals) */ > + *val2 = 100000; Anything to use from units.h? ... > + ret = regmap_write_bits(data->regmap, BMP380_REG_OSR, > + BMP380_OSRS_TEMP_MASK | BMP380_OSRS_PRESS_MASK, > + osrs); > + if (ret < 0) { Do all these ' < 0' parts make any sense? > + } ... > + { .compatible = "bosch,bmp380", .data = (void *)BMP380_CHIP_ID }, > { .compatible = "bosch,bme280", .data = (void *)BME280_CHIP_ID }, > { .compatible = "bosch,bmp280", .data = (void *)BMP280_CHIP_ID }, > { .compatible = "bosch,bmp180", .data = (void *)BMP180_CHIP_ID }, See below. ... > + {"bmp380", BMP380_CHIP_ID }, > {"bmp280", BMP280_CHIP_ID }, > {"bmp180", BMP180_CHIP_ID }, > {"bmp085", BMP180_CHIP_ID }, Can we actually keep them forward-ordered? (Add 380 after 280 here and in a separate patch sort the rest, or other way around, sort as a prerequisite patch) ... > +#define BMP380_MIN_TEMP -4000 > +#define BMP380_MAX_TEMP 8500 > +#define BMP380_MIN_PRES 3000000 > +#define BMP380_MAX_PRES 12500000 Units? -- With Best Regards, Andy Shevchenko
