Re: [Qemu-arm] [PATCH] hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()

[Philippe Mathieu-Daudé]

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
>