Date:   Thu, 25 Nov 2021 10:58:23 +0900
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     linux-iio@vger.kernel.org,
        Robin van der Gracht <robin@protonic.nl>,
        linux-kernel@vger.kernel.org,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        David Jander <david@protonic.nl>,
        Jonathan Cameron <jic23@kernel.org>, david@lechnology.com
Subject: Re: [PATCH v1] counter: interrupt-cnt: add counter_push_event()
Message-ID: <YZ7tv79LQwLL7h3T@shinobu>
References: <20211123134540.416695-1-o.rempel@pengutronix.de>
 <YZ3XAeYyfGblfaOi@shinobu>
 <20211124072720.GA30281@pengutronix.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="QhPtWNvbc3Joztu6"
Content-Disposition: inline
In-Reply-To: <20211124072720.GA30281@pengutronix.de>
Precedence: bulk


--QhPtWNvbc3Joztu6
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Nov 24, 2021 at 08:27:20AM +0100, Oleksij Rempel wrote:
> Hi William,
>=20
> On Wed, Nov 24, 2021 at 03:09:05PM +0900, William Breathitt Gray wrote:
> > On Tue, Nov 23, 2021 at 02:45:40PM +0100, Oleksij Rempel wrote:
> > > Add counter_push_event() to notify user space about new pulses
> > >=20
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > >  drivers/counter/interrupt-cnt.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >=20
> > > diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interr=
upt-cnt.c
> > > index 8514a87fcbee..b237137b552b 100644
> > > --- a/drivers/counter/interrupt-cnt.c
> > > +++ b/drivers/counter/interrupt-cnt.c
> > > @@ -31,6 +31,8 @@ static irqreturn_t interrupt_cnt_isr(int irq, void =
*dev_id)
> > > =20
> > >  	atomic_inc(&priv->count);
> > > =20
> > > +	counter_push_event(&priv->counter, COUNTER_EVENT_OVERFLOW, 0);
> > > +
> > >  	return IRQ_HANDLED;
> > >  }
> > > =20
> > > --=20
> > > 2.30.2
> >=20
> > Hi Oleksij,
> >=20
> > It looks like this is pushing a COUNTER_EVENT_OVERFLOW event every time
> > an interrupt is handled, which I suspect is not what you want to happen.
> > The COUNTER_EVENT_OVERFLOW event indicates a count value overflow event,
> > so you'll need to check for a count value overflow before pushing the
> > event.
> >=20
> > It would be good idea to implement a ceiling extension as well (you can
> > use the COUNTER_COMP_CEILING() macro) so that users can configure the
> > particular point where the value overflows.
>=20
> Thank you!
>=20
> What would be the best and resource effective strategy for periodically
> getting frequency of interrupts/pulses? This is actual information which =
is
> needed for my use case.
>=20
> So far, I was pushing every event to the user space, which is working
> but probably not the most resource effective method of doing it.
>=20
> Regards,
> Oleskij
> --=20
> 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 |

We could introduce a new Counter change-of-state event type which would
trigger whenever the count value changes, but I agree with you that this
is likely not the best way for us to derive the frequency of the
interrupts due to the indirection of handling and parsing the event
data.

Instead, perhaps introducing a "frequency" or "period" Count extension
would make more sense here. This extension could report the value delta
between counts, or alternatively the time delta from which you can
derive frequency. Regarding implementation, you can store the previous
value in a variable, updating it whenever an interrupt occurs, and
compute the particular delta every time a read is requested by the user.

David Lechner is implementing something similar for the TI eQEP driver
to expose speed, so I'm CCing them here in case this is of interest to
them.

William Breathitt Gray

--QhPtWNvbc3Joztu6
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAmGe7bQACgkQhvpINdm7
VJK0BRAAlF/X63tFON/5iQnbCUUnB1F+G6BoSoYdtyWa3it34l209vy0k/+CXtjp
hNQKViK6gek9U8a+7uwEtsxqNaZLFM3efuzCXEtQJEahpl8+DVukBsCG45abEhJ6
UQiUMABapCgmDOTgAk8zR5ItK9NjpJ8ELLpEzHm/8Zxe4KGmYGTTx0VpoxhnAW+6
WNtiyY7XO+J7Fp8bh/FsSsyPIV5kV0NOVbXSMiAhPzbKHMin+EFZZG5QmJ7CUP0l
kxKiHhdpNahqP/5KxDknxwO3+kqnuRHPxJbD06SJ2PKQDIih/CVuqQUQBpFlze5l
I5fBzONRjB/3MA9JNhp0/Qf+qDu/1ai1dXemw7KF5d9+nRZ9MJOT438clnhnGKqK
9VR/1R98ihBoUQgLpAC1jnk6KGH5Yyx20CoIHsMCTZJS4ZoN15gkiq8vzMb8rUkz
cIB0oQ1YBRw2ws4s5AV6Ooz9+CGpwS1OZ5lBudz/zyDh3GeXAzP4f9FMfFvzQNK1
ZYurZQ83lD2iXL8Y6TY9ZT30VinbAQcJWQYHlHvJJA8Z6YIhEKXocBoVxP/nNsY7
ygce+vFYQepiRN8f6bz840z8W4nbzRMfcwxbDDjOCsV1NCEXtnck+eC0TwPokX2F
uE66tF4f16xpunCuG5uaGk4wQvWPciAzZHMJXX64ZyLemgez5A8=
=Xn1H
-----END PGP SIGNATURE-----

--QhPtWNvbc3Joztu6--