[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_pro
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-arm] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link() |
Date: |
Thu, 22 Nov 2018 12:03:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
On 22/11/18 11:40, Li Qiang wrote:
> Peter Maydell <address@hidden
> <mailto:address@hidden>> 于2018年11月22日周四 下午6:38写道:
>
> On 22 November 2018 at 10:35, Philippe Mathieu-Daudé
> <address@hidden <mailto:address@hidden>> 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 <address@hidden <mailto:address@hidden>>
> >> ---
> >> 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é <address@hidden>
Regards,
Phil.
>
>
> Agree.
>
> Thanks,
> Li Qiang
>
>
> thanks
> -- PMM
>