qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jean-Christophe DUBOIS
Subject: Re: [Qemu-devel] [PULL 2/8] i.MX: Implement a more complete version of the GPT timer.
Date: Wed, 26 Jun 2013 20:58:11 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6

On 06/26/2013 09:21 AM, Peter Crosthwaite wrote:
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?
Peter (Maydell),

How do you want to work this out?

Do you revert it and we start over?

Or should I provide a patch on top of the actual file to change the "wrong name"?

Please advise.

JC


Regards,
Peter

Paolo






reply via email to

[Prev in Thread] Current Thread [Next in Thread]