Re: [Qemu-devel] [PATCH 3/4] serial-mcb: Add serial via MEN chameleon bus

[Philippe Mathieu-Daudé]

Hi Johannes,

On 08/08/2018 11:16 AM, Johannes Thumshirn wrote:
> Add MEN z125 UART over MEN Chameleon Bus emulation.
> 
> Signed-off-by: Johannes Thumshirn <address@hidden>
> ---
>  hw/char/Makefile.objs |  1 +
>  hw/char/serial-mcb.c  | 97 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
>  create mode 100644 hw/char/serial-mcb.c
> 
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index b57053129107..063f1720974d 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-$(CONFIG_PL011) += pl011.o
>  common-obj-$(CONFIG_SERIAL) += serial.o
>  common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
>  common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
> +common-obj-$(CONFIG_MCB) += serial-mcb.o
>  common-obj-$(CONFIG_VIRTIO_SERIAL) += virtio-console.o
>  common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
>  common-obj-$(CONFIG_XEN) += xen_console.o
> diff --git a/hw/char/serial-mcb.c b/hw/char/serial-mcb.c
> new file mode 100644
> index 000000000000..a1087bc369dd
> --- /dev/null
> +++ b/hw/char/serial-mcb.c
> @@ -0,0 +1,97 @@
> +/*
> + * QEMU MEN 16z125 UART over MCB emulation
> + *
> + * Copyright (C) 2016 Johannes Thumshirn <address@hidden>
> + *
> + * This code is licensed under the GNU GPL v2 or (at your opinion) any
> + * later version
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/char/serial.h"
> +#include "hw/mcb/mcb.h"
> +
> +typedef struct {

IMHO adding ...

       /*< private >*/


> +    MCBDevice dev;

       /*< public >*/

... makes it more explicit that MCBDevice contains the QOM parent
DeviceState. Since you don't seem to use it you can even name it parent_obj:

       /*< private >*/
       MCBDevice parent_obj;
       /*< public >*/
       ...

> +    SerialState state;
> +} MCBSerialState;
> +
> +static void serial_mcb_realize(DeviceState *dev, Error **errp)
> +{
> +    MCBDevice *mdev = MCB_DEVICE(dev);
> +    MCBSerialState *mss = DO_UPCAST(MCBSerialState, dev, mdev);
> +    MCBus *bus = MCB_BUS(qdev_get_parent_bus(DEVICE(dev)));
> +    SerialState *s = &mss->state;
> +    Error *err = 0;
> +
> +    mdev->gdd = mcb_new_chameleon_descriptor(bus, 125, mdev->rev,
> +                                             mdev->var, 0x10);
> +    if (!mdev->gdd) {
> +        return;
> +    }
> +
> +    s->baudbase = 115200;
> +    serial_realize_core(s, &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    s->irq = mcb_allocate_irq(&mss->dev);
> +    memory_region_init_io(&s->io, OBJECT(mss), &serial_io_ops, s, "serial", 
> 8);
> +    memory_region_add_subregion(&bus->mmio_region, mdev->gdd->offset, 
> &s->io);
> +}
> +
> +static void serial_mcb_unrealize(DeviceState *dev, Error **errp)
> +{
> +    MCBDevice *mdev = MCB_DEVICE(dev);
> +
> +    g_free(&mdev->gdd);
> +}
> +
> +static const VMStateDescription vmstate_mcb_serial = {
> +    .name = "mcb-serial",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_MCB_DEVICE(dev, MCBSerialState),
> +        VMSTATE_STRUCT(state, MCBSerialState, 0, vmstate_serial, 
> SerialState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static Property serial_mcb_properties[] = {
> +    DEFINE_PROP_CHR("chardev", MCBSerialState, state.chr),
> +    DEFINE_PROP_UINT8("rev", MCBSerialState, dev.rev, 0),
> +    DEFINE_PROP_UINT8("var", MCBSerialState, dev.var, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void serial_mcb_class_initfn(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    MCBDeviceClass *mc = MCB_DEVICE_CLASS(klass);
> +
> +    mc->realize = serial_mcb_realize;
> +    mc->unrealize = serial_mcb_unrealize;
> +
> +    set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +    dc->desc = "MEN 16z125 UART over MCB";
> +    dc->vmsd = &vmstate_mcb_serial;
> +    dc->props = serial_mcb_properties;
> +}
> +
> +static const TypeInfo serial_mcb_info = {
> +    .name = "mcb-serial",
> +    .parent = TYPE_MCB_DEVICE,
> +    .instance_size = sizeof(MCBSerialState),
> +    .class_init = serial_mcb_class_initfn,
> +};
> +
> +static void serial_mcb_register_types(void)
> +{
> +    type_register_static(&serial_mcb_info);
> +}
> +
> +type_init(serial_mcb_register_types);
>

signature.asc
Description: OpenPGP digital signature