From: Jonathan Cameron <jic23@kernel.org>
To: Petar Stoykov <pd.pstoykov@gmail.com>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
linux-iio@vger.kernel.org, Lars-Peter Clausen <lars@metafoo.de>,
Rob Herring <robh+dt@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Angel Iglesias <ang.iglesiasg@gmail.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/3] iio: pressure: Add driver for Sensirion SDP500
Date: Sun, 5 May 2024 18:58:09 +0100 [thread overview]
Message-ID: <20240505185809.7636403e@jic23-huawei> (raw)
In-Reply-To: <CADFWO8EF0WxAV=k-ZAJ1McmaYv4SD5G+O4FhoMDsVQaRqe6sdg@mail.gmail.com>
> > > +}
> > > +
> > > +static const struct iio_chan_spec sdp500_channels[] = {
> > > + {
> > > + .type = IIO_PRESSURE,
> > > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> >
> > Looks like a simple linear scale. Would be better to make scaling
> > a userspace / consumer problem and provide IIO_CHAN_INFO_RAW
> > and IIO_CHAN_INFO_SCALE.
>
> I prefer returning the pressure directly because there is no other calculation
> that the user of this driver can do. If they make the calculation differently
> then their pressure value would be wrong.
Ah. I missed this and just made the same comment on v2.
Let me give some more info than in the original review.
The documentation on how to apply scale is simple and this scaling is
pretty hard to get wrong.
There are a couple of reasons we prefer to make it a userspace problem
to do linear scaling and keep the actual channel value raw (if possible).
1) Logging applications typically store the scale once, and each data
point is then much cheaper to store as a u16 than as a floating point
number.
2) If you ever add buffered support, we do not support floating point
values in the buffer. That basically means we have to have both
_PROCESSED and _RAW provided so that _SCALE makes sense for the buffer.
Horribly messy ABI is the result.
Hence, push the scaling to userspace.
Note that we can't always do this as some conversion functions are
non linear and very hard to describe. In those cases _PROCESSED makes
sense. That's not true here.
Jonathan
prev parent reply other threads:[~2024-05-05 17:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-16 15:24 [PATCH 2/3] iio: pressure: Add driver for Sensirion SDP500 Petar Stoykov
2024-01-16 15:30 ` Krzysztof Kozlowski
2024-01-16 15:50 ` Krzysztof Kozlowski
2024-01-16 17:03 ` Jonathan Cameron
2024-04-30 15:11 ` Petar Stoykov
2024-05-05 17:58 ` Jonathan Cameron [this message]
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=20240505185809.7636403e@jic23-huawei \
--to=jic23@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=ang.iglesiasg@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pd.pstoykov@gmail.com \
--cc=robh+dt@kernel.org \
/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).