All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms
@ 2016-11-09 12:22 David Gibson
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: David Gibson @ 2016-11-09 12:22 UTC (permalink / raw)
  To: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa,
	qemu-devel, David Gibson

This is a rebase and revision of a series I wrote 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.

Unfortunately, a lot of the pieces here don't have a clear maintainer.
So, I'm hoping to get some Acked-bys from the maintainers of the
affected targets, then I intend to send a pull request direct to Peter.

Notes:
  * Patch 3/3 triggers a style warning, but that's just because I'm
    moving a C++ // comment verbatim from one file to another

Changes since v1:
  * Fixed some silly compile errors in 3/3 exposed by some
    changes in other headers

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                   | 181 +++++++++++++++++++++++++
 hw/timer/m48t59.c                       | 228 +++-----------------------------
 21 files changed, 305 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] 14+ messages in thread

* [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
@ 2016-11-09 12:22 ` David Gibson
  2016-11-10  7:29   ` Christian Borntraeger
  2016-11-17 16:11   ` Markus Armbruster
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out David Gibson
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: David Gibson @ 2016-11-09 12:22 UTC (permalink / raw)
  To: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa,
	qemu-devel, 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 67a9bca..f607125 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -52,6 +52,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] 14+ messages in thread

* [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
@ 2016-11-09 12:22 ` David Gibson
  2016-11-17 16:12   ` Markus Armbruster
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2016-11-09 12:22 UTC (permalink / raw)
  To: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa,
	qemu-devel, 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, openrisc, 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] 14+ messages in thread

* [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out David Gibson
@ 2016-11-09 12:22 ` David Gibson
  2016-11-10 14:57   ` Paolo Bonzini
  2016-11-10 10:51 ` [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms Edgar E. Iglesias
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2016-11-09 12:22 UTC (permalink / raw)
  To: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa,
	qemu-devel, 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      | 181 +++++++++++++++++++++++++++++++++++
 hw/timer/m48t59.c          | 228 ++++-----------------------------------------
 4 files changed, 284 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..32ae957
--- /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_INTERNAL_H
+#define HW_M48T59_INTERNAL_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_INTERNAL_H */
diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
new file mode 100644
index 0000000..ea1ba70
--- /dev/null
+++ b/hw/timer/m48t59-isa.c
@@ -0,0 +1,181 @@
+/*
+ * 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 "qemu/osdep.h"
+#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] 14+ messages in thread

* Re: [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
@ 2016-11-10  7:29   ` Christian Borntraeger
  2016-11-17 16:11   ` Markus Armbruster
  1 sibling, 0 replies; 14+ messages in thread
From: Christian Borntraeger @ 2016-11-10  7:29 UTC (permalink / raw)
  To: David Gibson, edgar.iglesias, michael, proljc, cornelia.huck,
	kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa, qemu-devel

On 11/09/2016 01:22 PM, David Gibson wrote:
> 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>

Should be fine for s390 (we do not even have serial)

Acked-by: Christian Borntraeger <borntraeger@de.ibm.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 67a9bca..f607125 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -52,6 +52,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
> 

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

* Re: [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
                   ` (2 preceding siblings ...)
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
@ 2016-11-10 10:51 ` Edgar E. Iglesias
  2016-11-10 13:49 ` Michael S. Tsirkin
  2016-11-13  3:13 ` no-reply
  5 siblings, 0 replies; 14+ messages in thread
From: Edgar E. Iglesias @ 2016-11-10 10:51 UTC (permalink / raw)
  To: David Gibson
  Cc: michael, proljc, borntraeger, cornelia.huck, kbastian, jcmvbkbc,
	agraf, mst, armbru, pbonzini, peter.maydell, veroniabahaa,
	qemu-devel

On Wed, Nov 09, 2016 at 11:22:01PM +1100, David Gibson wrote:
> This is a rebase and revision of a series I wrote 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.
> 
> Unfortunately, a lot of the pieces here don't have a clear maintainer.
> So, I'm hoping to get some Acked-bys from the maintainers of the
> affected targets, then I intend to send a pull request direct to Peter.

Looks good to me:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Best regards,
Edgar

> 
> Notes:
>   * Patch 3/3 triggers a style warning, but that's just because I'm
>     moving a C++ // comment verbatim from one file to another
> 
> Changes since v1:
>   * Fixed some silly compile errors in 3/3 exposed by some
>     changes in other headers
> 
> 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                   | 181 +++++++++++++++++++++++++
>  hw/timer/m48t59.c                       | 228 +++-----------------------------
>  21 files changed, 305 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] 14+ messages in thread

* Re: [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
                   ` (3 preceding siblings ...)
  2016-11-10 10:51 ` [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms Edgar E. Iglesias
@ 2016-11-10 13:49 ` Michael S. Tsirkin
  2016-11-13  3:13 ` no-reply
  5 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2016-11-10 13:49 UTC (permalink / raw)
  To: David Gibson
  Cc: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc, agraf, armbru, pbonzini, peter.maydell,
	veroniabahaa, qemu-devel

On Wed, Nov 09, 2016 at 11:22:01PM +1100, David Gibson wrote:
> This is a rebase and revision of a series I wrote 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.
> 
> Unfortunately, a lot of the pieces here don't have a clear maintainer.
> So, I'm hoping to get some Acked-bys from the maintainers of the
> affected targets, then I intend to send a pull request direct to Peter.
> 
> Notes:
>   * Patch 3/3 triggers a style warning, but that's just because I'm
>     moving a C++ // comment verbatim from one file to another
> 
> Changes since v1:
>   * Fixed some silly compile errors in 3/3 exposed by some
>     changes in other headers

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


> 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                   | 181 +++++++++++++++++++++++++
>  hw/timer/m48t59.c                       | 228 +++-----------------------------
>  21 files changed, 305 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] 14+ messages in thread

* Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
@ 2016-11-10 14:57   ` Paolo Bonzini
  2016-11-10 15:16     ` Mark Cave-Ayland
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2016-11-10 14:57 UTC (permalink / raw)
  To: David Gibson, edgar.iglesias, michael, proljc, borntraeger,
	cornelia.huck, kbastian, jcmvbkbc
  Cc: agraf, mst, armbru, peter.maydell, veroniabahaa, qemu-devel



On 09/11/2016 13:22, David Gibson wrote:
> 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>

Who needs the ISA M48T59?  Perhaps it should be a separate symbol
altogether.  Let's document that SPARC will stop providing it in 2.9,
for example, if it's only a PReP thing.

Paolo

> ---
>  hw/timer/Makefile.objs     |   3 +
>  hw/timer/m48t59-internal.h |  82 ++++++++++++++++
>  hw/timer/m48t59-isa.c      | 181 +++++++++++++++++++++++++++++++++++
>  hw/timer/m48t59.c          | 228 ++++-----------------------------------------
>  4 files changed, 284 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..32ae957
> --- /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_INTERNAL_H
> +#define HW_M48T59_INTERNAL_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_INTERNAL_H */
> diff --git a/hw/timer/m48t59-isa.c b/hw/timer/m48t59-isa.c
> new file mode 100644
> index 0000000..ea1ba70
> --- /dev/null
> +++ b/hw/timer/m48t59-isa.c
> @@ -0,0 +1,181 @@
> +/*
> + * 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 "qemu/osdep.h"
> +#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);
>      }
>  }
>  
> 

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

* Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-10 14:57   ` Paolo Bonzini
@ 2016-11-10 15:16     ` Mark Cave-Ayland
  2016-11-10 15:37       ` Paolo Bonzini
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Cave-Ayland @ 2016-11-10 15:16 UTC (permalink / raw)
  To: Paolo Bonzini, David Gibson, edgar.iglesias, michael, proljc,
	borntraeger, cornelia.huck, kbastian, jcmvbkbc
  Cc: veroniabahaa, peter.maydell, mst, armbru, qemu-devel, agraf

On 10/11/16 14:57, Paolo Bonzini wrote:
> 
> 
> On 09/11/2016 13:22, David Gibson wrote:
>> 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>
> 
> Who needs the ISA M48T59?  Perhaps it should be a separate symbol
> altogether.  Let's document that SPARC will stop providing it in 2.9,
> for example, if it's only a PReP thing.
> 
> Paolo

Hi Paolo,

The ISA M48T59 is still actively used by the sun4u machine on
qemu-system-sparc64. In real terms it's actually connected to the ebus,
but for all intents and purposes it's the same as an ISA bus connected
via a PCI bridge.


ATB,

Mark.

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

* Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-10 15:16     ` Mark Cave-Ayland
@ 2016-11-10 15:37       ` Paolo Bonzini
  2016-11-17 15:53         ` Markus Armbruster
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2016-11-10 15:37 UTC (permalink / raw)
  To: Mark Cave-Ayland
  Cc: David Gibson, edgar iglesias, michael, proljc, borntraeger,
	cornelia huck, kbastian, jcmvbkbc, veroniabahaa, peter maydell,
	mst, armbru, qemu-devel, agraf



----- Original Message -----
> From: "Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>
> To: "Paolo Bonzini" <pbonzini@redhat.com>, "David Gibson" <david@gibson.dropbear.id.au>, "edgar iglesias"
> <edgar.iglesias@gmail.com>, michael@walle.cc, proljc@gmail.com, borntraeger@de.ibm.com, "cornelia huck"
> <cornelia.huck@de.ibm.com>, kbastian@mail.uni-paderborn.de, jcmvbkbc@gmail.com
> Cc: veroniabahaa@gmail.com, "peter maydell" <peter.maydell@linaro.org>, mst@redhat.com, armbru@redhat.com,
> qemu-devel@nongnu.org, agraf@suse.de
> Sent: Thursday, November 10, 2016 4:16:36 PM
> Subject: Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
> 
> On 10/11/16 14:57, Paolo Bonzini wrote:
> > 
> > 
> > On 09/11/2016 13:22, David Gibson wrote:
> >> 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>
> > 
> > Who needs the ISA M48T59?  Perhaps it should be a separate symbol
> > altogether.  Let's document that SPARC will stop providing it in 2.9,
> > for example, if it's only a PReP thing.
> > 
> > Paolo
> 
> Hi Paolo,
> 
> The ISA M48T59 is still actively used by the sun4u machine on
> qemu-system-sparc64. In real terms it's actually connected to the ebus,
> but for all intents and purposes it's the same as an ISA bus connected
> via a PCI bridge.

sun4u is actually using the sysbus M48T59, and mapping it into the ebus
space:

    nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
    s = SYS_BUS_DEVICE(nvram);
    memory_region_add_subregion(get_system_io(), 0x2000,
                                sysbus_mmio_get_region(s, 0));

Paolo

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

* Re: [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms
  2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
                   ` (4 preceding siblings ...)
  2016-11-10 13:49 ` Michael S. Tsirkin
@ 2016-11-13  3:13 ` no-reply
  5 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2016-11-13  3:13 UTC (permalink / raw)
  To: david
  Cc: famz, edgar.iglesias, michael, proljc, borntraeger,
	cornelia.huck, kbastian, jcmvbkbc, veroniabahaa, peter.maydell,
	mst, qemu-devel, armbru, agraf, pbonzini

Hi,

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

Type: series
Subject: [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms
Message-id: 1478694124-15803-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'
4a36cd7 Split ISA and sysbus versions of m48t59 device
fb49803 Allow ISA bus to be configured out
eb63d84 Split serial-isa into its own config option

=== OUTPUT BEGIN ===
fatal: unrecognized argument: --no-patch
Checking PATCH 1/3: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 2/3: ...
fatal: unrecognized argument: --no-patch
Checking PATCH 3/3: ...
ERROR: do not use C99 // comments
#67: FILE: hw/timer/m48t59-internal.h:28:
+//#define DEBUG_NVRAM

total: 1 errors, 0 warnings, 614 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] 14+ messages in thread

* Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
  2016-11-10 15:37       ` Paolo Bonzini
@ 2016-11-17 15:53         ` Markus Armbruster
  0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2016-11-17 15:53 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Mark Cave-Ayland, veroniabahaa, peter maydell, proljc, jcmvbkbc,
	kbastian, agraf, mst, qemu-devel, borntraeger, michael,
	cornelia huck, edgar iglesias, David Gibson

Paolo Bonzini <pbonzini@redhat.com> writes:

> ----- Original Message -----
>> From: "Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>, "David Gibson" <david@gibson.dropbear.id.au>, "edgar iglesias"
>> <edgar.iglesias@gmail.com>, michael@walle.cc, proljc@gmail.com, borntraeger@de.ibm.com, "cornelia huck"
>> <cornelia.huck@de.ibm.com>, kbastian@mail.uni-paderborn.de, jcmvbkbc@gmail.com
>> Cc: veroniabahaa@gmail.com, "peter maydell" <peter.maydell@linaro.org>, mst@redhat.com, armbru@redhat.com,
>> qemu-devel@nongnu.org, agraf@suse.de
>> Sent: Thursday, November 10, 2016 4:16:36 PM
>> Subject: Re: [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device
>> 
>> On 10/11/16 14:57, Paolo Bonzini wrote:
>> > 
>> > 
>> > On 09/11/2016 13:22, David Gibson wrote:
>> >> 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>
>> > 
>> > Who needs the ISA M48T59?  Perhaps it should be a separate symbol
>> > altogether.  Let's document that SPARC will stop providing it in 2.9,
>> > for example, if it's only a PReP thing.
>> > 
>> > Paolo
>> 
>> Hi Paolo,
>> 
>> The ISA M48T59 is still actively used by the sun4u machine on
>> qemu-system-sparc64. In real terms it's actually connected to the ebus,
>> but for all intents and purposes it's the same as an ISA bus connected
>> via a PCI bridge.
>
> sun4u is actually using the sysbus M48T59, and mapping it into the ebus
> space:
>
>     nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
>     s = SYS_BUS_DEVICE(nvram);
>     memory_region_add_subregion(get_system_io(), 0x2000,
>                                 sysbus_mmio_get_region(s, 0));

As far as I can tell, only machine "prep" uses ISA M48T59.

Is David's patch okay as is, Paolo?

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

* Re: [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
  2016-11-10  7:29   ` Christian Borntraeger
@ 2016-11-17 16:11   ` Markus Armbruster
  1 sibling, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2016-11-17 16:11 UTC (permalink / raw)
  To: David Gibson
  Cc: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc, veroniabahaa, peter.maydell, mst, qemu-devel,
	agraf, pbonzini

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

> 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).

I poked them with nm, and the result supports your claims.

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

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

* Re: [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out
  2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out David Gibson
@ 2016-11-17 16:12   ` Markus Armbruster
  0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2016-11-17 16:12 UTC (permalink / raw)
  To: David Gibson
  Cc: edgar.iglesias, michael, proljc, borntraeger, cornelia.huck,
	kbastian, jcmvbkbc, veroniabahaa, peter.maydell, mst, qemu-devel,
	agraf, pbonzini

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

> 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, openrisc, 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>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

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

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

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-09 12:22 [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms David Gibson
2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 1/3] Split serial-isa into its own config option David Gibson
2016-11-10  7:29   ` Christian Borntraeger
2016-11-17 16:11   ` Markus Armbruster
2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 2/3] Allow ISA bus to be configured out David Gibson
2016-11-17 16:12   ` Markus Armbruster
2016-11-09 12:22 ` [Qemu-devel] [PATCHv2 3/3] Split ISA and sysbus versions of m48t59 device David Gibson
2016-11-10 14:57   ` Paolo Bonzini
2016-11-10 15:16     ` Mark Cave-Ayland
2016-11-10 15:37       ` Paolo Bonzini
2016-11-17 15:53         ` Markus Armbruster
2016-11-10 10:51 ` [Qemu-devel] [PATCHv2 0/3] Allow ISA to be disabled on some platforms Edgar E. Iglesias
2016-11-10 13:49 ` Michael S. Tsirkin
2016-11-13  3:13 ` 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.