Re: [Qemu-devel] [PATCH v8 6/8] register: Add block initialise helper

[Alistair Francis <alistair.francis@xilinx.com>]

On Mon, Jun 27, 2016 at 7:31 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 24 June 2016 at 16:42, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>> Add a helper that will scan a static RegisterAccessInfo Array
>> and populate a container MemoryRegion with registers as defined.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>
>> --- a/hw/core/register.c
>> +++ b/hw/core/register.c
>> @@ -228,6 +228,50 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
>>      return extract64(read_val, 0, size * 8);
>>  }
>>
>> +RegisterInfoArray *register_init_block32(DeviceState *owner,
>> +                                         const RegisterAccessInfo *rae,
>> +                                         int num, RegisterInfo *ri,
>> +                                         uint32_t *data,
>> +                                         const MemoryRegionOps *ops,
>> +                                         bool debug_enabled,
>> +                                         uint64_t memory_size)
>> +{
>> +    const char *device_prefix = object_get_typename(OBJECT(owner));
>> +    RegisterInfoArray *r_array = g_new0(RegisterInfoArray, 1);
>> +    int i;
>> +
>> +    r_array->r = g_new0(RegisterInfo *, num);
>> +    r_array->num_elements = num;
>> +    r_array->debug = debug_enabled;
>> +    r_array->prefix = device_prefix;
>> +
>> +    for (i = 0; i < num; i++) {
>> +        int index = rae[i].addr / 4;
>> +        RegisterInfo *r = &ri[index];
>> +
>> +        *r = (RegisterInfo) {
>> +            .data = &data[index],
>> +            .data_size = sizeof(uint32_t),
>> +            .access = &rae[i],
>> +            .opaque = owner,
>> +        };
>> +        register_init(r);
>> +
>> +        r_array->r[i] = r;
>> +    }
>> +
>> +    memory_region_init_io(&r_array->mem, OBJECT(owner), ops, r_array,
>> +                          device_prefix, memory_size);
>> +
>> +    return r_array;
>> +}
>> +
>> +void register_finlise_block(RegisterInfoArray *r_array)
>
> "finalize" (typo, and we prefer the -z- spelling in APIs.)

Sorry it's just a habit.

>
>> +/**
>> + * This function should be called to cleanup the registers that were initialized
>> + * when calling register_init_block32()
>> + *
>> + * @r_array: An structure containing all of the registers. The caller is in
>> + *           charge of cleaning up the memory region (r_array->mem).
>
> What cleanup does the memory region require?

The device init function that calls the register init function will
also call memory_region_add_subregion(). Doesn't it need to call
memory_region_del_subregion() afterwards?

Thanks,

Alistair

>
>> + */
>> +
>> +void register_finlise_block(RegisterInfoArray *r_array);
>> +
>>  /* Define constants for a 32 bit register */
>>
>>  /* This macro will define A_FOO, for the byte address of a register
>
> thanks
> -- PMM
>