Re: [RFC] GPIO API changes


Tomasz Bursztyka
 

Hi Johan,

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.
A perhaps slightly more compact solution (and a pattern I see a log e.g.
in Linux) is to take advantage of CONTAINER_OF. You'd first drop
user_data from the gpio_callback and pass the original struct to the
callback:

struct gpio_callback {
void (*func)(struct device *port, struct gpio_callback *cb, uint32_t pin);
struct gpio_callback *_next;
}

Then users of the API could just use this struct directly, or if they
need "user data" they can define their own structs that embed the gpio
one:

struct my_struct {
...
struct gpio_callback cb;
...
};

void my_func(struct device *port, struct gpio_callback *cb, uint32_t pins)
{
struct my_struct *data = CONTAINER_OF(cb, struct my_struct, cb);
}

{
struct my_struct s;

GPIO_INIT_CB(&s.cb, my_func);

gpio_set_callback(port, &s.cb);
}
I fully agree on that. It saves a pointer in case the user does not want
to access any private data,
and thus leaves space for adding pin mask in the struct.

Tomasz

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