[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