[PATCH 0/4] mc146818rtc: Allow call object_initialize(MC146818_RTC) instead of rtc_init()

[PATCH 0/4] mc146818rtc: Allow call object_initialize(MC146818_RTC) instead of rtc_init()

[Philippe Mathieu-Daudé]

Four RTC146818 patches extracted from a bigger series:
"hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge"
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg03685.html

This step is required to be able to create a MC146818_RTC within
a QOM object (which will be used in the "piix4: add a mc146818rtc
controller" patch later).

No changes since previous post:
$ git backport-diff -u pc_split_i440fx_piix
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/4:[----] [--] 'mc146818rtc: move structure to header file'
002/4:[----] [--] 'mc146818rtc: Move RTC_ISA_IRQ definition'
003/4:[----] [--] 'mc146818rtc: Include "mc146818rtc_regs.h" directly in mc146818rtc.c'
004/4:[----] [--] 'mc146818rtc: always register rtc to rtc list'

Hervé Poussineau (2):
  mc146818rtc: move structure to header file
  mc146818rtc: always register rtc to rtc list

Philippe Mathieu-Daudé (2):
  mc146818rtc: Move RTC_ISA_IRQ definition
  mc146818rtc: Include mc146818rtc_regs.h directly in mc146818rtc.c

 hw/timer/mc146818rtc.c              | 39 +++--------------------------
 include/hw/timer/mc146818rtc.h      | 36 +++++++++++++++++++++++++-
 include/hw/timer/mc146818rtc_regs.h |  2 --
 tests/rtc-test.c                    |  1 +
 4 files changed, 39 insertions(+), 39 deletions(-)

-- 
2.21.0

[PATCH 1/4] mc146818rtc: move structure to header file

[Philippe Mathieu-Daudé]

From: Hervé Poussineau <hpoussin@reactos.org>

We are now able to embed a timer in another object.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Message-Id: <20171216090228.28505-4-hpoussin@reactos.org>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/timer/mc146818rtc.c         | 30 ------------------------------
 include/hw/timer/mc146818rtc.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 6cb378751b..e40b54e743 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -71,36 +71,6 @@
 #define RTC_CLOCK_RATE            32768
 #define UIP_HOLD_LENGTH           (8 * NANOSECONDS_PER_SECOND / 32768)
 
-#define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
-
-typedef struct RTCState {
-    ISADevice parent_obj;
-
-    MemoryRegion io;
-    MemoryRegion coalesced_io;
-    uint8_t cmos_data[128];
-    uint8_t cmos_index;
-    int32_t base_year;
-    uint64_t base_rtc;
-    uint64_t last_update;
-    int64_t offset;
-    qemu_irq irq;
-    int it_shift;
-    /* periodic timer */
-    QEMUTimer *periodic_timer;
-    int64_t next_periodic_time;
-    /* update-ended timer */
-    QEMUTimer *update_timer;
-    uint64_t next_alarm_time;
-    uint16_t irq_reinject_on_ack_count;
-    uint32_t irq_coalesced;
-    uint32_t period;
-    QEMUTimer *coalesced_timer;
-    LostTickPolicy lost_tick_policy;
-    Notifier suspend_notifier;
-    QLIST_ENTRY(RTCState) link;
-} RTCState;
-
 static void rtc_set_time(RTCState *s);
 static void rtc_update_time(RTCState *s);
 static void rtc_set_cmos(RTCState *s, const struct tm *tm);
diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
index fe6ed63f71..0f1c886e5b 100644
--- a/include/hw/timer/mc146818rtc.h
+++ b/include/hw/timer/mc146818rtc.h
@@ -1,10 +1,43 @@
 #ifndef MC146818RTC_H
 #define MC146818RTC_H
 
+#include "qapi/qapi-types-misc.h"
+#include "qemu/queue.h"
+#include "qemu/timer.h"
 #include "hw/isa/isa.h"
 #include "hw/timer/mc146818rtc_regs.h"
 
 #define TYPE_MC146818_RTC "mc146818rtc"
+#define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
+
+typedef struct RTCState {
+    ISADevice parent_obj;
+
+    MemoryRegion io;
+    MemoryRegion coalesced_io;
+    uint8_t cmos_data[128];
+    uint8_t cmos_index;
+    int32_t base_year;
+    uint64_t base_rtc;
+    uint64_t last_update;
+    int64_t offset;
+    qemu_irq irq;
+    int it_shift;
+    /* periodic timer */
+    QEMUTimer *periodic_timer;
+    int64_t next_periodic_time;
+    /* update-ended timer */
+    QEMUTimer *update_timer;
+    uint64_t next_alarm_time;
+    uint16_t irq_reinject_on_ack_count;
+    uint32_t irq_coalesced;
+    uint32_t period;
+    QEMUTimer *coalesced_timer;
+    Notifier clock_reset_notifier;
+    LostTickPolicy lost_tick_policy;
+    Notifier suspend_notifier;
+    QLIST_ENTRY(RTCState) link;
+} RTCState;
 
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
                              qemu_irq intercept_irq);
-- 
2.21.0

[PATCH 2/4] mc146818rtc: Move RTC_ISA_IRQ definition

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The ISA default number for the RTC devices is not related to its
registers neither. Move this definition to "hw/timer/mc146818rtc.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/timer/mc146818rtc.h      | 2 ++
 include/hw/timer/mc146818rtc_regs.h | 2 --
 tests/rtc-test.c                    | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
index 0f1c886e5b..17761cf6d9 100644
--- a/include/hw/timer/mc146818rtc.h
+++ b/include/hw/timer/mc146818rtc.h
@@ -39,6 +39,8 @@ typedef struct RTCState {
     QLIST_ENTRY(RTCState) link;
 } RTCState;
 
+#define RTC_ISA_IRQ 8
+
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
                              qemu_irq intercept_irq);
 void rtc_set_memory(ISADevice *dev, int addr, int val);
diff --git a/include/hw/timer/mc146818rtc_regs.h b/include/hw/timer/mc146818rtc_regs.h
index bfbb57e570..631f71cfd9 100644
--- a/include/hw/timer/mc146818rtc_regs.h
+++ b/include/hw/timer/mc146818rtc_regs.h
@@ -27,8 +27,6 @@
 
 #include "qemu/timer.h"
 
-#define RTC_ISA_IRQ 8
-
 #define RTC_SECONDS             0
 #define RTC_SECONDS_ALARM       1
 #define RTC_MINUTES             2
diff --git a/tests/rtc-test.c b/tests/rtc-test.c
index 6309b0ef6c..18f895690f 100644
--- a/tests/rtc-test.c
+++ b/tests/rtc-test.c
@@ -15,6 +15,7 @@
 
 #include "libqtest-single.h"
 #include "qemu/timer.h"
+#include "hw/timer/mc146818rtc.h"
 #include "hw/timer/mc146818rtc_regs.h"
 
 #define UIP_HOLD_LENGTH           (8 * NANOSECONDS_PER_SECOND / 32768)
-- 
2.21.0

Re: [PATCH 2/4] mc146818rtc: Move RTC_ISA_IRQ definition

[Thomas Huth]

On 18/10/2019 15.35, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> The ISA default number for the RTC devices is not related to its
> registers neither. Move this definition to "hw/timer/mc146818rtc.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/hw/timer/mc146818rtc.h      | 2 ++
>  include/hw/timer/mc146818rtc_regs.h | 2 --
>  tests/rtc-test.c                    | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 3/4] mc146818rtc: Include mc146818rtc_regs.h directly in mc146818rtc.c

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Devices/boards wanting to use the MC146818 RTC don't need
the knowledge its internal registers. Move the "mc146818rtc_regs.h"
inclusion to mc146818rtc.c where it is required.

We can not move this file from include/hw/timer/ to hw/timer/ for
local inclusion because the ACPI FADT table use the RTC_CENTURY
register address.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/timer/mc146818rtc.c         | 1 +
 include/hw/timer/mc146818rtc.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index e40b54e743..0c04b74c2e 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -41,6 +41,7 @@
 #include "qapi/qapi-events-misc-target.h"
 #include "qapi/visitor.h"
 #include "exec/address-spaces.h"
+#include "hw/timer/mc146818rtc_regs.h"
 
 #ifdef TARGET_I386
 #include "hw/i386/apic.h"
diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
index 17761cf6d9..a857dcdc69 100644
--- a/include/hw/timer/mc146818rtc.h
+++ b/include/hw/timer/mc146818rtc.h
@@ -5,7 +5,6 @@
 #include "qemu/queue.h"
 #include "qemu/timer.h"
 #include "hw/isa/isa.h"
-#include "hw/timer/mc146818rtc_regs.h"
 
 #define TYPE_MC146818_RTC "mc146818rtc"
 #define MC146818_RTC(obj) OBJECT_CHECK(RTCState, (obj), TYPE_MC146818_RTC)
-- 
2.21.0

[PATCH 4/4] mc146818rtc: always register rtc to rtc list

[Philippe Mathieu-Daudé]

From: Hervé Poussineau <hpoussin@reactos.org>

We are not required anymore to use rtc_init() function.

Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Message-Id: <20171216090228.28505-5-hpoussin@reactos.org>
[PMD: rebased, fix OBJECT() value]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/timer/mc146818rtc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 0c04b74c2e..8f7d3a9cdf 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -963,17 +963,16 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     object_property_add_tm(OBJECT(s), "date", rtc_get_date, NULL);
 
     qdev_init_gpio_out(dev, &s->irq, 1);
+    QLIST_INSERT_HEAD(&rtc_devices, s, link);
 }
 
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
 {
     DeviceState *dev;
     ISADevice *isadev;
-    RTCState *s;
 
     isadev = isa_create(bus, TYPE_MC146818_RTC);
     dev = DEVICE(isadev);
-    s = MC146818_RTC(isadev);
     qdev_prop_set_int32(dev, "base_year", base_year);
     qdev_init_nofail(dev);
     if (intercept_irq) {
@@ -981,9 +980,8 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
     } else {
         isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
     }
-    QLIST_INSERT_HEAD(&rtc_devices, s, link);
 
-    object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(s),
+    object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(isadev),
                               "date", NULL);
 
     return isadev;
@@ -1015,8 +1013,6 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
     dc->reset = rtc_resetdev;
     dc->vmsd = &vmstate_rtc;
     dc->props = mc146818rtc_properties;
-    /* Reason: needs to be wired up by rtc_init() */
-    dc->user_creatable = false;
 }
 
 static const TypeInfo mc146818rtc_info = {
-- 
2.21.0

Re: [PATCH 0/4] mc146818rtc: Allow call object_initialize(MC146818_RTC) instead of rtc_init()

[Paolo Bonzini]

On 18/10/19 15:35, Philippe Mathieu-Daudé wrote:
> Four RTC146818 patches extracted from a bigger series:
> "hw/i386/pc: Split PIIX3 southbridge from i440FX northbridge"
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg03685.html
> 
> This step is required to be able to create a MC146818_RTC within
> a QOM object (which will be used in the "piix4: add a mc146818rtc
> controller" patch later).
> 
> No changes since previous post:
> $ git backport-diff -u pc_split_i440fx_piix
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/4:[----] [--] 'mc146818rtc: move structure to header file'
> 002/4:[----] [--] 'mc146818rtc: Move RTC_ISA_IRQ definition'
> 003/4:[----] [--] 'mc146818rtc: Include "mc146818rtc_regs.h" directly in mc146818rtc.c'
> 004/4:[----] [--] 'mc146818rtc: always register rtc to rtc list'
> 
> Hervé Poussineau (2):
>   mc146818rtc: move structure to header file
>   mc146818rtc: always register rtc to rtc list
> 
> Philippe Mathieu-Daudé (2):
>   mc146818rtc: Move RTC_ISA_IRQ definition
>   mc146818rtc: Include mc146818rtc_regs.h directly in mc146818rtc.c
> 
>  hw/timer/mc146818rtc.c              | 39 +++--------------------------
>  include/hw/timer/mc146818rtc.h      | 36 +++++++++++++++++++++++++-
>  include/hw/timer/mc146818rtc_regs.h |  2 --
>  tests/rtc-test.c                    |  1 +
>  4 files changed, 39 insertions(+), 39 deletions(-)
> 

Queued, thanks.

Paolo