GPIO interrupt fixes for atmega328p #7590
Conversation
lebrush
added
Platform: AVR
cpu/atmega_common/periph/gpio.c
Outdated
| @@ -148,6 +148,10 @@ int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank, | |||
| /* clear global interrupt flag */ | |||
| cli(); | |||
|
|
|||
| #ifdef CPU_ATMEGA328P | |||
| pin_num -= 2; | |||
There was a problem hiding this comment.
I don't question this, but could you please give a short reference/reasoning for this? Thx 😄
There was a problem hiding this comment.
Interrupt pins in atmega25.../12... are starting at 0 (e.g. PD0) but in atmega328 start at 2 (e.g. PC2). I'll add a comment :-) 👍
| GPIO_LOW = 0, /**< emit interrupt when pin low */ | ||
| GPIO_BOTH = 1, /**< emit interrupt on both flanks */ | ||
| GPIO_FALLING = 2, /**< emit interrupt on falling flank */ | ||
| GPIO_RISING = 3, /**< emit interrupt on rising flank */ |
There was a problem hiding this comment.
minor nit (and personal preference by me): either omit explicit definition of values or align them - looks nicer 😉
There was a problem hiding this comment.
Good point, will change 👍
lebrush
force-pushed
the
fix/atmega-gpio-misc
branch
from
5a65ca3
to
f6b6434
Compare
|
@smlng comments addressed |
| /* INT pins start at PD2 instead of at PD0 */ | ||
| pin_num -= 2; | ||
| #endif | ||
|
|
||
| EIMSK |= (1 << pin_num); |
There was a problem hiding this comment.
if I read the data sheet correctly there is only 0 and 1 allowed as pin_num for atmega328, maybe add an assert here?
There was a problem hiding this comment.
The previous assert already filters anything higher than 3 (line 138) maybe it's a good idea to add a check the lower bound as well. Will do 👍
cpu/atmega_common/periph/gpio.c
Outdated
| if (pin_num < 4) { | ||
| EICRA |= (1 << pin_num * 2); | ||
| } | ||
| if (flank > 3) { |
There was a problem hiding this comment.
3 looks like a magic number here, should be GPIO_FLANK_RISING or alternatively define GPIO_FLANK_MAX and check (flank < GPIO_FLANK_MAX).
lebrush
force-pushed
the
fix/atmega-gpio-misc
branch
from
f6b6434
to
1dd4b2e
Compare
lebrush
force-pushed
the
fix/atmega-gpio-misc
branch
from
1dd4b2e
to
6365466
Compare
|
@smlng all comments addressed. Also added extra commit fixing filter for other Atmegas. |
|
maybe squash? |
smlng
added
the
CI: ready for build
|
And go. The three commits tackle three different issues :-) |
One commit fixes the interrupt set-up and the other supports one more flank and compacts the code.