[Qemu-devel] [PATCH 0/4] use object link instead of qdev property

[Qemu-devel] [PATCH 0/4] use object link instead of qdev property

[Mao Zhongyi]

According to qdev-properties.h, properties of pointer type
should be avoid, so convert qdev property to link, Whilst we
are here, also update some hardcoded strings with already
defineded macros.

Mao Zhongyi (4):
  wm8750: remove duplicate macro
  audio: use TYPE_WM8750 instead of a hardcoded string
  audio: use object link instead of qdev property to pass wm8750
    reference
  audio: use existing macros istead of hardcoded strings

 hw/arm/musicpal.c          | 19 ++++++++++---------
 hw/audio/marvell_88w8618.c | 17 +++++++----------
 hw/audio/wm8750.c          | 18 ++++++++----------
 include/hw/audio/wm8750.h  |  1 +
 4 files changed, 26 insertions(+), 29 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/4] wm8750: remove duplicate macro

[Mao Zhongyi]

The header file wm8750.h contains '#define TYPE_WM8750 "wm8750"'
macro, but '#define CODEC "wm8750"' macro is redefined in wm8750.c,
just remove the local CODEC macro and replace it with TYPE_WM8750.

Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 hw/audio/wm8750.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
index f4aa838f62..4be3602079 100644
--- a/hw/audio/wm8750.c
+++ b/hw/audio/wm8750.c
@@ -15,8 +15,6 @@
 #define IN_PORT_N	3
 #define OUT_PORT_N	3
 
-#define CODEC		"wm8750"
-
 typedef struct {
     int adc;
     int adc_hz;
@@ -204,11 +202,11 @@ static void wm8750_set_format(WM8750State *s)
     in_fmt.fmt = AUD_FMT_S16;
 
     s->adc_voice[0] = AUD_open_in(&s->card, s->adc_voice[0],
-                    CODEC ".input1", s, wm8750_audio_in_cb, &in_fmt);
+                    TYPE_WM8750 ".input1", s, wm8750_audio_in_cb, &in_fmt);
     s->adc_voice[1] = AUD_open_in(&s->card, s->adc_voice[1],
-                    CODEC ".input2", s, wm8750_audio_in_cb, &in_fmt);
+                    TYPE_WM8750 ".input2", s, wm8750_audio_in_cb, &in_fmt);
     s->adc_voice[2] = AUD_open_in(&s->card, s->adc_voice[2],
-                    CODEC ".input3", s, wm8750_audio_in_cb, &in_fmt);
+                    TYPE_WM8750 ".input3", s, wm8750_audio_in_cb, &in_fmt);
 
     /* Setup output */
     out_fmt.endianness = 0;
@@ -217,12 +215,12 @@ static void wm8750_set_format(WM8750State *s)
     out_fmt.fmt = AUD_FMT_S16;
 
     s->dac_voice[0] = AUD_open_out(&s->card, s->dac_voice[0],
-                    CODEC ".speaker", s, wm8750_audio_out_cb, &out_fmt);
+                    TYPE_WM8750 ".speaker", s, wm8750_audio_out_cb, &out_fmt);
     s->dac_voice[1] = AUD_open_out(&s->card, s->dac_voice[1],
-                    CODEC ".headphone", s, wm8750_audio_out_cb, &out_fmt);
+                    TYPE_WM8750 ".headphone", s, wm8750_audio_out_cb, &out_fmt);
     /* MONOMIX is also in stereo for simplicity */
     s->dac_voice[2] = AUD_open_out(&s->card, s->dac_voice[2],
-                    CODEC ".monomix", s, wm8750_audio_out_cb, &out_fmt);
+                    TYPE_WM8750 ".monomix", s, wm8750_audio_out_cb, &out_fmt);
     /* no sense emulating OUT3 which is a mix of other outputs */
 
     wm8750_vol_update(s);
