On Thu, 7 Sep 2023 08:57:17 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> Morning Andy,
>
> Thanks for the review.
>
> On 9/6/23 18:48, Andy Shevchenko wrote:
> > On Wed, Sep 06, 2023 at 03:37:48PM +0300, Matti Vaittinen wrote:
> >> Support for the ROHM BM1390 pressure sensor. The BM1390GLV-Z can measure
> >> pressures ranging from 300 hPa to 1300 hPa with configurable measurement
> >> averaging and internal FIFO. The sensor does also provide temperature
> >> measurements.
> >>
> >> Sensor does also contain IIR filter implemented in HW. The data-sheet
> >> says the IIR filter can be configured to be "weak", "middle" or
> >> "strong". Some RMS noise figures are provided in data sheet but no
> >> accurate maths for the filter configurations is provided. Hence, the IIR
> >> filter configuration is not supported by this driver and the filter is
> >> configured to the "middle" setting (at least not for now).
> >>
> >> The FIFO measurement mode is only measuring the pressure and not the
> >> temperature. The driver measures temperature when FIFO is flushed and
> >> simply uses the same measured temperature value to all reported
> >> temperatures. This should not be a problem when temperature is not
> >> changing very rapidly (several degrees C / second) but allows users to
> >> get the temperature measurements from sensor without any additional logic.
> >
> > ...
> >
> >
> >> +struct bm1390_data {
> >> + int64_t timestamp, old_timestamp;
> >
> > Out of a sudden int64_t instead of u64?
>
> Judging the iio_push_to_buffers_with_timestamp() and iio_get_time_ns(),
> IIO operates with signed timestamps. One being s64, the other int64_t.
That's odd. Ah well. Should both be s64 as internal to the kernel only.
>
> >> + struct iio_trigger *trig;
> >> + struct regmap *regmap;
> >> + struct device *dev;
> >> + struct bm1390_data_buf buf;
> >> + int irq;
> >> + unsigned int state;
> >> + bool trigger_enabled;
> >
> >> + u8 watermark;
> >
> > Or u8 instead of uint8_t?
>
> So, uint8_t is preferred? I don't really care all that much which of
> these to use - which may even show up as a lack of consistency... I
> think I did use uint8_t when I learned about it - but at some point
> someone somewhere asked me to use u8 instead.. This somewhere might have
> been u-boot though...
>
> So, are you Suggesting I should replace u8 with uint8_t? Can do if it
> matters.
u8 preferred for internal to kernel stuff, uint8_t if a userspace header.
