From: Vasileios Amoiridis <vassilisamir@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vasileios Amoiridis <vassilisamir@gmail.com>,
jic23@kernel.org, lars@metafoo.de, ang.iglesiasg@gmail.com,
mazziesaccount@gmail.com, ak@it-klinger.de,
petre.rodan@subdimension.ro, phil@raspberrypi.com,
579lpy@gmail.com, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] iio: pressure: Add triggered buffer support for BMP280 driver
Date: Mon, 4 Mar 2024 20:08:38 +0100 [thread overview]
Message-ID: <20240304190838.GA4431@vamoiridPC> (raw)
In-Reply-To: <ZeW15Q2juyQuTw5q@smile.fi.intel.com>
On Mon, Mar 04, 2024 at 01:52:05PM +0200, Andy Shevchenko wrote:
> On Sun, Mar 03, 2024 at 05:53:00PM +0100, Vasileios Amoiridis wrote:
> > Add a buffer struct that will hold the values of the measurements
> > and will be pushed to userspace. Modify all read_* functions in order
> > to just read and compensate the data without though converting to the
> > required IIO measurement units which are used for the oneshot captures.
>
> > +#include <linux/iio/buffer.h>
> > #include <linux/iio/iio.h>
> > #include <linux/iio/sysfs.h>
> > +#include <linux/iio/trigger.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
>
> Yes, taking into account the comment against patch 1, this will become the part
> of iio/* group.
>
> ...
>
> > + /* val might be NULL if we're called by the buffer handler */
> > + if (val) {
> > + *val = comp_press;
> > + /* Compensated pressure is in cPa (centipascals) */
> > + *val2 = 100000;
>
> Here and everywhere else where it makes sense
>
> *val2 = CENTI * 1000; // (What is 1000 here?)
>
> from units.h?
>
I didn't change these values, the addition here is that I put them under an
if statement that checks if we were called by the buffer handler or by the
oneshot capture read_raw function. The point is that every sensor provides
values that need different scaling in order to have the IIO standard
measurement units. In the above code I guess since 1kPa=100000cPa that's
why *val2=100000.
The *val and *val2 values could be moved to the read_raw function as it will
already happen for the IIO_CHAN_INFO_SCALE values from chip_info arrays as
you proposed in Patch No.2. This would require though that all the functions
like this one you commented would need to change. Is that something that you
think as better?
> > + return IIO_VAL_FRACTIONAL;
> > + }
>
> ...
>
> > + struct {
> > + s32 temperature;
> > + u32 pressure;
> > + u32 humidity;
>
> > + s64 timestamp;
>
> Shouldn't this be aligned properly?
>
I saw that in some drivers it was added and in some it was not. What is the
difference of aligning just the timestamp of the kernel?
> > + } iio_buffer;
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
next prev parent reply other threads:[~2024-03-04 19:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-03 16:52 [PATCH 0/4] Series to add triggered buffer support to BMP280 driver Vasileios Amoiridis
2024-03-03 16:52 ` [PATCH 1/4] iio: pressure: BMP280 core driver headers sorting Vasileios Amoiridis
2024-03-04 11:36 ` Andy Shevchenko
2024-03-09 18:09 ` Jonathan Cameron
2024-03-03 16:52 ` [PATCH 2/4] iio: pressure: Add scale value for channels Vasileios Amoiridis
2024-03-04 11:42 ` Andy Shevchenko
2024-03-09 18:28 ` Jonathan Cameron
2024-03-03 16:52 ` [PATCH 3/4] iio: pressure: Add timestamp and scan_masks for BMP280 driver Vasileios Amoiridis
2024-03-04 11:47 ` Andy Shevchenko
2024-03-04 18:50 ` Vasileios Amoiridis
2024-03-04 19:07 ` Andy Shevchenko
2024-03-09 18:12 ` Jonathan Cameron
2024-03-03 16:53 ` [PATCH 4/4] iio: pressure: Add triggered buffer support " Vasileios Amoiridis
2024-03-04 11:52 ` Andy Shevchenko
2024-03-04 19:08 ` Vasileios Amoiridis [this message]
2024-03-04 19:18 ` Andy Shevchenko
2024-03-04 20:05 ` Vasileios Amoiridis
2024-03-09 18:19 ` Jonathan Cameron
2024-03-09 18:32 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240304190838.GA4431@vamoiridPC \
--to=vassilisamir@gmail.com \
--cc=579lpy@gmail.com \
--cc=ak@it-klinger.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=ang.iglesiasg@gmail.com \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mazziesaccount@gmail.com \
--cc=petre.rodan@subdimension.ro \
--cc=phil@raspberrypi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).