periph/gpio: merged dir and pushpull parameters #4862
Conversation
haukepetersen
added
Type: enhancement
| int res; | ||
|
|
||
| if (dir == GPIO_DIR_OUT) { | ||
| if (mode == GPIO_OUT) { |
There was a problem hiding this comment.
Using switch(mode){ case xyz:} instead of if() would improve the readability because of the similarities with other periph drivers.
There was a problem hiding this comment.
true. But it is true, that the compiler ends up with the same binary in this case, right?
There was a problem hiding this comment.
Yes, at least for a modern compiler such as gcc or clang I expect the generated machine code to be identical.
| int gpio_init_int(gpio_t pin, gpio_pp_t pullup, gpio_flank_t flank, | ||
| gpio_cb_t cb, void *arg) | ||
| int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank, | ||
| gpio_cb_t cb, void *arg) |
There was a problem hiding this comment.
remove the added whitespaces, alignment is messed up
haukepetersen
force-pushed
the
opt_periph_gpio_mode
branch
from
31ae5c7 to
08ce2a5
Compare
|
rebased and squashed |
haukepetersen
added
CI: ready for build
|
fixed limifrog |
haukepetersen
added
CI: ready for build
FYI, I asked Peter. We don't have one here. |
| typedef enum { | ||
| GPIO_IN = GPIO_MODE(0, 1), /**< input w/o pull R */ | ||
| GPIO_IN_PD = GPIO_MODE(0, 2), /**< input with pull-down */ | ||
| GPIO_IN_PU = GPIO_MODE(0, 2), /**< input with pull-up */ |
There was a problem hiding this comment.
It seems improbable that IN_PU and IN_PD should have the same value :) As in GPIO_IN_PD and GPIO_IN_PU would always have the same value now?
There was a problem hiding this comment.
nope, this is correct -> this only enables the pull resistor in general, see stm32f1/periph/gpio.c line 89/90, the output register value determines if PU or PD are used.
|
Pffeewwww long read 40 minutes or so haha, but seems ok apart from the F1 |
|
If the F1 is fixed ACK from my side :) |
|
The F1 is actually correct, Travis is happy -> go go go |
periph/gpio: merged dir and pushpull parameters
|
No because now the pull-up will always be enabled ;)
|
|
No, when you use |
| port->CR[pin_num >> 3] |= (mode << ((pin_num & 0x7) * 4)); | ||
| /* set initial state of output register */ | ||
| port->BRR = (1 << pin_num); | ||
| if (mode == GPIO_IN_PU) { |
There was a problem hiding this comment.
because
+ GPIO_IN_PD = GPIO_MODE(0, 2), /**< input with pull-down */
+ GPIO_IN_PU = GPIO_MODE(0, 2), /**< input with pull-up */
is the same value, this if statement is also true for PD :)
|
Yeah that's true, sorry for the mis-communication. I meant the the pull-up is always enabled, even for pull-down :) |
|
Oh, seems I wearing my pink glasses today, you are perfectly right! Will provide a fix ASAP |
|
haha np, happens on Friday ;) |
|
-> #5104 |
This PR solves #4472
Please have a look at the interface change first (commit 57ba62e) -> this is what came out of the discussion in #4472 (or what I interpreted of it :-) ).
I adjusted all CPUs except thekinetis_common. For the latter I am waiting on #4830 to prevent duplicate work...If you all agree, I guess some extensive testing is in order...