Patchwork [RFC,1/6] adlib: qdev-ify

login
register
mail settings
Submitter Paolo Bonzini
Date April 2, 2013, 2:50 p.m.
Message ID <1364914261-4237-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/233029/
State New
Headers show

Comments

Paolo Bonzini - April 2, 2013, 2:50 p.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/adlib.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 18 deletions(-)
Andreas Färber - April 9, 2013, 5:28 p.m.
Am 02.04.2013 16:50, schrieb Paolo Bonzini:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/adlib.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 64 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/adlib.c b/hw/adlib.c
> index e6bce59..ca47b55 100644
> --- a/hw/adlib.c
> +++ b/hw/adlib.c
> @@ -31,6 +31,12 @@
>  
>  #define ADLIB_KILL_TIMERS 1
>  
> +#ifdef HAS_YMF262
> +#define ADLIB_DESC "Yamaha YMF262 (OPL3)"
> +#else
> +#define ADLIB_DESC "Yamaha YM3812 (OPL2)"
> +#endif

Shouldn't in the process of QOM'ifying this be split into two devices?
Is HAS_YMF262 user-selected or dependent on libraries or something?

> +
>  #ifdef DEBUG
>  #include "qemu/timer.h"
>  #endif
> @@ -56,13 +62,11 @@ void YMF262UpdateOneQEMU (int which, INT16 *dst, int length);
>  #define IO_WRITE_PROTO(name) \
>      void name (void *opaque, uint32_t nport, uint32_t val)
>  
> -static struct {
> -    int port;
> -    int freq;
> -} conf = {0x220, 44100};
> -
>  typedef struct {
> +    ISADevice dev;

parent_obj and white line please. :)

