enhance gpio periphal driver

[punchcard60]

gpio.h in $(RIOTBASE)/drivers/include/periph should define mechanisms for setting the pin to high impedance mode (tristate) and a mechanism to define a pin as "open source" (aka open collector).

[haukepetersen]

When initially designing this interface, I did not include these options as they are not available on all platforms and as I wanted to keep out everything that might bloat this interface (to keep it as slim and easy to use as possible). But there were also some earlier discussions with @PeterKietzmann about integrating this.

Now that I see it, most MCUs actually let you choose at least between push-pull and open-drain when in output mode. Tristate I have actually not yet seen (though I might not have looked very hard). In the contrary to the pull resistor selection, these modes are only applicable when in output mode.

So the question is how we could integrate this so it doesn't bloat the interface and so that it can be very efficiently implemented. I see these options:
1a. Add this as parameter to the init function:

int gpio_init(gpio_t pin, gpio_dir_t dir, gpio_omode_t omode, gpio_pp_t pull);
with
typedef enum {
    GPIO_OMODE_PUSHPULL,
    GPIO_OMODE_OPENDRAIN,
    GPIO_OMODE_TRISTATE,
} gpio_omode_t;

1b. Integrate these options into the dir parameter:

typedef enum {
    GPIO_DIR_IN = 0,    
    GPIO_DIR_OUT_PP,
    GPIO_DIR_OUT_OD,
    GPIO_DIR_OUT_TS,
} gpio_dir_t;

1. Add a special function for this and document that this function should only be used when pin is configured to be output

void gpio_outmode(gpio_t pin, gpio_omode_t mode);

Looking at this I would probably prefer option 1b, though it might take some beauty from some gpio initialization code...

@kaspar030, @gebart, @jfischer-phytec-iot, @PeterKietzmann: what are your opinions on this?

[PeterKietzmann]

I do also prefer 1b. It doesn't touch and bloat the API. Maybe we should leave GPIO_DIR_OUT in as a default parameter for not so experienced users (and document this properly)

[punchcard60]

I like 1b the best as well, but I think _PP, _OD, and _TS could be made clearer with comments..

typedef enum {
    GPIO_DIR_IN = 0,    
    GPIO_DIR_OUT_PP, /* Push Pull - normally driven output pin */
    GPIO_DIR_OUT_OD, /* Open Drain or Open Collector */
    GPIO_DIR_OUT_TS, /* TriState or high impedance */
} gpio_dir_t;

[haukepetersen]

I like 1b the best as well, but I think _PP, _OD, and _TS could be made clearer with comments..

of course... Just did not add them in the 'pseudo-code' above...

But seems like we have a tendency here, lets hear if there are different opinions.

[sgso]

1b is the natural choice imho. I'd like to see a GPIO_DIR_INVALID value as well, so that one could define:

typedef enum {
    GPIO_DIR_IN = 0,    
    GPIO_DIR_OUT_PP = 1,
    GPIO_DIR_OUT_OD = 0xF,
    GPIO_DIR_OUT_TS = 0xF,
    GPIO_DIR_INVALID = 0xF
} gpio_dir_t;

and application code could figure out the implemented modes this way. May sound like a weird request and useless in most code but I'm thinking about portable shell commands interacting with GPIOs.

[punchcard60]

Oops! Didn't think this through. Tristate is the pin defined as OD and a value set to 1.

typedef enum {
    GPIO_DIR_IN = 0,     /* input pin */
    GPIO_DIR_OUT = 1,    /* normal push-pull output pin */
    GPIO_DIR_OUT_OD = 2  /* open drain or open collector pin */
} gpio_dir_t;

This is a global definition so GPIO_DIR_OUT_OD is always defined even if the hardware won't support it. Calling gpio_init() function to set a pin as OD when hardware doesn't support it should return -1.

[DipSwitch]

I vote for 1b aswell :D

[haukepetersen]

@punchcard60: thought there was something funny in defining a tri-state output mode...

@sgso: IMHO an invalid dir makes no sense here: why should you call gpio_init(xxx, INVALID, ...)? When writing something like the shell command you describe, you can just call gpio_init with OD and PP and look at the return values if which modes are applicable.

Concerning a default value: I would suggest to define something like this:

typedef enum {
    GPIO_IN = 0,    
    GPIO_OUT = 1,
    GPIO_OUT_PP = 1,
    GPIO_OUT_OD = 2,
} gpio_dir_t;

where GPIO_OUT and GPIO_OUT_PP have the same value, ergo GPIO_OUT_PP is the default... I would also tend to drop the _DIR_ in the enum names, as this is more consistent with other GPIO config enums (e.g. flank and pull resistor config).

[punchcard60]

@haukepetersen I agree with the desire to drop the redundant DIR, but if we preserve

GPIO_DIR_IN = 0,     /* input pin */
GPIO_DIR_OUT = 1,    /* normal push-pull output pin */

then no existing code will be affected.

[haukepetersen]

True, but at the current state, there is not too much code that is affected, and we should make sure not to start piling up legacy code already...

[jnohlgard]

I would like to argue that in option 1b, with new values for direction out with open drain, we should also do away with the pushpull parameter to gpio_init and expand the GPIO_DIR_IN argument to include GPIO_DIR_IN_PULL_UP, GPIO_DIR_IN_PULL_DOWN. This in order for the API to be consistent with itself. I don't see why there would need to be a pushpull parameter for an output pin since there is no point in pulling the signal towards either end if it is driven by the MCU. If the MCU silicon does allow the configuration then it would only add a bit of unnecessary current burn.

[haukepetersen]

