All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms
@ 2016-10-17  4:23 David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option David Gibson
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: David Gibson @ 2016-10-17  4:23 UTC (permalink / raw)
  To: mst, pbonzini; +Cc: qemu-devel, armbru, David Gibson

This is a rebase/repost of a series I sent quite some time ago.  This
makes some cleanups that are a start on allowing ISA to be compiled
out for platforms which don't use it.

David Gibson (3):
  Split serial-isa into its own config option
  Allow ISA bus to be configured out
  Split ISA and sysbus versions of m48t59 device

 default-configs/alpha-softmmu.mak       |   1 +
 default-configs/arm-softmmu.mak         |   1 +
 default-configs/i386-softmmu.mak        |   1 +
 default-configs/mips-softmmu-common.mak |   1 +
 default-configs/moxie-softmmu.mak       |   2 +
 default-configs/pci.mak                 |   3 +
 default-configs/ppc-softmmu.mak         |   1 +
 default-configs/ppc64-softmmu.mak       |   1 +
 default-configs/ppcemb-softmmu.mak      |   1 +
 default-configs/sh4-softmmu.mak         |   1 +
 default-configs/sh4eb-softmmu.mak       |   1 +
 default-configs/sparc-softmmu.mak       |   1 +
 default-configs/sparc64-softmmu.mak     |   1 +
 default-configs/unicore32-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak      |   1 +
 hw/char/Makefile.objs                   |   3 +-
 hw/isa/Makefile.objs                    |   2 +-
 hw/timer/Makefile.objs                  |   3 +
 hw/timer/m48t59-internal.h              |  82 ++++++++++++
 hw/timer/m48t59-isa.c                   | 180 +++++++++++++++++++++++++
 hw/timer/m48t59.c                       | 228 +++-----------------------------
 21 files changed, 304 insertions(+), 212 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

-- 
2.7.4

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

