Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
From: Andrea Merello <andrea.merello@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Couret Charles-Antoine <charles-antoine.couret@essensium.com>,
	"Ardelean, Alexandru" <alexandru.Ardelean@analog.com>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"lars@metafoo.de" <lars@metafoo.de>,
	"pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
	"knaack.h@gmx.de" <knaack.h@gmx.de>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Subject: Re: [PATCH 4/4] iio: ad7949: fix channels mixups
Date: Mon, 23 Sep 2019 10:21:49 +0200
Message-ID: <CAN8YU5O5ouLXnpi=f1jHfbbVXGjtFOT00cG+fggWWbxDco111w@mail.gmail.com> (raw)
In-Reply-To: <20190921181253.43fa0071@archlinux>

Il giorno sab 21 set 2019 alle ore 19:12 Jonathan Cameron
<jic23@kernel.org> ha scritto:

> >
> > If we skip the configuration rewrite when the channel doesn't change -
> > as discussed above - then we actually _terminate_ the acquisition when
> > the IIO read is triggered, that is we are converting the value sampled
> > right before the IIO read.
> >
> > If this is OK then I'll go on, otherwise I think that we should always
> > do the three cycles (so that triggering IIO read always waits also for
> > a new acquisition phase)

I had a discussion about this with a HW engineer, he said that it's
probably not necessary to perform a full extra cycle (i.e. SPI xfer +
udelay(2)), rather, since the HW is already in acquisition, it should
be enough to perform the udelay(2) to make sure the internal capacitor
settles (if we change channel of course we need also the SPI xfer to
update the CFG).

So indeed it seems to me that:
- if CFG (channel) changes: we need three full SPI xfer (actual SPI
xfer + delay(2))
- if CFG (channel) doesn't change: we need a delay(2) [*]- to
guarantee the user sees a value sampled after the IIO read, as
discussed - and two full SPI xfer (actual SPI xfer + delay(2))

.. Indeed I also wonder if it would be enough to cycle the CS, without
performing a full SPI xfer, in order to start the conversion.. But
given that this whole thing seems to me a bit complicated and unclear,
I would stick to the dummy cycle for now..

> An excellent point.  I agree and suspect we may have this wrong in other
> sensors.  The question gets more interesting if running in buffered mode
> as we have had systems using a trigger generated by an external process.
> I suppose in that case we just have to deal with the offset in the fifo
> etc in userspace.

Yes. I'm not familiar with IIO buffered mode, but for a streaming,
continuous, read mode I guess that the user would expect some latency
anyway (might be due to the ADC or the buffering mechanism itself or
whatever).

I didn't look into this buffered thing to see how it works, but maybe
we can skip the first udelay(2) [*] in the driver in case of buffered
access?

> Maybe we should think about adding a note to be careful of that.  Not
> really sure where we would note it though...

IMHO clarifying what the API guarantees and what it doesn't in IIO
DocBook could be helpful (I didn't see it, but I might have missed
it)..

So, we could state that a single shot read must return a value sampled
after the read has been shot, and that on the other hand, when using
buffered mode one should expect some latency.. But indeed, since you
said that we might have a number of IIO drivers that actually behave
in a different way, then I'm not sure that it's a good idea; maybe we
could just drop this requirement and allow (and document) that a
single shot IIO read could just _terminate_ the sampling and trigger
the conversion on the current sampled signal? (so also in this driver
we can just not care about this)..

> Thanks,
>
> Jonathan
>
>

  reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-12 14:43 [PATCH 0/4] Fixes for ad7949 Andrea Merello
2019-09-12 14:43 ` [PATCH 1/4] iio: ad7949: kill pointless "readback"-handling code Andrea Merello
2019-09-13  6:37   ` Ardelean, Alexandru
2019-09-15 10:26     ` Jonathan Cameron
2019-09-12 14:43 ` [PATCH 2/4] iio: ad7949: fix incorrect SPI xfer len Andrea Merello
2019-09-13  6:46   ` Ardelean, Alexandru
2019-09-13  7:56     ` Andrea Merello
2019-09-13  8:28       ` Ardelean, Alexandru
2019-09-15 10:36       ` Jonathan Cameron
2019-09-16  7:51         ` Ardelean, Alexandru
2019-09-21 17:16           ` Jonathan Cameron
2019-09-12 14:43 ` [PATCH 3/4] iio: ad7949: fix SPI xfer delays Andrea Merello
2019-09-13  6:59   ` Ardelean, Alexandru
2019-09-13  8:23     ` Andrea Merello
2019-09-13  8:43       ` Ardelean, Alexandru
2019-09-12 14:43 ` [PATCH 4/4] iio: ad7949: fix channels mixups Andrea Merello
2019-09-13  7:19   ` Ardelean, Alexandru
2019-09-13  8:30     ` Andrea Merello
2019-09-13 11:30       ` Couret Charles-Antoine
2019-09-13 11:40         ` Andrea Merello
2019-09-20  7:45         ` Andrea Merello
2019-09-21 17:12           ` Jonathan Cameron
2019-09-23  8:21             ` Andrea Merello [this message]
2019-10-05  9:55               ` Jonathan Cameron
     [not found]                 ` <CAN8YU5PRO5Y5EeEj2SZGm5XfuKSB1rtS7nKdu6wWxXYDOfexqw@mail.gmail.com>
2019-10-22  8:56                   ` Jonathan Cameron
2019-11-04 14:12                     ` Andrea Merello
2019-11-09 11:58                       ` Jonathan Cameron
2019-11-12 15:09                       ` Couret Charles-Antoine
2019-12-02 14:13                         ` [v2] " Andrea Merello
2019-12-02 15:36                           ` Couret Charles-Antoine
2019-12-04 11:06                             ` Jonathan Cameron
2019-12-04 11:13                               ` Couret Charles-Antoine
2019-12-06 16:45                                 ` Jonathan Cameron
2019-09-13  7:24 ` [PATCH 0/4] Fixes for ad7949 Ardelean, Alexandru
2019-09-13 14:00   ` Couret Charles-Antoine
2019-09-15 10:49     ` Jonathan Cameron
2019-09-16  7:39       ` Andrea Merello
2019-09-16  7:48         ` Ardelean, Alexandru
2019-09-16  7:50           ` Ardelean, Alexandru
2019-09-16  7:34     ` Andrea Merello

Reply instructions:

You may reply publically 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='CAN8YU5O5ouLXnpi=f1jHfbbVXGjtFOT00cG+fggWWbxDco111w@mail.gmail.com' \
    --to=andrea.merello@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=alexandru.Ardelean@analog.com \
    --cc=charles-antoine.couret@essensium.com \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    /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

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git