@@ -584,7 +582,7 @@ static int wm8750_post_load(void *opaque, int version_id)
 }
 
 static const VMStateDescription vmstate_wm8750 = {
-    .name = CODEC,
+    .name = TYPE_WM8750,
     .version_id = 0,
     .minimum_version_id = 0,
     .pre_save = wm8750_pre_save,
@@ -621,7 +619,7 @@ static void wm8750_realize(DeviceState *dev, Error **errp)
 {
     WM8750State *s = WM8750(dev);
 
-    AUD_register_card(CODEC, &s->card);
+    AUD_register_card(TYPE_WM8750, &s->card);
     wm8750_reset(I2C_SLAVE(s));
 }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/4] wm8750: remove duplicate macro

[Philippe Mathieu-Daudé]

Hi Mao,

On 11/10/2018 11:00, Mao Zhongyi wrote:
> The header file wm8750.h contains '#define TYPE_WM8750 "wm8750"'
> macro, but '#define CODEC "wm8750"' macro is redefined in wm8750.c,
> just remove the local CODEC macro and replace it with TYPE_WM8750.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  hw/audio/wm8750.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
> index f4aa838f62..4be3602079 100644
> --- a/hw/audio/wm8750.c
> +++ b/hw/audio/wm8750.c
> @@ -15,8 +15,6 @@
>  #define IN_PORT_N	3
>  #define OUT_PORT_N	3
>  
> -#define CODEC		"wm8750"
> -
>  typedef struct {
>      int adc;
>      int adc_hz;
> @@ -204,11 +202,11 @@ static void wm8750_set_format(WM8750State *s)
>      in_fmt.fmt = AUD_FMT_S16;
>  
>      s->adc_voice[0] = AUD_open_in(&s->card, s->adc_voice[0],
> -                    CODEC ".input1", s, wm8750_audio_in_cb, &in_fmt);
> +                    TYPE_WM8750 ".input1", s, wm8750_audio_in_cb, &in_fmt);

I don't think this is correct. The TYPE_name could change, but the CODEC
shouldn't change. Both definitions are different.

Regards,

Phil.

