[Qemu-devel] [PATCH] Revert "audio: fix pc speaker init"

Gerd Hoffmann posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190328071121.21147-1-kraxel@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/audio/pcspk.c | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
[Qemu-devel] [PATCH] Revert "audio: fix pc speaker init"
Posted by Gerd Hoffmann 5 years ago
This reverts commit bd56d378842c238c8901536c06c20a4a51ee9761.

Turned out it isn't that simple as the device needs the pit object link.
So "-device isa-pcspk" isn't going wo work anyway.  We are in freeze, so
just reverting the thing is the best way to handle this for now, trying
to come up with something better can be done in the 4.1 devel cycle.

Also add a comment noting the object link.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/pcspk.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index fdbb4b6e9923..9c7fd74aeba7 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -57,6 +57,7 @@ typedef struct {
 } PCSpkState;
 
 static const char *s_spk = "pcspk";
+static PCSpkState *pcspk_state;
 
 static inline void generate_samples(PCSpkState *s)
 {
@@ -110,6 +111,22 @@ static void pcspk_callback(void *opaque, int free)
     }
 }
 
+static int pcspk_audio_init(ISABus *bus)
+{
+    PCSpkState *s = pcspk_state;
+    struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
+
+    AUD_register_card(s_spk, &s->card);
+
+    s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as);
+    if (!s->voice) {
+        AUD_log(s_spk, "Could not open voice\n");
+        return -1;
+    }
+
+    return 0;
+}
+
 static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
                               unsigned size)
 {
@@ -162,20 +179,12 @@ static void pcspk_initfn(Object *obj)
 
 static void pcspk_realizefn(DeviceState *dev, Error **errp)
 {
-    struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
     ISADevice *isadev = ISA_DEVICE(dev);
     PCSpkState *s = PC_SPEAKER(dev);
 
     isa_register_ioport(isadev, &s->ioport, s->iobase);
 
-    AUD_register_card(s_spk, &s->card);
-
-    s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as);
-    if (!s->voice) {
-        error_setg(errp, "Initializing audio voice failed");
-        AUD_remove_card(&s->card);
-        return;
-    }
+    pcspk_state = s;
 }
 
 static bool migrate_needed(void *opaque)
@@ -212,6 +221,9 @@ static void pcspk_class_initfn(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
     dc->vmsd = &vmstate_spk;
     dc->props = pcspk_properties;
+    /* Reason: realize sets global pcspk_state */
+    /* Reason: pit object link */
+    dc->user_creatable = false;
 }
 
 static const TypeInfo pcspk_info = {
@@ -222,12 +234,6 @@ static const TypeInfo pcspk_info = {
     .class_init     = pcspk_class_initfn,
 };
 
-static int pcspk_audio_init(ISABus *bus)
-{
-    isa_create_simple(bus, TYPE_PC_SPEAKER);
-    return 0;
-}
-
 static void pcspk_register(void)
 {
     type_register_static(&pcspk_info);
-- 
2.18.1


Re: [Qemu-devel] [PATCH] Revert "audio: fix pc speaker init"
Posted by Philippe Mathieu-Daudé 5 years ago
Le jeu. 28 mars 2019 08:12, Gerd Hoffmann <kraxel@redhat.com> a écrit :

> This reverts commit bd56d378842c238c8901536c06c20a4a51ee9761.
>
> Turned out it isn't that simple as the device needs the pit object link.
> So "-device isa-pcspk" isn't going wo work anyway.  We are in freeze, so
> just reverting the thing is the best way to handle this for now, trying
> to come up with something better can be done in the 4.1 devel cycle.
>
> Also add a comment noting the object link.
>
> Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/audio/pcspk.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
> index fdbb4b6e9923..9c7fd74aeba7 100644
> --- a/hw/audio/pcspk.c
> +++ b/hw/audio/pcspk.c
> @@ -57,6 +57,7 @@ typedef struct {
>  } PCSpkState;
>
>  static const char *s_spk = "pcspk";
> +static PCSpkState *pcspk_state;
>
>  static inline void generate_samples(PCSpkState *s)
>  {
> @@ -110,6 +111,22 @@ static void pcspk_callback(void *opaque, int free)
>      }
>  }
>
> +static int pcspk_audio_init(ISABus *bus)
> +{
> +    PCSpkState *s = pcspk_state;
> +    struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
> +
> +    AUD_register_card(s_spk, &s->card);
> +
> +    s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
> &as);
> +    if (!s->voice) {
> +        AUD_log(s_spk, "Could not open voice\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  static uint64_t pcspk_io_read(void *opaque, hwaddr addr,
>                                unsigned size)
>  {
> @@ -162,20 +179,12 @@ static void pcspk_initfn(Object *obj)
>
>  static void pcspk_realizefn(DeviceState *dev, Error **errp)
>  {
> -    struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
>      ISADevice *isadev = ISA_DEVICE(dev);
>      PCSpkState *s = PC_SPEAKER(dev);
>
>      isa_register_ioport(isadev, &s->ioport, s->iobase);
>
> -    AUD_register_card(s_spk, &s->card);
> -
> -    s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback,
> &as);
> -    if (!s->voice) {
> -        error_setg(errp, "Initializing audio voice failed");
> -        AUD_remove_card(&s->card);
> -        return;
> -    }
> +    pcspk_state = s;
>  }
>
>  static bool migrate_needed(void *opaque)
> @@ -212,6 +221,9 @@ static void pcspk_class_initfn(ObjectClass *klass,
> void *data)
>      set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
>      dc->vmsd = &vmstate_spk;
>      dc->props = pcspk_properties;
> +    /* Reason: realize sets global pcspk_state */
> +    /* Reason: pit object link */
> +    dc->user_creatable = false;
>  }
>
>  static const TypeInfo pcspk_info = {
> @@ -222,12 +234,6 @@ static const TypeInfo pcspk_info = {
>      .class_init     = pcspk_class_initfn,
>  };
>
> -static int pcspk_audio_init(ISABus *bus)
> -{
> -    isa_create_simple(bus, TYPE_PC_SPEAKER);
> -    return 0;
> -}
> -
>  static void pcspk_register(void)
>  {
>      type_register_static(&pcspk_info);
> --
> 2.18.1
>

*Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org <f4bug@amsat.org>>*

>