On Fri, 18 Oct 2019 at 16:42, Marc-André Lureau
<marcandre.lur...@redhat.com> wrote:
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> 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.
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.
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
doing. Is the existence of PROP_PTR getting in your way
for a change you want to make ?
thanks
-- PMM
