devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Esteban Blanc" <eblanc@baylibre.com>
To: "Jonathan Cameron" <jic23@kernel.org>
Cc: "Lars-Peter Clausen" <lars@metafoo.de>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Nuno Sa" <nuno.sa@analog.com>, <linux-iio@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	"David Lechner" <dlechner@baylibre.com>
Subject: Re: [PATCH RFC 0/5] iio: adc: ad4030: new driver for AD4030 and similar ADCs
Date: Wed, 14 Aug 2024 15:02:42 +0200	[thread overview]
Message-ID: <D3FNL323ZXLQ.2D0QLACO67VTP@baylibre.com> (raw)
In-Reply-To: <20240629174039.3e6053e5@jic23-huawei>

On Sat Jun 29, 2024 at 6:40 PM CEST, Jonathan Cameron wrote:
> On Thu, 27 Jun 2024 13:59:11 +0200
> Esteban Blanc <eblanc@baylibre.com> wrote:
>
> > This is adding DT bindings and a new driver for AD4030, AD4630 and
> > AD4632 ADCs.
> > 
> > This work is being done in collaboration with Analog Devices Inc.,
> > hence they are listed as maintainers rather than me.
> > 
> > The code has been tested on a Zedboard with an EVAL-AD4030-24FMCZ,
> > an EVAL-AD4630-24FMCZ and an EVAL-AD4630-16FMCZ. As there is no eval
> > board for AD4632 the support can't be tested at the moment. The main
> > difference is the reduced throughput.
> > 
> > This series is taged as RFC because I think I'm misusing
> > IIO_CHAN_INFO_CALIB*. For CALIBBIAS the doc in sysfs-bus-iio says
> > "Hardware applied calibration offset (assumed to fix production
> > inaccuracies)" but AD4030 offset in on 24bits and I would argue that at
> > this point it's not just here to fix production inaccuracies. Same this
> > for CALIBSCALE. What IIO attributes should I use instead?
>
> Interesting.   So awkward question for you.  What's the point in applying
> a digital offset?  calibbias is normally about tweaking the Analog side.
> This just seems to be adding a value on.  I'm not sure it affects what
> can actually be captured without saturation.

True, both scale and offset applied with thoses registers can lead to
saturation.

> Maybe it has influence by changing the input range and scale for the
> block averaging filter?  I'm not sure.
>
> You can use offset for this given it's a simple linear value and not
> anything to do with calibration. It's a little awkward though as that
> is post scale rather than the other way around which is rather more
> common.
> Controls are in the form
> voltage = (raw + offset) * scale 
>
> So here
> voltage = (raw + offset_reg / (gain_reg * other scaling)) * gain_reg * otherscaling.
>
> Hence your offset is a bit fiddly to compute.

After talking to ADI engineer about this, the conclusion is that I was
wrong and this is indeed mostly for calibration. They left the range
of values quite wide in case a user wanted to use this to apply an
offset or scale to the raw value directly in order to avoid doing some
post processing later on. But the main goal is calibration.

If that's ok with you I will keep CALIBBIAS and CALIBSCALE for the next
round and remove the RFC tag.

Thanks for your time and sorry for the confusion,

-- 
Esteban "Skallwar" Blanc
BayLibre


  reply	other threads:[~2024-08-14 13:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-27 11:59 [PATCH RFC 0/5] iio: adc: ad4030: new driver for AD4030 and similar ADCs Esteban Blanc
2024-06-27 11:59 ` [PATCH RFC 1/5] dt-bindings: iio: adc: add ADI ad4030 and ad4630 Esteban Blanc
2024-06-28 16:55   ` Conor Dooley
2024-06-27 11:59 ` [PATCH RFC 2/5] iio: adc: ad4030: add driver for ad4030-24 Esteban Blanc
2024-06-28  8:35   ` Nuno Sá
2024-06-29 16:40     ` Jonathan Cameron
2024-07-26 13:38     ` Esteban Blanc
2024-07-28 15:04       ` Jonathan Cameron
2024-07-31  9:07         ` Nuno Sá
2024-08-03  9:48           ` Jonathan Cameron
2024-07-31  8:56       ` Nuno Sá
2024-08-03  9:51         ` Jonathan Cameron
2024-08-05  9:38           ` Esteban Blanc
2024-06-29 16:39   ` Jonathan Cameron
2024-07-29 14:42     ` Esteban Blanc
2024-07-29 20:13       ` Jonathan Cameron
2024-07-29 21:34   ` Christophe JAILLET
2024-06-27 11:59 ` [PATCH RFC 3/5] iio: adc: ad4030: add averaging support Esteban Blanc
2024-06-27 11:59 ` [PATCH RFC 4/5] iio: adc: ad4030: add support for ad4630-24 and ad4630-16 Esteban Blanc
2024-06-29 16:55   ` Jonathan Cameron
2024-07-29 14:57     ` Esteban Blanc
2024-06-27 11:59 ` [PATCH RFC 5/5] iio: adc: ad4030: add support for ad4632-16 and ad4632-24 Esteban Blanc
2024-06-29 16:40 ` [PATCH RFC 0/5] iio: adc: ad4030: new driver for AD4030 and similar ADCs Jonathan Cameron
2024-08-14 13:02   ` Esteban Blanc [this message]
2024-08-14 18:38     ` 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=D3FNL323ZXLQ.2D0QLACO67VTP@baylibre.com \
    --to=eblanc@baylibre.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@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 NNTP newsgroup(s).