qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
@ 2020-09-14 10:24 Philippe Mathieu-Daudé
  2020-09-16  2:50 ` David Gibson
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-14 10:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Mark Cave-Ayland, Philippe Mathieu-Daudé,
	Hervé Poussineau, qemu-ppc, Artyom Tarasenko, David Gibson

As the 'io_base' argument of m48t59_init() is unused (set to 0),
remove it to simplify.
To create a device on the ISA bus, m48t59_init_isa() is the
preferred function to use.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/rtc/m48t59.h |  2 +-
 hw/ppc/ppc405_boards.c  |  2 +-
 hw/rtc/m48t59.c         | 10 ++--------
 hw/sparc/sun4m.c        |  2 +-
 hw/sparc64/sun4u.c      |  2 +-
 5 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
index 04abedf3b2b..62297ee0db1 100644
--- a/include/hw/rtc/m48t59.h
+++ b/include/hw/rtc/m48t59.h
@@ -50,7 +50,7 @@ struct NvramClass {
 Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
                        int base_year, int type);
 Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                   uint32_t io_base, uint16_t size, int base_year,
+                   uint16_t size, int base_year,
                    int type);
 
 #endif /* HW_M48T59_H */
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 6198ec1035b..93ffee801a3 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
     /* Register FPGA */
     ref405ep_fpga_init(sysmem, 0xF0300000);
     /* Register NVRAM */
-    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
+    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
     /* Load kernel */
     linux_boot = (kernel_filename != NULL);
     if (linux_boot) {
diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index 6525206976b..2d6a095c4e4 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
 
 /* Initialisation routine */
 Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                   uint32_t io_base, uint16_t size, int base_year,
+                   uint16_t size, int base_year,
                    int model)
 {
     DeviceState *dev;
@@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
         s = SYS_BUS_DEVICE(dev);
         sysbus_realize_and_unref(s, &error_fatal);
         sysbus_connect_irq(s, 0, IRQ);
-        if (io_base != 0) {
-            memory_region_add_subregion(get_system_io(), io_base,
-                                        sysbus_mmio_get_region(s, 1));
-        }
-        if (mem_base != 0) {
-            sysbus_mmio_map(s, 0, mem_base);
-        }
+        sysbus_mmio_map(s, 0, mem_base);
 
         return NVRAM(s);
     }
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 947b69d1597..56a0d38f274 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
         create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
     }
 
-    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
+    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
 
     slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index b4aabfc076f..1cc57b030a7 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     pci_ide_create_devs(pci_dev);
 
     /* Map NVRAM into I/O (ebus) space */
-    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
+    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
     s = SYS_BUS_DEVICE(nvram);
     memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
                                 sysbus_mmio_get_region(s, 0));
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-14 10:24 [PATCH] hw/rtc/m48t59: Simplify m48t59_init() Philippe Mathieu-Daudé
@ 2020-09-16  2:50 ` David Gibson
  2020-09-24 14:53   ` Philippe Mathieu-Daudé
  2020-09-30  8:35 ` Mark Cave-Ayland
  2020-09-30 11:25 ` Laurent Vivier
  2 siblings, 1 reply; 10+ messages in thread
From: David Gibson @ 2020-09-16  2:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-trivial, Mark Cave-Ayland, qemu-devel, Hervé Poussineau,
	qemu-ppc, Artyom Tarasenko

[-- Attachment #1: Type: text/plain, Size: 4258 bytes --]

On Mon, Sep 14, 2020 at 12:24:25PM +0200, Philippe Mathieu-Daudé wrote:
> As the 'io_base' argument of m48t59_init() is unused (set to 0),
> remove it to simplify.
> To create a device on the ISA bus, m48t59_init_isa() is the
> preferred function to use.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/rtc/m48t59.h |  2 +-
>  hw/ppc/ppc405_boards.c  |  2 +-
>  hw/rtc/m48t59.c         | 10 ++--------
>  hw/sparc/sun4m.c        |  2 +-
>  hw/sparc64/sun4u.c      |  2 +-
>  5 files changed, 6 insertions(+), 12 deletions(-)

ppc part
Acked-by: David Gibson <david@gibson.dropbear.id.au>

> 
> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
> index 04abedf3b2b..62297ee0db1 100644
> --- a/include/hw/rtc/m48t59.h
> +++ b/include/hw/rtc/m48t59.h
> @@ -50,7 +50,7 @@ struct NvramClass {
>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>                         int base_year, int type);
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int type);
>  
>  #endif /* HW_M48T59_H */
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 6198ec1035b..93ffee801a3 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>      /* Register FPGA */
>      ref405ep_fpga_init(sysmem, 0xF0300000);
>      /* Register NVRAM */
> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>      /* Load kernel */
>      linux_boot = (kernel_filename != NULL);
>      if (linux_boot) {
> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
> index 6525206976b..2d6a095c4e4 100644
> --- a/hw/rtc/m48t59.c
> +++ b/hw/rtc/m48t59.c
> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>  
>  /* Initialisation routine */
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int model)
>  {
>      DeviceState *dev;
> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>          s = SYS_BUS_DEVICE(dev);
>          sysbus_realize_and_unref(s, &error_fatal);
>          sysbus_connect_irq(s, 0, IRQ);
> -        if (io_base != 0) {
> -            memory_region_add_subregion(get_system_io(), io_base,
> -                                        sysbus_mmio_get_region(s, 1));
> -        }
> -        if (mem_base != 0) {
> -            sysbus_mmio_map(s, 0, mem_base);
> -        }
> +        sysbus_mmio_map(s, 0, mem_base);
>  
>          return NVRAM(s);
>      }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 947b69d1597..56a0d38f274 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>      }
>  
> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>  
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>  
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b4aabfc076f..1cc57b030a7 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      pci_ide_create_devs(pci_dev);
>  
>      /* Map NVRAM into I/O (ebus) space */
> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
>      s = SYS_BUS_DEVICE(nvram);
>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>                                  sysbus_mmio_get_region(s, 0));

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-16  2:50 ` David Gibson
@ 2020-09-24 14:53   ` Philippe Mathieu-Daudé
  2020-09-24 15:00     ` Laurent Vivier
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-24 14:53 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-trivial, Mark Cave-Ayland, qemu-devel, Hervé Poussineau,
	qemu-ppc, Artyom Tarasenko

