Re: [PATCH v7 1/5] xen/arm: Add support for GIC v3

[Julien Grall <julien.grall@linaro.org>]

On 22/07/14 11:43, Ian Campbell wrote:
> On Tue, 2014-07-22 at 11:01 +0100, Julien Grall wrote:
>>
>> On 22/07/14 10:48, Vijay Kilari wrote:
>>>>> +/*
>>>>> + * Additional registers defined in GIC v3.
>>>>> + * Common GICD registers are defined in gic.h
>>>>> + */
>>>>> +
>>>>> +#define GICD_STATUSR                 (0x010)
>>>>> [...][
>>>>> +#define GICV3_GICD_PIDR0             (0x92)
>>>>
>>>> What is the distinction between variables with GIC[DR]_ prefixes and
>>>> those with GICV3_GIC[DR]_ ones?
>>>
>>> GICV3 is prefixed for indicating that there are values not the addresses.
>>> In anycase I will remove GICV3 prefixes and postfix _VAL
>>
>> Those value are GICV3 specific. If you drop the prefix we won't know
>> what are their purpose...
>
> I'm not so sure that's the case. We will know it is the value to use for
> GICD_PIDR0 for any GIC which includes that register, which is at least
> v3 and v4 right now. Calling it V3 is equally misleading as leaving it
> out.
> Given that we've decided to share the #defines across versions I think
> we should leave the prefix off. The alternative is to make sure
> everything is prefixed and to duplicate the definitions for each
> version, which is an approach we previously moved away from I think, I
> don't see a strong reason to go back on that decision now.


On GICv2, this field is called ICPIDR0 (same register offset) and is 
equal to 0x90.

If those values are only used for the vgic v3 driver, then they should 
live in the c files and not in the common header.

Regards,

-- 
Julien Grall