>      s->adc_voice[1] = AUD_open_in(&s->card, s->adc_voice[1],
> -                    CODEC ".input2", s, wm8750_audio_in_cb, &in_fmt);
> +                    TYPE_WM8750 ".input2", s, wm8750_audio_in_cb, &in_fmt);
>      s->adc_voice[2] = AUD_open_in(&s->card, s->adc_voice[2],
> -                    CODEC ".input3", s, wm8750_audio_in_cb, &in_fmt);
> +                    TYPE_WM8750 ".input3", s, wm8750_audio_in_cb, &in_fmt);
>  
>      /* Setup output */
>      out_fmt.endianness = 0;
> @@ -217,12 +215,12 @@ static void wm8750_set_format(WM8750State *s)
>      out_fmt.fmt = AUD_FMT_S16;
>  
>      s->dac_voice[0] = AUD_open_out(&s->card, s->dac_voice[0],
> -                    CODEC ".speaker", s, wm8750_audio_out_cb, &out_fmt);
> +                    TYPE_WM8750 ".speaker", s, wm8750_audio_out_cb, &out_fmt);
>      s->dac_voice[1] = AUD_open_out(&s->card, s->dac_voice[1],
> -                    CODEC ".headphone", s, wm8750_audio_out_cb, &out_fmt);
> +                    TYPE_WM8750 ".headphone", s, wm8750_audio_out_cb, &out_fmt);
>      /* MONOMIX is also in stereo for simplicity */
>      s->dac_voice[2] = AUD_open_out(&s->card, s->dac_voice[2],
> -                    CODEC ".monomix", s, wm8750_audio_out_cb, &out_fmt);
> +                    TYPE_WM8750 ".monomix", s, wm8750_audio_out_cb, &out_fmt);
>      /* no sense emulating OUT3 which is a mix of other outputs */
>  
>      wm8750_vol_update(s);
> @@ -584,7 +582,7 @@ static int wm8750_post_load(void *opaque, int version_id)
>  }
>  
>  static const VMStateDescription vmstate_wm8750 = {
> -    .name = CODEC,
> +    .name = TYPE_WM8750,
>      .version_id = 0,
>      .minimum_version_id = 0,
>      .pre_save = wm8750_pre_save,
> @@ -621,7 +619,7 @@ static void wm8750_realize(DeviceState *dev, Error **errp)
>  {
>      WM8750State *s = WM8750(dev);
>  
> -    AUD_register_card(CODEC, &s->card);
> +    AUD_register_card(TYPE_WM8750, &s->card);
>      wm8750_reset(I2C_SLAVE(s));
>  }
>  
> 

Re: [Qemu-devel] [PATCH 1/4] wm8750: remove duplicate macro

[maozy]

On 10/11/18 6:39 PM, Philippe Mathieu-Daudé wrote:
> Hi Mao,
> 
> On 11/10/2018 11:00, Mao Zhongyi wrote:
>> The header file wm8750.h contains '#define TYPE_WM8750 "wm8750"'
>> macro, but '#define CODEC "wm8750"' macro is redefined in wm8750.c,
>> just remove the local CODEC macro and replace it with TYPE_WM8750.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>>   hw/audio/wm8750.c | 18 ++++++++----------
>>   1 file changed, 8 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
>> index f4aa838f62..4be3602079 100644
>> --- a/hw/audio/wm8750.c
>> +++ b/hw/audio/wm8750.c
>> @@ -15,8 +15,6 @@
>>   #define IN_PORT_N	3
>>   #define OUT_PORT_N	3
>>   
>> -#define CODEC		"wm8750"
>> -
>>   typedef struct {
>>       int adc;
>>       int adc_hz;
>> @@ -204,11 +202,11 @@ static void wm8750_set_format(WM8750State *s)
>>       in_fmt.fmt = AUD_FMT_S16;
>>   
>>       s->adc_voice[0] = AUD_open_in(&s->card, s->adc_voice[0],
>> -                    CODEC ".input1", s, wm8750_audio_in_cb, &in_fmt);
>> +                    TYPE_WM8750 ".input1", s, wm8750_audio_in_cb, &in_fmt);
> 
> I don't think this is correct. The TYPE_name could change, but the CODEC
> shouldn't change. Both definitions are different.

OK, I will remove this patch.

Thanks,
Mao

> 
> Regards,
> 
> Phil.
> 
>>       s->adc_voice[1] = AUD_open_in(&s->card, s->adc_voice[1],
>> -                    CODEC ".input2", s, wm8750_audio_in_cb, &in_fmt);
>> +                    TYPE_WM8750 ".input2", s, wm8750_audio_in_cb, &in_fmt);
>>       s->adc_voice[2] = AUD_open_in(&s->card, s->adc_voice[2],
>> -                    CODEC ".input3", s, wm8750_audio_in_cb, &in_fmt);
>> +                    TYPE_WM8750 ".input3", s, wm8750_audio_in_cb, &in_fmt);
>>   
>>       /* Setup output */
>>       out_fmt.endianness = 0;
>> @@ -217,12 +215,12 @@ static void wm8750_set_format(WM8750State *s)
>>       out_fmt.fmt = AUD_FMT_S16;
>>   
>>       s->dac_voice[0] = AUD_open_out(&s->card, s->dac_voice[0],
>> -                    CODEC ".speaker", s, wm8750_audio_out_cb, &out_fmt);
>> +                    TYPE_WM8750 ".speaker", s, wm8750_audio_out_cb, &out_fmt);
>>       s->dac_voice[1] = AUD_open_out(&s->card, s->dac_voice[1],
>> -                    CODEC ".headphone", s, wm8750_audio_out_cb, &out_fmt);
>> +                    TYPE_WM8750 ".headphone", s, wm8750_audio_out_cb, &out_fmt);
>>       /* MONOMIX is also in stereo for simplicity */
>>       s->dac_voice[2] = AUD_open_out(&s->card, s->dac_voice[2],
>> -                    CODEC ".monomix", s, wm8750_audio_out_cb, &out_fmt);
>> +                    TYPE_WM8750 ".monomix", s, wm8750_audio_out_cb, &out_fmt);
>>       /* no sense emulating OUT3 which is a mix of other outputs */
>>   
>>       wm8750_vol_update(s);
>> @@ -584,7 +582,7 @@ static int wm8750_post_load(void *opaque, int version_id)
>>   }
>>   
>>   static const VMStateDescription vmstate_wm8750 = {
>> -    .name = CODEC,
>> +    .name = TYPE_WM8750,
>>       .version_id = 0,
>>       .minimum_version_id = 0,
>>       .pre_save = wm8750_pre_save,
>> @@ -621,7 +619,7 @@ static void wm8750_realize(DeviceState *dev, Error **errp)
>>   {
>>       WM8750State *s = WM8750(dev);
>>   
>> -    AUD_register_card(CODEC, &s->card);
>> +    AUD_register_card(TYPE_WM8750, &s->card);
>>       wm8750_reset(I2C_SLAVE(s));
>>   }
>>   
>>
> 

[Qemu-devel] [PATCH 2/4] audio: use TYPE_WM8750 instead of a hardcoded string

[Mao Zhongyi]

Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-arm@nongnu.org

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 hw/arm/musicpal.c          | 2 +-
 hw/audio/marvell_88w8618.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index c807010e83..3dafb41b0b 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1695,7 +1695,7 @@ static void musicpal_init(MachineState *machine)
     wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR);
     dev = qdev_create(NULL, "mv88w8618_audio");
     s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_ptr(dev, "wm8750", wm8750_dev);