On 9/16/20 4:50 AM, David Gibson wrote:
> On Mon, Sep 14, 2020 at 12:24:25PM +0200, Philippe Mathieu-Daudé wrote:
>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>> remove it to simplify.
>> To create a device on the ISA bus, m48t59_init_isa() is the
>> preferred function to use.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/rtc/m48t59.h |  2 +-
>>  hw/ppc/ppc405_boards.c  |  2 +-
>>  hw/rtc/m48t59.c         | 10 ++--------
>>  hw/sparc/sun4m.c        |  2 +-
>>  hw/sparc64/sun4u.c      |  2 +-
>>  5 files changed, 6 insertions(+), 12 deletions(-)
> 
> ppc part
> Acked-by: David Gibson <david@gibson.dropbear.id.au>

Thanks!

Can this go via qemu-trivial@?

> 
>>
>> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
>> index 04abedf3b2b..62297ee0db1 100644
>> --- a/include/hw/rtc/m48t59.h
>> +++ b/include/hw/rtc/m48t59.h
>> @@ -50,7 +50,7 @@ struct NvramClass {
>>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>>                         int base_year, int type);
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int type);
>>  
>>  #endif /* HW_M48T59_H */
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 6198ec1035b..93ffee801a3 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>>      /* Register FPGA */
>>      ref405ep_fpga_init(sysmem, 0xF0300000);
>>      /* Register NVRAM */
>> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
>> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>>      /* Load kernel */
>>      linux_boot = (kernel_filename != NULL);
>>      if (linux_boot) {
>> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
>> index 6525206976b..2d6a095c4e4 100644
>> --- a/hw/rtc/m48t59.c
>> +++ b/hw/rtc/m48t59.c
>> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>>  
>>  /* Initialisation routine */
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int model)
>>  {
>>      DeviceState *dev;
>> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>          s = SYS_BUS_DEVICE(dev);
>>          sysbus_realize_and_unref(s, &error_fatal);
>>          sysbus_connect_irq(s, 0, IRQ);
>> -        if (io_base != 0) {
>> -            memory_region_add_subregion(get_system_io(), io_base,
>> -                                        sysbus_mmio_get_region(s, 1));
>> -        }
>> -        if (mem_base != 0) {
>> -            sysbus_mmio_map(s, 0, mem_base);
>> -        }
>> +        sysbus_mmio_map(s, 0, mem_base);
>>  
>>          return NVRAM(s);
>>      }
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 947b69d1597..56a0d38f274 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>>      }
>>  
>> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>>  
>>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>>  
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index b4aabfc076f..1cc57b030a7 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>      pci_ide_create_devs(pci_dev);
>>  
>>      /* Map NVRAM into I/O (ebus) space */
>> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
>> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
>>      s = SYS_BUS_DEVICE(nvram);
>>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>>                                  sysbus_mmio_get_region(s, 0));
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-24 14:53   ` Philippe Mathieu-Daudé
@ 2020-09-24 15:00     ` Laurent Vivier
  2020-09-29 10:21       ` Philippe Mathieu-Daudé
  2020-09-30  8:26       ` Mark Cave-Ayland
  0 siblings, 2 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-09-24 15:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, David Gibson
  Cc: qemu-trivial, Mark Cave-Ayland, qemu-devel, Hervé Poussineau,
	qemu-ppc, Artyom Tarasenko