>      QEMUSoundCard card;
> +    uint32_t freq;
> +    uint32_t port;
>      int ticking[2];
>      int enabled;
>      int active;
> @@ -80,7 +84,7 @@ typedef struct {
>  #endif
>  } AdlibState;
>  
> -static AdlibState glob_adlib;
> +static AdlibState *glob_adlib;
>  
>  static void adlib_stop_opl_timer (AdlibState *s, size_t n)
>  {
> @@ -150,7 +154,7 @@ static IO_READ_PROTO (adlib_read)
>  
>  static void timer_handler (int c, double interval_Sec)
>  {
> -    AdlibState *s = &glob_adlib;
> +    AdlibState *s = glob_adlib;
>      unsigned n = c & 1;
>  #ifdef DEBUG
>      double interval;
> @@ -275,14 +279,21 @@ static void Adlib_fini (AdlibState *s)
>      AUD_remove_card (&s->card);
>  }
>  
> -int Adlib_init (ISABus *bus)
> +static int Adlib_initfn (ISADevice *dev)

Hm, obviously that conflicts with my series converting ISADevices to QOM
realize that I was hoping to still get into 1.5...

>  {
> -    AdlibState *s = &glob_adlib;
> +    AdlibState *s;
>      struct audsettings as;
>  
> +    if (glob_adlib) {
> +        dolog ("Cannot create more than 1 adlib device\n");
> +        return -1;
> +    }
> +    s = DO_UPCAST (AdlibState, dev, dev);

No new DO_UPCAST() for QOM objects please!

> +    glob_adlib = s;
> +
>  #ifdef HAS_YMF262
> -    if (YMF262Init (1, 14318180, conf.freq)) {
> -        dolog ("YMF262Init %d failed\n", conf.freq);
> +    if (YMF262Init (1, 14318180, s->freq)) {
> +        dolog ("YMF262Init %d failed\n", s->freq);
>          return -1;
>      }
>      else {
> @@ -290,9 +301,9 @@ int Adlib_init (ISABus *bus)
>          s->enabled = 1;
>      }
>  #else
> -    s->opl = OPLCreate (OPL_TYPE_YM3812, 3579545, conf.freq);
> +    s->opl = OPLCreate (OPL_TYPE_YM3812, 3579545, s->freq);
>      if (!s->opl) {
> -        dolog ("OPLCreate %d failed\n", conf.freq);
> +        dolog ("OPLCreate %d failed\n", s->freq);
>          return -1;
>      }
>      else {
> @@ -301,7 +312,7 @@ int Adlib_init (ISABus *bus)
>      }
>  #endif
>  
> -    as.freq = conf.freq;
> +    as.freq = s->freq;
>      as.nchannels = SHIFT;
>      as.fmt = AUD_FMT_S16;
>      as.endianness = AUDIO_HOST_ENDIANNESS;
> @@ -327,11 +338,46 @@ int Adlib_init (ISABus *bus)
>      register_ioport_read (0x388, 4, 1, adlib_read, s);
>      register_ioport_write (0x388, 4, 1, adlib_write, s);
>  
> -    register_ioport_read (conf.port, 4, 1, adlib_read, s);
> -    register_ioport_write (conf.port, 4, 1, adlib_write, s);
> +    register_ioport_read (s->port, 4, 1, adlib_read, s);
> +    register_ioport_write (s->port, 4, 1, adlib_write, s);
>  
> -    register_ioport_read (conf.port + 8, 2, 1, adlib_read, s);
> -    register_ioport_write (conf.port + 8, 2, 1, adlib_write, s);
> +    register_ioport_read (s->port + 8, 2, 1, adlib_read, s);
> +    register_ioport_write (s->port + 8, 2, 1, adlib_write, s);
>  
>      return 0;
>  }
> +
> +static Property adlib_properties[] = {
> +    DEFINE_PROP_HEX32  ("iobase",  AdlibState, port, 0x220),
> +    DEFINE_PROP_UINT32 ("freq",    AdlibState, freq,  44100),
> +    DEFINE_PROP_END_OF_LIST (),
> +};
> +
> +static void adlib_class_initfn (ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS (klass);
> +    ISADeviceClass *ic = ISA_DEVICE_CLASS (klass);
> +    ic->init = Adlib_initfn;
> +    dc->desc = ADLIB_DESC;
> +    dc->props = adlib_properties;
> +}
> +
> +static TypeInfo adlib_info = {

static const please.

> +    .name          = "adlib",
> +    .parent        = TYPE_ISA_DEVICE,
> +    .instance_size = sizeof (AdlibState),
> +    .class_init    = adlib_class_initfn,
> +};
> +
> +int Adlib_init (ISABus *bus)
> +{
> +    isa_create_simple (bus, "adlib");
> +    return 0;
> +}
> +
> +static void adlib_register_types (void)
> +{
> +    type_register_static (&adlib_info);
> +}
> +
> +type_init (adlib_register_types)

Andreas
Paolo Bonzini - April 10, 2013, 8:03 a.m.
Il 09/04/2013 19:28, Andreas Färber ha scritto:
>> > +#ifdef HAS_YMF262
>> > +#define ADLIB_DESC "Yamaha YMF262 (OPL3)"
>> > +#else
>> > +#define ADLIB_DESC "Yamaha YM3812 (OPL2)"
>> > +#endif
> Shouldn't in the process of QOM'ifying this be split into two devices?
> Is HAS_YMF262 user-selected or dependent on libraries or something?
> 

IIRC the OPL3 code is not under a GPL-compatible license (perhaps the
MAME license?), the code is just there for people who want to combine it
themselves.

Paolo

Patch

diff --git a/hw/adlib.c b/hw/adlib.c
index e6bce59..ca47b55 100644
--- a/hw/adlib.c
+++ b/hw/adlib.c
@@ -31,6 +31,12 @@ 
 
 #define ADLIB_KILL_TIMERS 1
 
+#ifdef HAS_YMF262
+#define ADLIB_DESC "Yamaha YMF262 (OPL3)"
+#else
+#define ADLIB_DESC "Yamaha YM3812 (OPL2)"
+#endif
+
 #ifdef DEBUG
 #include "qemu/timer.h"
 #endif
@@ -56,13 +62,11 @@  void YMF262UpdateOneQEMU (int which, INT16 *dst, int length);
 #define IO_WRITE_PROTO(name) \
     void name (void *opaque, uint32_t nport, uint32_t val)
 
-static struct {
-    int port;
-    int freq;
-} conf = {0x220, 44100};
-
 typedef struct {
+    ISADevice dev;
     QEMUSoundCard card;
+    uint32_t freq;
+    uint32_t port;
     int ticking[2];
     int enabled;
     int active;
@@ -80,7 +84,7 @@  typedef struct {
 #endif
 } AdlibState;
 
-static AdlibState glob_adlib;
+static AdlibState *glob_adlib;
 
 static void adlib_stop_opl_timer (AdlibState *s, size_t n)
 {
@@ -150,7 +154,7 @@  static IO_READ_PROTO (adlib_read)
 
 static void timer_handler (int c, double interval_Sec)
 {
-    AdlibState *s = &glob_adlib;
+    AdlibState *s = glob_adlib;
     unsigned n = c & 1;
 #ifdef DEBUG
     double interval;
@@ -275,14 +279,21 @@  static void Adlib_fini (AdlibState *s)
     AUD_remove_card (&s->card);
 }
 
-int Adlib_init (ISABus *bus)
+static int Adlib_initfn (ISADevice *dev)
 {
-    AdlibState *s = &glob_adlib;
+    AdlibState *s;
     struct audsettings as;
 
+    if (glob_adlib) {
+        dolog ("Cannot create more than 1 adlib device\n");
+        return -1;
+    }
+    s = DO_UPCAST (AdlibState, dev, dev);
+    glob_adlib = s;
+
 #ifdef HAS_YMF262
-    if (YMF262Init (1, 14318180, conf.freq)) {
-        dolog ("YMF262Init %d failed\n", conf.freq);
+    if (YMF262Init (1, 14318180, s->freq)) {
+        dolog ("YMF262Init %d failed\n", s->freq);
         return -1;
     }
     else {
@@ -290,9 +301,9 @@  int Adlib_init (ISABus *bus)
         s->enabled = 1;
     }
 #else
-    s->opl = OPLCreate (OPL_TYPE_YM3812, 3579545, conf.freq);
+    s->opl = OPLCreate (OPL_TYPE_YM3812, 3579545, s->freq);
     if (!s->opl) {
-        dolog ("OPLCreate %d failed\n", conf.freq);
+        dolog ("OPLCreate %d failed\n", s->freq);
         return -1;
     }
     else {
@@ -301,7 +312,7 @@  int Adlib_init (ISABus *bus)
     }
 #endif
 
-    as.freq = conf.freq;
+    as.freq = s->freq;
     as.nchannels = SHIFT;
     as.fmt = AUD_FMT_S16;
     as.endianness = AUDIO_HOST_ENDIANNESS;
@@ -327,11 +338,46 @@  int Adlib_init (ISABus *bus)
     register_ioport_read (0x388, 4, 1, adlib_read, s);
     register_ioport_write (0x388, 4, 1, adlib_write, s);
 
-    register_ioport_read (conf.port, 4, 1, adlib_read, s);
-    register_ioport_write (conf.port, 4, 1, adlib_write, s);
+    register_ioport_read (s->port, 4, 1, adlib_read, s);
+    register_ioport_write (s->port, 4, 1, adlib_write, s);
 
-    register_ioport_read (conf.port + 8, 2, 1, adlib_read, s);
-    register_ioport_write (conf.port + 8, 2, 1, adlib_write, s);
+    register_ioport_read (s->port + 8, 2, 1, adlib_read, s);
+    register_ioport_write (s->port + 8, 2, 1, adlib_write, s);
 
     return 0;
 }
+
+static Property adlib_properties[] = {
+    DEFINE_PROP_HEX32  ("iobase",  AdlibState, port, 0x220),
+    DEFINE_PROP_UINT32 ("freq",    AdlibState, freq,  44100),
+    DEFINE_PROP_END_OF_LIST (),
+};
+
+static void adlib_class_initfn (ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS (klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS (klass);
+    ic->init = Adlib_initfn;
+    dc->desc = ADLIB_DESC;
+    dc->props = adlib_properties;
+}
+
+static TypeInfo adlib_info = {
+    .name          = "adlib",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof (AdlibState),
+    .class_init    = adlib_class_initfn,
+};
+
+int Adlib_init (ISABus *bus)
+{
+    isa_create_simple (bus, "adlib");
+    return 0;
+}
+
+static void adlib_register_types (void)
+{
+    type_register_static (&adlib_info);
+}
+
+type_init (adlib_register_types)