+    qdev_prop_set_ptr(dev, TYPE_WM8750, wm8750_dev);
     qdev_init_nofail(dev);
     sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
     sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c
index e546892d3c..cf6ce6979b 100644
--- a/hw/audio/marvell_88w8618.c
+++ b/hw/audio/marvell_88w8618.c
@@ -280,7 +280,7 @@ static const VMStateDescription mv88w8618_audio_vmsd = {
 };
 
 static Property mv88w8618_audio_properties[] = {
-    DEFINE_PROP_PTR("wm8750", mv88w8618_audio_state, wm),
+    DEFINE_PROP_PTR(TYPE_WM8750, mv88w8618_audio_state, wm),
     {/* end of list */},
 };
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/4] audio: use TYPE_WM8750 instead of a hardcoded string

[Philippe Mathieu-Daudé]

On 11/10/2018 11:00, Mao Zhongyi wrote:
> Cc: Jan Kiszka <jan.kiszka@web.de>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> To: qemu-arm@nongnu.org
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/arm/musicpal.c          | 2 +-
>  hw/audio/marvell_88w8618.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index c807010e83..3dafb41b0b 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -1695,7 +1695,7 @@ static void musicpal_init(MachineState *machine)
>      wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR);
>      dev = qdev_create(NULL, "mv88w8618_audio");
>      s = SYS_BUS_DEVICE(dev);
> -    qdev_prop_set_ptr(dev, "wm8750", wm8750_dev);
> +    qdev_prop_set_ptr(dev, TYPE_WM8750, wm8750_dev);
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
>      sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
> diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c
> index e546892d3c..cf6ce6979b 100644
> --- a/hw/audio/marvell_88w8618.c
> +++ b/hw/audio/marvell_88w8618.c
> @@ -280,7 +280,7 @@ static const VMStateDescription mv88w8618_audio_vmsd = {
>  };
>  
>  static Property mv88w8618_audio_properties[] = {
> -    DEFINE_PROP_PTR("wm8750", mv88w8618_audio_state, wm),
> +    DEFINE_PROP_PTR(TYPE_WM8750, mv88w8618_audio_state, wm),
>      {/* end of list */},
>  };
>  
> 

[Qemu-devel] [PATCH 3/4] audio: use object link instead of qdev property to pass wm8750 reference

[Mao Zhongyi]