Le 24/09/2020 à 16:53, Philippe Mathieu-Daudé a écrit :
> On 9/16/20 4:50 AM, David Gibson wrote:
>> On Mon, Sep 14, 2020 at 12:24:25PM +0200, Philippe Mathieu-Daudé wrote:
>>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>>> remove it to simplify.
>>> To create a device on the ISA bus, m48t59_init_isa() is the
>>> preferred function to use.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  include/hw/rtc/m48t59.h |  2 +-
>>>  hw/ppc/ppc405_boards.c  |  2 +-
>>>  hw/rtc/m48t59.c         | 10 ++--------
>>>  hw/sparc/sun4m.c        |  2 +-
>>>  hw/sparc64/sun4u.c      |  2 +-
>>>  5 files changed, 6 insertions(+), 12 deletions(-)
>>
>> ppc part
>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Thanks!
> 
> Can this go via qemu-trivial@?

Yes, but more reviewers would help. Mark?

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-24 15:00     ` Laurent Vivier
@ 2020-09-29 10:21       ` Philippe Mathieu-Daudé
  2020-09-30  8:26       ` Mark Cave-Ayland
  1 sibling, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-29 10:21 UTC (permalink / raw)
  To: Laurent Vivier, David Gibson
  Cc: qemu-trivial, Mark Cave-Ayland, qemu-devel, Hervé Poussineau,
	qemu-ppc, Artyom Tarasenko

On 9/24/20 5:00 PM, Laurent Vivier wrote:
> Le 24/09/2020 à 16:53, Philippe Mathieu-Daudé a écrit :
>> On 9/16/20 4:50 AM, David Gibson wrote:
>>> On Mon, Sep 14, 2020 at 12:24:25PM +0200, Philippe Mathieu-Daudé wrote:
>>>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>>>> remove it to simplify.
>>>> To create a device on the ISA bus, m48t59_init_isa() is the
>>>> preferred function to use.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>  include/hw/rtc/m48t59.h |  2 +-
>>>>  hw/ppc/ppc405_boards.c  |  2 +-
>>>>  hw/rtc/m48t59.c         | 10 ++--------
>>>>  hw/sparc/sun4m.c        |  2 +-
>>>>  hw/sparc64/sun4u.c      |  2 +-
>>>>  5 files changed, 6 insertions(+), 12 deletions(-)
>>>
>>> ppc part
>>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> Thanks!
>>
>> Can this go via qemu-trivial@?
> 
> Yes, but more reviewers would help. Mark?

Or Artyom :)

> 
> Thanks,
> Laurent
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-24 15:00     ` Laurent Vivier
  2020-09-29 10:21       ` Philippe Mathieu-Daudé
@ 2020-09-30  8:26       ` Mark Cave-Ayland
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Cave-Ayland @ 2020-09-30  8:26 UTC (permalink / raw)
  To: Laurent Vivier, Philippe Mathieu-Daudé, David Gibson
  Cc: qemu-trivial, qemu-ppc, Hervé Poussineau, qemu-devel,
	Artyom Tarasenko

