All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Rosin <peda@axentia.se>
To: Liam Beguin <liambeguin@gmail.com>,
	jic23@kernel.org, lars@metafoo.de, pmeerw@pmeerw.net
Cc: linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	devicetree@vger.kernel.org, robh+dt@kernel.org
Subject: Re: [PATCH v1 4/9] iio: afe: rescale: add offset support
Date: Mon, 31 May 2021 18:25:55 +0200	[thread overview]
Message-ID: <ca30e3d2-7d9a-1c9d-9ae5-beefa2cd6492@axentia.se> (raw)
In-Reply-To: <CBRIK3PI2AMD.3KUD7EI7NJ2EB@shaak>

On 2021-05-31 16:51, Liam Beguin wrote:
> On Mon May 31, 2021 at 10:08 AM EDT, Peter Rosin wrote:
>> On 2021-05-31 15:36, Liam Beguin wrote:
>>> Hi Peter,
>>>
>>> On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
>>>> Hi!
>>>>
>>>> Thanks for the patch!
>>>>
>>>> On 2021-05-30 02:59, Liam Beguin wrote:
>>>>> From: Liam Beguin <lvb@xiphos.com>
>>>>>
>>>>> This is a preparatory change required for the addition of temperature
>>>>> sensing front ends.
>>>>
>>>> I think this is too simplistic. I think that if the upstream iio-dev has
>>>> an offset, it should be dealt with (i.e. be rescaled). The rescale
>>>> driver
>>>> cannot ignore such an upstream offset and then throw in some other
>>>> unrelated offset of its own. That would be thoroughly confusing.
>>>
>>> I'm not sure I fully understand. The upstream offset should be dealt
>>> with when calling iio_read_channel_processed().  That was my main
>>> motivation behind using the IIO core to get a processed value.
>>
>> You can rescale a channel with an offset, but without using processed
>> values. I.e. the upstream channel provides raw values, a scale and an
>> offset. The current rescale code ignores the upstream offset. I did not
>> need that when I created the driver, and at a glace it felt "difficult".
>> So I punted.
> 
> I understand what you meant now.
> 
> At first, I tried to apply the upstream offset from inside the rescaler.
> As you said it felt difficult and it felt like this must've been
> implemented somewhere else before.
> 
> After looking around, I noticed that the code to do that was already
> part of inkern.c and exposed through iio_read_channel_processed().
> If the upstream channel doesn't provide a processed value, the upstream
> offset and scale are automatically applied.
> 
> So with the changes in [3/9] the rescaler's raw value becomes the
> upstream channel's processed value.
> 
> This seems like an easier and probably cleaner way of adding offset
> support in the rescaler.
> 
> Does that make sense?

Yes, it does. Doing generic calculations like this efficiently with
integer math without losing precision is ... difficult.

I think that perhaps IF the upstream channel has an offset, the
rescaler could revert to always use the upstream processed channel in
preference of the raw channel. That would fix the missing support for
upstream offset and still not penalize the sweet case of no upstream
offset. Because the processed channel costs processing for each and
every sample and I think it should be avoided as much as possible.

Does that make sense?

Or are a bunch of drivers adding an explicit zero offset "just because"?
That would be a nuisance.

Cheers,
Peter

  reply	other threads:[~2021-05-31 17:06 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-30  0:59 [PATCH v1 0/9] iio: afe: add temperature rescaling support Liam Beguin
2021-05-30  0:59 ` [PATCH v1 1/9] iio: inkern: always apply scale requested by consumer Liam Beguin
2021-05-31 13:39   ` Peter Rosin
2021-05-30  0:59 ` [PATCH v1 2/9] iio: inkern: error out on unsupported offset type Liam Beguin
2021-05-31  9:45   ` Peter Rosin
2021-05-31 13:31     ` Liam Beguin
2021-05-30  0:59 ` [PATCH v1 3/9] iio: afe: rescale: use core to get processed value Liam Beguin
2021-05-31  7:09   ` Peter Rosin
2021-05-31 13:23     ` Liam Beguin
2021-06-01 16:22   ` Jonathan Cameron
2021-05-30  0:59 ` [PATCH v1 4/9] iio: afe: rescale: add offset support Liam Beguin
2021-05-31  8:52   ` Peter Rosin
2021-05-31 13:36     ` Liam Beguin
2021-05-31 14:08       ` Peter Rosin
2021-05-31 14:51         ` Liam Beguin
2021-05-31 16:25           ` Peter Rosin [this message]
2021-05-31 17:42             ` Liam Beguin
2021-06-01 16:31               ` Jonathan Cameron
2021-05-30  0:59 ` [PATCH v1 5/9] iio: afe: rescale: add support for temperature sensors Liam Beguin
2021-06-01 16:34   ` Jonathan Cameron
2021-05-30  0:59 ` [PATCH v1 6/9] dt-bindings: iio: afe: update MAINTAINERS file Liam Beguin
2021-05-31  7:57   ` Peter Rosin
2021-05-30  0:59 ` [PATCH v1 7/9] dt-bindings: iio: afe: add binding for temperature-sense-rtd Liam Beguin
2021-06-01 16:43   ` Jonathan Cameron
2021-06-04 21:17   ` Rob Herring
2021-06-05 14:58     ` Jonathan Cameron
2021-05-30  0:59 ` [PATCH v1 8/9] dt-bindings: iio: afe: add binding for temperature-sense-current Liam Beguin
2021-05-31  7:28   ` Peter Rosin
2021-05-31  8:58     ` Peter Rosin
2021-05-31 13:41       ` Liam Beguin
2021-06-01 16:47   ` Jonathan Cameron
2021-06-04 21:21   ` Rob Herring
2021-05-30  0:59 ` [PATCH v1 9/9] dt-bindings: iio: afe: add binding for temperature-sense-amplifier Liam Beguin
2021-05-31  7:32   ` Peter Rosin
2021-05-31 14:03     ` Liam Beguin
2021-06-01 16:02       ` Jonathan Cameron
2021-06-01 16:07         ` Jonathan Cameron
2021-06-01 15:59   ` 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=ca30e3d2-7d9a-1c9d-9ae5-beefa2cd6492@axentia.se \
    --to=peda@axentia.se \
    --cc=devicetree@vger.kernel.org \
    --cc=jic23@kernel.org \
    --cc=lars@metafoo.de \
    --cc=liambeguin@gmail.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.