According to qdev-properties.h, properties of pointer type should
be avoided, it seems a link type property is a good substitution.

Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-arm@nongnu.org

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 hw/arm/musicpal.c          |  3 ++-
 hw/audio/marvell_88w8618.c | 14 ++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 3dafb41b0b..ac266f9253 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1695,7 +1695,8 @@ static void musicpal_init(MachineState *machine)
     wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR);
     dev = qdev_create(NULL, "mv88w8618_audio");
     s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_ptr(dev, TYPE_WM8750, wm8750_dev);
+    object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
+                             TYPE_WM8750, NULL);
     qdev_init_nofail(dev);
     sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
     sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c
index cf6ce6979b..baab4a3d53 100644
--- a/hw/audio/marvell_88w8618.c
+++ b/hw/audio/marvell_88w8618.c
@@ -15,6 +15,7 @@
 #include "hw/i2c/i2c.h"
 #include "hw/audio/wm8750.h"
 #include "audio/audio.h"
+#include "qapi/error.h"
 
 #define MP_AUDIO_SIZE           0x00001000
 
@@ -252,6 +253,11 @@ static void mv88w8618_audio_init(Object *obj)
     memory_region_init_io(&s->iomem, obj, &mv88w8618_audio_ops, s,
                           "audio", MP_AUDIO_SIZE);
     sysbus_init_mmio(dev, &s->iomem);
+
+    object_property_add_link(OBJECT(dev), "mv88w8618", TYPE_WM8750,
+                             (Object **) &s->wm,
+                             qdev_prop_allow_set_link_before_realize,
+                             0, &error_abort);
 }
 
 static void mv88w8618_audio_realize(DeviceState *dev, Error **errp)
@@ -279,11 +285,6 @@ static const VMStateDescription mv88w8618_audio_vmsd = {
     }
 };
 
-static Property mv88w8618_audio_properties[] = {
-    DEFINE_PROP_PTR(TYPE_WM8750, mv88w8618_audio_state, wm),
-    {/* end of list */},
-};
-
 static void mv88w8618_audio_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -291,9 +292,6 @@ static void mv88w8618_audio_class_init(ObjectClass *klass, void *data)
     dc->realize = mv88w8618_audio_realize;
     dc->reset = mv88w8618_audio_reset;
     dc->vmsd = &mv88w8618_audio_vmsd;
-    dc->props = mv88w8618_audio_properties;
-    /* Reason: pointer property "wm8750" */
-    dc->user_creatable = false;
 }
 
 static const TypeInfo mv88w8618_audio_info = {
-- 
2.17.1

[Qemu-devel] [PATCH 4/4] audio: use existing macros istead of hardcoded strings

[Mao Zhongyi]

Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-arm@nongnu.org

Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
 hw/arm/musicpal.c          | 16 ++++++++--------
 hw/audio/marvell_88w8618.c |  3 +--
 include/hw/audio/wm8750.h  |  1 +
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index ac266f9253..6425f1d50f 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -406,7 +406,7 @@ static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
 }
 
 static const VMStateDescription mv88w8618_eth_vmsd = {
-    .name = "mv88w8618_eth",
+    .name = TYPE_MV88W8618_ETH,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -645,7 +645,7 @@ static void musicpal_lcd_init(Object *obj)
 }
 
 static const VMStateDescription musicpal_lcd_vmsd = {
-    .name = "musicpal_lcd",
+    .name = TYPE_MUSICPAL_LCD,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -771,7 +771,7 @@ static void mv88w8618_pic_init(Object *obj)
 }
 
 static const VMStateDescription mv88w8618_pic_vmsd = {
-    .name = "mv88w8618_pic",
+    .name = TYPE_MV88W8618_PIC,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -951,7 +951,7 @@ static const VMStateDescription mv88w8618_timer_vmsd = {
 };
 
 static const VMStateDescription mv88w8618_pit_vmsd = {
-    .name = "mv88w8618_pit",
+    .name = TYPE_MV88W8618_PIT,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -1038,7 +1038,7 @@ static void mv88w8618_flashcfg_init(Object *obj)
 }
 
 static const VMStateDescription mv88w8618_flashcfg_vmsd = {
-    .name = "mv88w8618_flashcfg",
+    .name = TYPE_MV88W8618_FLASHCFG,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -1375,7 +1375,7 @@ static void musicpal_gpio_init(Object *obj)
 }
 
 static const VMStateDescription musicpal_gpio_vmsd = {
-    .name = "musicpal_gpio",
+    .name = TYPE_MUSICPAL_GPIO,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -1539,7 +1539,7 @@ static void musicpal_key_init(Object *obj)
 }
 
 static const VMStateDescription musicpal_key_vmsd = {
-    .name = "musicpal_key",
+    .name = TYPE_MUSICPAL_KEY,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
@@ -1693,7 +1693,7 @@ static void musicpal_init(MachineState *machine)
     }
 
     wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR);
