* [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
@ 2018-11-22 10:00 Li Qiang
2018-11-22 10:35 ` Philippe Mathieu-Daudé
2018-11-30 12:11 ` Peter Maydell
0 siblings, 2 replies; 6+ messages in thread
From: Li Qiang @ 2018-11-22 10:00 UTC (permalink / raw)
To: jan.kiszka, peter.maydell
Cc: qemu-arm, qemu-devel, maozhongyi, philmd, Li Qiang
The third argument of object_property_set_link() is the name of
property, not related with the QOM type name, using the constant
string instead.
Signed-off-by: Li Qiang <liq3ea@gmail.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 9648b3af44..726ae29394 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState *machine)
dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
s = SYS_BUS_DEVICE(dev);
object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
- TYPE_WM8750, NULL);
+ "wm8750", NULL);
qdev_init_nofail(dev);
sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
2018-11-22 10:00 [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link() Li Qiang
@ 2018-11-22 10:35 ` Philippe Mathieu-Daudé
2018-11-22 10:38 ` Peter Maydell
2018-11-30 12:11 ` Peter Maydell
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-22 10:35 UTC (permalink / raw)
To: Li Qiang, jan.kiszka, peter.maydell
Cc: qemu-arm, qemu-devel, maozhongyi, Markus Armbruster
Hi Li,
On 22/11/18 11:00, Li Qiang wrote:
> The third argument of object_property_set_link() is the name of
> property, not related with the QOM type name, using the constant
> string instead.
You are correct.
>
> Signed-off-by: Li Qiang <liq3ea@gmail.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 9648b3af44..726ae29394 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState *machine)
> dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
> s = SYS_BUS_DEVICE(dev);
> object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
> - TYPE_WM8750, NULL);
> + "wm8750", NULL);
Since this property is not related to migration, maybe we can resolve
this the other way, using TYPE_WM8750 in mv88w8618_audio_init():
-- >8 --
@@ -255,3 +255,3 @@ static void mv88w8618_audio_init(Object *obj)
- object_property_add_link(OBJECT(dev), "wm8750", TYPE_WM8750,
+ object_property_add_link(OBJECT(dev), TYPE_WM8750, TYPE_WM8750,
(Object **) &s->wm,
---
So using the same definition would protect someone to accidentaly rename
the property name.
What do you think?
Regards,
Phil.
> qdev_init_nofail(dev);
> sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
> sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
2018-11-22 10:35 ` Philippe Mathieu-Daudé
@ 2018-11-22 10:38 ` Peter Maydell
2018-11-22 10:40 ` Li Qiang
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2018-11-22 10:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Li Qiang, Jan Kiszka, qemu-arm, QEMU Developers, Mao Zhongyi,
Markus Armbruster
On 22 November 2018 at 10:35, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> Hi Li,
>
> On 22/11/18 11:00, Li Qiang wrote:
>> The third argument of object_property_set_link() is the name of
>> property, not related with the QOM type name, using the constant
>> string instead.
>
> You are correct.
>
>>
>> Signed-off-by: Li Qiang <liq3ea@gmail.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 9648b3af44..726ae29394 100644
>> --- a/hw/arm/musicpal.c
>> +++ b/hw/arm/musicpal.c
>> @@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState *machine)
>> dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
>> s = SYS_BUS_DEVICE(dev);
>> object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
>> - TYPE_WM8750, NULL);
>> + "wm8750", NULL);
>
> Since this property is not related to migration, maybe we can resolve
> this the other way, using TYPE_WM8750 in mv88w8618_audio_init():
>
> -- >8 --
> @@ -255,3 +255,3 @@ static void mv88w8618_audio_init(Object *obj)
>
> - object_property_add_link(OBJECT(dev), "wm8750", TYPE_WM8750,
> + object_property_add_link(OBJECT(dev), TYPE_WM8750, TYPE_WM8750,
> (Object **) &s->wm,
> ---
>
> So using the same definition would protect someone to accidentaly rename
> the property name.
I think I prefer Li's patch -- the property name is just
an arbitrary name not related to the type name necessarily.
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
2018-11-22 10:38 ` Peter Maydell
@ 2018-11-22 10:40 ` Li Qiang
2018-11-22 11:03 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Li Qiang @ 2018-11-22 10:40 UTC (permalink / raw)
To: Peter Maydell
Cc: philmd, jan.kiszka, qemu-arm, Qemu Developers, maozhongyi,
Markus Armbruster
Peter Maydell <peter.maydell@linaro.org> 于2018年11月22日周四 下午6:38写道：
> On 22 November 2018 at 10:35, Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
> > Hi Li,
> >
> > On 22/11/18 11:00, Li Qiang wrote:
> >> The third argument of object_property_set_link() is the name of
> >> property, not related with the QOM type name, using the constant
> >> string instead.
> >
> > You are correct.
> >
> >>
> >> Signed-off-by: Li Qiang <liq3ea@gmail.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 9648b3af44..726ae29394 100644
> >> --- a/hw/arm/musicpal.c
> >> +++ b/hw/arm/musicpal.c
> >> @@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState *machine)
> >> dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
> >> s = SYS_BUS_DEVICE(dev);
> >> object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
> >> - TYPE_WM8750, NULL);
> >> + "wm8750", NULL);
> >
> > Since this property is not related to migration,
This property is not related with migration.
> maybe we can resolve
> > this the other way, using TYPE_WM8750 in mv88w8618_audio_init():
> >
> > -- >8 --
> > @@ -255,3 +255,3 @@ static void mv88w8618_audio_init(Object *obj)
> >
> > - object_property_add_link(OBJECT(dev), "wm8750", TYPE_WM8750,
> > + object_property_add_link(OBJECT(dev), TYPE_WM8750, TYPE_WM8750,
> > (Object **) &s->wm,
>
No, just as Peter point out, the property can be arbitrary name, here use
'TYPE_WM8750'
makes confusion.
> > ---
> >
> > So using the same definition would protect someone to accidentaly rename
> > the property name.
>
> I think I prefer Li's patch -- the property name is just
> an arbitrary name not related to the type name necessarily.
>
>
Agree.
Thanks,
Li Qiang
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
2018-11-22 10:40 ` Li Qiang
@ 2018-11-22 11:03 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-11-22 11:03 UTC (permalink / raw)
To: Li Qiang, Peter Maydell
Cc: jan.kiszka, qemu-arm, Qemu Developers, maozhongyi,
Markus Armbruster
On 22/11/18 11:40, Li Qiang wrote:
> Peter Maydell <peter.maydell@linaro.org
> <mailto:peter.maydell@linaro.org>> 于2018年11月22日周四 下午6:38写道：
>
> On 22 November 2018 at 10:35, Philippe Mathieu-Daudé
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> > Hi Li,
> >
> > On 22/11/18 11:00, Li Qiang wrote:
> >> The third argument of object_property_set_link() is the name of
> >> property, not related with the QOM type name, using the constant
> >> string instead.
> >
> > You are correct.
> >
> >>
> >> Signed-off-by: Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.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 9648b3af44..726ae29394 100644
> >> --- a/hw/arm/musicpal.c
> >> +++ b/hw/arm/musicpal.c
> >> @@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState
> *machine)
> >> dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
> >> s = SYS_BUS_DEVICE(dev);
> >> object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
> >> - TYPE_WM8750, NULL);
> >> + "wm8750", NULL);
> >
> > Since this property is not related to migration,
>
>
> This property is not related with migration.
>
>
> maybe we can resolve
> > this the other way, using TYPE_WM8750 in mv88w8618_audio_init():
> >
> > -- >8 --
> > @@ -255,3 +255,3 @@ static void mv88w8618_audio_init(Object *obj)
> >
> > - object_property_add_link(OBJECT(dev), "wm8750", TYPE_WM8750,
> > + object_property_add_link(OBJECT(dev), TYPE_WM8750, TYPE_WM8750,
> > (Object **) &s->wm,
>
>
> No, just as Peter point out, the property can be arbitrary name, here
> use 'TYPE_WM8750'
> makes confusion.
>
>
>
> > ---
> >
> > So using the same definition would protect someone to accidentaly
> rename
> > the property name.
>
> I think I prefer Li's patch -- the property name is just
> an arbitrary name not related to the type name necessarily.
OK, fine then!
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Regards,
Phil.
>
>
> Agree.
>
> Thanks,
> Li Qiang
>
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
2018-11-22 10:00 [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link() Li Qiang
2018-11-22 10:35 ` Philippe Mathieu-Daudé
@ 2018-11-30 12:11 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2018-11-30 12:11 UTC (permalink / raw)
To: Li Qiang
Cc: Jan Kiszka, qemu-arm, QEMU Developers, Mao Zhongyi,
Philippe Mathieu-Daudé
On Thu, 22 Nov 2018 at 10:00, Li Qiang <liq3ea@gmail.com> wrote:
>
> The third argument of object_property_set_link() is the name of
> property, not related with the QOM type name, using the constant
> string instead.
>
> Signed-off-by: Li Qiang <liq3ea@gmail.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 9648b3af44..726ae29394 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -1696,7 +1696,7 @@ static void musicpal_init(MachineState *machine)
> dev = qdev_create(NULL, TYPE_MV88W8618_AUDIO);
> s = SYS_BUS_DEVICE(dev);
> object_property_set_link(OBJECT(dev), OBJECT(wm8750_dev),
> - TYPE_WM8750, NULL);
> + "wm8750", NULL);
> qdev_init_nofail(dev);
> sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
> sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
> --
Applied to target-arm.next for 4.0, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-30 12:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-22 10:00 [Qemu-devel] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link() Li Qiang
2018-11-22 10:35 ` Philippe Mathieu-Daudé
2018-11-22 10:38 ` Peter Maydell
2018-11-22 10:40 ` Li Qiang
2018-11-22 11:03 ` Philippe Mathieu-Daudé
2018-11-30 12:11 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).