Hi, I'm attempting to use the Zephyr GPIO API for the first time.  It seems full featured, but I think this has caused me some confusion.  I was debugging my application and found that I got stuck in an infinite loop in _gpio_fire_callbacks().  (For what it's worth, I'm using an nRF52832 chip, so this was called from gpio_nrfx.c)

I am trying to interface with an IRQ pin on a peripheral, so I wrote a couple of enable/disable functions for my driver that look similar to this:

https://gist.github.com/Lncn/49174feb32c2d96dc4e82924b6163c8f

This code was ported from another platform & RTOS where I simply reconfigured the pin entirely when enabling/disabling the device IRQ.  In hindsight this may have been heavy handed.  I was unknowingly calling my "x_enable_irq" function twice in the process of initializing my application and I think this caused my infinite loop.  It appears you cannot call gpio_add_callback twice with the same callback struct, as sys_slist_prepend() will create a circular linked list if the struct gpio_callback reference is already in the list.

Knowing this, I have a couple of questions:

1. First, is this already a known and accepted issue that a user is expected to know to avoid? (Obviously, this would be at the price of having the most efficient list insertion which seems like an acceptable tradeoff)
2. Once my IRQ pin is configured and my callback added, it looks like I should ONLY ever use gpio_pin_[enable|disable]_callback() to enable or disable the IRQ?  This is okay, but requires me to maintain more state in my peripheral device driver.  It would be nice if something catastrophic happens, I could blindly reinitialize my driver and either (a) have a GPIO api to check if a callback is already registered before calling gpio_add_callback() or (b) the gpio_add_callback() function will do nothing if the callback is already in the list.

Any recommendation or comments are appreciated!  Or perhaps I'm off base on something, let me know!

Thanks,
Lincoln Simmons
lincolnsimmons@...

Hi,

Knowing this, I have a couple of questions:

First, is this already a known and accepted issue that a user is
expected to know to avoid? (Obviously, this would be at the price of
having the most efficient list insertion which seems like an
acceptable tradeoff)

Actually you found a bug. It's not up to the user to know that the cb
is already installed (even though, logically it does not make sense to
add the same cb many times), in other words: gpio should not blindly
install an already installed cb.

I'll open an issue. I think it should check the status of the node, if
already set up, it will try to find it and return an error: -EALREADY
if found, or -EINVAL as it might have been inserted into another
controller's list and we should not touch it.

Once my IRQ pin is configured and my callback added, it looks like I
should ONLY ever use gpio_pin_[enable|disable]_callback() to enable
or disable the IRQ? This is okay, but requires me to maintain more
state in my peripheral device driver. It would be nice if something
catastrophic happens, I could blindly reinitialize my driver and
either (a) have a GPIO api to check if a callback is already
registered before calling gpio_add_callback() or (b) the
gpio_add_callback() function will do nothing if the callback is
already in the list.

I think my fix proposal above should clarify that.
If it returns 0 or -EALREADY, all will be fine.

Any recommendation or comments are appreciated! Or perhaps I'm off
base on something, let me know!

No that was a good catch!

Thanks,

Tomasz

Hi Lincoln,

Can you try https://github.com/zephyrproject-rtos/zephyr/pull/11396
(1st commit)

That should solve your issue.

Instead of returning -EALREADY if already installed, I just return 0.

Tomasz

Nice, thanks Tomasz!