[PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state

Thomas Huth posted 1 patch 11 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231221122939.11001-1-huth@tuxfamily.org
Maintainers: Thomas Huth <huth@tuxfamily.org>
hw/m68k/mcf5206.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
[PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state
Posted by Thomas Huth 11 months, 1 week ago
There's no need to explicitely allocate the memory here, we can
simply embed it into the m5206_mbar_state instead.

Signed-off-by: Thomas Huth <huth@tuxfamily.org>
---
 hw/m68k/mcf5206.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
index a46a23538d..183fd3cc08 100644
--- a/hw/m68k/mcf5206.c
+++ b/hw/m68k/mcf5206.c
@@ -148,15 +148,11 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val)
     m5206_timer_update(s);
 }
 
-static m5206_timer_state *m5206_timer_init(qemu_irq irq)
+static void m5206_timer_init(m5206_timer_state *s, qemu_irq irq)
 {
-    m5206_timer_state *s;
-
-    s = g_new0(m5206_timer_state, 1);
     s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_LEGACY);
     s->irq = irq;
     m5206_timer_reset(s);
-    return s;
 }
 
 /* System Integration Module.  */
@@ -167,7 +163,7 @@ typedef struct {
     M68kCPU *cpu;
     MemoryRegion iomem;
     qemu_irq *pic;
-    m5206_timer_state *timer[2];
+    m5206_timer_state timer[2];
     DeviceState *uart[2];
     uint8_t scr;
     uint8_t icr[14];
@@ -293,9 +289,9 @@ static uint64_t m5206_mbar_read(m5206_mbar_state *s,
                                 uint16_t offset, unsigned size)
 {
     if (offset >= 0x100 && offset < 0x120) {
-        return m5206_timer_read(s->timer[0], offset - 0x100);
+        return m5206_timer_read(&s->timer[0], offset - 0x100);
     } else if (offset >= 0x120 && offset < 0x140) {
-        return m5206_timer_read(s->timer[1], offset - 0x120);
+        return m5206_timer_read(&s->timer[1], offset - 0x120);
     } else if (offset >= 0x140 && offset < 0x160) {
         return mcf_uart_read(s->uart[0], offset - 0x140, size);
     } else if (offset >= 0x180 && offset < 0x1a0) {
@@ -333,10 +329,10 @@ static void m5206_mbar_write(m5206_mbar_state *s, uint16_t offset,
                              uint64_t value, unsigned size)
 {
     if (offset >= 0x100 && offset < 0x120) {
-        m5206_timer_write(s->timer[0], offset - 0x100, value);
+        m5206_timer_write(&s->timer[0], offset - 0x100, value);
         return;
     } else if (offset >= 0x120 && offset < 0x140) {
-        m5206_timer_write(s->timer[1], offset - 0x120, value);
+        m5206_timer_write(&s->timer[1], offset - 0x120, value);
         return;
     } else if (offset >= 0x140 && offset < 0x160) {
         mcf_uart_write(s->uart[0], offset - 0x140, value, size);
@@ -598,8 +594,8 @@ static void mcf5206_mbar_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
 
     s->pic = qemu_allocate_irqs(m5206_mbar_set_irq, s, 14);
-    s->timer[0] = m5206_timer_init(s->pic[9]);
-    s->timer[1] = m5206_timer_init(s->pic[10]);
+    m5206_timer_init(&s->timer[0], s->pic[9]);
+    m5206_timer_init(&s->timer[1], s->pic[10]);
     s->uart[0] = mcf_uart_create(s->pic[12], serial_hd(0));
     s->uart[1] = mcf_uart_create(s->pic[13], serial_hd(1));
 }
-- 
2.43.0
Re: [PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state
Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago
On 21/12/23 13:29, Thomas Huth wrote:
> There's no need to explicitely allocate the memory here, we can
> simply embed it into the m5206_mbar_state instead.
> 
> Signed-off-by: Thomas Huth <huth@tuxfamily.org>
> ---
>   hw/m68k/mcf5206.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)

Patch merged as commit 5e077a7767.
Re: [PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state
Posted by Mark Cave-Ayland 11 months ago
On 21/12/2023 12:29, Thomas Huth wrote:

> There's no need to explicitely allocate the memory here, we can
> simply embed it into the m5206_mbar_state instead.
> 
> Signed-off-by: Thomas Huth <huth@tuxfamily.org>
> ---
>   hw/m68k/mcf5206.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c
> index a46a23538d..183fd3cc08 100644
> --- a/hw/m68k/mcf5206.c
> +++ b/hw/m68k/mcf5206.c
> @@ -148,15 +148,11 @@ static void m5206_timer_write(m5206_timer_state *s, uint32_t addr, uint32_t val)
>       m5206_timer_update(s);
>   }
>   
> -static m5206_timer_state *m5206_timer_init(qemu_irq irq)
> +static void m5206_timer_init(m5206_timer_state *s, qemu_irq irq)
>   {
> -    m5206_timer_state *s;
> -
> -    s = g_new0(m5206_timer_state, 1);
>       s->timer = ptimer_init(m5206_timer_trigger, s, PTIMER_POLICY_LEGACY);
>       s->irq = irq;
>       m5206_timer_reset(s);
> -    return s;
>   }
>   
>   /* System Integration Module.  */
> @@ -167,7 +163,7 @@ typedef struct {
>       M68kCPU *cpu;
>       MemoryRegion iomem;
>       qemu_irq *pic;
> -    m5206_timer_state *timer[2];
> +    m5206_timer_state timer[2];
>       DeviceState *uart[2];
>       uint8_t scr;
>       uint8_t icr[14];
> @@ -293,9 +289,9 @@ static uint64_t m5206_mbar_read(m5206_mbar_state *s,
>                                   uint16_t offset, unsigned size)
>   {
>       if (offset >= 0x100 && offset < 0x120) {
> -        return m5206_timer_read(s->timer[0], offset - 0x100);
> +        return m5206_timer_read(&s->timer[0], offset - 0x100);
>       } else if (offset >= 0x120 && offset < 0x140) {
> -        return m5206_timer_read(s->timer[1], offset - 0x120);
> +        return m5206_timer_read(&s->timer[1], offset - 0x120);
>       } else if (offset >= 0x140 && offset < 0x160) {
>           return mcf_uart_read(s->uart[0], offset - 0x140, size);
>       } else if (offset >= 0x180 && offset < 0x1a0) {
> @@ -333,10 +329,10 @@ static void m5206_mbar_write(m5206_mbar_state *s, uint16_t offset,
>                                uint64_t value, unsigned size)
>   {
>       if (offset >= 0x100 && offset < 0x120) {
> -        m5206_timer_write(s->timer[0], offset - 0x100, value);
> +        m5206_timer_write(&s->timer[0], offset - 0x100, value);
>           return;
>       } else if (offset >= 0x120 && offset < 0x140) {
> -        m5206_timer_write(s->timer[1], offset - 0x120, value);
> +        m5206_timer_write(&s->timer[1], offset - 0x120, value);
>           return;
>       } else if (offset >= 0x140 && offset < 0x160) {
>           mcf_uart_write(s->uart[0], offset - 0x140, value, size);
> @@ -598,8 +594,8 @@ static void mcf5206_mbar_realize(DeviceState *dev, Error **errp)
>       sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>   
>       s->pic = qemu_allocate_irqs(m5206_mbar_set_irq, s, 14);
> -    s->timer[0] = m5206_timer_init(s->pic[9]);
> -    s->timer[1] = m5206_timer_init(s->pic[10]);
> +    m5206_timer_init(&s->timer[0], s->pic[9]);
> +    m5206_timer_init(&s->timer[1], s->pic[10]);
>       s->uart[0] = mcf_uart_create(s->pic[12], serial_hd(0));
>       s->uart[1] = mcf_uart_create(s->pic[13], serial_hd(1));
>   }

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This reminds me that there is one still remaining memory region left to convert in 
q800.c...


ATB,

Mark.
Re: [PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state
Posted by Philippe Mathieu-Daudé 11 months ago
On 21/12/23 13:29, Thomas Huth wrote:
> There's no need to explicitely allocate the memory here, we can
> simply embed it into the m5206_mbar_state instead.
> 
> Signed-off-by: Thomas Huth <huth@tuxfamily.org>
> ---
>   hw/m68k/mcf5206.c | 20 ++++++++------------
>   1 file changed, 8 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>