cpu, cc2538: gpio helper functions #7316
Conversation
smlng
added
Type: cleanup
smlng
force-pushed
the
cpu/cc2538/periph_gpio
branch
from
873d309
to
4d103cd
Compare
smlng
force-pushed
the
cpu/cc2538/periph_gpio
branch
from
4d103cd
to
69f1584
Compare
smlng
added
the
CI: ready for build
cpu/cc2538/include/periph_cpu.h
Outdated
| @@ -14,6 +15,7 @@ | |||
| * @brief CPU specific definitions for internal peripheral handling | |||
| * | |||
| * @author Hauke Petersen <[email protected]> | |||
| * @author Sebastian Meiling <[email protected]> | |||
cpu/cc2538/include/periph_cpu.h
Outdated
| { | ||
| return (int)(pin & PIN_MASK); | ||
| } | ||
| /** |
| } | ||
|
|
||
| /** | ||
| * @brief Helper function to enable gpio software control |
There was a problem hiding this comment.
Helper function to disable gpio software control ?
There was a problem hiding this comment.
no? it disables hardware control, as it is the reverse of gpio_hw_ctrl() above.
cpu/cc2538/include/periph_cpu.h
Outdated
| static inline int gpio_port_num(gpio_t pin) | ||
| { | ||
| return (int)((pin & PORTNUM_MASK) >> PORTNUM_SHIFT) - 1; | ||
| } |
There was a problem hiding this comment.
The return value of this function is for any gpio pin -1. Detected by a hardware test.
There was a problem hiding this comment.
I can't reproduce that. I added debug output to gpio_init, you can try tests/periph_gpio for example and init one of the LEDs with init_put 3 4 you will see debug output showing real pin address (uint32_t) as well as port and pin recovered from that. For me looks like that:
> init_out 3 4
2017-07-06 16:52:33,730 - INFO # init_out 3 4
2017-07-06 16:52:33,731 - INFO # GPIO 1074642948, PORT: 3, PIN: 4
cpu/cc2538/include/periph_cpu.h
Outdated
| static inline int gpio_pp_num(gpio_t pin) | ||
| { | ||
| return (gpio_port_num(pin) * 8) + gpio_pin_num(pin); | ||
| } |
There was a problem hiding this comment.
The return value of this function is because of gpio_port_num() wrong.
| } | ||
|
|
||
| /** | ||
| * @brief Helper function to enable gpio software control |
cpu/cc2538/include/periph_cpu.h
Outdated
| */ | ||
| static inline uint8_t gpio_pin_num(gpio_t pin) | ||
| { | ||
| return (int)(pin & PIN_MASK); |
There was a problem hiding this comment.
The conversion is wrong. It should be uint8_t.
smlng
force-pushed
the
cpu/cc2538/periph_gpio
branch
from
75e2c23
to
e946725
Compare
|
squashed, lets see if murdock gets happy |
|
@PeterKietzmann I assume @dnahm is not allow[ed] to(officially) approve for RIOT PRs [yet], correct? Can you help and proxy-approve - this PR is kind of important for all follow ups, which I'd like to rebase after this merge. |
There was a problem hiding this comment.
Code-wise looks fine and @dnahm approved testing. ACK and go
|
Hej, nice effort cleaning up the cc2538 code after all. But I fear this PR is going only half the way and is not really helping in separating concerns: the actual solution for simplification and stricter separation of periph drivers is not to export internal GPIO driver functionality to other GPIO drivers, but to extend the interface locally with CPU internal functions for configuring pins in alternate function mode and so on. Simply have a look at e.g. |
|
@haukepetersen I'm not sure if I completely get what you mean, but I guess you meant to hide such things in the I2C driver: here? |
|
exactly. For the concrete situation, I would imagine the i2c driver doing something like: ...
gpio_init_af(i2c_config[dev].scl, IOC_I2CMSSCL, I2C_CMSSCL, IOC_OVERRIDE_PUE);
gpio_init_af(i2c_config[dev].sda, IOC_I2CMSSDA, I2C_CMSSDA, IOC_OVERRIDE_PUE);This way the gpio init code in the i2c driver is simplified a bit, and more importantly, the |
|
ah, thanks for clarification! I'll mark the other periph driver reworks as WIP and will adapt ... |
|
nice, thanks! |
this PR make some periph-gpio helper function accessible by other periph drivers of CC2538, such as I2C, SPI, and UART. Follow up PRs will adapt named periph drivers to use RIOTs gpio API instead of the cc2538 internal API, to simplify usage and periph_conf.