* [Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option
  2016-10-17  4:23 [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms David Gibson
@ 2016-10-17  4:23 ` David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 2/3] Allow ISA bus to be configured out David Gibson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2016-10-17  4:23 UTC (permalink / raw)
  To: mst, pbonzini; +Cc: qemu-devel, armbru, David Gibson

At present, the core device model code for 8250-like serial ports
(serial.c) and the code for serial ports attached to ISA-style legacy IO
(serial-isa.c) are both controlled by the CONFIG_SERIAL variable.

There are lots and lots of embedded platforms that have 8250-like serial
ports but have never had anything resembling ISA legacy IO.  Therefore,
split serial-isa into its own CONFIG_SERIAL_ISA option so it can be
disabled for platforms where it's not appropriate.

For now, I enabled CONFIG_SERIAL_ISA in every default-config where
CONFIG_SERIAL is enabled, excepting microblaze, or32, and xtensa.  As best
as I can tell, those platforms never used legacy ISA, and also don't
include PCI support (which would allow connection of a PCI->ISA bridge
and/or a southbridge including legacy ISA serial ports).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 default-configs/alpha-softmmu.mak       | 1 +
 default-configs/arm-softmmu.mak         | 1 +
 default-configs/i386-softmmu.mak        | 1 +
 default-configs/mips-softmmu-common.mak | 1 +
 default-configs/moxie-softmmu.mak       | 1 +
 default-configs/pci.mak                 | 1 +
 default-configs/ppc-softmmu.mak         | 1 +
 default-configs/ppc64-softmmu.mak       | 1 +
 default-configs/ppcemb-softmmu.mak      | 1 +
 default-configs/sh4-softmmu.mak         | 1 +
 default-configs/sh4eb-softmmu.mak       | 1 +
 default-configs/sparc64-softmmu.mak     | 1 +
 default-configs/x86_64-softmmu.mak      | 1 +
 hw/char/Makefile.objs                   | 3 ++-
 14 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/default-configs/alpha-softmmu.mak b/default-configs/alpha-softmmu.mak
index 7f6161e..e0d75e3 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 include usb.mak
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8254=y
 CONFIG_PCKBD=y
 CONFIG_VGA_CIRRUS=y
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6de3e16..dcbcea7 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -6,6 +6,7 @@ CONFIG_VGA=y
 CONFIG_NAND=y
 CONFIG_ECC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PTIMER=y
 CONFIG_SD=y
 CONFIG_MAX7310=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..3f2e820 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -15,6 +15,7 @@ CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_KCS=y
 CONFIG_ISA_IPMI_BT=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
index 0394514..5b8b0c9 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -9,6 +9,7 @@ CONFIG_VGA_ISA_MM=y
 CONFIG_VGA_CIRRUS=y
 CONFIG_VMWARE_VGA=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak
index 1a95476..7e22863 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -2,4 +2,5 @@
 
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_VGA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index fff7ce3..d8d6548 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -27,6 +27,7 @@ CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_SERIAL_PCI=y
 CONFIG_IPACK=y
 CONFIG_WDT_IB6300ESB=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index d4d0f9b..13eb94f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -45,5 +45,6 @@ CONFIG_PLATFORM_BUS=y
 CONFIG_ETSEC=y
 CONFIG_LIBDECNUMBER=y
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index db5a4d6..7a3b10e 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_XICS=$(CONFIG_PSERIES)
 CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
 CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
+CONFIG_SERIAL_ISA=y
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
 CONFIG_MEM_HOTPLUG=y
diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
index 54acc4d..7f56004 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -5,6 +5,7 @@ include sound.mak
 include usb.mak
 CONFIG_M48T59=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_I8257=y
 CONFIG_OPENPIC=y
 CONFIG_PFLASH_CFI01=y
diff --git a/default-configs/sh4-softmmu.mak b/default-configs/sh4-softmmu.mak
index 8e00390..546d855 100644
--- a/default-configs/sh4-softmmu.mak
+++ b/default-configs/sh4-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 include usb.mak
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PTIMER=y
 CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
diff --git a/default-configs/sh4eb-softmmu.mak b/default-configs/sh4eb-softmmu.mak
index efdd058..2d3fd49 100644
--- a/default-configs/sh4eb-softmmu.mak
+++ b/default-configs/sh4eb-softmmu.mak
@@ -3,6 +3,7 @@
 include pci.mak
 include usb.mak
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PTIMER=y
 CONFIG_PFLASH_CFI02=y
 CONFIG_SH4=y
diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak
index c0cdd64..922db55 100644
--- a/default-configs/sparc64-softmmu.mak
+++ b/default-configs/sparc64-softmmu.mak
@@ -5,6 +5,7 @@ include usb.mak
 CONFIG_M48T59=y
 CONFIG_PTIMER=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_PCKBD=y
 CONFIG_FDC=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 7f89503..f34bc80 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -15,6 +15,7 @@ CONFIG_IPMI_EXTERN=y
 CONFIG_ISA_IPMI_KCS=y
 CONFIG_ISA_IPMI_BT=y
 CONFIG_SERIAL=y
+CONFIG_SERIAL_ISA=y
 CONFIG_PARALLEL=y
 CONFIG_I8254=y
 CONFIG_PCSPK=y
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 69a553c..6ea76fe 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -2,7 +2,8 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_PL011) += pl011.o
-common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
+common-obj-$(CONFIG_SERIAL) += serial.o
+common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
 common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
 common-obj-$(CONFIG_VIRTIO) += virtio-console.o
 common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
-- 
2.7.4

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

* [Qemu-devel] [PATCH 2/3] Allow ISA bus to be configured out
  2016-10-17  4:23 [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option David Gibson
@ 2016-10-17  4:23 ` David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
  2016-10-17  4:36 ` [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms no-reply
  3 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2016-10-17  4:23 UTC (permalink / raw)
  To: mst, pbonzini; +Cc: qemu-devel, armbru, David Gibson

Currently, the code to handle the legacy ISA bus is always included in
qemu.  However there are lots of platforms that don't include ISA legacy
devies, and quite a few that have never used ISA legacy devices at all.

This patch allows the ISA bus code to be disabled in the configuration for
platforms where it doesn't make sense.

For now, the default configs are adjusted to include ISA on all platforms
including PCI: anything with PCI can at least in principle add an i82378
PCI->ISA bridge.  Also, CONFIG_IDE_CORE which is already in pci.mak
requires ISA support.

We also explicitly enable ISA on some other non-PCI platforms which include
ISA devices: moxie, sparc and unicore32.  We may want to pare this down in
future.

The platforms that will lose ISA by default are: cris, lm32, microblazeel,
microblaze, or32, s390x, tricore, xtensaeb, xtensa.  As far as I can tell
none of these ever used ISA.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 default-configs/moxie-softmmu.mak     | 1 +
 default-configs/pci.mak               | 2 ++
 default-configs/sparc-softmmu.mak     | 1 +
 default-configs/unicore32-softmmu.mak | 1 +
 hw/isa/Makefile.objs                  | 2 +-
 5 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/default-configs/moxie-softmmu.mak b/default-configs/moxie-softmmu.mak
index 7e22863..e00d099 100644
--- a/default-configs/moxie-softmmu.mak
+++ b/default-configs/moxie-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for moxie-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_MC146818RTC=y
 CONFIG_SERIAL=y
 CONFIG_SERIAL_ISA=y
diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index d8d6548..60dc651 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -1,4 +1,6 @@
 CONFIG_PCI=y
+# For now, CONFIG_IDE_CORE requires ISA, so we enable it here
+CONFIG_ISA_BUS=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO=y
 CONFIG_USB_UHCI=y
diff --git a/default-configs/sparc-softmmu.mak b/default-configs/sparc-softmmu.mak
index ab796b3..004b0f4 100644
--- a/default-configs/sparc-softmmu.mak
+++ b/default-configs/sparc-softmmu.mak
@@ -1,5 +1,6 @@
 # Default configuration for sparc-softmmu
 
+CONFIG_ISA_BUS=y
 CONFIG_ECC=y
 CONFIG_ESP=y
 CONFIG_ESCC=y
diff --git a/default-configs/unicore32-softmmu.mak b/default-configs/unicore32-softmmu.mak
index de38577..5f6c4a8 100644
--- a/default-configs/unicore32-softmmu.mak
+++ b/default-configs/unicore32-softmmu.mak
@@ -1,4 +1,5 @@
 # Default configuration for unicore32-softmmu
+CONFIG_ISA_BUS=y
 CONFIG_PUV3=y
 CONFIG_PTIMER=y
 CONFIG_PCKBD=y
diff --git a/hw/isa/Makefile.objs b/hw/isa/Makefile.objs
index 9164556..fb37c55 100644
--- a/hw/isa/Makefile.objs
+++ b/hw/isa/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += isa-bus.o
+common-obj-$(CONFIG_ISA_BUS) += isa-bus.o
 common-obj-$(CONFIG_APM) += apm.o
 common-obj-$(CONFIG_I82378) += i82378.o
 common-obj-$(CONFIG_PC87312) += pc87312.o
-- 
2.7.4

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

* [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device
  2016-10-17  4:23 [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option David Gibson
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 2/3] Allow ISA bus to be configured out David Gibson
@ 2016-10-17  4:23 ` David Gibson
  2016-11-04 10:22   ` Markus Armbruster
  2016-10-17  4:36 ` [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms no-reply
  3 siblings, 1 reply; 8+ messages in thread
From: David Gibson @ 2016-10-17  4:23 UTC (permalink / raw)
  To: mst, pbonzini; +Cc: qemu-devel, armbru, David Gibson

The m48t59 device supports both ISA and direct sysbus attached versions of
the device in the one .c file.  This can be awkward for some embedded
machine types which need the sysbus M48T59, but don't want to pull in the
ISA bus code and its other dependencies.

Therefore, this patch splits out the code for the ISA attached M48T59 into
its own C file.  It will be built when both CONFIG_M48T59 and
CONFIG_ISA_BUS are enabled.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/timer/Makefile.objs     |   3 +
 hw/timer/m48t59-internal.h |  82 ++++++++++++++++
 hw/timer/m48t59-isa.c      | 180 +++++++++++++++++++++++++++++++++++
 hw/timer/m48t59.c          | 228 ++++-----------------------------------------
 4 files changed, 283 insertions(+), 210 deletions(-)
 create mode 100644 hw/timer/m48t59-internal.h
 create mode 100644 hw/timer/m48t59-isa.c

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 7ba8c23..bf3ea3c 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -6,6 +6,9 @@ common-obj-$(CONFIG_DS1338) += ds1338.o
 common-obj-$(CONFIG_HPET) += hpet.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_M48T59) += m48t59.o
+ifeq ($(CONFIG_ISA_BUS),y)
+common-obj-$(CONFIG_M48T59) += m48t59-isa.o
+endif
 common-obj-$(CONFIG_PL031) += pl031.o
 common-obj-$(CONFIG_PUV3) += puv3_ost.o
 common-obj-$(CONFIG_TWL92230) += twl92230.o
diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
new file mode 100644
index 0000000..c50f134
--- /dev/null
+++ b/hw/timer/m48t59-internal.h
@@ -0,0 +1,82 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (common header)
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef HW_M48T59_H
+#define HW_M48T59_H 1
+
+//#define DEBUG_NVRAM
+
+#if defined(DEBUG_NVRAM)
+#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
+#else
+#define NVRAM_PRINTF(fmt, ...) do { } while (0)
+#endif
+
+/*
+ * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
+ * alarm and a watchdog timer and related control registers. In the
+ * PPC platform there is also a nvram lock function.
+ */
+
+typedef struct M48txxInfo {
+    const char *bus_name;
+    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+    uint32_t size;
+} M48txxInfo;
+
+typedef struct M48t59State {
+    /* Hardware parameters */
+    qemu_irq IRQ;
+    MemoryRegion iomem;
+    uint32_t size;
+    int32_t base_year;
+    /* RTC management */
+    time_t   time_offset;
+    time_t   stop_time;
+    /* Alarm & watchdog */
+    struct tm alarm;
+    QEMUTimer *alrm_timer;
+    QEMUTimer *wd_timer;
+    /* NVRAM storage */
+    uint8_t *buffer;
+    /* Model parameters */
+    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+    /* NVRAM storage */
+    uint16_t addr;
+    uint8_t  lock;
+} M48t59State;
+
+uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr);
+void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val);
+void m48t59_reset_common(M48t59State *NVRAM);
+void m48t59_realize_common(M48t59State *s, Error **errp);
+
+static inline void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
+{
+    NVRAM->lock ^= 1 << lock;
+}
+
+extern const MemoryRegionOps m48t59_io_ops;
+
+#endif /* HW_M48T59_H */
diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
new file mode 100644
index 0000000..3a521dc
--- /dev/null
+++ b/hw/timer/m48t59-isa.c
@@ -0,0 +1,180 @@
+/*
+ * QEMU M48T59 and M48T08 NVRAM emulation (ISA bus interface
+ *
+ * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "hw/isa/isa.h"
+#include "hw/timer/m48t59.h"
+#include "m48t59-internal.h"
+
+#define TYPE_M48TXX_ISA "isa-m48txx"
+#define M48TXX_ISA_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(M48txxISADeviceClass, (obj), TYPE_M48TXX_ISA)
+#define M48TXX_ISA_CLASS(klass) \
+    OBJECT_CLASS_CHECK(M48txxISADeviceClass, (klass), TYPE_M48TXX_ISA)
+#define M48TXX_ISA(obj) \
+    OBJECT_CHECK(M48txxISAState, (obj), TYPE_M48TXX_ISA)
+
+typedef struct M48txxISAState {
+    ISADevice parent_obj;
+    M48t59State state;
+    uint32_t io_base;
+    MemoryRegion io;
+} M48txxISAState;
+
+typedef struct M48txxISADeviceClass {
+    ISADeviceClass parent_class;
+    M48txxInfo info;
+} M48txxISADeviceClass;
+
+static M48txxInfo m48txx_isa_info[] = {
+    {
+        .bus_name = "isa-m48t59",
+        .model = 59,
+        .size = 0x2000,
+    }
+};
+
+Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
+                       int base_year, int model)
+{
+    DeviceState *dev;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(m48txx_isa_info); i++) {
+        if (m48txx_isa_info[i].size != size ||
+            m48txx_isa_info[i].model != model) {
+            continue;
+        }
+
+        dev = DEVICE(isa_create(bus, m48txx_isa_info[i].bus_name));
+        qdev_prop_set_uint32(dev, "iobase", io_base);
+        qdev_prop_set_int32(dev, "base-year", base_year);
+        qdev_init_nofail(dev);
+        return NVRAM(dev);
+    }
+
+    assert(false);
+    return NULL;
+}
+
+static uint32_t m48txx_isa_read(Nvram *obj, uint32_t addr)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    return m48t59_read(&d->state, addr);
+}
+
+static void m48txx_isa_write(Nvram *obj, uint32_t addr, uint32_t val)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    m48t59_write(&d->state, addr, val);
+}
+
+static void m48txx_isa_toggle_lock(Nvram *obj, int lock)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    m48t59_toggle_lock(&d->state, lock);
+}
+
+static Property m48t59_isa_properties[] = {
+    DEFINE_PROP_INT32("base-year", M48txxISAState, state.base_year, 0),
+    DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void m48t59_reset_isa(DeviceState *d)
+{
+    M48txxISAState *isa = M48TXX_ISA(d);
+    M48t59State *NVRAM = &isa->state;
+
+    m48t59_reset_common(NVRAM);
+}
+
+static void m48t59_isa_realize(DeviceState *dev, Error **errp)
+{
+    M48txxISADeviceClass *u = M48TXX_ISA_GET_CLASS(dev);
+    ISADevice *isadev = ISA_DEVICE(dev);
+    M48txxISAState *d = M48TXX_ISA(dev);
+    M48t59State *s = &d->state;
+
+    s->model = u->info.model;
+    s->size = u->info.size;
+    isa_init_irq(isadev, &s->IRQ, 8);
+    m48t59_realize_common(s, errp);
+    memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4);
+    if (d->io_base != 0) {
+        isa_register_ioport(isadev, &d->io, d->io_base);
+    }
+}
+
+static void m48txx_isa_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    NvramClass *nc = NVRAM_CLASS(klass);
+
+    dc->realize = m48t59_isa_realize;
+    dc->reset = m48t59_reset_isa;
+    dc->props = m48t59_isa_properties;
+    nc->read = m48txx_isa_read;
+    nc->write = m48txx_isa_write;
+    nc->toggle_lock = m48txx_isa_toggle_lock;
+}
+
+static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
+{
+    M48txxISADeviceClass *u = M48TXX_ISA_CLASS(klass);
+    M48txxInfo *info = data;
+
+    u->info = *info;
+}
+
+static const TypeInfo m48txx_isa_type_info = {
+    .name = TYPE_M48TXX_ISA,
+    .parent = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(M48txxISAState),
+    .abstract = true,
+    .class_init = m48txx_isa_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_NVRAM },
+        { }
+    }
+};
+
+static void m48t59_isa_register_types(void)
+{
+    TypeInfo isa_type_info = {
+        .parent = TYPE_M48TXX_ISA,
+        .class_size = sizeof(M48txxISADeviceClass),
+        .class_init = m48txx_isa_concrete_class_init,
+    };
+    int i;
+
+    type_register_static(&m48txx_isa_type_info);
+
+    for (i = 0; i < ARRAY_SIZE(m48txx_isa_info); i++) {
+        isa_type_info.name = m48txx_isa_info[i].bus_name;
+        isa_type_info.class_data = &m48txx_isa_info[i];
+        type_register(&isa_type_info);
+    }
+}
+
+type_init(m48t59_isa_register_types)
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index e46ca88..0157977 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -29,17 +29,10 @@
 #include "qemu/timer.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
-#include "hw/isa/isa.h"
 #include "exec/address-spaces.h"
 #include "qemu/bcd.h"
 
-//#define DEBUG_NVRAM
-
-#if defined(DEBUG_NVRAM)
-#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
-#else
-#define NVRAM_PRINTF(fmt, ...) do { } while (0)
-#endif
+#include "m48t59-internal.h"
 
 #define TYPE_M48TXX_SYS_BUS "sysbus-m48txx"
 #define M48TXX_SYS_BUS_GET_CLASS(obj) \
@@ -49,27 +42,6 @@
 #define M48TXX_SYS_BUS(obj) \
     OBJECT_CHECK(M48txxSysBusState, (obj), TYPE_M48TXX_SYS_BUS)
 
-#define TYPE_M48TXX_ISA "isa-m48txx"
-#define M48TXX_ISA_GET_CLASS(obj) \
-    OBJECT_GET_CLASS(M48txxISADeviceClass, (obj), TYPE_M48TXX_ISA)
-#define M48TXX_ISA_CLASS(klass) \
-    OBJECT_CLASS_CHECK(M48txxISADeviceClass, (klass), TYPE_M48TXX_ISA)
-#define M48TXX_ISA(obj) \
-    OBJECT_CHECK(M48txxISAState, (obj), TYPE_M48TXX_ISA)
-
-/*
- * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
- * alarm and a watchdog timer and related control registers. In the
- * PPC platform there is also a nvram lock function.
- */
-
-typedef struct M48txxInfo {
-    const char *isa_name;
-    const char *sysbus_name;
-    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
-    uint32_t size;
-} M48txxInfo;
-
 /*
  * Chipset docs:
  * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
@@ -77,40 +49,6 @@ typedef struct M48txxInfo {
  * http://www.st.com/stonline/products/literature/od/7001/m48t59y.pdf
  */
 
-typedef struct M48t59State {
-    /* Hardware parameters */
-    qemu_irq IRQ;
-    MemoryRegion iomem;
-    uint32_t size;
-    int32_t base_year;
-    /* RTC management */
-    time_t   time_offset;
-    time_t   stop_time;
-    /* Alarm & watchdog */
-    struct tm alarm;
-    QEMUTimer *alrm_timer;
-    QEMUTimer *wd_timer;
-    /* NVRAM storage */
-    uint8_t *buffer;
-    /* Model parameters */
-    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
-    /* NVRAM storage */
-    uint16_t addr;
-    uint8_t  lock;
-} M48t59State;
-
-typedef struct M48txxISAState {
-    ISADevice parent_obj;
-    M48t59State state;
-    uint32_t io_base;
-    MemoryRegion io;
-} M48txxISAState;
-
-typedef struct M48txxISADeviceClass {
-    ISADeviceClass parent_class;
-    M48txxInfo info;
-} M48txxISADeviceClass;
-
 typedef struct M48txxSysBusState {
     SysBusDevice parent_obj;
     M48t59State state;
@@ -122,21 +60,17 @@ typedef struct M48txxSysBusDeviceClass {
     M48txxInfo info;
 } M48txxSysBusDeviceClass;
 
-static M48txxInfo m48txx_info[] = {
+static M48txxInfo m48txx_sysbus_info[] = {
     {
-        .sysbus_name = "sysbus-m48t02",
+        .bus_name = "sysbus-m48t02",
         .model = 2,
         .size = 0x800,
     },{
-        .sysbus_name = "sysbus-m48t08",
+        .bus_name = "sysbus-m48t08",
         .model = 8,
         .size = 0x2000,
     },{
-        .sysbus_name = "sysbus-m48t59",
-        .model = 59,
-        .size = 0x2000,
-    },{
-        .isa_name = "isa-m48t59",
+        .bus_name = "sysbus-m48t59",
         .model = 59,
         .size = 0x2000,
     }
@@ -248,7 +182,7 @@ static void set_up_watchdog(M48t59State *NVRAM, uint8_t value)
 }
 
 /* Direct access to NVRAM */
-static void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val)
+void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val)
 {
     struct tm tm;
     int tmp;
@@ -413,7 +347,7 @@ static void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr)
+uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr)
 {
     struct tm tm;
     uint32_t retval = 0xFF;
@@ -517,11 +451,6 @@ static uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr)
     return retval;
 }
 