On 24/09/2020 16:00, Laurent Vivier wrote:

> Le 24/09/2020 à 16:53, Philippe Mathieu-Daudé a écrit :
>> On 9/16/20 4:50 AM, David Gibson wrote:
>>> On Mon, Sep 14, 2020 at 12:24:25PM +0200, Philippe Mathieu-Daudé wrote:
>>>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>>>> remove it to simplify.
>>>> To create a device on the ISA bus, m48t59_init_isa() is the
>>>> preferred function to use.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>  include/hw/rtc/m48t59.h |  2 +-
>>>>  hw/ppc/ppc405_boards.c  |  2 +-
>>>>  hw/rtc/m48t59.c         | 10 ++--------
>>>>  hw/sparc/sun4m.c        |  2 +-
>>>>  hw/sparc64/sun4u.c      |  2 +-
>>>>  5 files changed, 6 insertions(+), 12 deletions(-)
>>>
>>> ppc part
>>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> Thanks!
>>
>> Can this go via qemu-trivial@?
> 
> Yes, but more reviewers would help. Mark?

Ooof sorry I missed this one. Let me take a quick look now...


ATB,

Mark.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-14 10:24 [PATCH] hw/rtc/m48t59: Simplify m48t59_init() Philippe Mathieu-Daudé
  2020-09-16  2:50 ` David Gibson
@ 2020-09-30  8:35 ` Mark Cave-Ayland
  2020-09-30 10:22   ` Philippe Mathieu-Daudé
  2020-09-30 11:25 ` Laurent Vivier
  2 siblings, 1 reply; 10+ messages in thread
From: Mark Cave-Ayland @ 2020-09-30  8:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, qemu-ppc, Hervé Poussineau, Artyom Tarasenko,
	David Gibson

On 14/09/2020 11:24, Philippe Mathieu-Daudé wrote:

> As the 'io_base' argument of m48t59_init() is unused (set to 0),
> remove it to simplify.
> To create a device on the ISA bus, m48t59_init_isa() is the
> preferred function to use.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/rtc/m48t59.h |  2 +-
>  hw/ppc/ppc405_boards.c  |  2 +-
>  hw/rtc/m48t59.c         | 10 ++--------
>  hw/sparc/sun4m.c        |  2 +-
>  hw/sparc64/sun4u.c      |  2 +-
>  5 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
> index 04abedf3b2b..62297ee0db1 100644
> --- a/include/hw/rtc/m48t59.h
> +++ b/include/hw/rtc/m48t59.h
> @@ -50,7 +50,7 @@ struct NvramClass {
>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>                         int base_year, int type);
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int type);
>  
>  #endif /* HW_M48T59_H */
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 6198ec1035b..93ffee801a3 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>      /* Register FPGA */
>      ref405ep_fpga_init(sysmem, 0xF0300000);
>      /* Register NVRAM */
> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>      /* Load kernel */
>      linux_boot = (kernel_filename != NULL);
>      if (linux_boot) {
> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
> index 6525206976b..2d6a095c4e4 100644
> --- a/hw/rtc/m48t59.c
> +++ b/hw/rtc/m48t59.c
> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>  
>  /* Initialisation routine */
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int model)
>  {
>      DeviceState *dev;
> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>          s = SYS_BUS_DEVICE(dev);
>          sysbus_realize_and_unref(s, &error_fatal);
>          sysbus_connect_irq(s, 0, IRQ);
> -        if (io_base != 0) {
> -            memory_region_add_subregion(get_system_io(), io_base,
> -                                        sysbus_mmio_get_region(s, 1));
> -        }
> -        if (mem_base != 0) {
> -            sysbus_mmio_map(s, 0, mem_base);
> -        }
> +        sysbus_mmio_map(s, 0, mem_base);
>  
>          return NVRAM(s);
>      }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 947b69d1597..56a0d38f274 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>      }
>  
> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>  
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>  
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b4aabfc076f..1cc57b030a7 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      pci_ide_create_devs(pci_dev);
>  
>      /* Map NVRAM into I/O (ebus) space */
> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
>      s = SYS_BUS_DEVICE(nvram);
>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>                                  sysbus_mmio_get_region(s, 0));

