Re: [Xen-devel] [PATCH v3 07/16] xen/arm: segregate and split GIC low level functionality

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 04/23/2014 04:01 PM, Julien Grall wrote:
> On 04/23/2014 03:55 PM, Ian Campbell wrote:
>> On Tue, 2014-04-15 at 19:35 +0100, Julien Grall wrote:
>>>> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
>>>> index eba41ee..2387e38 100644
>>>> --- a/xen/include/asm-arm/gic.h
>>>> +++ b/xen/include/asm-arm/gic.h
>>>> @@ -43,12 +43,41 @@
>>>>  #define SGI_TARGET_OTHERS  1
>>>>  #define SGI_TARGET_SELF    2
>>>>  
>>>> +#define GICH_LR_PENDING    1
>>>> +#define GICH_LR_ACTIVE     2
>>>> +
>>>
>>> Prefixing by GICH_ is confusing. I though we were using it for to set
>>> the value in the hardware. Can you rename it?
>>
>> This is code motion of an existing definition, definitely please don't
>> rename it at the same time.
>>
>> But anyway, these are bits relating to the v2 GICH_LR register, which is
>> the same on v3 as it happens. So I think the names are fine.
> 
> Reading again the code, you are right. I was confused then of the value
> and the position at the same time.

After thinking it would be nice to keep the shift in it smth like:

#define GICH_LR_PENDING (1 << 0)
#define GICH_LR_ACTION  (1 << 1)

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel