* [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).