Re: [Xen-devel] [PATCH 02/12 v3] xen/arm: vpl011: Define generic vreg_reg* access functions in vreg.h

[Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>]

Hi Julien,


>>
>> -/*
>> - * 64 bits registers are only supported on platform with 64-bit long.
>> - * This is also allow us to optimize the 32 bit case by using
>> - * unsigned long rather than uint64_t
>> - */
>> -#if BITS_PER_LONG == 64
>> -VGIC_REG_HELPERS(64, 0x7);
>> -#endif
>> -VGIC_REG_HELPERS(32, 0x3);
>> -
>> -#undef VGIC_REG_HELPERS
>> -
>
I will move this to patch #1.

>
> Why this is moved out here? And not in patch #1?
>
>>  enum gic_sgi_mode;
>>
>>  /*
>> diff --git a/xen/include/asm-arm/vreg.h b/xen/include/asm-arm/vreg.h
>> index 1442c58..e127114 100644
>> --- a/xen/include/asm-arm/vreg.h
>> +++ b/xen/include/asm-arm/vreg.h
>
>
> [...]
>
>> -{                                                                       \
>> -    unsigned long tmp = *reg;                                           \
>> -                                                                        \
>> -    vgic_reg_clearbits(&tmp, bits, info->gpa & offmask,                 \
>> -                       info->dabt.size);                                \
>> -                                                                        \
>> -    *reg = tmp;                                                         \
>> +#define DEFINE_VREG_REG_HELPERS(sz, offmask)
>> \
>
>
> Nack. There are no reason to rename the name of the macro more than
> s/vgic/vreg/. It also makes to patch diff more difficult to read.
>
I will rename it back to VREG_REG_HELPERS.

Regards,
Bhupinder

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