Looks good to me. In fact, if you're working in this area there are a couple of other
quick wins here too:

- Remove m48t59_init() and replace with QOM/qdev properties

- Remove m48t59_init_isa() as this seems to be unused, along with the associated
complexity of handling the m48txx_isa_info[] array


Anyhow:

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


ATB,

Mark.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-30  8:35 ` Mark Cave-Ayland
@ 2020-09-30 10:22   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-30 10:22 UTC (permalink / raw)
  To: Mark Cave-Ayland, qemu-devel
  Cc: qemu-trivial, Hervé Poussineau, qemu-ppc, Artyom Tarasenko,
	David Gibson

On 9/30/20 10:35 AM, Mark Cave-Ayland wrote:
> On 14/09/2020 11:24, Philippe Mathieu-Daudé wrote:
> 
>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>> remove it to simplify.
>> To create a device on the ISA bus, m48t59_init_isa() is the
>> preferred function to use.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/rtc/m48t59.h |  2 +-
>>  hw/ppc/ppc405_boards.c  |  2 +-
>>  hw/rtc/m48t59.c         | 10 ++--------
>>  hw/sparc/sun4m.c        |  2 +-
>>  hw/sparc64/sun4u.c      |  2 +-
>>  5 files changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
>> index 04abedf3b2b..62297ee0db1 100644
>> --- a/include/hw/rtc/m48t59.h
>> +++ b/include/hw/rtc/m48t59.h
>> @@ -50,7 +50,7 @@ struct NvramClass {
>>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>>                         int base_year, int type);
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int type);
>>  
>>  #endif /* HW_M48T59_H */
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 6198ec1035b..93ffee801a3 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>>      /* Register FPGA */
>>      ref405ep_fpga_init(sysmem, 0xF0300000);
>>      /* Register NVRAM */
>> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
>> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>>      /* Load kernel */
>>      linux_boot = (kernel_filename != NULL);
>>      if (linux_boot) {
>> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
>> index 6525206976b..2d6a095c4e4 100644
>> --- a/hw/rtc/m48t59.c
>> +++ b/hw/rtc/m48t59.c
>> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>>  
>>  /* Initialisation routine */
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int model)
>>  {
>>      DeviceState *dev;
>> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>          s = SYS_BUS_DEVICE(dev);
>>          sysbus_realize_and_unref(s, &error_fatal);
>>          sysbus_connect_irq(s, 0, IRQ);
>> -        if (io_base != 0) {
>> -            memory_region_add_subregion(get_system_io(), io_base,
>> -                                        sysbus_mmio_get_region(s, 1));
>> -        }
>> -        if (mem_base != 0) {
>> -            sysbus_mmio_map(s, 0, mem_base);
>> -        }
>> +        sysbus_mmio_map(s, 0, mem_base);
>>  
>>          return NVRAM(s);
>>      }
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 947b69d1597..56a0d38f274 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>>      }
>>  
>> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>>  
>>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>>  
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index b4aabfc076f..1cc57b030a7 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>      pci_ide_create_devs(pci_dev);
>>  
>>      /* Map NVRAM into I/O (ebus) space */
>> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
>> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
>>      s = SYS_BUS_DEVICE(nvram);
>>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>>                                  sysbus_mmio_get_region(s, 0));
> 
> Looks good to me. In fact, if you're working in this area there are a couple of other
> quick wins here too:
> 
> - Remove m48t59_init() and replace with QOM/qdev properties
> 
> - Remove m48t59_init_isa() as this seems to be unused, along with the associated
> complexity of handling the m48txx_isa_info[] array

OK.

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

Thanks!

> 
> 
> ATB,
> 
> Mark.
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-14 10:24 [PATCH] hw/rtc/m48t59: Simplify m48t59_init() Philippe Mathieu-Daudé
  2020-09-16  2:50 ` David Gibson
  2020-09-30  8:35 ` Mark Cave-Ayland
@ 2020-09-30 11:25 ` Laurent Vivier
  2020-10-12 21:18   ` Laurent Vivier
  2 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2020-09-30 11:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Mark Cave-Ayland, Hervé Poussineau, qemu-ppc,
	Artyom Tarasenko, David Gibson

Le 14/09/2020 à 12:24, Philippe Mathieu-Daudé a écrit :
> As the 'io_base' argument of m48t59_init() is unused (set to 0),
> remove it to simplify.
> To create a device on the ISA bus, m48t59_init_isa() is the
> preferred function to use.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/rtc/m48t59.h |  2 +-
>  hw/ppc/ppc405_boards.c  |  2 +-
>  hw/rtc/m48t59.c         | 10 ++--------
>  hw/sparc/sun4m.c        |  2 +-
>  hw/sparc64/sun4u.c      |  2 +-
>  5 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
> index 04abedf3b2b..62297ee0db1 100644
> --- a/include/hw/rtc/m48t59.h
> +++ b/include/hw/rtc/m48t59.h
> @@ -50,7 +50,7 @@ struct NvramClass {
>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>                         int base_year, int type);
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int type);
>  
>  #endif /* HW_M48T59_H */
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 6198ec1035b..93ffee801a3 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>      /* Register FPGA */
>      ref405ep_fpga_init(sysmem, 0xF0300000);
>      /* Register NVRAM */
> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>      /* Load kernel */
>      linux_boot = (kernel_filename != NULL);
>      if (linux_boot) {
> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
> index 6525206976b..2d6a095c4e4 100644
> --- a/hw/rtc/m48t59.c
> +++ b/hw/rtc/m48t59.c
> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>  
>  /* Initialisation routine */
>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> -                   uint32_t io_base, uint16_t size, int base_year,
> +                   uint16_t size, int base_year,
>                     int model)
>  {
>      DeviceState *dev;
> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>          s = SYS_BUS_DEVICE(dev);
>          sysbus_realize_and_unref(s, &error_fatal);
>          sysbus_connect_irq(s, 0, IRQ);
> -        if (io_base != 0) {
> -            memory_region_add_subregion(get_system_io(), io_base,
> -                                        sysbus_mmio_get_region(s, 1));
> -        }
> -        if (mem_base != 0) {
> -            sysbus_mmio_map(s, 0, mem_base);
> -        }
> +        sysbus_mmio_map(s, 0, mem_base);
>  
>          return NVRAM(s);
>      }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 947b69d1597..56a0d38f274 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>      }
>  
> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>  
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>  
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b4aabfc076f..1cc57b030a7 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      pci_ide_create_devs(pci_dev);
>  
>      /* Map NVRAM into I/O (ebus) space */
> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);
>      s = SYS_BUS_DEVICE(nvram);
>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>                                  sysbus_mmio_get_region(s, 0));
> 

Applied to my trivial-patches branch.

Thanks,
Laurent



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] hw/rtc/m48t59: Simplify m48t59_init()
  2020-09-30 11:25 ` Laurent Vivier
