Re: [Qemu-devel] [PULL 2/8] i.MX: Implement a more complete version of the GPT timer.

[Peter Crosthwaite]

Hi

On Wed, Jun 26, 2013 at 4:56 PM, Paolo Bonzini <address@hidden> wrote:
> Il 25/06/2013 22:53, Peter Maydell ha scritto:
>> On 25 June 2013 19:42, Paolo Bonzini <address@hidden> wrote:
>>> Il 25/06/2013 20:21, Peter Maydell ha scritto:
>>>> @@ -416,7 +513,7 @@ static int imx_timerg_init(SysBusDevice *dev)
>>>>
>>>>      sysbus_init_irq(dev, &s->irq);
>>>>      memory_region_init_io(&s->iomem, &imx_timerg_ops,
>>>> -                          s, "imxg-timer",
>>>> +                          s, TYPE_IMX_GPT,
>>>>                            0x00001000);
>>>>      sysbus_init_mmio(dev, &s->iomem);
>>>>
>>>
>>> There was some agreement that this is not a good change.
>>
>> I agree (and more so regarding the use of the macro in the
>> vmstate name), but nobody actually posted any comment to
>> that effect against any of the versions of this patch that
>> got sent out for review...
>
> Yeah, the timing was bad... Can you post a revert, though?
>

The original string being replaced was a poor choice as well. IIUC the
consensus was string field of the memory regions is supposed to
indicate the purpose of the memory region for the device. Good
examples would be "Control regs" or "MMIO". Naming the memory region
after the device type is a redundancy as that info will come via
memory region owners.

So rather than revert could you just choose something more descriptive?

Regards,
Peter

> Paolo
>