Re: [PATCH v4 11/16] xen/arm: calculate vgic irq rank based on register size

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

On 30/05/14 11:24, Vijay Kilari wrote:
> On Fri, May 30, 2014 at 3:28 PM, Julien Grall <julien.grall@linaro.org> wrote:
>> Hi Vijay,
>>
>>>>>
>>>>>        case GICD_ISENABLER ... GICD_ISENABLERN:
>>>>> -        if ( dabt.size != 2 ) goto bad_width;
>>>>> -        rank = vgic_irq_rank(v, 1, gicd_reg - GICD_ISENABLER);
>>>>> +        if ( dabt.size != DABT_WORD ) goto bad_width;
>>>>> +        rank = vgic_irq_rank(v, 1, gicd_reg - GICD_ISENABLER,
>>>>> DABT_WORD);
>>>>
>>>>
>>>> In your commit message you explicitly say that use DABT_* will help you
>>>> to get the register offset but... you still hardcode the size.
>>>>
>>>> Why can't you use dabt.size here? And all the other places.
>>>
>>>
>>>      dabt.size gives the current register access size but not the actual
>>> register size.
>>
>>
>> In this specific case, the register access size and the actual register size
>> is the same...
>
> Yes, in most of the cases it is same. But there are some register
> access that supports
> both byte and word size access. In that case we have to choose always
> the register size DABT_*
>
> To be consistent I have not used dabt.size.  In case if byte access to
> particular register
> is added then one can go wrong.

With your explanation, I don't see any reason to replace all the 
dabt.size != number by dat.size != DABT_*.

Regards,

-- 
Julien Grall