@ 2020-10-12 21:18   ` Laurent Vivier
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2020-10-12 21:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: qemu-trivial, Mark Cave-Ayland, Hervé Poussineau, qemu-ppc,
	Artyom Tarasenko, David Gibson

Le 30/09/2020 à 13:25, Laurent Vivier a écrit :
> Le 14/09/2020 à 12:24, Philippe Mathieu-Daudé a écrit :
>> As the 'io_base' argument of m48t59_init() is unused (set to 0),
>> remove it to simplify.
>> To create a device on the ISA bus, m48t59_init_isa() is the
>> preferred function to use.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/rtc/m48t59.h |  2 +-
>>  hw/ppc/ppc405_boards.c  |  2 +-
>>  hw/rtc/m48t59.c         | 10 ++--------
>>  hw/sparc/sun4m.c        |  2 +-
>>  hw/sparc64/sun4u.c      |  2 +-
>>  5 files changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/hw/rtc/m48t59.h b/include/hw/rtc/m48t59.h
>> index 04abedf3b2b..62297ee0db1 100644
>> --- a/include/hw/rtc/m48t59.h
>> +++ b/include/hw/rtc/m48t59.h
>> @@ -50,7 +50,7 @@ struct NvramClass {
>>  Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
>>                         int base_year, int type);
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int type);
>>  
>>  #endif /* HW_M48T59_H */
>> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
>> index 6198ec1035b..93ffee801a3 100644
>> --- a/hw/ppc/ppc405_boards.c
>> +++ b/hw/ppc/ppc405_boards.c
>> @@ -227,7 +227,7 @@ static void ref405ep_init(MachineState *machine)
>>      /* Register FPGA */
>>      ref405ep_fpga_init(sysmem, 0xF0300000);
>>      /* Register NVRAM */
>> -    m48t59_init(NULL, 0xF0000000, 0, 8192, 1968, 8);
>> +    m48t59_init(NULL, 0xF0000000, 8192, 1968, 8);
>>      /* Load kernel */
>>      linux_boot = (kernel_filename != NULL);
>>      if (linux_boot) {
>> diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
>> index 6525206976b..2d6a095c4e4 100644
>> --- a/hw/rtc/m48t59.c
>> +++ b/hw/rtc/m48t59.c
>> @@ -566,7 +566,7 @@ const MemoryRegionOps m48t59_io_ops = {
>>  
>>  /* Initialisation routine */
>>  Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>> -                   uint32_t io_base, uint16_t size, int base_year,
>> +                   uint16_t size, int base_year,
>>                     int model)
>>  {
>>      DeviceState *dev;
>> @@ -584,13 +584,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
>>          s = SYS_BUS_DEVICE(dev);
>>          sysbus_realize_and_unref(s, &error_fatal);
>>          sysbus_connect_irq(s, 0, IRQ);
>> -        if (io_base != 0) {
>> -            memory_region_add_subregion(get_system_io(), io_base,
>> -                                        sysbus_mmio_get_region(s, 1));
>> -        }
>> -        if (mem_base != 0) {

You can't remove the "if ()"...

>> -            sysbus_mmio_map(s, 0, mem_base);
>> -        }
>> +        sysbus_mmio_map(s, 0, mem_base);
>>  
>>          return NVRAM(s);
>>      }
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 947b69d1597..56a0d38f274 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -970,7 +970,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>          create_unimplemented_device("SUNW,sx", hwdef->sx_base, 0x2000);
>>      }
>>  
>> -    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0, 0x2000, 1968, 8);
>> +    nvram = m48t59_init(slavio_irq[0], hwdef->nvram_base, 0x2000, 1968, 8);
>>  
>>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], slavio_cpu_irq, smp_cpus);
>>  
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index b4aabfc076f..1cc57b030a7 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -675,7 +675,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>      pci_ide_create_devs(pci_dev);
>>  
>>      /* Map NVRAM into I/O (ebus) space */
>> -    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
>> +    nvram = m48t59_init(NULL, 0, NVRAM_SIZE, 1968, 59);

... because mem_base is 0 here.

>>      s = SYS_BUS_DEVICE(nvram);
>>      memory_region_add_subregion(pci_address_space_io(ebus), 0x2000,
>>                                  sysbus_mmio_get_region(s, 0));
>>
> 
> Applied to my trivial-patches branch.

I'm removing it from my branch because it breaks check-qtest-sparc64:

Running test qtest-sparc64: endianness-test
qemu-system-sparc64: ../../../Projects/qemu/softmmu/memory.c:2398:
memory_region_add_subregion_common: Assertion `!subregion->container'
failed.

Thanks,
Laurent




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-12 21:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-14 10:24 [PATCH] hw/rtc/m48t59: Simplify m48t59_init() Philippe Mathieu-Daudé
2020-09-16  2:50 ` David Gibson
2020-09-24 14:53   ` Philippe Mathieu-Daudé
2020-09-24 15:00     ` Laurent Vivier
2020-09-29 10:21       ` Philippe Mathieu-Daudé
2020-09-30  8:26       ` Mark Cave-Ayland
2020-09-30  8:35 ` Mark Cave-Ayland
2020-09-30 10:22   ` Philippe Mathieu-Daudé
2020-09-30 11:25 ` Laurent Vivier
2020-10-12 21:18   ` Laurent Vivier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).