* [Qemu-devel] [PATCH v2 0/3] use object link instead of qdev property
@ 2018-10-12 8:30 Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string Mao Zhongyi
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Mao Zhongyi @ 2018-10-12 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Mao Zhongyi, Jan Kiszka, Peter Maydell, Gerd Hoffmann
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.
v2:
-removed the patch1
-removed the TYPE_name in VMStateDescription.name
Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Mao Zhongyi (3):
audio: use TYPE_WM8750 instead of a hardcoded string
audio: use object link instead of qdev property to pass wm8750
reference
audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string
hw/arm/musicpal.c | 5 +++--
hw/audio/marvell_88w8618.c | 14 ++++++--------
2 files changed, 9 insertions(+), 10 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string
2018-10-12 8:30 [Qemu-devel] [PATCH v2 0/3] use object link instead of qdev property Mao Zhongyi
@ 2018-10-12 8:30 ` Mao Zhongyi
2018-10-12 9:50 ` Philippe Mathieu-Daudé
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string Mao Zhongyi
2 siblings, 1 reply; 13+ messages in thread
From: Mao Zhongyi @ 2018-10-12 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Mao Zhongyi, Jan Kiszka, Peter Maydell, Gerd Hoffmann
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 */},
};
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference
2018-10-12 8:30 [Qemu-devel] [PATCH v2 0/3] use object link instead of qdev property Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string Mao Zhongyi
@ 2018-10-12 8:30 ` Mao Zhongyi
2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string Mao Zhongyi
2 siblings, 1 reply; 13+ messages in thread
From: Mao Zhongyi @ 2018-10-12 8:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Mao Zhongyi, Jan Kiszka, Peter Maydell, Gerd Hoffmann
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string
2018-10-12 8:30 [Qemu-devel] [PATCH v2 0/3] use object link instead of qdev property Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference Mao Zhongyi
@ 2018-10-12 8:30 ` Mao Zhongyi
2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 9:58 ` maozy
2 siblings, 2 replies; 13+ messages in thread
From: Mao Zhongyi @ 2018-10-12 8:30 UTC (permalink / raw)
To: qemu-devel
Cc: Mao Zhongyi, Jan Kiszka, Philippe Mathieu-Daudé, Peter Maydell
Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
To: qemu-arm@nongnu.org
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
---
hw/arm/musicpal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index ac266f9253..9648b3af44 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -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);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string Mao Zhongyi
@ 2018-10-12 9:50 ` Philippe Mathieu-Daudé
2018-10-12 10:14 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of ahardcoded string maozy
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-12 9:50 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: Peter Maydell, Jan Kiszka, Gerd Hoffmann
On 12/10/2018 10:30, 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
"To: qemu-arm@nongnu.org" is probably not relevant in the commit message.
The Linux kernel describes the 'Cc:' line in the "Submitting patches:
the essential guide to getting your code into the kernel" document as:
If a person has had the opportunity to comment on a patch, but has not
provided such comments, you may optionally add a Cc: tag to the patch.
This is the only tag which might be added without an explicit action
by the person it names - but it should indicate that this person was
copied on the patch. This tag documents that potentially interested
parties have been included in the discussion.
>
> 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 */},
> };
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference Mao Zhongyi
@ 2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 11:51 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev " maozy
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-12 9:53 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: Peter Maydell, Jan Kiszka, Gerd Hoffmann
Hi Mao,
On 12/10/2018 10:30, Mao Zhongyi wrote:
> 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;
Having a link property isn't it the same restriction?
> }
>
> static const TypeInfo mv88w8618_audio_info = {
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string Mao Zhongyi
@ 2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 9:58 ` maozy
1 sibling, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-12 9:53 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: Jan Kiszka, Peter Maydell
On 12/10/2018 10:30, Mao Zhongyi wrote:
> Cc: Jan Kiszka <jan.kiszka@web.de>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> 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 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index ac266f9253..9648b3af44 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -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);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string Mao Zhongyi
2018-10-12 9:53 ` Philippe Mathieu-Daudé
@ 2018-10-12 9:58 ` maozy
1 sibling, 0 replies; 13+ messages in thread
From: maozy @ 2018-10-12 9:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Jan Kiszka, Philippe Mathieu-Daudé, Peter Maydell
Sorry for the noise, there is something wrong with this patch,
I will fix it and resend this patchset.
Thanks,
Mao
On 10/12/18 4:30 PM, Mao Zhongyi wrote:
> Cc: Jan Kiszka <jan.kiszka@web.de>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> To: qemu-arm@nongnu.org
>
> Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
> ---
> hw/arm/musicpal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index ac266f9253..9648b3af44 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -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);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of ahardcoded string
2018-10-12 9:50 ` Philippe Mathieu-Daudé
@ 2018-10-12 10:14 ` maozy
0 siblings, 0 replies; 13+ messages in thread
From: maozy @ 2018-10-12 10:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Jan Kiszka, Gerd Hoffmann
Hi, Philippe
On 10/12/18 5:50 PM, Philippe Mathieu-Daudé wrote:
> On 12/10/2018 10:30, 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
>
> "To: qemu-arm@nongnu.org" is probably not relevant in the commit message.
>
> The Linux kernel describes the 'Cc:' line in the "Submitting patches:
> the essential guide to getting your code into the kernel" document as:
>
> If a person has had the opportunity to comment on a patch, but has not
> provided such comments, you may optionally add a Cc: tag to the patch.
> This is the only tag which might be added without an explicit action
> by the person it names - but it should indicate that this person was
> copied on the patch. This tag documents that potentially interested
> parties have been included in the discussion.
I got it, thank you very much for the detailed explanation. I will
remove it. :)
Thanks,
Mao
>>
>> 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 */},
>> };
>>
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev property to pass wm8750 reference
2018-10-12 9:53 ` Philippe Mathieu-Daudé
@ 2018-10-12 11:51 ` maozy
2018-10-12 12:30 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: maozy @ 2018-10-12 11:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Jan Kiszka, Gerd Hoffmann
Hi, Philippe
On 10/12/18 5:53 PM, Philippe Mathieu-Daudé wrote:
> Hi Mao,
>
> On 12/10/2018 10:30, Mao Zhongyi wrote:
>> 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;
>
> Having a link property isn't it the same restriction?
This task can found in
https://wiki.qemu.org/Contribute/BiteSizedTasks#Device_models
Convert qdev pointer properties (defined with DEFINE_PROP_PTR) to QOM
links. Example: commit 873b4d3.
Thanks,
Mao
>
>> }
>>
>> static const TypeInfo mv88w8618_audio_info = {
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev property to pass wm8750 reference
2018-10-12 11:51 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev " maozy
@ 2018-10-12 12:30 ` Philippe Mathieu-Daudé
2018-10-12 18:40 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-12 12:30 UTC (permalink / raw)
To: maozy, qemu-devel, Peter Maydell, Eduardo Habkost, Thomas Huth
Cc: Jan Kiszka, Gerd Hoffmann
Cc'ing Eduardo and Thomas.
On 12/10/2018 13:51, maozy wrote:
> Hi, Philippe
>
> On 10/12/18 5:53 PM, Philippe Mathieu-Daudé wrote:
>> Hi Mao,
>>
>> On 12/10/2018 10:30, Mao Zhongyi wrote:
>>> 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;
>>
>> Having a link property isn't it the same restriction?
>
> This task can found in
> https://wiki.qemu.org/Contribute/BiteSizedTasks#Device_models
>
> Convert qdev pointer properties (defined with DEFINE_PROP_PTR) to QOM
> links. Example: commit 873b4d3.
I agree with the QOM conversion (the rest of this patch), what I'm
wondering is if declaring this device now user_creatable is correct. I
don't think we can set link property from command line, but maybe I'm
wrong because I never investigate/tried to do it.
Maybe devices having link property are automatically marked as
user_creatable = false, then your change would be correct (except you
should explicit that in the commit message).
I'll post another mail on the list to ask about that.
Regards,
Phil.
>
> Thanks,
> Mao
>
>>
>>> }
>>> static const TypeInfo mv88w8618_audio_info = {
>>>
>>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev property to pass wm8750 reference
2018-10-12 12:30 ` Philippe Mathieu-Daudé
@ 2018-10-12 18:40 ` Philippe Mathieu-Daudé
2018-10-15 2:14 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdevproperty " maozy
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-10-12 18:40 UTC (permalink / raw)
To: maozy, qemu-devel, Peter Maydell, Eduardo Habkost, Thomas Huth
Cc: Jan Kiszka, Gerd Hoffmann
Hi Mao,
On 12/10/2018 14:30, Philippe Mathieu-Daudé wrote:
> Cc'ing Eduardo and Thomas.
>
> On 12/10/2018 13:51, maozy wrote:
>> Hi, Philippe
>>
>> On 10/12/18 5:53 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Mao,
>>>
>>> On 12/10/2018 10:30, Mao Zhongyi wrote:
>>>> 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;
>>>
>>> Having a link property isn't it the same restriction?
>>
>> This task can found in
>> https://wiki.qemu.org/Contribute/BiteSizedTasks#Device_models
>>
>> Convert qdev pointer properties (defined with DEFINE_PROP_PTR) to QOM
>> links. Example: commit 873b4d3.
>
> I agree with the QOM conversion (the rest of this patch), what I'm
> wondering is if declaring this device now user_creatable is correct. I
> don't think we can set link property from command line, but maybe I'm
> wrong because I never investigate/tried to do it.
>
> Maybe devices having link property are automatically marked as
> user_creatable = false, then your change would be correct (except you
> should explicit that in the commit message).
>
> I'll post another mail on the list to ask about that.
On https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02531.html
Peter answered:
I think whether a device with a link property is
user creatable might depend on what the property is
for and whether the device has a useful fallback for
"link not connected".
So if you look the realize function, there is no check on the linked object:
static void mv88w8618_audio_realize(DeviceState *dev, Error **errp)
{
mv88w8618_audio_state *s = MV88W8618_AUDIO(dev);
wm8750_data_req_set(s->wm, mv88w8618_audio_callback, s);
}
Then the called function dereference the WM8750State:
void wm8750_data_req_set(DeviceState *dev, data_req_cb *data_req,
void *opaque)
{
WM8750State *s = WM8750(dev);
s->data_req = data_req;
s->opaque = opaque;
}
So this patch is incorrect. You should either keep the
'dc->user_creatable = false' line, or add a check in the realize()
function, as here:
static void bcm2836_realize(DeviceState *dev, Error **errp)
{
Object *obj;
Error *err = NULL;
obj = object_property_get_link(OBJECT(dev), "ram", &err);
if (obj == NULL) {
error_setg(errp, "%s: required ram link not found: %s",
__func__, error_get_pretty(err));
return;
}
...
Regards,
Phil.
>
> Regards,
>
> Phil.
>
>>
>> Thanks,
>> Mao
>>
>>>
>>>> }
>>>> static const TypeInfo mv88w8618_audio_info = {
>>>>
>>>
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdevproperty to pass wm8750 reference
2018-10-12 18:40 ` Philippe Mathieu-Daudé
@ 2018-10-15 2:14 ` maozy
0 siblings, 0 replies; 13+ messages in thread
From: maozy @ 2018-10-15 2:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
qemu-devel, Peter Maydell, Eduardo Habkost, Thomas Huth
Cc: Jan Kiszka, Gerd Hoffmann
Hi, Philippe
On 10/13/18 2:40 AM, Philippe Mathieu-Daudé wrote:
> Hi Mao,
>
> On 12/10/2018 14:30, Philippe Mathieu-Daudé wrote:
>> Cc'ing Eduardo and Thomas.
>>
>> On 12/10/2018 13:51, maozy wrote:
>>> Hi, Philippe
>>>
>>> On 10/12/18 5:53 PM, Philippe Mathieu-Daudé wrote:
>>>> Hi Mao,
>>>>
>>>> On 12/10/2018 10:30, Mao Zhongyi wrote:
>>>>> 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;
>>>>
>>>> Having a link property isn't it the same restriction?
>>>
>>> This task can found in
>>> https://wiki.qemu.org/Contribute/BiteSizedTasks#Device_models
>>>
>>> Convert qdev pointer properties (defined with DEFINE_PROP_PTR) to QOM
>>> links. Example: commit 873b4d3.
>>
>> I agree with the QOM conversion (the rest of this patch), what I'm
>> wondering is if declaring this device now user_creatable is correct. I
>> don't think we can set link property from command line, but maybe I'm
>> wrong because I never investigate/tried to do it.
>>
>> Maybe devices having link property are automatically marked as
>> user_creatable = false, then your change would be correct (except you
>> should explicit that in the commit message).
>>
>> I'll post another mail on the list to ask about that.
>
> On https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02531.html
>
> Peter answered:
>
> I think whether a device with a link property is
> user creatable might depend on what the property is
> for and whether the device has a useful fallback for
> "link not connected".
>
> So if you look the realize function, there is no check on the linked object:
>
> static void mv88w8618_audio_realize(DeviceState *dev, Error **errp)
> {
> mv88w8618_audio_state *s = MV88W8618_AUDIO(dev);
>
> wm8750_data_req_set(s->wm, mv88w8618_audio_callback, s);
> }
>
> Then the called function dereference the WM8750State:
>
> void wm8750_data_req_set(DeviceState *dev, data_req_cb *data_req,
> void *opaque)
> {
> WM8750State *s = WM8750(dev);
>
> s->data_req = data_req;
> s->opaque = opaque;
> }
>
> So this patch is incorrect. You should either keep the
> 'dc->user_creatable = false' line, or add a check in the realize()
> function, as here:
>
> static void bcm2836_realize(DeviceState *dev, Error **errp)
> {
> Object *obj;
> Error *err = NULL;
>
> obj = object_property_get_link(OBJECT(dev), "ram", &err);
> if (obj == NULL) {
> error_setg(errp, "%s: required ram link not found: %s",
> __func__, error_get_pretty(err));
> return;
> }
> ...
>
Sorry for the delay in response, I was away for the holiday weekend.
I saw the mail you discussed and reviewed the code, I will fix it in
the next version. Thank you! I really appreciate it.
Thanks,
Mao
> Regards,
>
> Phil.
>
>>
>> Regards,
>>
>> Phil.
>>
>>>
>>> Thanks,
>>> Mao
>>>
>>>>
>>>>> }
>>>>> static const TypeInfo mv88w8618_audio_info = {
>>>>>
>>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-15 2:15 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 8:30 [Qemu-devel] [PATCH v2 0/3] use object link instead of qdev property Mao Zhongyi
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of a hardcoded string Mao Zhongyi
2018-10-12 9:50 ` Philippe Mathieu-Daudé
2018-10-12 10:14 ` [Qemu-devel] [PATCH v2 1/3] audio: use TYPE_WM8750 instead of ahardcoded string maozy
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead of qdev property to pass wm8750 reference Mao Zhongyi
2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 11:51 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdev " maozy
2018-10-12 12:30 ` Philippe Mathieu-Daudé
2018-10-12 18:40 ` Philippe Mathieu-Daudé
2018-10-15 2:14 ` [Qemu-devel] [PATCH v2 2/3] audio: use object link instead ofqdevproperty " maozy
2018-10-12 8:30 ` [Qemu-devel] [PATCH v2 3/3] audio: use TYPE_MV88W8618_AUDIO instead of hardcoded string Mao Zhongyi
2018-10-12 9:53 ` Philippe Mathieu-Daudé
2018-10-12 9:58 ` maozy
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.