Re: [PATCH v1 07/12] ArmPkg: Fix return value for ArmGicV2AcknowledgeInterrupt

["Sami Mujawar" <sami.mujawar@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 1647 bytes --]

Hi Pedro,

Thank you for the feedback.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 23/05/2023 02:32 pm, Pedro Falcato wrote:
> On Tue, May 23, 2023 at 2:04 PM Sami Mujawar<sami.mujawar@arm.com>  wrote:
>> The IAR register of the Gic CPU interface is 32 bit, while
>> the value returned by ArmGicV2AcknowledgeInterrupt() is
>> UINTN. Therefore, typecast the return value to UINTN before
>> returning.
> Since IAR is 32-bit, doesn't it make sense to return UINT32 here? Is
> this for compatibility with GICv3 or...?

[SAMI] IAR in GICv3 is 32-bit as well. I think this is done to keep 
compatibility with HARDWARE_INTERRUPT_SOURCE which is UINTN.

>> Signed-off-by: Sami Mujawar<sami.mujawar@arm.com>
>> ---
>>   ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
>> index f403bec367b5254c248e620e56471904e520f9f2..80115b243afabd5e4faad88089af738b19ce4cd1 100644
>> --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
>> +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c
>> @@ -16,7 +16,7 @@ ArmGicV2AcknowledgeInterrupt (
>>     )
>>   {
>>     // Read the Interrupt Acknowledge Register
>> -  return MmioRead32 (GicInterruptInterfaceBase + ARM_GIC_ICCIAR);
>> +  return (UINTN)MmioRead32 (GicInterruptInterfaceBase + ARM_GIC_ICCIAR);
>>   }
> You don't need to cast from UINT32 to UINTN IMO. UINTN is, as far as I
> know, always going to be at least 32-bits, so casting makes little
> sense here, in my view, as UINTN >= UINT32.
[SAMI] Ack. I will drop this patch.
>

[-- Attachment #2: Type: text/html, Size: 2897 bytes --]