[RFC] GPIO API changes


Tomasz Bursztyka
 

Hi,

I would like to propose some changes to the public GPIO API.
Addressing major 2 issues which I faced while writing some code using
the API. And an third one, related to the consistency of the API.

API issues:
==========

1) the callback might need to know about the context: Let's say you
have a driver which sets a gpio callback. As soon as there is 2
instances of this driver, it won't be able to tell which instance
does that callback call belongs to (unless keeping book about which
gpio port/pin relates to which inner instance which is ugly)

2) 2+ different sub-system might need to set a callback: One
sub-system might be interested to get notified when pin X generated
an interrupt, when another would be interested by pin Y.

3) Currently, you can set either a callback for the whole port or
per-pin. The major difference is found in how callback is called:

-> port callback: the pin parameter of the callback is a bitfield,
each bit telling which pin generated and int.

-> pin callback: the pin parameter is the pin number and not anymore
a bit in a bitfield.


API changes:
===========

1) Let's add a user data pointer parameter to the callback:

typedef void (*gpio_callback_t)(struct device *port,
uint32_t pin,
void *user_data);

gpio_set_callback() would then require a void *user_data parameter as
well.


2) The callback would not be only a function, but a structure in order
to be able to manage a simple linked-list of callbacks.

Johan had a good input on that one, as he and bt team have done that
in the BT stack. Instead of a function pointer you would pass a
pointer of:

struct gpio_callback {
void (*func)(struct device *port, uint32_t pin, void *user_data);
void *user_data;

struct gpio_callback *_next;
}

This avoid to manage any memory slots in the driver. All would be
statically allowed. Also, being a generic list, the code could be
centralized in a private header in drivers/gpio thus we would not have
to re-implement such management in every driver.

3) I would like to normalize that: uint32_t pin would becomes uint32_t
pins and it would be always a bitfield.

Then instead of:
-> port cb: if (pin & BIT(the_pin_to_look_up)) { ... }
and
-> pin cb: if (pin == the_pin_to_look_up) { ... }

We would end up with a unique way of handling such parameter in the
callback:

if (pin & BIT(the_pin_to_look_up)) { ... }


Another idea:
============

Having multiple callback, I see one issue: it's up to the callback to
filter out which pins it's interested in. I mean, when setting a
callback nothing tells which pins it's meant for. Once you enabled pin
X, Y or Z to generate interrupts, all callbacks will be called
whatever pin is generating an interrupt.

If you have a callback interested by pin X and Y but not Z, it will be
called for the 3 of them.

It's not a big issue on the callback side since it will most likely
filter on X or Y this way:

if (pins & BIT(pin_X) {
...
} else if (pins & BIT(pin_y)) {
...
}

So we could add a pin mask in the struct gpio_callback: uint32_t
pin_mask.

That one would be used in gpio driver ISR handler to filter which
callback is relevant to call. I see 2 advantages:

- less callback calls, only relevant ones (gpio int are then handled
in a quicker way also)
- it would be a tiny bit more simple to filter out pins in the
callback: instead of the last "else if", you would just get a "else".
It's not much but still.

The drawback is it bloats a bit more the struct gpio_callback.

However, on complex hw setup, with different sub-system being
interested by the same gpio port and different pins, this could make
thing performing better. The less we stay in an ISR context, the
better.

From API functions point of view, I want to keep most of it as it is,
only parameters will change a bit. For instance I still want a
unique gpio_set_callback() function. I still think it's relevant to
keep gpio_pin_<enable/disable>_callback() no matter what pins are the
callback interested in. Same for gpi_port_enable_callback().

Finally, only the gpio_callback_t type will change. I still have to
figure out how to keep the old API behavior, probably through Kconfig
option. It would be either/or, not both. Or should it enable old
behavior always?

Tomasz

Join devel@lists.zephyrproject.org to automatically receive all group messages.