-    dev = qdev_create(NULL, "mv88w8618_audio");
+    dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
     s = SYS_BUS_DEVICE(dev);
     object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
                              TYPE_WM8750, NULL);
diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c
index baab4a3d53..dbdddf8ef4 100644
--- a/hw/audio/marvell_88w8618.c
+++ b/hw/audio/marvell_88w8618.c
@@ -39,7 +39,6 @@
 #define MP_AUDIO_CLOCK_24MHZ    (1 << 9)
 #define MP_AUDIO_MONO           (1 << 14)
 
-#define TYPE_MV88W8618_AUDIO "mv88w8618_audio"
 #define MV88W8618_AUDIO(obj) \
     OBJECT_CHECK(mv88w8618_audio_state, (obj), TYPE_MV88W8618_AUDIO)
 
@@ -268,7 +267,7 @@ static void mv88w8618_audio_realize(DeviceState *dev, Error **errp)
 }
 
 static const VMStateDescription mv88w8618_audio_vmsd = {
-    .name = "mv88w8618_audio",
+    .name = TYPE_MV88W8618_AUDIO,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
diff --git a/include/hw/audio/wm8750.h b/include/hw/audio/wm8750.h
index 84e7a119bb..e12cb886d1 100644
--- a/include/hw/audio/wm8750.h
+++ b/include/hw/audio/wm8750.h
@@ -17,6 +17,7 @@
 #include "hw/hw.h"
 
 #define TYPE_WM8750 "wm8750"
+#define TYPE_MV88W8618_AUDIO "mv88w8618_audio"
 
 typedef void data_req_cb(void *opaque, int free_out, int free_in);
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 4/4] audio: use existing macros istead of hardcoded strings

[Philippe Mathieu-Daudé]

