public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: "David Lechner" <david@lechnology.com>,
	linux-iio@vger.kernel.org,
	"Robin van der Gracht" <robin@protonic.nl>,
	"David Jander" <david@protonic.nl>,
	linux-kernel@vger.kernel.org,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Jonathan Cameron" <jic23@kernel.org>
Subject: Re: [PATCH v1] counter: interrupt-cnt: add counter_push_event()
Date: Thu, 3 Feb 2022 11:40:36 +0100	[thread overview]
Message-ID: <20220203104036.GB12695@pengutronix.de> (raw)
In-Reply-To: <YfuJXrxpas1ufzp2@shinobu>

Hi William,

On Thu, Feb 03, 2022 at 04:50:54PM +0900, William Breathitt Gray wrote:
> > Hm...
> > 
> > To detect pulse frequency, I need a burst of sequential time-stamps
> > without drops. In case the pulse frequency is higher then the use space
> > is able to get it out of FIFO, we will get high number of drops. 
> > So, we do not need all time stamps. Only bunch of them without drops in
> > the middle.
> > 
> > I know, at some frequency we wont be able to collect all pulses any way.
> > Internal FIFO is just increasing the max detectable frequency. So, it is
> > sort of optimization.
> > 
> > My current driver version has own FIFO which is filled directly by the
> > IRQ handler and user space trigger flush_cb to push all collected
> > time stamps. The main question is: how the flush procedure should be
> > controlled. We have following options:
> > 
> > - Attach it to the read(). The disadvantage: at high frequencies, we
> >   wont be able to get a burst with time stamps without drops in the
> >   middle
> > - Trigger flush from user space. In this case, we make user space a bit
> >   more complicated and cant really get all advantages of poll().
> > - kernel driver is using own timer to trigger flush. The timer can be
> >   configured from user space. The advantage of it, the user space is
> >   simple and has full advantage of using poll()
> > 
> > Regards,
> > Oleksij
> 
> Hi Oleksij,
> 
> Earlier in this thread, Jonathan Cameron suggested using the RCU macros
> to protect access to the events. Taking an RCU approach would eliminate
> the need for spinlocks because the memory barriers are built-in to the
> macros, so I assume flushing would no longer be necessary. Would RCU be
> a viable solution for your needs?

IMO, RCU is the wrong word in this context. It provide an advantage
where we need to reuse/read less frequently changed data. In this use
case we need to move data ASAP, so KFIFO seems to work just fine here.

In any case, after implementi double FIFO and more testing I would
prefer to stay with my initial patch. On a single core system, with have
no waiting time at all. No concurrent access. And on the SMP system
(iMX6Q), currently I can measure higher frequency with initial not
optimized driver:
- with counter_push_event() directly from IRQ: max freq 30-35kHz
- with double FIFO, i have max freq of ~25kHz

Your suggestion was to add COUNTER_EVENT_CHANGE_OF_STATE and use it for
my use case. Correct?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

      reply	other threads:[~2022-02-03 10:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 13:45 [PATCH v1] counter: interrupt-cnt: add counter_push_event() Oleksij Rempel
2021-11-24  6:09 ` William Breathitt Gray
2021-11-24  7:27   ` Oleksij Rempel
2021-11-25  1:58     ` William Breathitt Gray
2021-11-25  7:27       ` David Jander
2021-12-06 19:24       ` David Lechner
2021-12-07  7:16         ` David Jander
2021-12-08 13:59           ` Uwe Kleine-König
2021-12-08 16:10             ` David Jander
2021-12-15  8:48               ` William Breathitt Gray
2021-12-15  9:08                 ` David Jander
2021-12-25  4:07                   ` William Breathitt Gray
2021-12-27 15:16                     ` David Lechner
2021-12-29  9:26                       ` William Breathitt Gray
2021-12-29 16:45                         ` Jonathan Cameron
2022-02-02 12:32                     ` Oleksij Rempel
2022-02-02 15:17                       ` David Lechner
2022-02-03  7:24                         ` Oleksij Rempel
2022-02-03  7:50                           ` William Breathitt Gray
2022-02-03 10:40                             ` Oleksij Rempel [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=20220203104036.GB12695@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=david@lechnology.com \
    --cc=david@protonic.nl \
    --cc=jic23@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin@protonic.nl \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=vilhelm.gray@gmail.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