[riot-notifications] [RIOT-OS/RIOT] periph/gpio: enable API usage for expanders (#9690)

Kaspar Schleiser notifications at github.com
Tue Jan 8 15:13:10 CET 2019

I'd like to take another look at the redirection method.

I haven't taken an in-depth look at the API changes other than for gpio, so this applies mostly to gpio.

The reason why we want to change all the API's in the first place is that we need to support multiple "providers" of the functionality (I2C gpio extenders, bitbanging soft bus implementations, UART-over-USB, external ADC/DAC, ..). Ideally we manage to do that while

- keeping the current user-facing API (so we don't need to change all drivers)
- integrate it into the current API (so drivers can seamlessly use the new "providers")
- don't break existing applications. This includes the fact that the current implementation, when compiled with LTO, optimizes "gpio_set/get/..." to a single memory write if called with constant PIN parameters
- blah blah maintainability, code size, ram usage, ...

If I understand the current method correctly:

- the current periph code is renamed to "gpio_*_cpu". IMO this is good. (although I'd prefer "gpio_cpu_*", but let's talk concept first).

- the interception happens in wrapper functions named like the legacy "gpio_*" API (where else? so good.)

- If "gpio_ext" is not used, the redirection API directly calls "gpio_*_cpu" (good, optimization should make sure that existing applications lead to mostly the same binary code).

So far, so good.

- if gpio_ext_* is compiled in, should the pin number be above an MCU specific threshold, instead of calling the CPU functions, "gpio_*_redir" is called.
- gpio_*_redir calls gpio_ext_pin/dev to get driver and pin, then calls the corresponding (typical C OO) driver
- gpio_ext_dev get's the driver's "ops" struct by consulting a const static list

I propose the following:

- we rename the current gpio_t struct to "gpio_cpu_t"
- if gpio_ext is not compiled in, gpio_t is typedef'ed to gpio_cpu_t, and redirection is direct as above
- else we define "gpio_t as omething like this:

    typedef struct {
      gpio_ext_t *driver;
      union {
        unsigned pin;
        gpio_cpu_t cpu_pin;
    } gpio_t;

- in the dispatch functions, if "driver" is NULL we assume gpio_cpu_* and call those functions with the "cpu_pin" parameter
- else, we directly call "driver->get/set/...(gpio_t, ...)"

I think this way it might be possible to save quite a lot of the redirection code. It would remove the necessity for (MCU specific) "GPIO_EXT_THRESH". It would allow creation of portable GPIO_PIN(GPIO_EXT_A, pin-nr) macros. It would remove the need for a pin->driver mapping.

@ZetaR60 @gschorcht what do you think?

Another slightly more complicated option would be to have an MCU specific "gpio_is_ext(gpio_t)" function and use that instead of "driver==NULL" to do the redirection. That way gpio_*_cpu could make use of the driver field.

Another idea, if we introduce "gpio_ext", would be to go all the way and introduce "gpio_port_*", exposing whole ports even for the MCU internal gpio ports.

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190108/c5c2526b/attachment-0001.html>

More information about the notifications mailing list