[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/14] sm501: replace PROP_PTR with PROP_LINK
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH 01/14] sm501: replace PROP_PTR with PROP_LINK |
Date: |
Fri, 18 Oct 2019 18:36:43 +0200 |
Hi
On Fri, Oct 18, 2019 at 6:22 PM Peter Maydell <address@hidden> wrote:
>
> On Fri, 18 Oct 2019 at 16:42, Marc-André Lureau
> <address@hidden> wrote:
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > hw/display/sm501.c | 5 +++--
> > hw/sh4/r2d.c | 3 ++-
> > 2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> > index 1f33c87e65..a87d18efab 100644
> > --- a/hw/display/sm501.c
> > +++ b/hw/display/sm501.c
> > @@ -1930,7 +1930,7 @@ typedef struct {
> > SM501State state;
> > uint32_t vram_size;
> > uint32_t base;
> > - void *chr_state;
> > + Chardev *chr_state;
> > } SM501SysBusState;
> >
> > static void sm501_realize_sysbus(DeviceState *dev, Error **errp)
> > @@ -1968,7 +1968,8 @@ static void sm501_realize_sysbus(DeviceState *dev,
> > Error **errp)
> > static Property sm501_sysbus_properties[] = {
> > DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
> > DEFINE_PROP_UINT32("base", SM501SysBusState, base, 0),
> > - DEFINE_PROP_PTR("chr-state", SM501SysBusState, chr_state),
> > + DEFINE_PROP_LINK("chr-state", SM501SysBusState, chr_state,
> > + TYPE_CHARDEV, Chardev *),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> > index ee0840f380..5780ee85d9 100644
> > --- a/hw/sh4/r2d.c
> > +++ b/hw/sh4/r2d.c
> > @@ -272,7 +272,8 @@ static void r2d_init(MachineState *machine)
> > busdev = SYS_BUS_DEVICE(dev);
> > qdev_prop_set_uint32(dev, "vram-size", SM501_VRAM_SIZE);
> > qdev_prop_set_uint32(dev, "base", 0x10000000);
> > - qdev_prop_set_ptr(dev, "chr-state", serial_hd(2));
> > + object_property_set_link(OBJECT(dev), OBJECT(serial_hd(2)),
> > + "chr-state", &error_abort);
> > qdev_init_nofail(dev);
> > sysbus_mmio_map(busdev, 0, 0x10000000);
> > sysbus_mmio_map(busdev, 1, 0x13e00000);
>
> We have a typed way of passing a Chardev to devices:
> use qdev_prop_set_chr(). Unfortunately it's not trivially
> easy to drop in here, because it sets a property defined
> with DEFINE_PROP_CHR to set a field of type CharBackend
> (note, not Chardev, and not a pointer) in the device struct.
> But serial_mm_init() wants a Chardev*, because it is a
> non-QOM interface to the serial device and is manually
> doing the qemu_chr_fe_init() that connects the Chardev
> to its own CharBackend. The QOM CHR property setter wants
> to do that qemu_chr_fe_init() itself.
>
> So I think the right fix here is to properly QOMify the
> code which is not QOMified, ie hw/char/serial.c, in a
> way that means that the various "memory mapped 16650-ish
> devices" we have can use it and can define a
> TYPE_CHARDEV property.
I see, I can look at that.
> In general I think our uses of PROP_PTR are code smells
> that indicate places where we have not properly converted
> code over to the general approach that the QOM/qdev
> design desires; but we should be getting rid of PROP_PTR
> by actually doing all those (difficult) conversions.
I think all user_creatable = false are smelly in that regard.
> Merely removing PROP_PTR itself by rephrasing the dubious
> inter-device connections in a way that makes them harder
> to grep for doesn't seem to me to be necessarily worth
grep for user_creatable = false
> doing. Is the existence of PROP_PTR getting in your way
> for a change you want to make ?
Yes, I am looking at improving the qdev vs object and class vs
instance properties. I have a larger series of wip refactoring. This
initial series is preliminary cleanup that would help.
- [PATCH 00/14] Clean-ups: remove QDEV_PROP_PTR, Marc-André Lureau, 2019/10/18
- [PATCH 02/14] vmmouse: replace PROP_PTR with PROP_LINK, Marc-André Lureau, 2019/10/18
- [PATCH 03/14] lance: replace PROP_PTR with PROP_LINK, Marc-André Lureau, 2019/10/18
- [PATCH 04/14] etraxfs: remove PROP_PTR usage, Marc-André Lureau, 2019/10/18
- Re: [PATCH 04/14] etraxfs: remove PROP_PTR usage, Peter Maydell, 2019/10/21