| Message ID | 20240912233234.45519-1-vassilisamir@gmail.com |
|---|---|
| Headers | show |
| Series | pressure: bmp280: Minor cleanup and interrupt support | expand |
On Fri, Sep 13, 2024 at 01:32:31AM +0200, Vasileios Amoiridis wrote: > Add forced mode support in sensors BMP28x, BME28x, BMP3xx and BMP58x. > Sensors BMP18x and BMP085 are old and do not support this feature so > their operation is not affected at all. > > Essentially, up to now, the rest of the sensors were used in normal mode > all the time. This means that they are continuously doing measurements > even though these measurements are not used. Even though the sensor does > provide PM support, to cover all the possible use cases, the sensor needs > to go into sleep mode and wake up whenever necessary. > > The idea is that the sensor is by default in sleep mode, wakes up in > forced mode when a oneshot capture is requested, or in normal mode > when the buffer is enabled. The difference lays in the fact that in > forced mode, the sensor does only one conversion and goes back to sleep > while in normal mode, the sensor does continuous measurements with the > frequency that was set in the ODR registers. > > The bmpX_chip_config() functions which are responsible for applying > the requested configuration to the sensor, are modified accordingly > in order to set the sensor by default in sleep mode. > > DEEP STANDBY, Low Power NORMAL and CONTINUOUS modes, supported only by > the BMP58x version, are not added. ... > + if (!((reg & BMP380_STATUS_DRDY_PRESS_MASK) && > + (reg & BMP380_STATUS_DRDY_TEMP_MASK))) { I would add one more space to make the indentation follow the logic. (no need to resend until Jonathan asks for it, otherwise I believe he can amend this whilst applying) > + dev_err(data->dev, "Measurement cycle didn't complete.\n"); > + return -EBUSY; > + } ... > + /* > + * According to the BMP3 Sensor API, the sensor needs 5000us Can we use 5ms... > + * in order to go to the sleep mode. > + */ > + fsleep(5000); ...and 5 * USEC_PER_MSEC here respectively?
On Fri, Sep 13, 2024 at 01:32:30AM +0200, Vasileios Amoiridis wrote: > Depends on this: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> for patches 1 & 3. Dunno if a couple of nit-picks warrants the v7, so I leave it to Jonathan to decide.
On Fri, Sep 13, 2024 at 12:51:55PM +0300, Andy Shevchenko wrote: > On Fri, Sep 13, 2024 at 01:32:31AM +0200, Vasileios Amoiridis wrote: > > Add forced mode support in sensors BMP28x, BME28x, BMP3xx and BMP58x. > > Sensors BMP18x and BMP085 are old and do not support this feature so > > their operation is not affected at all. > > > > Essentially, up to now, the rest of the sensors were used in normal mode > > all the time. This means that they are continuously doing measurements > > even though these measurements are not used. Even though the sensor does > > provide PM support, to cover all the possible use cases, the sensor needs > > to go into sleep mode and wake up whenever necessary. > > > > The idea is that the sensor is by default in sleep mode, wakes up in > > forced mode when a oneshot capture is requested, or in normal mode > > when the buffer is enabled. The difference lays in the fact that in > > forced mode, the sensor does only one conversion and goes back to sleep > > while in normal mode, the sensor does continuous measurements with the > > frequency that was set in the ODR registers. > > > > The bmpX_chip_config() functions which are responsible for applying > > the requested configuration to the sensor, are modified accordingly > > in order to set the sensor by default in sleep mode. > > > > DEEP STANDBY, Low Power NORMAL and CONTINUOUS modes, supported only by > > the BMP58x version, are not added. > > ... > > > + if (!((reg & BMP380_STATUS_DRDY_PRESS_MASK) && > > + (reg & BMP380_STATUS_DRDY_TEMP_MASK))) { > > I would add one more space to make the indentation follow the logic. > > (no need to resend until Jonathan asks for it, otherwise I believe > he can amend this whilst applying) > > > + dev_err(data->dev, "Measurement cycle didn't complete.\n"); > > + return -EBUSY; > > + } > > ... > > > + /* > > + * According to the BMP3 Sensor API, the sensor needs 5000us > > Can we use 5ms... > > > + * in order to go to the sleep mode. > > + */ > > + fsleep(5000); > > ...and 5 * USEC_PER_MSEC here respectively? > > -- > With Best Regards, > Andy Shevchenko > > Hi Andy, Thanks for the time for the review once again. I think I can manage to find the time to do it today. Cheers, Vasilis
On Fri, Sep 13, 2024 at 12:54:59PM +0300, Andy Shevchenko wrote: > On Fri, Sep 13, 2024 at 01:32:30AM +0200, Vasileios Amoiridis wrote: > > Depends on this: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > for patches 1 & 3. > Dunno if a couple of nit-picks warrants the v7, so I leave it to Jonathan to > decide. > > -- > With Best Regards, > Andy Shevchenko > > Hi Andy! Thanks for the reviews! Jonathan, I just sent a v7 with these small changes so you can go and pick up the v7! Cheers, Vasilis
Depends on this: https://lore.kernel.org/linux-iio/20240823172017.9028-1-vassilisamir@gmail.com Changes in v6: - First 3 patches were applied already, last 4 remain. [PATCH v6 1/4]: - Remove outer parentheses and change indentation in mathematical expressions. - Use De-Morgan's Law to make !A OR !B = !(A AND B) [PATCH v6 2/4]: - Remove extra line - Use contains keyword where it's needed [PATCH v6 3/4]: - Change string from small to capital letters - Use better naming for interrupt pin config --- v5: https://lore.kernel.org/linux-iio/20240902184222.24874-1-vassilisamir@gmail.com v4: https://lore.kernel.org/linux-iio/20240828205128.92145-1-vassilisamir@gmail.com v3: https://lore.kernel.org/linux-iio/20240823181714.64545-1-vassilisamir@gmail.com v2: https://lore.kernel.org/linux-iio/20240725231039.614536-1-vassilisamir@gmail.com v1: https://lore.kernel.org/linux-iio/20240711211558.106327-1-vassilisamir@gmail.com Vasileios Amoiridis (4): iio: pressure: bmp280: Use sleep and forced mode for oneshot captures dt-bindings: iio: pressure: bmp085: Add interrupts for BMP3xx and BMP5xx devices iio: pressure: bmp280: Add data ready trigger support iio: pressure: bmp280: Move bmp085 interrupt to new configuration .../bindings/iio/pressure/bmp085.yaml | 22 +- drivers/iio/pressure/bmp280-core.c | 580 ++++++++++++++++-- drivers/iio/pressure/bmp280-i2c.c | 4 +- drivers/iio/pressure/bmp280-spi.c | 4 +- drivers/iio/pressure/bmp280.h | 43 ++ 5 files changed, 613 insertions(+), 40 deletions(-) base-commit: fec496684388685647652ab4213454fbabdab099 prerequisite-patch-id: e4f81f31f4fbb2aa872c0c74ed4511893eee0c9a