On 11/10/2018 11:00, Mao Zhongyi wrote:
> Cc: Jan Kiszka <jan.kiszka@web.de>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> To: qemu-arm@nongnu.org
> 
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
>  hw/arm/musicpal.c          | 16 ++++++++--------
>  hw/audio/marvell_88w8618.c |  3 +--
>  include/hw/audio/wm8750.h  |  1 +
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index ac266f9253..6425f1d50f 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -406,7 +406,7 @@ static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static const VMStateDescription mv88w8618_eth_vmsd = {
> -    .name = "mv88w8618_eth",
> +    .name = TYPE_MV88W8618_ETH,

I understand TYPE_device name might changed, but once used,
VMStateDescription shouldn't, else this would break migration.
Thus I wouldn't recommend using TYPE_name in VMStateDescription.name.

Since I'm not sure I cc'ed the migration maintainers.

>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -645,7 +645,7 @@ static void musicpal_lcd_init(Object *obj)
>  }
>  
>  static const VMStateDescription musicpal_lcd_vmsd = {
> -    .name = "musicpal_lcd",
> +    .name = TYPE_MUSICPAL_LCD,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -771,7 +771,7 @@ static void mv88w8618_pic_init(Object *obj)
>  }
>  
>  static const VMStateDescription mv88w8618_pic_vmsd = {
> -    .name = "mv88w8618_pic",
> +    .name = TYPE_MV88W8618_PIC,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -951,7 +951,7 @@ static const VMStateDescription mv88w8618_timer_vmsd = {
>  };
>  
>  static const VMStateDescription mv88w8618_pit_vmsd = {
> -    .name = "mv88w8618_pit",
> +    .name = TYPE_MV88W8618_PIT,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -1038,7 +1038,7 @@ static void mv88w8618_flashcfg_init(Object *obj)
>  }
>  
>  static const VMStateDescription mv88w8618_flashcfg_vmsd = {
> -    .name = "mv88w8618_flashcfg",
> +    .name = TYPE_MV88W8618_FLASHCFG,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -1375,7 +1375,7 @@ static void musicpal_gpio_init(Object *obj)
>  }
>  
>  static const VMStateDescription musicpal_gpio_vmsd = {
> -    .name = "musicpal_gpio",
> +    .name = TYPE_MUSICPAL_GPIO,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -1539,7 +1539,7 @@ static void musicpal_key_init(Object *obj)
>  }
>  
>  static const VMStateDescription musicpal_key_vmsd = {
> -    .name = "musicpal_key",
> +    .name = TYPE_MUSICPAL_KEY,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> @@ -1693,7 +1693,7 @@ static void musicpal_init(MachineState *machine)
>      }
>  
>      wm8750_dev = i2c_create_slave(i2c, TYPE_WM8750, MP_WM_ADDR);
> -    dev = qdev_create(NULL, "mv88w8618_audio");
> +    dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);

This change is correct.

>      s = SYS_BUS_DEVICE(dev);
>      object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
>                               TYPE_WM8750, NULL);
> diff --git a/hw/audio/marvell_88w8618.c b/hw/audio/marvell_88w8618.c
> index baab4a3d53..dbdddf8ef4 100644
> --- a/hw/audio/marvell_88w8618.c
> +++ b/hw/audio/marvell_88w8618.c
> @@ -39,7 +39,6 @@
>  #define MP_AUDIO_CLOCK_24MHZ    (1 << 9)
>  #define MP_AUDIO_MONO           (1 << 14)
>  
> -#define TYPE_MV88W8618_AUDIO "mv88w8618_audio"
>  #define MV88W8618_AUDIO(obj) \
>      OBJECT_CHECK(mv88w8618_audio_state, (obj), TYPE_MV88W8618_AUDIO)
>  
> @@ -268,7 +267,7 @@ static void mv88w8618_audio_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static const VMStateDescription mv88w8618_audio_vmsd = {
> -    .name = "mv88w8618_audio",
> +    .name = TYPE_MV88W8618_AUDIO,
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> diff --git a/include/hw/audio/wm8750.h b/include/hw/audio/wm8750.h
> index 84e7a119bb..e12cb886d1 100644
> --- a/include/hw/audio/wm8750.h
> +++ b/include/hw/audio/wm8750.h
> @@ -17,6 +17,7 @@
>  #include "hw/hw.h"
>  
>  #define TYPE_WM8750 "wm8750"
> +#define TYPE_MV88W8618_AUDIO "mv88w8618_audio"
>  
>  typedef void data_req_cb(void *opaque, int free_out, int free_in);
>  
> 

Regards,

Phil.

Re: [Qemu-devel] [PATCH 4/4] audio: use existing macros istead of hardcoded strings

[Peter Maydell]

On 11 October 2018 at 11:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 11/10/2018 11:00, Mao Zhongyi wrote:
>> Cc: Jan Kiszka <jan.kiszka@web.de>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> To: qemu-arm@nongnu.org
>>
>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>> ---
>>  hw/arm/musicpal.c          | 16 ++++++++--------
>>  hw/audio/marvell_88w8618.c |  3 +--
>>  include/hw/audio/wm8750.h  |  1 +
>>  3 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
>> index ac266f9253..6425f1d50f 100644
>> --- a/hw/arm/musicpal.c
>> +++ b/hw/arm/musicpal.c
>> @@ -406,7 +406,7 @@ static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
>>  }
>>
>>  static const VMStateDescription mv88w8618_eth_vmsd = {
>> -    .name = "mv88w8618_eth",
>> +    .name = TYPE_MV88W8618_ETH,
>
> I understand TYPE_device name might changed, but once used,
> VMStateDescription shouldn't, else this would break migration.
> Thus I wouldn't recommend using TYPE_name in VMStateDescription.name.
>
> Since I'm not sure I cc'ed the migration maintainers.

Yes, that's the usual approach -- the vmstate struct names
aren't inherently the same as the QOM type names, and so
we keep them decoupled to avoid accidentally breaking migration.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/4] audio: use existing macros istead ofhardcoded strings

