Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 27 Nov 2020 21:30:24 -0300
From:      "Dr. Rolf Jansen" <freebsd-rj@obsigna.com>
To:        Ian Lepore <ian@freebsd.org>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: User Space GPIO Interrupt programming - GSoC-2018
Message-ID:  <3F7009F8-549C-4580-B4D2-9C8471890CE6@obsigna.com>
In-Reply-To: <ed2079c342f64b611e014fb5331e2be8beeed547.camel@freebsd.org>
References:  <2B01780F-D367-48A3-A827-B479030A496D@obsigna.com> <c55d7f332631b69c3241a60538a6a7b5475d93b9.camel@freebsd.org> <FBEF19B1-0504-4CDF-976C-C50707E06584@obsigna.com> <ed2079c342f64b611e014fb5331e2be8beeed547.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

Am 27.11.2020 um 14:32 schrieb Ian Lepore <ian@freebsd.org>:

> On Thu, 2020-11-26 at 22:18 -0300, Dr. Rolf Jansen wrote:
>>> Am 26.11.2020 um 16:56 schrieb Ian Lepore <ian@freebsd.org>:
>>>=20
>>> On Tue, 2020-11-24 at 17:14 -0300, Dr. Rolf Jansen wrote:
>>>> Hello
>>>>=20
>>>> Has anything of the GSoC-2018 efforts made it into the current
>>>> code
>>>> base?
>>>>=20
>>>>=20
>>>=20
>>>=20
> =
https://wiki.freebsd.org/SummerOfCode2018Projects/UserSpaceGPIOinterrupts
>>>>=20
>>>> I installed the recent 13.0-CURRENT snapshot (2020-11-19) on a
>>>> BeagleBone Black which was one of the implementation targets of
>>>> said
>>>> project, but when running the test tools, I either see cannot
>>>> read/kevent/poll/aio_read - Operation not supported by device or
>>>> Inappropriate ioctl for device.
>>>>=20
>>>> Perhaps I need to pull the project=1B$B!-=1B(Bs changes into the =
kernel by
>>>> myself. However, before this I would like to ask whether it is
>>>> worth
>>>> the effort.
>>>>=20
>>>> Please, can anyone shed some light on this.
>>>>=20
>>>> Best regards
>>>>=20
>>>> Rolf
>>>>=20
>>>=20
>>> I made some time this morning to review the gsoc2018 code.  It
>>> turns
>>> out this code is very high quality, nearly ready to commit as-
>>> is.  The
>>> main thing it needs is some style cleanup in its comment blocks,
>>> and
>>> documentation.  I'd be inclined to commit the code first and write
>>> the
>>> documentation over the next little while and commit it separately.
>>>=20
>>> If you'd like to give it a try, here's a diff that should apply and
>>> build cleanly on freebsd 12 or 13:
>>>=20
>>> https://people.freebsd.org/~ian/gpio_gsoc2018.diff
>>>=20
>>> While there isn't any documentation yet, there is a test program (I
>>> haven't run it yet) that demonstrates all the features:
>>>=20
>>>=20
> =
https://github.com/ckraemer/gsoc2018-utils/blob/master/src/gpioc_intr_test=
.c
>>>=20
>>> Right now the code will let you block waiting for a pin-change
>>> event
>>> using select(), poll() or kevents, or to be notified via SIGIO, but
>>> after being notified that something happened, you still have to
>>> call
>>> read() to see which pin changed.  I think if the pin changes state
>>> multiple times between calls to read(), you'll lose track of some
>>> changes (I'm not positive of that, I don't understand the kevent
>>> stuff
>>> well).
>>>=20
>>> I'd like to add some features so that you can configure it to track
>>> pin
>>> changes in counting-mode and timestamping-mode.  In counting mode,
>>> when
>>> you do a read() you would get back a pair of values, the pin number
>>> and
>>> how many times its interrupt fired since the last read.  In
>>> timestamping mode, every read would return a pin number and an
>>> associated timespec giving the exact time the interrupt happened
>>> (there
>>> would need to be a way to configure how many events it could
>>> buffer,
>>> but I think even allowing a thousand buffered events would only use
>>> a
>>> few kbytes of memory).
>>=20
>> I got it working as well, please see my other post from yesterday. I
>> used gpioc_intr_test.c.
>>=20
>> I see hundreds of warning messages when I press the test button a few
>> times. May these warnings be safely ignored. The kernel module of
>> Oskar Holmund works quite nice as well (for what I need), and with
>> that one, I don=1B$B!G=1B(Bt see warnings.
>>=20
>> The counting- and timestamping-mode for sure would be very useful.
>> Perhaps by implementing this, there won=1B$B!G=1B(Bt be no unhandled =
interrupts
>> anymore, and hence there won=1B$B!G=1B(Bt be any warnings either.
>>=20
>> Best regards
>>=20
>> Rolf
>=20
> I'm sorry, I somehow overlooked your previous message about using
> gpioc_intr_test.c.
>=20
> Those warning messages are definitely not a good thing, in some =
changes
> I've made to the original patches they are changed to debugging-only
> output that won't normally show up.  Printing messages from within an
> interrupt handler is pretty much always a bad idea. :)
>=20
> I was thinking about the various interrupt options and realized we
> cannot support level-triggered interrupts in this code without a ton =
of
> work.  It's just a recipe for an interrupt storm (which will happen at
> the rate of tens of thousands of interrupts per second once the =
printfs
> aren't there to slow things down to console IO speed).  To use level-
> triggered interrupts from userland, the gpioc would have to mask the
> interrupt from within the interrupt handler (we don't have an internal
> API for doing that with gpio interrupts right now), and provide some
> sort of EOI (end-of-interrupt) acknowledgement interface to userland =
to
> unmask them once it had done something to make the interrupt stop
> asserting.  Basically this would extend the way device drivers handle
> level-triggered interrupts into userland, and I just don't see much
> value in all the work that would be involved to make that happen.
>=20
> I think instead we should just run in counting-mode by default, and =
add
> new code similar to what Vladimir Goncharov has proposed to handle
> detailed reporting of each event if the app requests it.
>=20
> I can't make up my mind on the issue of debouncing.  My gut tells me
> that building in some kind of debounce logic as a per-pin configurable
> option would be nice, but it might also get really complicated.  An =
app
> could handle debouncing for itself if it requested detailed event
> reporting, because the timestamps on the events could help it decide
> what to do.

After working a bit with the various "User Space GPIO Interrupt" =
incarnations in the last few days, I found that I don't need level =
interrupts. The hardware colleague of our present project told me that I =
do not need to bother with button debouncing in software either. There =
are IC's which manage this, for example: MAX6818 - =
https://datasheets.maximintegrated.com/en/ds/1896.pdf. So, the GPIO =
would see only perfect square wave signals when a button has been =
pressed.

I still think that counting-mode and timestamp-mode could be very =
useful.

Regarding the memory allocation, I am with you that it should be done =
once and not for any interrupt trail. As a matter of fact I did kind of =
this already for a kernel module for FreeBSD-x86-64 targets which I =
wrote for National Instruments M- and X-Series PCI and PCIe DAQ boards. =
Here I allocate huge blocks of DMA memory (256 MB for the AI channels =
and 48 MB for the AO channels) in the course of xxx_attach() and this =
can be mapped into user space by mmap(). I added sysctls which allow to =
resize said memory blocks, and ioctls for clearing and synching the =
buffers. I do all the math on the measurement data in user space by =
directly accessing the DMA blocks in the course of the measurements.

Thinking about it, wouldn't it be nice to mmap() the =
interrupt/counter/timestamp table into user space? Programs could read =
the data in the table directly. It could be a round robin table, perhaps =
of 100 kB to 1 MB. Perhaps my code in my kernel module for DMA buffer =
management, which could be easily switched to any-kind-of-memory =
management, does not exactly meet the high quality and style of FreeBSD =
code, and I am even hesitant to offer it. However, if you are curious, =
please send me a note.

Best regards

Rolf=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3F7009F8-549C-4580-B4D2-9C8471890CE6>