Re: [Qemu-devel] [PATCH v4 13/21] target-arm: Use dedicated CPU state fields for ARM946 access bit registers

[Peter Maydell]

On 17 March 2014 05:20, Peter Crosthwaite <address@hidden> wrote:
> On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <address@hidden> wrote:
>>  static const ARMCPRegInfo pmsav5_cp_reginfo[] = {
>>      { .name = "DATA_AP", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
>>        .access = PL1_RW, .type = ARM_CP_NO_MIGRATE,
>> -      .fieldoffset = offsetof(CPUARMState, cp15.c5_data), .resetvalue = 0,
>> +      .fieldoffset = offsetof(CPUARMState, cp15.pmsav5_data_ap),
>> +      .resetvalue = 0,
>
> I know its just motion of existing code, but what's the policy on zero
> resets? Can we leave them out for brevity? Checkpatch complains when a
> global is explictly 0 initialized, so it seems sane that the same rule
> applies to individual fields (just checkpatch probably has hard time
> figuring this one out).

The semantics allow you to leave out .resetvalue if it is 0; I think
generally when I've written reginfo definitions I've tended to put
in the .resetvalue as an indication of "I know this reginfo needs
a reset value and it is zero" as opposed to "I didn't think about
reset when I wrote this".

We definitely don't want checkpatch to warn about explicit zero
initializers in structs like this, otherwise we wouldn't be
able to say ".crm = 0, .opc1 = 0, .opc2 = 0", which would be
less comprehensible than writing them out explicitly I think.

thanks
-- PMM