On Sun, Jul 28, 2024 at 04:57:24PM +0100, Jonathan Cameron wrote:
> On Fri, 26 Jul 2024 01:10:36 +0200
> Vasileios Amoiridis <vassilisamir@xxxxxxxxx> wrote:
>
> > This commit adds 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.
> >
> > This commit, adds sleep and forced mode support. Essentially, the sensor
> > sleeps all the time except for when a measurement is requested. When there
> > is a request for a measurement, the sensor is put into forced mode, starts
> > the measurement and after it is done we read the output and we put it again
> > in sleep mode.
> >
> > For really fast and more deterministic measurements, the triggered buffer
> > interface can be used, since the sensor is still used in normal mode for
> > that use case.
> >
> > This commit does not add though support for DEEP STANDBY, Low Power NORMAL
> > and CONTINUOUS modes, supported only by the BMP58x version.
> >
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@xxxxxxxxx>
> One question inline about the corner case of buffered capture in progress
> when the machine is suspended. We'd like the device to carry on feeding
> us data on resume. Does that happen?
>
> Jonathan
>
Hi Jonathan,
This is actually a corner case that I couldn't think of. I will have to think
it a bit more and come back on that.
>
> > .trigger_handler = bmp380_trigger_handler,
> > @@ -2085,6 +2239,64 @@ static int bmp580_preinit(struct bmp280_data *data)
> > return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
> > }
> >
> > +static const u8 bmp580_operation_mode[] = { BMP580_MODE_SLEEP,
> > + BMP580_MODE_FORCED,
> > + BMP580_MODE_NORMAL };
> > +
>
>
> > +
> > +static int bmp580_wait_conv(struct bmp280_data *data)
> > +{
> > + /*
> > + * Taken from datasheet, Section 2 "Specification, Table 3 "Electrical
> > + * characteristics
> > + */
> > + const int time_conv_press[] = { 0, 1050, 1785, 3045, 5670, 10920, 21420,
> > + 42420, 84420};
> > + const int time_conv_temp[] = { 0, 1050, 1105, 1575, 2205, 3465, 6090,
> > + 11340, 21840};
> space before }
>
> Also stick a static in front of them or Colin will ;)
> Aim being to makes sure they aren't pointlessly allocated on the stack
> if the compiler doesn't do something clever with them.
>
Ack.
> > + int meas_time;
> > +
> > + meas_time = 4000 + time_conv_temp[data->oversampling_temp] +
> > + time_conv_press[data->oversampling_press];
> > +
> > + usleep_range(meas_time, meas_time * 12 / 10);
> > +
> > + return 0;
> > +}
> >
> >
> > +/* Keep compatibility with future generations of the sensor */
> > +static int bmp180_set_mode(struct bmp280_data *data, enum bmp280_op_mode mode)
> > +{
> > + return 0;
> > +}
> > +
> > +/* Keep compatibility with future generations of the sensor */
> > +static int bmp180_wait_conv(struct bmp280_data *data)
> > +{
> > + return 0;
> > +}
> > +
> > +/* Keep compatibility with future generations of the sensor */
>
> What does this comment mean? I'm in favour of course, but don't understand
> why it is here and above the stub calls.
>
>
This is for the bm(p/e)(2/3/5)80 devices which actually use those functions.
Maybe instead of "future" I should have put "newer".
> > @@ -2825,6 +3048,9 @@ static int bmp280_runtime_suspend(struct device *dev)
> > struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > struct bmp280_data *data = iio_priv(indio_dev);
> >
> > + data->chip_info->set_mode(data, BMP280_SLEEP);
>
> What happens if the device is in buffered mode and you suspend?
> I'd expect to see the power mode stashed somewhere and restored in resume.
>
As said before, I will investigate it and come back with more info.
Cheers,
Vasilis
> > +
> > + usleep_range(2500, 3000);
> > return regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> > }
> >
