On 01/31/2012 11:41 AM, Jan Kiszka wrote:
Convert the PC speaker device to a qdev ISA model. Move the public
interface to a dedicated header file at this chance.
Signed-off-by: Jan Kiszka<jan.kiszka@xxxxxxxxxxx>
Heh, I did this too more or less the same way. Some comments below:
---
arch_init.c | 1 +
hw/i82378.c | 3 +-
hw/mips_jazz.c | 3 +-
hw/pc.c | 3 +-
hw/pc.h | 4 ---
hw/pcspk.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++---------
hw/pcspk.h | 45 ++++++++++++++++++++++++++++++++++++++++
7 files changed, 104 insertions(+), 17 deletions(-)
create mode 100644 hw/pcspk.h
diff --git a/arch_init.c b/arch_init.c
index 2366511..a45485b 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -42,6 +42,7 @@
#include "gdbstub.h"
#include "hw/smbios.h"
#include "exec-memory.h"
+#include "hw/pcspk.h"
#ifdef TARGET_SPARC
int graphic_width = 1024;
diff --git a/hw/i82378.c b/hw/i82378.c
index 127cadf..79fa8b7 100644
--- a/hw/i82378.c
+++ b/hw/i82378.c
@@ -20,6 +20,7 @@
#include "pci.h"
#include "pc.h"
#include "i8254.h"
+#include "pcspk.h"
//#define DEBUG_I82378
@@ -195,7 +196,7 @@ static void i82378_init(DeviceState *dev, I82378State *s)
pit = pit_init(isabus, 0x40, 0, NULL);
/* speaker */
- pcspk_init(pit);
+ pcspk_init(isabus, pit);
/* 2 82C37 (dma) */
DMA_init(1,&s->out[1]);
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index b61b218..65608dc 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -37,6 +37,7 @@
#include "loader.h"
#include "mc146818rtc.h"
#include "i8254.h"
+#include "pcspk.h"
#include "blockdev.h"
#include "sysbus.h"
#include "exec-memory.h"
@@ -193,7 +194,7 @@ static void mips_jazz_init(MemoryRegion *address_space,
cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
DMA_init(0, cpu_exit_irq);
pit = pit_init(isa_bus, 0x40, 0, NULL);
- pcspk_init(pit);
+ pcspk_init(isa_bus, pit);
/* ISA IO space at 0x90000000 */
isa_mmio_init(0x90000000, 0x01000000);
diff --git a/hw/pc.c b/hw/pc.c
index 6fb1de7..f6a91a9 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -37,6 +37,7 @@
#include "multiboot.h"
#include "mc146818rtc.h"
#include "i8254.h"
+#include "pcspk.h"
#include "msi.h"
#include "sysbus.h"
#include "sysemu.h"
@@ -1169,7 +1170,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
/* connect PIT to output control line of the HPET */
qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(&pit->qdev, 0));
}
- pcspk_init(pit);
+ pcspk_init(isa_bus, pit);
for(i = 0; i< MAX_SERIAL_PORTS; i++) {
if (serial_hds[i]) {
diff --git a/hw/pc.h b/hw/pc.h
index b08708d..1b47bbd 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -149,10 +149,6 @@ void piix4_smbus_register_device(SMBusDevice *dev, uint8_t addr);
/* hpet.c */
extern int no_hpet;
-/* pcspk.c */
-void pcspk_init(ISADevice *pit);
-int pcspk_audio_init(ISABus *bus);
-
/* piix_pci.c */
struct PCII440FXState;
typedef struct PCII440FXState PCII440FXState;
diff --git a/hw/pcspk.c b/hw/pcspk.c
index 43df818..5bd9e32 100644
--- a/hw/pcspk.c
+++ b/hw/pcspk.c
@@ -28,6 +28,7 @@
#include "audio/audio.h"
#include "qemu-timer.h"
#include "i8254.h"
+#include "pcspk.h"
#define PCSPK_BUF_LEN 1792
#define PCSPK_SAMPLE_RATE 32000
@@ -35,10 +36,13 @@
#define PCSPK_MIN_COUNT ((PIT_FREQ + PCSPK_MAX_FREQ - 1) / PCSPK_MAX_FREQ)
typedef struct {
+ ISADevice dev;
+ MemoryRegion ioport;
+ uint32_t iobase;
uint8_t sample_buf[PCSPK_BUF_LEN];
QEMUSoundCard card;
SWVoiceOut *voice;
- ISADevice *pit;
+ void *pit;
unsigned int pit_count;
unsigned int samples;
unsigned int play_pos;
@@ -47,7 +51,7 @@ typedef struct {
} PCSpkState;
static const char *s_spk = "pcspk";
-static PCSpkState pcspk_state;
+static PCSpkState *pcspk_state;
static inline void generate_samples(PCSpkState *s)
{
@@ -99,7 +103,7 @@ static void pcspk_callback(void *opaque, int free)
int pcspk_audio_init(ISABus *bus)
{
- PCSpkState *s =&pcspk_state;
+ PCSpkState *s = pcspk_state;
struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUD_FMT_U8, 0};
This can be a follow up, but what this is really doing is enabling audio for
this device. ISABus is unused as a parameter.
I think it would be better to make this a qdev bool property (audio_enabled) and
then modify the code that calls this function to either set the property as a
global, or use the QOM path to specifically set it on this device.
Either way, I think setting an audio_enabled flag via qdev makes a whole lot
more sense conceptionally than using the -soundhw option.
AUD_register_card(s_spk,&s->card);
@@ -113,7 +117,8 @@ int pcspk_audio_init(ISABus *bus)
return 0;
}
-static uint32_t pcspk_ioport_read(void *opaque, uint32_t addr)
+static uint64_t pcspk_io_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
{
PCSpkState *s = opaque;
int out;
@@ -124,7 +129,8 @@ static uint32_t pcspk_ioport_read(void *opaque, uint32_t addr)
return pit_get_gate(s->pit, 2) | (s->data_on<< 1) | s->dummy_refresh_clock | out;
}
-static void pcspk_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void pcspk_io_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+ unsigned size)
{
PCSpkState *s = opaque;
const int gate = val& 1;
@@ -138,11 +144,47 @@ static void pcspk_ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
}
-void pcspk_init(ISADevice *pit)
+static const MemoryRegionOps pcspk_io_ops = {
+ .read = pcspk_io_read,
+ .write = pcspk_io_write,
+ .impl = {
+ .min_access_size = 1,
+ .max_access_size = 1,
+ },
+};
+
+static int pcspk_initfn(ISADevice *dev)
+{
+ PCSpkState *s = DO_UPCAST(PCSpkState, dev, dev);
+
+ memory_region_init_io(&s->ioport,&pcspk_io_ops, s, "elcr", 1);
+ isa_register_ioport(NULL,&s->ioport, s->iobase);
Should pass dev as the first argument to isa_register_ioport. Otherwise the
resource won't be cleaned up during destruction.
+
+ pcspk_state = s;
+
+ return 0;
+}
+
+static void pcspk_class_initfn(ObjectClass *klass, void *data)
{
- PCSpkState *s =&pcspk_state;
+ ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+ ic->init = pcspk_initfn;
+}
- s->pit = pit;
- register_ioport_read(0x61, 1, 1, pcspk_ioport_read, s);
- register_ioport_write(0x61, 1, 1, pcspk_ioport_write, s);
+static DeviceInfo pcspk_info = {
+ .name = "isa-pcspk",
+ .size = sizeof(PCSpkState),
+ .no_user = 1,
+ .props = (Property[]) {
+ DEFINE_PROP_HEX32("iobase", PCSpkState, iobase, -1),
+ DEFINE_PROP_PTR("pit", PCSpkState, pit),
Please don't introduce a pointer property here. They cannot be used in a
meaningful way in qdev. Why not register a link<TYPE_PIT> in instance_init?
Regards,
Anthony Liguori
+ DEFINE_PROP_END_OF_LIST(),
+ },
+ .class_init = pcspk_class_initfn,
+};
+
+static void pcspk_register(void)
+{
+ isa_qdev_register(&pcspk_info);
}
+device_init(pcspk_register)
diff --git a/hw/pcspk.h b/hw/pcspk.h
new file mode 100644
index 0000000..7f42bac
--- /dev/null
+++ b/hw/pcspk.h
@@ -0,0 +1,45 @@
+/*
+ * QEMU PC speaker emulation
+ *
+ * Copyright (c) 2006 Joachim Henke
+ *
+ * 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_PCSPK_H
+#define HW_PCSPK_H
+
+#include "hw.h"
+#include "isa.h"
+
+static inline ISADevice *pcspk_init(ISABus *bus, ISADevice *pit)
+{
+ ISADevice *dev;
+
+ dev = isa_create(bus, "isa-pcspk");
+ qdev_prop_set_uint32(&dev->qdev, "iobase", 0x61);
+ qdev_prop_set_ptr(&dev->qdev, "pit", pit);
+ qdev_init_nofail(&dev->qdev);
+
+ return dev;
+}
+
+int pcspk_audio_init(ISABus *bus);
+
+#endif /* !HW_PCSPK_H */
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