[maozy]

On 10/11/18 7:05 PM, Peter Maydell wrote:
> On 11 October 2018 at 11:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> On 11/10/2018 11:00, Mao Zhongyi wrote:
>>> Cc: Jan Kiszka <jan.kiszka@web.de>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> To: qemu-arm@nongnu.org
>>>
>>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>>> ---
>>>   hw/arm/musicpal.c          | 16 ++++++++--------
>>>   hw/audio/marvell_88w8618.c |  3 +--
>>>   include/hw/audio/wm8750.h  |  1 +
>>>   3 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
>>> index ac266f9253..6425f1d50f 100644
>>> --- a/hw/arm/musicpal.c
>>> +++ b/hw/arm/musicpal.c
>>> @@ -406,7 +406,7 @@ static void mv88w8618_eth_realize(DeviceState *dev, Error **errp)
>>>   }
>>>
>>>   static const VMStateDescription mv88w8618_eth_vmsd = {
>>> -    .name = "mv88w8618_eth",
>>> +    .name = TYPE_MV88W8618_ETH,
>>
>> I understand TYPE_device name might changed, but once used,
>> VMStateDescription shouldn't, else this would break migration.
>> Thus I wouldn't recommend using TYPE_name in VMStateDescription.name.
>>
>> Since I'm not sure I cc'ed the migration maintainers.
> 
> Yes, that's the usual approach -- the vmstate struct names
> aren't inherently the same as the QOM type names, and so
> we keep them decoupled to avoid accidentally breaking migration.

OK, I got it, will fix it in the next, thanks for the clarification.

thanks
Mao

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH 4/4] audio: use existing macros istead of hardcoded strings

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 11 October 2018 at 11:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> On 11/10/2018 11:00, Mao Zhongyi wrote:
>>> Cc: Jan Kiszka <jan.kiszka@web.de>
>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>> To: qemu-arm@nongnu.org
>>>
>>> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
>>> ---
>>>  hw/arm/musicpal.c          | 16 ++++++++--------
>>>  hw/audio/marvell_88w8618.c |  3 +--
>>>  include/hw/audio/wm8750.h  |  1 +
>>>  3 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
>>> index ac266f9253..6425f1d50f 100644
>>> --- a/hw/arm/musicpal.c
>>> +++ b/hw/arm/musicpal.c
>>> @@ -406,7 +406,7 @@ static void mv88w8618_eth_realize(DeviceState
>>> *dev, Error **errp)
>>>  }
>>>
>>>  static const VMStateDescription mv88w8618_eth_vmsd = {
>>> -    .name = "mv88w8618_eth",
>>> +    .name = TYPE_MV88W8618_ETH,
>>
>> I understand TYPE_device name might changed, but once used,
>> VMStateDescription shouldn't, else this would break migration.
>> Thus I wouldn't recommend using TYPE_name in VMStateDescription.name.
>>
>> Since I'm not sure I cc'ed the migration maintainers.
>
> Yes, that's the usual approach -- the vmstate struct names
> aren't inherently the same as the QOM type names, and so
> we keep them decoupled to avoid accidentally breaking migration.

Yeap, I agree.

Later, Juan.