[PATCH 2/2] bsps: Remove uses of BSP-specific interrupt API

[Chris Johns]

On 16/6/2023 3:36 pm, Sebastian Huber wrote:
> On 16.06.23 07:04, Chris Johns wrote:
>> On 16/6/2023 2:51 pm, Sebastian Huber wrote:
>>> On 16.06.23 03:49, Chris Johns wrote:
>>>>> diff --git a/bsps/shared/grlib/drvmgr/ambapp_bus_grlib.c
>>>>> b/bsps/shared/grlib/drvmgr/ambapp_bus_grlib.c
>>>>> index 96b77907a6..bc211e37b6 100644
>>>>> --- a/bsps/shared/grlib/drvmgr/ambapp_bus_grlib.c
>>>>> +++ b/bsps/shared/grlib/drvmgr/ambapp_bus_grlib.c
>>>>> @@ -41,7 +41,7 @@
>>>>>    #include <grlib/genirq.h>
>>>>>      #include <bsp.h>
>>>>> -#include <bsp/irq.h>
>>>>> +#include <bsp/irq-generic.h>
>>>>>      #include <grlib/grlib_impl.h>
>>>>>    @@ -227,7 +227,7 @@ static int ambapp_grlib_int_clear
>>>>>        struct drvmgr_dev *dev,
>>>>>        int irq)
>>>>>    {
>>>>> -    BSP_shared_interrupt_clear(irq);
>>>>> +    (void) rtems_interrupt_clear(irq);
>>>>>        return DRVMGR_OK;
>>>> Why ignore the return code of the clear and assume the result is OK?
>>>>
>>>> This pattern is repeated in other places.
>>>
>>> This is how it is. I didn't want to introduce functional or API changes with
>>> this patch set.
>>
>> I do not see the API change.
>>
>> If the code here is looking through the interface provided to the implementation
>> so it knows no error will be returned then it is functionally equivalent to what
>> exists if the error is checked. Nothing changes. The problem with the change is
>> using the interface implies the error is being checked and a change in the
>> implementation to return an error would not be handled here. Leaving a latent
>> issue like that does not seem right?
> 
> The BSP_shared_interrupt*() functions have no return value and they didn't check
> anything. The patch was supposed to be a simple change from one API to another.
> I added some error checks to v2 of the patch set.

I appreciate this. The interfaces that return void have no where to go and that
is fine. The ones discussed and updated are different. I was not explicit about
this. The v2 means it is one less thing to deal with later.

Chris