Re: [Qemu-devel] [PATCH V8 09/18] pc: remove a global variable, RTCState *rtc_state.

[Isaku Yamahata <yamahata@valinux.co.jp>]

On Mon, Dec 07, 2009 at 10:15:17AM +0100, Gerd Hoffmann wrote:
> On 12/04/09 06:50, Isaku Yamahata wrote:
>> remove a global variable, RTCState *rtc_state.
>> Only the cmos_set_s3_resume_init() needs it global.
>> So introduce a registering function and make it local.
>> As for other function which references the variable, pass it
>> as a function argument.
>>
>> Signed-off-by: Isaku Yamahata<yamahata@valinux.co.jp>
>> Cc: Paolo Bonzini<bonzini@gnu.org>
>
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -64,8 +64,6 @@
> [ ... ]
>> -static RTCState *rtc_state;
> [ ... ]
>> +static RTCState *rtc_state;
> [ ... ]
>
> Hmm?  Patch description says something else ...

How about the following patch?
(This is on top of V9 patch.)

>From 60499ae68c2e187374393534ce85a68ef4095cab Mon Sep 17 00:00:00 2001
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Wed, 9 Dec 2009 11:34:13 +0900
Subject: [PATCH] pc: remove global variable rtc_state by using qemu_irq.

Following d9c3231019a0fbacbe15dcb26a0e3708b726af77 which uses qemu_irq
for powerdown to eliminate nasty #ifdef (TARGET_xxx),
this patch removes #ifdef(TARGET_I386) and global variable rtc_state.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/acpi_piix4.c |    8 ++++----
 hw/mips_malta.c |    3 ++-
 hw/pc.c         |   15 +++------------
 hw/pc.h         |    5 ++---
 hw/pc_piix.c    |    5 ++++-
 5 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 2b913c7..fb892a2 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -51,6 +51,7 @@ typedef struct PIIX4PMState {
     PCSMBus smb;
 
     qemu_irq irq;
+    qemu_irq cmos_s3_resume;
 } PIIX4PMState;
 
 #define ACPI_ENABLE 0xf1
@@ -146,9 +147,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
                     s->pmsts |= (ACPI_BITMASK_WAKE_STATUS |
                                  ACPI_BITMASK_POWER_BUTTON_STATUS);
                     qemu_system_reset_request();
-#if defined(TARGET_I386)
-                    cmos_set_s3_resume();
-#endif
+                    qemu_irq_raise(s->cmos_s3_resume);
                 default:
                     break;
                 }
@@ -313,7 +312,7 @@ static void piix4_powerdown(void *opaque, int irq, int power_failing)
 }
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-                       qemu_irq sci_irq)
+                       qemu_irq sci_irq, qemu_irq cmos_s3_resume)
 {
     PIIX4PMState *s;
     uint8_t *pci_conf;
@@ -367,6 +366,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
 
     pc_smbus_init(&s->smb);
     s->irq = sci_irq;
+    s->cmos_s3_resume = cmos_s3_resume;
     qemu_register_reset(piix4_reset, s);
 
     return s->smb.smbus;
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 7cff0c0..8f33e74 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -928,7 +928,8 @@ void mips_malta_init (ram_addr_t ram_size,
     isa_bus_irqs(i8259);
     pci_piix4_ide_init(pci_bus, hd, piix4_devfn + 1);
     usb_uhci_piix4_init(pci_bus, piix4_devfn + 2);
-    smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100, isa_reserve_irq(9));
+    smbus = piix4_pm_init(pci_bus, piix4_devfn + 3, 0x1100,
+                          isa_reserve_irq(9), NULL);
     eeprom_buf = qemu_mallocz(8 * 256); /* XXX: make this persistent */
     for (i = 0; i < 8; i++) {
         /* TODO: Populate SPD eeprom data.  */
diff --git a/hw/pc.c b/hw/pc.c
index 6f25ba8..7cb4f14 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1135,7 +1135,6 @@ void pc_basic_device_init(qemu_irq *isa_irq,
     register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL);
 
     *rtc_state = rtc_init(2000);
-    cmos_set_s3_resume_init(*rtc_state);
 
     qemu_register_boot_set(pc_boot_set, *rtc_state);
 
@@ -1189,16 +1188,8 @@ void pc_pci_device_init(PCIBus *pci_bus)
 
 /* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
    BIOS will read it and start S3 resume at POST Entry */
-static ISADevice *rtc_state; /* RTCState device */
-void cmos_set_s3_resume_init(ISADevice *s)
+void cmos_set_s3_resume_fn(void *opaque, int n, int level)
 {
-    rtc_state = s;
-}
-
-void cmos_set_s3_resume(void)
-{
-#if defined(TARGET_I386)
-    if (rtc_state)
-        rtc_set_memory(rtc_state, 0xF, 0xFE);
-#endif
+    ISADevice *rtc_state = opaque;
+    rtc_set_memory(rtc_state, 0xF, 0xFE);
 }
diff --git a/hw/pc.h b/hw/pc.h
index be5230e..74f551e 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -89,8 +89,7 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
                    target_phys_addr_t mask);
 
 /* pc.c */
-void cmos_set_s3_resume_init(ISADevice *s);
-void cmos_set_s3_resume(void);
+void cmos_set_s3_resume_fn(void *opaque, int n, int level);
 
 extern int fd_bootchk;
 
@@ -139,7 +138,7 @@ int acpi_table_add(const char *table_desc);
 
 /* acpi_piix.c */
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
-                       qemu_irq sci_irq);
+                       qemu_irq sci_irq, qemu_irq cmos_set_s3_resume);
 void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
 void piix4_acpi_system_hot_add_init(PCIBus *bus);
 
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 2f776f8..a20d052 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -143,9 +143,12 @@ static void pc_init1(ram_addr_t ram_size,
         uint8_t *eeprom_buf = qemu_mallocz(8 * 256); /* XXX: make this persistent */
         i2c_bus *smbus;
 
+        qemu_irq cmos_s3_resume =
+            *qemu_allocate_irqs(cmos_set_s3_resume_fn, rtc_state, 1);
+
         /* TODO: Populate SPD eeprom data.  */
         smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
-                              isa_reserve_irq(9));
+                              isa_reserve_irq(9), cmos_s3_resume);
         for (i = 0; i < 8; i++) {
             DeviceState *eeprom;
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
-- 
1.6.5.4




-- 
yamahata