Hm, if you configure the pin to open-drain output, a pull-up resistor does still make sense and is a vaild use-case, right?

so let's see what could make sense:

input: pull-up, pull-down, no-pull
output (push-pull): no-pull
output (open-drain): pull-up, no-pull

So all in all, I see 6 valid GPIO configurations:

GPIO_IN
GPIO_IN_PULLUP
GPIO_IN_PULLDOWN
GPIO_PP
GPIO_OD
GPIO_OD_PULLUP

do you agree with these modes?

Then further I agree that it would make sense, to get rid of the pull resistor parameter for GPIO init. But next question: how do we treat gpio_init_int? Here we still need the pull parameter I guess.

[haukepetersen]

ping @Everone: how should we proceed on this? Is there an agreement to go with this changed GPIO interface:

gpio_init(gpio_t pin, gpio_mode_t mode);

typedef enum {
    GPIO_IN,
    GPIO_IN_PULLDOWN,
    GPIO_IN_PULLUP,
    GPIO_PP,
    GPIO_OD,
    GPIO_OD_PULLUP
} gpio_mode_t;

We should try to decide on the change and implement it before the next release!

[PeterKietzmann]

I'm fine with this solution. For gpio_init_int() I think we can go with the first three options and document it properly.

[jnohlgard]

I agree with @PeterKietzmann. Let's not complicate matters by introducing a separate parameter that is only used on gpio_init_int, but rather check the parameter that the user isn't trying to do init_int with an output mode.
Also, I'd like the names for the modes to be a bit more verbose, GPIO_PP doesn't really say much about the functionality without having read this discussion thread beforehand.

[PeterKietzmann]

IMO GPIO_PUSHPULL, GPIO_OPENDRAIN and GPIO_OPENDRAIN_PULLUPare acceptable as well

[haukepetersen]

Actually I would opt for keeping the names short. I think in the context of GPIO the abbreviations are quite sufficient and clear, but my reception might be clouded by looking at this stuff for too long :-)

we could maybe write GPIO_OUT instead of GPIO_PP, as this might be easier to understand for beginners (and OD mode is more of an advanced feature anyway :-))

[PeterKietzmann]

Well, I agree in that point. Having simple GPIO_IN as well as GPIO_OUT could make sense. Concerning long or short form of PP or OD I don't really care as long as it's documented. So I leave the decision up to you :-)

[haukepetersen]

then I choose this naming:

typedef enum {
    GPIO_IN,
    GPIO_IN_PD,
    GPIO_IN_PU,
    GPIO_OUT,
    GPIO_OD,
    GPIO_OD_PU
} gpio_mode_t;

short, precise and as the context is fixed, I would argue that these types should suffice. @gebart are you ok with this?

[jnohlgard]

@haukepetersen 👍

[haukepetersen]

nice, I have already started on a PR, but as this touches all CPUs this might take a little while, hope to get it done by the end of the week.

[jnohlgard]

@haukepetersen I will try to review #4830 ASAP to allow you to base your new PR on that rewrite

[haukepetersen]

very nice!

[haukepetersen]

Fix for this is done: #4862. This is gonna be a constant rebase target :-)

[lebrush]

@haukepetersen for stm32l1 open drain pulldown (as output) is also a valid combination. So we need an extra GPIO_OD_PD

[jnohlgard]

@lebrush how does that work in practise? is the pin driven in the 1 state and high impedance in the 0 state?

[lebrush]

I just happened to have the user manual open on the GPIO page and the functional description states:

each port bit of the general-purpose I/O (GPIO) ports can be individually configured by software in several modes:
• Input floating
• Input pull-up
• Input-pull-down
• Analog
• Output open-drain with pull-up or pull-down capability
• Output push-pull with pull-up or pull-down capability
• Alternate function push-pull with pull-up or pull-down capability
• Alternate function open-drain with pull-up or pull-down capability

That's why I mention it, if this MCU supports this combination others might do it as well. Right now I've no clue how it works and can't think of a possible use case...

[haukepetersen]

Technical speaking does OD with PD not make much sense. Though I don't doubt that some MCUs do support it, that does not mean that it does make sense to use it (same for push-pull with pull resistor -> simply uses more energy...).

[jnohlgard]

@haukepetersen It is possible, but I guess it would be called an open-source circuit, not open-drain.

if you remove the lower transistor and replace it with a pull-down resistor the behaviour should be to drive the 1 and HiZ the 0.

[jnohlgard]

Though I don't know if any MCU actually has this capability or if it just allows setting a pull-down resistor in parallel with the lower transistor in the figure above.

[haukepetersen]

Asking google about 'open-source output' is not very helpful :-)

Though I can electrically draw up how an 'open-drain with high-side FET and pull-down' would look like, I have never seen this in any circuit/MCU before. But that does not necessarily mean much :-)

I am a little bit undecided on this issue - I think I tend to hold it with the good old: 'let's care about this once somebody really needs to use this'... I don't like adding options to the API because there is a change that some point in time somebody might need this is.

[jnohlgard]

@lebrush do you have a use case or a need for an open drain with pull down GPIO?

[lebrush]

I was not expecting to get such attention in the topic and didn't think so much about it. As I said by chance I had that page open of the user guide 😄

I agree with @haukepetersen and the code in #4862 looks generic enough to allow adding it if anyone ever needs it. Good for me!

[haukepetersen]

cool, then we leave it as is for now and don't hesitate to open an issue/PR once the need for the OD-PD mode arrives :-)

[haukepetersen]

As #4862 was merged some time ago, I consider this issue closed.

@punchcard60 please re-open of you disagree.