[PATCH 0/1] grlib/genirq: Issue when enabling/disabling interrupt

[Daniel Hellstrom]

Hi Jan,

I think it is a correct analysis of the bad return value, when using 
shared interrupt routines on the same interrupt source. I'm fine with 
the patch, maybe it could have been simplified, like this:

+++ b/bsps/shared/grlib/irq/genirq.c
@@ -188,8 +188,8 @@ static int genirq_set_active(
                                 return 1;
                         }
                         e = isrentry;
-               } else {
-                       enabled += isrentry->enabled;
+               } else if (isrentry->enabled) {
+                       enabled = 1;
                 }
                 isrentry = isrentry->next;
         }

Kind Regards,
Daniel

On 2021-04-15 09:07, Jan.Sommer at dlr.de wrote:
> Does someone have any objections to this patch?
> I would like to push it to master and 5.
> Or should we reach out to someone at Gaisler to check if it is ok?
>
> Best regards,
>
>      Jan
>
>> -----Original Message-----
>> From: devel <devel-bounces at rtems.org> On Behalf Of
>> Gabriel.Moyano at dlr.de
>> Sent: Tuesday, April 13, 2021 10:09 AM
>> To: Moyano Heredia, Victor Gabriel <Gabriel.Moyano at dlr.de>;
>> gedare at rtems.org
>> Cc: devel at rtems.org
>> Subject: AW: [PATCH 0/1] grlib/genirq: Issue when enabling/disabling
>> interrupt
>>
>>> Yes, rtems 5 also has it. I can create a ticket.
>> Here is the link to the ticket https://devel.rtems.org/ticket/4385#ticket.
>> BTW the proposed solution for the master branch can be also applied to 5 as
>> well.
>>
>>> Regarding the proposed solution, I wanted to start a thread for discussing it
>> (maybe there is better was to do it).
>>>
>>>> Is this a bug in rtems 5 also?
>>>>
>>>> If so, does it warrant a back-port fix and a ticket?
>>>>
>>>> On Mon, Apr 12, 2021 at 3:16 AM Moyano, Gabriel
>> <gabriel.moyano at dlr.de> wrote:
>>>>> Hello everyone,
>>>>>
>>>>> I've found what can be an issue in the function genirq_set_active():
>> under some conditions it can return a value greater than 1.
>>>>> This function is used by genirq_enable() and genirq_disable() and both
>> of them returns the value returned by genirq_set_active(). According to the
>> documentation in genirq.h, they should return -1, 0 or 1.
>>>>> When this issue can happen? If there are 3 entries in the list of IRQ and 2
>> of them are already enabled, the variable `enabled` would be 2, because of
>> `enabled += isrentry->enabled`.
>>>>> As a possible solution, the value of `enabled` can changed to 1 if it's
>> greater than 1 (see the patch) or maybe improve the search.
>>>>> Thanks in advance,
>>>>>
>>>>> Gabriel Moyano
>>>>>
>>>>>
>>>>>
>>>>> Moyano, Gabriel (1):
>>>>>    grlib/genirq: Taking into account that it could be more than one ISR
>>>>>      enabled/disabled
>>>>>
>>>>>   bsps/shared/grlib/irq/genirq.c | 5 +++++
>>>>>   1 file changed, 5 insertions(+)
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel