Re: [PATCH v4 3/6] vt82c686: Add VT8231_SUPERIO based on VIA_SUPERIO

[Philippe Mathieu-Daudé]

On 2/25/21 8:47 PM, BALATON Zoltan wrote:
> The VT8231 south bridge is very similar to VT82C686B but there are
> some differences in register addresses and functionality, e.g. the
> VT8231 only has one serial port. This commit adds VT8231_SUPERIO
> subclass based on the abstract VIA_SUPERIO class to emulate the
> superio part of VT8231.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/isa/vt82c686.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 121 insertions(+)
> 
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 9b2ffecc79..72234bc4d1 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -489,6 +489,126 @@ static const TypeInfo vt82c686b_superio_info = {
>  };
>  
>  
> +#define TYPE_VT8231_SUPERIO "vt8231-superio"
> +
> +static void vt8231_superio_cfg_write(void *opaque, hwaddr addr,
> +                                     uint64_t data, unsigned size)
> +{
> +    ViaSuperIOState *sc = opaque;
> +    uint8_t idx = sc->regs[0];
> +
> +    if (addr == 0) { /* config index register */
> +        sc->regs[0] = data;
> +        return;
> +    }
> +
> +    /* config data register */
> +    trace_via_superio_write(idx, data);
> +    switch (idx) {
> +    case 0x00 ... 0xdf:
> +    case 0xe7 ... 0xef:
> +    case 0xf0 ... 0xf1:
> +    case 0xf5:
> +    case 0xf8:
> +    case 0xfd:
> +        /* ignore write to read only registers */
> +        return;
> +    case 0xf2: /* Function select */
> +    {
> +        data &= 0x17;

I'd prefer a definition for this mask (and @0xf4) but well...
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +        if (data & BIT(2)) { /* Serial port enable */
> +            ISADevice *dev = sc->superio.serial[0];
> +            if (!memory_region_is_mapped(sc->serial_io[0])) {
> +                memory_region_add_subregion(isa_address_space_io(dev),
> +                                            dev->ioport_id, 
> sc->serial_io[0]);
> +            }
> +        } else {
> +            MemoryRegion *io = isa_address_space_io(sc->superio.serial[0]);
> +            if (memory_region_is_mapped(sc->serial_io[0])) {
> +                memory_region_del_subregion(io, sc->serial_io[0]);
> +            }
> +        }
> +        break;
> +    }
> +    case 0xf4: /* Serial port io base address */
> +    {
> +        data &= 0xfe;
> +        sc->superio.serial[0]->ioport_id = data << 2;
> +        if (memory_region_is_mapped(sc->serial_io[0])) {
> +            memory_region_set_address(sc->serial_io[0], data << 2);
> +        }
> +        break;
> +    }
> +    default:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "via_superio_cfg: unimplemented register 0x%x\n", idx);
> +        break;
> +    }
> +    sc->regs[idx] = data;
> +}