-static void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
-{
-    NVRAM->lock ^= 1 << lock;
-}
-
 /* IO access to NVRAM */
 static void NVRAM_writeb(void *opaque, hwaddr addr, uint64_t val,
                          unsigned size)
@@ -639,7 +568,7 @@ static const VMStateDescription vmstate_m48t59 = {
     }
 };
 
-static void m48t59_reset_common(M48t59State *NVRAM)
+void m48t59_reset_common(M48t59State *NVRAM)
 {
     NVRAM->addr = 0;
     NVRAM->lock = 0;
@@ -650,14 +579,6 @@ static void m48t59_reset_common(M48t59State *NVRAM)
         timer_del(NVRAM->wd_timer);
 }
 
-static void m48t59_reset_isa(DeviceState *d)
-{
-    M48txxISAState *isa = M48TXX_ISA(d);
-    M48t59State *NVRAM = &isa->state;
-
-    m48t59_reset_common(NVRAM);
-}
-
 static void m48t59_reset_sysbus(DeviceState *d)
 {
     M48txxSysBusState *sys = M48TXX_SYS_BUS(d);
@@ -666,7 +587,7 @@ static void m48t59_reset_sysbus(DeviceState *d)
     m48t59_reset_common(NVRAM);
 }
 
-static const MemoryRegionOps m48t59_io_ops = {
+const MemoryRegionOps m48t59_io_ops = {
     .read = NVRAM_readb,
     .write = NVRAM_writeb,
     .impl = {
@@ -685,14 +606,13 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
     SysBusDevice *s;
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
-        if (!m48txx_info[i].sysbus_name ||
-            m48txx_info[i].size != size ||
-            m48txx_info[i].model != model) {
+    for (i = 0; i < ARRAY_SIZE(m48txx_sysbus_info); i++) {
+        if (m48txx_sysbus_info[i].size != size ||
+            m48txx_sysbus_info[i].model != model) {
             continue;
         }
 
-        dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
+        dev = qdev_create(NULL, m48txx_sysbus_info[i].bus_name);
         qdev_prop_set_int32(dev, "base-year", base_year);
         qdev_init_nofail(dev);
         s = SYS_BUS_DEVICE(dev);
@@ -712,31 +632,7 @@ Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
     return NULL;
 }
 
-Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                       int base_year, int model)
-{
-    DeviceState *dev;
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
-        if (!m48txx_info[i].isa_name ||
-            m48txx_info[i].size != size ||
-            m48txx_info[i].model != model) {
-            continue;
-        }
-
-        dev = DEVICE(isa_create(bus, m48txx_info[i].isa_name));
-        qdev_prop_set_uint32(dev, "iobase", io_base);
-        qdev_prop_set_int32(dev, "base-year", base_year);
-        qdev_init_nofail(dev);
-        return NVRAM(dev);
-    }
-
-    assert(false);
-    return NULL;
-}
-
-static void m48t59_realize_common(M48t59State *s, Error **errp)
+void m48t59_realize_common(M48t59State *s, Error **errp)
 {
     s->buffer = g_malloc0(s->size);
     if (s->model == 59) {
@@ -748,23 +644,6 @@ static void m48t59_realize_common(M48t59State *s, Error **errp)
     vmstate_register(NULL, -1, &vmstate_m48t59, s);
 }
 
-static void m48t59_isa_realize(DeviceState *dev, Error **errp)
-{
-    M48txxISADeviceClass *u = M48TXX_ISA_GET_CLASS(dev);
-    ISADevice *isadev = ISA_DEVICE(dev);
-    M48txxISAState *d = M48TXX_ISA(dev);
-    M48t59State *s = &d->state;
-
-    s->model = u->info.model;
-    s->size = u->info.size;
-    isa_init_irq(isadev, &s->IRQ, 8);
-    m48t59_realize_common(s, errp);
-    memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4);
-    if (d->io_base != 0) {
-        isa_register_ioport(isadev, &d->io, d->io_base);
-    }
-}
-
 static int m48t59_init1(SysBusDevice *dev)
 {
     M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_GET_CLASS(dev);
@@ -791,51 +670,6 @@ static int m48t59_init1(SysBusDevice *dev)
     return 0;
 }
 
-static uint32_t m48txx_isa_read(Nvram *obj, uint32_t addr)
-{
-    M48txxISAState *d = M48TXX_ISA(obj);
-    return m48t59_read(&d->state, addr);
-}
-
-static void m48txx_isa_write(Nvram *obj, uint32_t addr, uint32_t val)
-{
-    M48txxISAState *d = M48TXX_ISA(obj);
-    m48t59_write(&d->state, addr, val);
-}
-
-static void m48txx_isa_toggle_lock(Nvram *obj, int lock)
-{
-    M48txxISAState *d = M48TXX_ISA(obj);
-    m48t59_toggle_lock(&d->state, lock);
-}
-
-static Property m48t59_isa_properties[] = {
-    DEFINE_PROP_INT32("base-year", M48txxISAState, state.base_year, 0),
-    DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void m48txx_isa_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    NvramClass *nc = NVRAM_CLASS(klass);
-
-    dc->realize = m48t59_isa_realize;
-    dc->reset = m48t59_reset_isa;
-    dc->props = m48t59_isa_properties;
-    nc->read = m48txx_isa_read;
-    nc->write = m48txx_isa_write;
-    nc->toggle_lock = m48txx_isa_toggle_lock;
-}
-
-static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
-{
-    M48txxISADeviceClass *u = M48TXX_ISA_CLASS(klass);
-    M48txxInfo *info = data;
-
-    u->info = *info;
-}
-
 static uint32_t m48txx_sysbus_read(Nvram *obj, uint32_t addr)
 {
     M48txxSysBusState *d = M48TXX_SYS_BUS(obj);
@@ -899,18 +733,6 @@ static const TypeInfo m48txx_sysbus_type_info = {
     }
 };
 
-static const TypeInfo m48txx_isa_type_info = {
-    .name = TYPE_M48TXX_ISA,
-    .parent = TYPE_ISA_DEVICE,
-    .instance_size = sizeof(M48txxISAState),
-    .abstract = true,
-    .class_init = m48txx_isa_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { TYPE_NVRAM },
-        { }
-    }
-};
-
 static void m48t59_register_types(void)
 {
     TypeInfo sysbus_type_info = {
@@ -918,29 +740,15 @@ static void m48t59_register_types(void)
         .class_size = sizeof(M48txxSysBusDeviceClass),
         .class_init = m48txx_sysbus_concrete_class_init,
     };
-    TypeInfo isa_type_info = {
-        .parent = TYPE_M48TXX_ISA,
-        .class_size = sizeof(M48txxISADeviceClass),
-        .class_init = m48txx_isa_concrete_class_init,
-    };
     int i;
 
     type_register_static(&nvram_info);
     type_register_static(&m48txx_sysbus_type_info);
-    type_register_static(&m48txx_isa_type_info);
 
-    for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
-        if (m48txx_info[i].sysbus_name) {
-            sysbus_type_info.name = m48txx_info[i].sysbus_name;
-            sysbus_type_info.class_data = &m48txx_info[i];
-            type_register(&sysbus_type_info);
-        }
-
-        if (m48txx_info[i].isa_name) {
-            isa_type_info.name = m48txx_info[i].isa_name;
-            isa_type_info.class_data = &m48txx_info[i];
-            type_register(&isa_type_info);
-        }
+    for (i = 0; i < ARRAY_SIZE(m48txx_sysbus_info); i++) {
+        sysbus_type_info.name = m48txx_sysbus_info[i].bus_name;
+        sysbus_type_info.class_data = &m48txx_sysbus_info[i];
+        type_register(&sysbus_type_info);
     }
 }
 
-- 
2.7.4

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

* Re: [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms
  2016-10-17  4:23 [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms David Gibson
                   ` (2 preceding siblings ...)
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
@ 2016-10-17  4:36 ` no-reply
  3 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2016-10-17  4:36 UTC (permalink / raw)
  To: david; +Cc: famz, mst, pbonzini, qemu-devel, armbru

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms
Type: series
Message-id: 1476678204-11511-1-git-send-email-david@gibson.dropbear.id.au

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
c2105ef Split ISA and sysbus versions of m48t59 device
d3ad698 Allow ISA bus to be configured out
2b39254 Split serial-isa into its own config option

=== OUTPUT BEGIN ===
Checking PATCH 1/3: Split serial-isa into its own config option...
Checking PATCH 2/3: Allow ISA bus to be configured out...
Checking PATCH 3/3: Split ISA and sysbus versions of m48t59 device...
ERROR: do not use C99 // comments
#65: FILE: hw/timer/m48t59-internal.h:28:
+//#define DEBUG_NVRAM

total: 1 errors, 0 warnings, 613 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

* Re: [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device
  2016-10-17  4:23 ` [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
@ 2016-11-04 10:22   ` Markus Armbruster
  2016-11-04 13:28     ` Eric Blake
  0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2016-11-04 10:22 UTC (permalink / raw)
  To: David Gibson; +Cc: mst, pbonzini, qemu-devel

Needs a rebase.  First error:

  CC      hw/timer/m48t59.o
In file included from /work/armbru/qemu/include/exec/cpu-common.h:7:0,
                 from /work/armbru/qemu/include/exec/memory.h:24,
                 from /work/armbru/qemu/include/hw/isa/isa.h:6,
                 from /work/armbru/qemu/hw/timer/m48t59-isa.c:25:
/work/armbru/qemu/include/exec/hwaddr.h:11:9: error: unknown type name ‘uint64_t’
 typedef uint64_t hwaddr;
         ^

David Gibson <david@gibson.dropbear.id.au> writes:

> The m48t59 device supports both ISA and direct sysbus attached versions of
> the device in the one .c file.  This can be awkward for some embedded
> machine types which need the sysbus M48T59, but don't want to pull in the
> ISA bus code and its other dependencies.
>
> Therefore, this patch splits out the code for the ISA attached M48T59 into
> its own C file.  It will be built when both CONFIG_M48T59 and
> CONFIG_ISA_BUS are enabled.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/timer/Makefile.objs     |   3 +
>  hw/timer/m48t59-internal.h |  82 ++++++++++++++++
>  hw/timer/m48t59-isa.c      | 180 +++++++++++++++++++++++++++++++++++
>  hw/timer/m48t59.c          | 228 ++++-----------------------------------------
>  4 files changed, 283 insertions(+), 210 deletions(-)
>  create mode 100644 hw/timer/m48t59-internal.h
>  create mode 100644 hw/timer/m48t59-isa.c
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 7ba8c23..bf3ea3c 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -6,6 +6,9 @@ common-obj-$(CONFIG_DS1338) += ds1338.o
>  common-obj-$(CONFIG_HPET) += hpet.o
>  common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
>  common-obj-$(CONFIG_M48T59) += m48t59.o
> +ifeq ($(CONFIG_ISA_BUS),y)
> +common-obj-$(CONFIG_M48T59) += m48t59-isa.o
> +endif
>  common-obj-$(CONFIG_PL031) += pl031.o
>  common-obj-$(CONFIG_PUV3) += puv3_ost.o
>  common-obj-$(CONFIG_TWL92230) += twl92230.o
> diff --git a/hw/timer/m48t59-internal.h b/hw/timer/m48t59-internal.h
> new file mode 100644
> index 0000000..c50f134
> --- /dev/null
> +++ b/hw/timer/m48t59-internal.h
> @@ -0,0 +1,82 @@
> +/*
> + * QEMU M48T59 and M48T08 NVRAM emulation (common header)
> + *
> + * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
> + * Copyright (c) 2013 Hervé Poussineau
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#ifndef HW_M48T59_H
> +#define HW_M48T59_H 1
> +
> +//#define DEBUG_NVRAM
> +
> +#if defined(DEBUG_NVRAM)
> +#define NVRAM_PRINTF(fmt, ...) do { printf(fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define NVRAM_PRINTF(fmt, ...) do { } while (0)
> +#endif
> +
> +/*
> + * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
> + * alarm and a watchdog timer and related control registers. In the
> + * PPC platform there is also a nvram lock function.
> + */
> +
> +typedef struct M48txxInfo {
> +    const char *bus_name;
> +    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
> +    uint32_t size;
> +} M48txxInfo;
> +
> +typedef struct M48t59State {
> +    /* Hardware parameters */
> +    qemu_irq IRQ;
> +    MemoryRegion iomem;
> +    uint32_t size;
> +    int32_t base_year;
> +    /* RTC management */
> +    time_t   time_offset;
> +    time_t   stop_time;
> +    /* Alarm & watchdog */
> +    struct tm alarm;
> +    QEMUTimer *alrm_timer;
> +    QEMUTimer *wd_timer;
> +    /* NVRAM storage */
> +    uint8_t *buffer;
> +    /* Model parameters */
> +    uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
> +    /* NVRAM storage */
> +    uint16_t addr;
> +    uint8_t  lock;
> +} M48t59State;
> +
> +uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr);
> +void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val);
> +void m48t59_reset_common(M48t59State *NVRAM);
> +void m48t59_realize_common(M48t59State *s, Error **errp);
> +
> +static inline void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
> +{
> +    NVRAM->lock ^= 1 << lock;
> +}
> +
> +extern const MemoryRegionOps m48t59_io_ops;
> +
> +#endif /* HW_M48T59_H */
> diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
> new file mode 100644
> index 0000000..3a521dc
> --- /dev/null
> +++ b/hw/timer/m48t59-isa.c
> @@ -0,0 +1,180 @@
> +/*
> + * QEMU M48T59 and M48T08 NVRAM emulation (ISA bus interface
> + *
> + * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
> + * Copyright (c) 2013 Hervé Poussineau
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "hw/isa/isa.h"
> +#include "hw/timer/m48t59.h"
> +#include "m48t59-internal.h"
> +
> +#define TYPE_M48TXX_ISA "isa-m48txx"
> +#define M48TXX_ISA_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(M48txxISADeviceClass, (obj), TYPE_M48TXX_ISA)
> +#define M48TXX_ISA_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(M48txxISADeviceClass, (klass), TYPE_M48TXX_ISA)
> +#define M48TXX_ISA(obj) \
> +    OBJECT_CHECK(M48txxISAState, (obj), TYPE_M48TXX_ISA)
> +
> +typedef struct M48txxISAState {
> +    ISADevice parent_obj;
> +    M48t59State state;
> +    uint32_t io_base;
> +    MemoryRegion io;
> +} M48txxISAState;
> +
> +typedef struct M48txxISADeviceClass {
> +    ISADeviceClass parent_class;
> +    M48txxInfo info;
> +} M48txxISADeviceClass;
> +
> +static M48txxInfo m48txx_isa_info[] = {
> +    {
> +        .bus_name = "isa-m48t59",
> +        .model = 59,
> +        .size = 0x2000,
> +    }
> +};
> +
> +Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
> +                       int base_year, int model)
> +{
> +    DeviceState *dev;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(m48txx_isa_info); i++) {
> +        if (m48txx_isa_info[i].size != size ||
> +            m48txx_isa_info[i].model != model) {
> +            continue;
> +        }
> +
> +        dev = DEVICE(isa_create(bus, m48txx_isa_info[i].bus_name));

isa_create()'s second argument is a device model name, not a bus name.
Please rename .bus_name accordingly.

> +        qdev_prop_set_uint32(dev, "iobase", io_base);
> +        qdev_prop_set_int32(dev, "base-year", base_year);
> +        qdev_init_nofail(dev);
> +        return NVRAM(dev);
> +    }
> +
> +    assert(false);
> +    return NULL;
> +}
[...]

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

* Re: [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-04 10:22   ` Markus Armbruster
@ 2016-11-04 13:28     ` Eric Blake
  2016-11-08  6:12       ` David Gibson
  0 siblings, 1 reply; 8+ messages in thread
From: Eric Blake @ 2016-11-04 13:28 UTC (permalink / raw)
  To: Markus Armbruster, David Gibson; +Cc: pbonzini, qemu-devel, mst

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

On 11/04/2016 05:22 AM, Markus Armbruster wrote:
> Needs a rebase.  First error:
> 
>   CC      hw/timer/m48t59.o
> In file included from /work/armbru/qemu/include/exec/cpu-common.h:7:0,
>                  from /work/armbru/qemu/include/exec/memory.h:24,
>                  from /work/armbru/qemu/include/hw/isa/isa.h:6,
>                  from /work/armbru/qemu/hw/timer/m48t59-isa.c:25:
> /work/armbru/qemu/include/exec/hwaddr.h:11:9: error: unknown type name ‘uint64_t’
>  typedef uint64_t hwaddr;
>          ^
> 

>> index 0000000..3a521dc
>> --- /dev/null
>> +++ b/hw/timer/m48t59-isa.c

>> + * THE SOFTWARE.
>> + */
>> +#include "hw/isa/isa.h"

Probably because you forgot osdep.h as the first include.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-04 13:28     ` Eric Blake
@ 2016-11-08  6:12       ` David Gibson
  0 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2016-11-08  6:12 UTC (permalink / raw)
  To: Eric Blake; +Cc: Markus Armbruster, pbonzini, qemu-devel, mst

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

On Fri, Nov 04, 2016 at 08:28:25AM -0500, Eric Blake wrote:
> On 11/04/2016 05:22 AM, Markus Armbruster wrote:
> > Needs a rebase.  First error:
> > 
> >   CC      hw/timer/m48t59.o
> > In file included from /work/armbru/qemu/include/exec/cpu-common.h:7:0,
> >                  from /work/armbru/qemu/include/exec/memory.h:24,
> >                  from /work/armbru/qemu/include/hw/isa/isa.h:6,
> >                  from /work/armbru/qemu/hw/timer/m48t59-isa.c:25:
> > /work/armbru/qemu/include/exec/hwaddr.h:11:9: error: unknown type name ‘uint64_t’
> >  typedef uint64_t hwaddr;
> >          ^
> > 
> 
> >> index 0000000..3a521dc
> >> --- /dev/null
> >> +++ b/hw/timer/m48t59-isa.c
> 
> >> + * THE SOFTWARE.
> >> + */
> >> +#include "hw/isa/isa.h"
> 
> Probably because you forgot osdep.h as the first include.

That, plus I stupidly re-used the same double-include prevention
symbol from an existing header file.  I've fixed that up in my branch
now.

Of course, I still have no idea who I really need to convince to
actually get this merged.

-- 
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: 819 bytes --]

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

end of thread, other threads:[~2016-11-08  6:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17  4:23 [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms David Gibson
2016-10-17  4:23 ` [Qemu-devel] [PATCH 1/3] Split serial-isa into its own config option David Gibson
2016-10-17  4:23 ` [Qemu-devel] [PATCH 2/3] Allow ISA bus to be configured out David Gibson
2016-10-17  4:23 ` [Qemu-devel] [PATCH 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
2016-11-04 10:22   ` Markus Armbruster
2016-11-04 13:28     ` Eric Blake
2016-11-08  6:12       ` David Gibson
2016-10-17  4:36 ` [Qemu-devel] [PATCH 0/3] Allow ISA to be disabled on some platforms no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.