[En-Nut-Discussion] STM32 share EXTI can not work properly, need some fix below
Harald Kipp
harald.kipp at egnite.de
Thu Sep 24 21:03:17 CEST 2015
Hi,
On 20.09.2015 06:03, kontais wrote:
> - int port_nr = (port & 0x3fff) >> 10;
> + int port_nr = (port-GPIOA_BASE) >> 10;
Much better than a dubious hardcode mask 0x3ff. Just one thing I don't
like: It should be "(port - GPIOA_BASE)". See
http://www.ethernut.de/en/documents/programming-style-guide.html
part "Spaces and operators".
> sig = malloc(sizeof(GPIO_SIGNAL));
> if (sig) {
> int rc;
> @@ -248,18 +248,20 @@
> GPIO_SIGNAL *sig_chain;
> sig_chain = isrhandler->ir_arg;
> while (sig_chain) {
> - if(sig_chain->ios_pin != bit) {
> - /* Pin is not already mapped */
> - if (0 == sig_chain->sig_next) {
> - /* we have a free slot*/
> - /* Clear any pending interrupts and mask */
> - EXTI->PR = (1 << bit);
> - EXTI->IMR &= ~(1 << bit);
> - sig_chain->sig_next = sig;
> - rc = 0;
> - break;
> - }
> + /* Pin is already mapped */
> + if (sig_chain->ios_pin == bit) {
> + break;
> }
Added a comment and avoid too much intending, very good.
> + /* Last one */
> + if (sig_chain->sig_next == NULL) {
Follows the Nut/OS coding rules (we never use '1 == x', always 'x == 1')
and yes, sig_next is a pointer and should be compared to a pointer
(NULL) instead of an integer (0).
However, refactoring and program flow changes should never come with the
same patch.
Regards,
Harald
More information about the En-Nut-Discussion
mailing list