linux-kernel - Re: [PATCH 2/3] iio: pressure: Support ROHM BU1390

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230908193329.0ddd39e2@jic23-huawei>
Date:   Fri, 8 Sep 2023 19:33:29 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Angel Iglesias <ang.iglesiasg@...il.com>,
        Andreas Klinger <ak@...klinger.de>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] iio: pressure: Support ROHM BU1390

On Thu, 7 Sep 2023 08:57:17 +0300
Matti Vaittinen <mazziesaccount@...il.com> 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.