Re: [Qemu-devel] [PATCH v2 2/6] MIPS: Initial support of vt82686b south bridge used by fulong mini pc

[Isaku Yamahata]

Some comments below.
NOTE: I just checked pci related code and I'm not familiar
      with those chips.


On Wed, May 19, 2010 at 10:27:23PM +0800, Huacai Chen wrote:
> Signed-off-by: Huacai Chen <address@hidden>
> ---
>  Makefile.target |    2 +-
>  hw/pc.h         |    7 +
>  hw/pci_ids.h    |    8 +
>  hw/vt82c686.c   |  786 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 802 insertions(+), 1 deletions(-)
>  create mode 100644 hw/vt82c686.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index 247a2eb..9ed4a8d 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -221,7 +221,7 @@ obj-mips-y += dma.o vga.o i8259.o
>  obj-mips-y += g364fb.o jazz_led.o
>  obj-mips-y += gt64xxx.o pckbd.o mc146818rtc.o
>  obj-mips-y += piix4.o cirrus_vga.o
> -obj-mips-$(CONFIG_FULONG) += bonito.o
> +obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o
>  
>  obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>  
> diff --git a/hw/pc.h b/hw/pc.h
> index 654b7b3..7f0730b 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -143,6 +143,13 @@ void i440fx_init_memory_mappings(PCII440FXState *d);
>  extern PCIDevice *piix4_dev;
>  int piix4_init(PCIBus *bus, int devfn);
>  
> +/* vt82c686.c */
> +int vt82c686b_init(PCIBus * bus, int devfn);
> +void vt82c686b_ac97_init(PCIBus *bus, int devfn);
> +void vt82c686b_mc97_init(PCIBus *bus, int devfn);
> +i2c_bus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> +            qemu_irq sci_irq);
> +

vt82c686.h?

>  /* vga.c */
>  enum vga_retrace_method {
>      VGA_RETRACE_DUMB,
> diff --git a/hw/pci_ids.h b/hw/pci_ids.h
> index fe7a121..39e9f1d 100644
> --- a/hw/pci_ids.h
> +++ b/hw/pci_ids.h
> @@ -78,6 +78,14 @@
>  
>  #define PCI_VENDOR_ID_XILINX             0x10ee
>  
> +#define PCI_VENDOR_ID_VIA                0x1106
> +#define PCI_DEVICE_ID_VIA_ISA_BRIDGE     0x0686
> +#define PCI_DEVICE_ID_VIA_IDE            0x0571
> +#define PCI_DEVICE_ID_VIA_UHCI           0x3038
> +#define PCI_DEVICE_ID_VIA_ACPI           0x3057
> +#define PCI_DEVICE_ID_VIA_AC97           0x3058
> +#define PCI_DEVICE_ID_VIA_MC97           0x3068
> +
>  #define PCI_VENDOR_ID_MARVELL            0x11ab
>  
>  #define PCI_VENDOR_ID_ENSONIQ            0x1274
> diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> new file mode 100644
> index 0000000..1045467
> --- /dev/null
> +++ b/hw/vt82c686.c
> @@ -0,0 +1,786 @@
> +/*
> + * VT82C686B south bridge support
> + *
> + * Copyright (c) 2008 yajin (address@hidden)
> + * Copyright (c) 2009 chenming (address@hidden)
> + * Copyright (c) 2010 Huacai Chen (address@hidden)
> + * This code is licensed under the GNU GPL v2.
> + */
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "i2c.h"
> +#include "smbus.h"
> +#include "pci.h"
> +#include "isa.h"
> +#include "sysbus.h"
> +#include "mips.h"
> +
> +typedef uint32_t pci_addr_t;
> +#include "pci_host.h"
> +//#define DEBUG_VT82C686B
> +
> +#ifdef DEBUG_VT82C686B
> +#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __FUNCTION__, 
> ##__VA_ARGS__)
> +#else
> +#define DPRINTF(fmt, ...)
> +#endif
> +
> +typedef struct SuperIOConfig
> +{
> +    uint8_t config[0xff];
> +    uint8_t index;
> +    uint8_t data;
> +} SuperIOConfig;
> +
> +typedef struct VT82C686BState {
> +    PCIDevice dev;
> +    SuperIOConfig *superio_conf;
> +} VT82C686BState;
> +
> +uint32_t smb_data[16];
> +static void superio_ioport_writeb(void *opaque, uint32_t addr, uint32_t data)
> +{
> +    int can_write;
> +    SuperIOConfig *superio_conf = (SuperIOConfig *)opaque;
> +
> +    DPRINTF("superio_ioport_writeb  address 0x%x  val 0x%x  \n", addr, data);
> +    if (addr == 0x3f0) {
> +        superio_conf->index = data & 0xff;
> +    } else {
> +        /* 0x3f1 */
> +        switch (superio_conf->index) {
> +        case 0x00 ... 0xdf:
> +        case 0xe4:
> +        case 0xe5:
> +        case 0xe9 ... 0xed:
> +        case 0xf3:
> +        case 0xf5:
> +        case 0xf7:
> +        case 0xf9 ... 0xfb:
> +        case 0xfd ... 0xff:
> +            can_write = 0;
> +            break;
> +        default:
> +            can_write = 1;
> +
> +            if (can_write) {
> +                switch (superio_conf->index) {
> +                case 0xe7:
> +                    if ((data & 0xff) != 0xfe) {
> +                        DPRINTF("chage uart 1 base. unsupported yet \n");
> +                    }
> +                    break;
> +                case 0xe8:
> +                    if ((data & 0xff) != 0xbe) {
> +                        DPRINTF("chage uart 2 base. unsupported yet \n");
> +                    }
> +                    break;
> +
> +                default:
> +                    superio_conf->config[superio_conf->index] = data & 0xff;
> +                }
> +            }
> +        }
> +        superio_conf->config[superio_conf->index] = data & 0xff;
> +    }
> +}
> +
> +static uint32_t superio_ioport_readb(void *opaque, uint32_t addr)
> +{
> +    SuperIOConfig *superio_conf = (SuperIOConfig *)opaque;
> +
> +    DPRINTF("superio_ioport_readb  address 0x%x   \n", addr);
> +    return (superio_conf->config[superio_conf->index]);
> +}
> +
> +static void vt82c686b_reset(void * opaque)
> +{
> +    PCIDevice *d = opaque;
> +    uint8_t *pci_conf = d->config;
> +    VT82C686BState *vt82c = DO_UPCAST(VT82C686BState, dev, d);
> +
> +    pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x000000c0);
> +
> +    pci_conf[0x04] = 0x87; /* master, memory, I/O and special */
> +    pci_conf[0x05] = 0x00;

Please use symbolic value and accessors.
Something like
pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_xxx | ...);


> +    pci_conf[0x06] = 0x00;
> +    pci_conf[0x07] = 0x02;

pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_xxx | ...)

> +    /* Miscellaneous Control, Port 70/74 Access Status,IDE interrupt Routing 
> */
> +    pci_conf[0x48] = 0x01;
> +    pci_conf[0x4a] = 0x04;
> +    pci_conf[0x4f] = 0x03;  /* DMA/Master Mem Access Control 1/2/3 */
> +    /* IDE Primary Channel IRQ Routing -> IRQ 14 */
> +    /* IDE Secondary Channel IRQ Routing -> IRQ 15 */
> +    /* PnP DMA Request Control, PnP Routing for LPT/FDC IRQ,
> +     * PnP Routing for COM2/COM1 IRQ */
> +    pci_conf[0x50] = 0x2d;
> +    pci_conf[0x59] = 0x04;
> +    pci_conf[0x5a] = 0x04;
> +    pci_conf[0x5f] = 0x04;
> +    pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
> +    pci_conf[0x84] = 0x10;
> +
> +    vt82c->superio_conf->config[0xe0] = 0x3c;
> +    vt82c->superio_conf->config[0xe2] = 0x03;
> +    vt82c->superio_conf->config[0xe3] = 0xfc;
> +    vt82c->superio_conf->config[0xe6] = 0xde;
> +    vt82c->superio_conf->config[0xe7] = 0xfe;
> +    vt82c->superio_conf->config[0xe8] = 0xbe;
> +}
> +
> +/* read config pci function0 registers. PCI-ISA bridge */
> +static uint32_t vt82c686b_read_config(PCIDevice * d, uint32_t address, int 
> len)
> +{
> +    uint32_t val;
> +
> +    DPRINTF("vt82c686b_read_config  address 0x%x  len 0x%x \n", address, 
> len);
> +    switch (len) {
> +    default:
> +    case 4:
> +        if (address <= 0xfc) {
> +            val = le32_to_cpu(*(uint32_t *) (d->config + address));
> +            break;
> +        }
> +        /* fall through */
> +    case 2:
> +        if (address <= 0xfe) {
> +            val = le16_to_cpu(*(uint16_t *) (d->config + address));
> +            break;
> +        }
> +        /* fall through */
> +    case 1:
> +        val = d->config[address];
> +        break;
> +    }
> +    return val;
> +}

pci_default_read_config() should work.
So delete this function and leave via_info::config_read NULL.


> +
> +/* write config pci function0 registers. PCI-ISA bridge */
> +static void vt82c686b_write_config(PCIDevice * d, uint32_t address,
> +                                   uint32_t val, int len)
> +{
> +    int can_write, i;
> +    uint32_t  addr;
> +    VT82C686BState *vt82c = DO_UPCAST(VT82C686BState, dev, d);
> +
> +    DPRINTF("vt82c686b_write_config  address 0x%x  val 0x%x len 0x%x \n",
> +           address, val, len);
> +
> +    /* not efficient, but simple */
> +    addr = address;
> +    for (i = 0; i < len; i++) {
> +        /* default read/write accesses */
> +        switch (addr) {
> +        case 0x00 ... 0x03:
> +        case 0x08 ... 0x3f:
> +            can_write = 0;
> +            break;
> +        default:
> +            can_write = 1;
> +            break;
> +        }
> +        if (can_write) {
> +            switch (addr) {
> +            case 0x85:
> +                /* enable or disable super IO configure */
> +                if (val & 0x2) {
> +                    /* floppy also uses 0x3f0 and 0x3f1.
> +                     * But we do not emulate flopy,so just set it here. */
> +                    isa_unassign_ioport(0x3f0, 2);
> +                    register_ioport_read(0x3f0, 2, 1, superio_ioport_readb,
> +                                         vt82c->superio_conf);
> +                    register_ioport_write(0x3f0, 2, 1, superio_ioport_writeb,
> +                                          vt82c->superio_conf);
> +                } else {
> +                    isa_unassign_ioport(0x3f0, 2);
> +                }
> +                break;
> +            }
> +
> +            d->config[addr] = val;
> +        } else {
> +            fprintf(stderr, "warning. vt82c686b_write_config: "
> +                    "write to read only pci conf register addr 0x%x\n", 
> addr);
> +        }
> +        if (++addr > 0xff)
> +            break;
> +        val >>= 8;
> +    }
> +}

Hmm, you may want to set PCIDevice::wmask appropriately.
and call pci_default_write_config().


> +
> +static void vt82c686b_save(QEMUFile * f, void *opaque)
> +{
> +    PCIDevice *d = opaque;
> +    pci_device_save(d, f);
> +}
> +
> +static int vt82c686b_load(QEMUFile * f, void *opaque, int version_id)
> +{
> +    PCIDevice *d = opaque;
> +    if (version_id != 1)
> +        return -EINVAL;
> +    return pci_device_load(d, f);
> +}
> +
> +
> +#define PM_FREQ 3579545

Please use definitions in acpi.h, PM_TIMER_FREQUENCY.

> +
> +#define ACPI_DBG_IO_ADDR  0xb044
> +
> +typedef struct VT686PMState {
> +    PCIDevice dev;
> +    uint16_t pmsts;
> +    uint16_t pmen;
> +    uint16_t pmcntrl;
> +    uint8_t apmc;
> +    uint8_t apms;

Please use APMState in apm and its helper functions.

> +    QEMUTimer *tmr_timer;
> +    int64_t tmr_overflow_time;
> +    i2c_bus *smbus;
> +    uint8_t smb_stat;
> +    uint8_t smb_ctl;
> +    uint8_t smb_cmd;
> +    uint8_t smb_addr;
> +    uint8_t smb_data0;
> +    uint8_t smb_data1;
> +    uint8_t smb_data[32];
> +    uint8_t smb_index;

Please use PMSMBus in pm_smbus and its helper functions.


> +} VT686PMState;
> +
> +typedef struct VT686AC97State {
> +    PCIDevice dev;
> +    int unused;
> +} VT686AC97State;
> +
> +typedef struct VT686MC97State {
> +    PCIDevice dev;
> +    int unused;
> +} VT686MC97State;
> +
> +#define RTC_EN (1 << 10)
> +#define PWRBTN_EN (1 << 8)
> +#define GBL_EN (1 << 5)
> +#define TMROF_EN (1 << 0)
> +
> +#define SCI_EN (1 << 0)
> +
> +#define SUS_EN (1 << 13)
> +
> +#define ACPI_ENABLE 0xf1
> +#define ACPI_DISABLE 0xf0
> +
> +#define SMBHSTSTS 0x00
> +#define SMBHSTCNT 0x02
> +#define SMBHSTCMD 0x03
> +#define SMBHSTADD 0x04
> +#define SMBHSTDAT0 0x05
> +#define SMBHSTDAT1 0x06
> +#define SMBBLKDAT 0x07
> +
> +static uint32_t get_pmtmr(VT686PMState *s)
> +{
> +    uint32_t d;
> +    d = muldiv64(qemu_get_clock(vm_clock), PM_FREQ, get_ticks_per_sec());
> +    return d & 0xffffff;
> +}
> +
> +static int get_pmsts(VT686PMState *s)
> +{
> +    int64_t d;
> +    int pmsts;
> +    pmsts = s->pmsts;
> +    d = muldiv64(qemu_get_clock(vm_clock), PM_FREQ, get_ticks_per_sec());
> +    if (d >= s->tmr_overflow_time)
> +        s->pmsts |= TMROF_EN;
> +    return pmsts;
> +}
> +
> +static void pm_update_sci(VT686PMState *s)
> +{
> +    int sci_level, pmsts;
> +    int64_t expire_time;
> +
> +    pmsts = get_pmsts(s);
> +    sci_level = (((pmsts & s->pmen) &
> +                  (RTC_EN | PWRBTN_EN | GBL_EN | TMROF_EN)) != 0);
> +    qemu_set_irq(s->dev.irq[0], sci_level);
> +    /* schedule a timer interruption if needed */
> +    if ((s->pmen & TMROF_EN) && !(pmsts & TMROF_EN)) {
> +        expire_time = muldiv64(s->tmr_overflow_time, get_ticks_per_sec(), 
> PM_FREQ);
> +        qemu_mod_timer(s->tmr_timer, expire_time);
> +    } else {
> +        qemu_del_timer(s->tmr_timer);
> +    }
> +}
> +
> +static void pm_tmr_timer(void *opaque)
> +{
> +    VT686PMState *s = opaque;
> +    pm_update_sci(s);
> +}
> +
> +static void pm_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    VT686PMState *s = opaque;
> +
> +    addr &= 0x0f;
> +    switch (addr) {
> +    case 0x00:
> +        {
> +            int64_t d;
> +            int pmsts;
> +            pmsts = get_pmsts(s);
> +            if (pmsts & val & TMROF_EN) {
> +                /* if TMRSTS is reset, then compute the new overflow time */
> +                d = muldiv64(qemu_get_clock(vm_clock), PM_FREQ, 
> get_ticks_per_sec());
> +                s->tmr_overflow_time = (d + 0x800000LL) & ~0x7fffffLL;
> +            }
> +            s->pmsts &= ~val;
> +            pm_update_sci(s);
> +        }
> +        break;
> +    case 0x02:
> +        s->pmen = val;
> +        pm_update_sci(s);
> +        break;
> +    case 0x04:
> +        {
> +            int sus_typ;
> +            s->pmcntrl = val & ~(SUS_EN);
> +            if (val & SUS_EN) {
> +                /* change suspend type */
> +                sus_typ = (val >> 10) & 3;
> +                switch (sus_typ) {
> +                case 0: /* soft power off */
> +                    qemu_system_shutdown_request();
> +                    break;
> +                default:
> +                    break;
> +                }
> +            }
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +    DPRINTF("PM writew port=0x%04x val=0x%02x\n", addr, val);
> +}
> +
> +static uint32_t pm_ioport_readw(void *opaque, uint32_t addr)
> +{
> +    VT686PMState *s = opaque;
> +    uint32_t val;
> +
> +    addr &= 0x0f;
> +    switch (addr) {
> +    case 0x00:
> +        val = get_pmsts(s);
> +        break;
> +    case 0x02:
> +        val = s->pmen;
> +        break;
> +    case 0x04:
> +        val = s->pmcntrl;
> +        break;
> +    default:
> +        val = 0;
> +        break;
> +    }
> +    DPRINTF("PM readw port=0x%04x val=0x%02x\n", addr, val);
> +    return val;
> +}
> +
> +static void pm_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    addr &= 0x0f;
> +    DPRINTF("PM writel port=0x%04x val=0x%08x\n", addr, val);
> +}
> +
> +static uint32_t pm_ioport_readl(void *opaque, uint32_t addr)
> +{
> +    VT686PMState *s = opaque;
> +    uint32_t val;
> +
> +    addr &= 0x0f;
> +    switch (addr) {
> +    case 0x08:
> +        val = get_pmtmr(s);
> +        break;
> +    default:
> +        val = 0;
> +        break;
> +    }
> +    DPRINTF("PM readl port=0x%04x val=0x%08x\n", addr, val);
> +    return val;
> +}
> +
> +static void smb_transaction(VT686PMState *s)
> +{
> +    uint8_t prot = (s->smb_ctl >> 2) & 0x07;
> +    uint8_t read = s->smb_addr & 0x01;
> +    uint8_t cmd = s->smb_cmd;
> +    uint8_t addr = s->smb_addr >> 1;
> +    i2c_bus *bus = s->smbus;
> +
> +    DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
> +    switch (prot) {
> +    case 0x0:
> +        smbus_quick_command(bus, addr, read);
> +        break;
> +    case 0x1:
> +        if (read) {
> +            s->smb_data0 = smbus_receive_byte(bus, addr);
> +        } else {
> +            smbus_send_byte(bus, addr, cmd);
> +        }
> +        break;
> +    case 0x2:
> +        if (read) {
> +            s->smb_data0 = smbus_read_byte(bus, addr, cmd);
> +            printf("smbus_read_byte:0x%04x\n",s->smb_data0);
> +        } else {
> +            smbus_write_byte(bus, addr, cmd, s->smb_data0);
> +            printf("smbus_write_byte\n");
> +        }
> +        break;
> +    case 0x3:
> +        if (read) {
> +            uint16_t val;
> +            val = smbus_read_word(bus, addr, cmd);
> +            s->smb_data0 = val;
> +            s->smb_data1 = val >> 8;
> +        } else {
> +            smbus_write_word(bus, addr, cmd, (s->smb_data1 << 8) | 
> s->smb_data0);
> +        }
> +        break;
> +    case 0x5:
> +        if (read) {
> +            s->smb_data0 = smbus_read_block(bus, addr, cmd, s->smb_data);
> +        } else {
> +            smbus_write_block(bus, addr, cmd, s->smb_data, s->smb_data0);
> +        }
> +        break;
> +    default:
> +        goto error;
> +    }
> +    return;
> +
> +  error:
> +    s->smb_stat |= 0x04;
> +}
> +
> +static void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    VT686PMState *s = opaque;
> +
> +    addr &= 0x0f;
> +    DPRINTF("SMB writeb port=0x%04x val=0x%02x\n", addr, val);
> +
> +    switch (addr) {
> +    case SMBHSTSTS:
> +        s->smb_stat = 0;
> +        s->smb_index = 0;
> +        break;
> +    case SMBHSTCNT:
> +        s->smb_ctl = val;
> +        if (val & 0x40) {
> +            smb_transaction(s);
> +        }
> +        break;
> +    case SMBHSTCMD:
> +        s->smb_cmd = val;
> +        break;
> +    case SMBHSTADD:
> +        s->smb_addr = val;
> +        break;
> +    case SMBHSTDAT0:
> +        s->smb_data0 = val;
> +        break;
> +    case SMBHSTDAT1:
> +        s->smb_data1 = val;
> +        break;
> +    case SMBBLKDAT:
> +        s->smb_data[s->smb_index++] = val;
> +        if (s->smb_index > 31) {
> +            s->smb_index = 0;
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +static uint32_t smb_ioport_readb(void *opaque, uint32_t addr)
> +{
> +    VT686PMState *s = opaque;
> +    uint32_t val;
> +    addr &= 0x0f;
> +
> +    switch (addr) {
> +    case SMBHSTSTS:
> +        val = s->smb_stat;
> +        break;
> +    case SMBHSTCNT:
> +        s->smb_index = 0;
> +        val = s->smb_ctl & 0x1f;
> +        break;
> +    case SMBHSTCMD:
> +        val = s->smb_cmd;
> +        break;
> +    case SMBHSTADD:
> +        val = s->smb_addr;
> +        break;
> +    case SMBHSTDAT0:
> +        val = s->smb_data0;
> +        break;
> +    case SMBHSTDAT1:
> +        val = s->smb_data1;
> +        break;
> +    case SMBBLKDAT:
> +        val = s->smb_data[s->smb_index++];
> +        if (s->smb_index > 31)
> +            s->smb_index = 0;
> +        break;
> +    default:
> +        val = 0;
> +        break;
> +    }
> +    DPRINTF("SMB readb port=0x%04x val=0x%02x\n", addr, val);
> +    return val;
> +}
> +
> +static void pm_io_space_update(VT686PMState *s)
> +{
> +    uint32_t pm_io_base;
> +
> +    if (s->dev.config[0x80] & 1) {
> +        pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));

pci_get_long()


> +        pm_io_base &= 0xffc0;
> +
> +        /* XXX: need to improve memory and ioport allocation */
> +        DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
> +        register_ioport_write(pm_io_base, 64, 2, pm_ioport_writew, s);
> +        register_ioport_read(pm_io_base, 64, 2, pm_ioport_readw, s);
> +        register_ioport_write(pm_io_base, 64, 4, pm_ioport_writel, s);
> +        register_ioport_read(pm_io_base, 64, 4, pm_ioport_readl, s);
> +    }
> +}
> +
> +/* read config pci function4 registers. PM*/
> +static uint32_t pm_read_config(PCIDevice * d, uint32_t address, int len)
> +{
> +    uint32_t val;
> +
> +    DPRINTF("pm_read_config  address 0x%x  len 0x%x \n", address, len);
> +    switch (len) {
> +    default:
> +    case 4:
> +        if (address <= 0xfc) {
> +            val = le32_to_cpu(*(uint32_t *) (d->config + address));
> +            break;
> +        }
> +        /* fall through */
> +    case 2:
> +        if (address <= 0xfe) {
> +            val = le16_to_cpu(*(uint16_t *) (d->config + address));
> +            break;
> +        }
> +        /* fall through */
> +    case 1:
> +        val = d->config[address];
> +        break;
> +    }
> +    return val;
> +
> +}

pci_default_read_config() should work.
just pass NULL to pci_register_device()

> +
> +static void pm_write_config(PCIDevice *d,
> +                            uint32_t address, uint32_t val, int len)
> +{
> +    DPRINTF("pm_write_config  address 0x%x  val 0x%x len 0x%x \n",
> +           address, val, len);
> +    pci_default_write_config(d, address, val, len);
> +}
> +
> +static void pm_save(QEMUFile* f,void *opaque)
> +{
> +    VT686PMState *s = opaque;
> +
> +    pci_device_save(&s->dev, f);
> +
> +    qemu_put_be16s(f, &s->pmsts);
> +    qemu_put_be16s(f, &s->pmen);
> +    qemu_put_be16s(f, &s->pmcntrl);
> +    qemu_put_8s(f, &s->apmc);
> +    qemu_put_8s(f, &s->apms);
> +    qemu_put_timer(f, s->tmr_timer);
> +    qemu_put_be64(f, s->tmr_overflow_time);
> +}
> +
> +static int pm_load(QEMUFile* f,void* opaque,int version_id)
> +{
> +    VT686PMState *s = opaque;
> +    int ret;
> +
> +    if (version_id > 1)
> +        return -EINVAL;
> +
> +    ret = pci_device_load(&s->dev, f);
> +    if (ret < 0)
> +        return ret;
> +
> +    qemu_get_be16s(f, &s->pmsts);
> +    qemu_get_be16s(f, &s->pmen);
> +    qemu_get_be16s(f, &s->pmcntrl);
> +    qemu_get_8s(f, &s->apmc);
> +    qemu_get_8s(f, &s->apms);
> +    qemu_get_timer(f, s->tmr_timer);
> +    s->tmr_overflow_time=qemu_get_be64(f);
> +
> +    pm_io_space_update(s);
> +
> +    return 0;
> +}

qdevfy and vmstatefy, please.

> +
> +/*
> + * TODO: vt82c686b_ac97_init() and vt82c686b_mc97_init()
> + * just register a PCI device now, functionalities will be implemented later.
> + */
> +
> +void vt82c686b_ac97_init(PCIBus *bus, int devfn)
> +{
> +    VT686AC97State *s;
> +    uint8_t *pci_conf;
> +
> +    s = (VT686AC97State *)pci_register_device(bus,
> +                                         "vt82c686b_AC97", 
> sizeof(VT686AC97State),
> +                                         devfn, NULL, NULL);
> +    pci_conf = s->dev.config;
> +    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_VIA);
> +    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_VIA_AC97);
> +    pci_config_set_class(pci_conf, PCI_CLASS_MULTIMEDIA_AUDIO);
> +    pci_config_set_revision(pci_conf, 0x50);
> +
> +    pci_set_long(pci_conf + 0x04, 0x02100050);
> +    pci_set_long(pci_conf + 0x10, 0x00000001);
> +    pci_set_long(pci_conf + 0x14, 0x00000001);
> +    pci_set_long(pci_conf + 0x3c, 0x00000300);
> +    pci_set_long(pci_conf + 0x40, 0x1c000000);

Please use defines in pci_regs.h


> +}
> +
> +void vt82c686b_mc97_init(PCIBus *bus, int devfn)
> +{
> +    VT686MC97State *s;
> +    uint8_t *pci_conf;
> +
> +    s = (VT686MC97State *)pci_register_device(bus,
> +                                         "vt82c686b_MC97", 
> sizeof(VT686MC97State),
> +                                         devfn, NULL, NULL);
> +    pci_conf = s->dev.config;
> +    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_VIA);
> +    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_VIA_MC97);
> +    pci_config_set_class(pci_conf, PCI_CLASS_COMMUNICATION_OTHER);
> +    pci_config_set_revision(pci_conf, 0x30);
> +
> +    pci_set_long(pci_conf + 0x04, 0x02000030);
> +    pci_set_long(pci_conf + 0x10, 0x00000001);
> +    pci_set_long(pci_conf + 0x14, 0x00000001);
> +    pci_set_long(pci_conf + 0x3c, 0x00000300);
> +    pci_set_long(pci_conf + 0x40, 0x1c000000);

Please use defines in pci_regs.h

> +
> +}
> +
> +/* vt82c686 pm init */
> +i2c_bus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
> +                     qemu_irq sci_irq)
> +{
> +    VT686PMState *s;
> +    uint8_t *pci_conf;
> +
> +    s = (VT686PMState *)pci_register_device(bus,
> +                                         "vt82c686b_PM", 
> sizeof(VT686PMState),
> +                                         devfn, pm_read_config, 
> pm_write_config);
> +    pci_conf = s->dev.config;
> +    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_VIA);
> +    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_VIA_ACPI);
> +    pci_config_set_revision(pci_conf, 0x40);
> +    pci_conf[0x04] = 0x00;
> +    pci_conf[0x05] = 0x00;

pci_set_word(pci_conf + PCI_COMMAND, ...)

> +    pci_conf[0x06] = 0x80;
> +    pci_conf[0x07] = 0x02;

pci_set_word(pci_conf + PCI_SATUS, ...)

> +
> +    pci_set_long(pci_conf + 0x34, 0x00000068);
> +    pci_set_long(pci_conf + 0x48, 0x00000001);
> +    pci_set_long(pci_conf + 0x68, 0x00020001);
> +    pci_set_long(pci_conf + 0x70, 0x00000001);

Please use defines in pci_regs.h

> +
> +    /* SMB ports:0xeee0~0xeeef */
> +    smb_io_base =((smb_io_base & 0xfff0) + 0x0);
> +    pci_conf[0x90] = smb_io_base | 1;
> +    pci_conf[0x91] = smb_io_base >> 8;
> +    pci_conf[0xd2] = 0x90;
> +    register_ioport_write(smb_io_base, 0xf, 1, smb_ioport_writeb, s);
> +    register_ioport_read(smb_io_base, 0xf, 1, smb_ioport_readb, s);
> +
> +    s->tmr_timer = qemu_new_timer(vm_clock, pm_tmr_timer, s);
> +
> +    register_savevm("vt82c686b_PM", 0, 1, pm_save, pm_load, s);
> +
> +    s->smbus = i2c_init_bus(NULL, "i2c");
> +    return s->smbus;
> +}
> +
> +
> +
> +/* init the PCI-to-ISA bridge */
> +static int vt82c686b_initfn(PCIDevice *d)
> +{
> +    uint8_t *pci_conf;
> +
> +    isa_bus_new(&d->qdev);
> +    register_savevm("vt82c686b", 0, 1, vt82c686b_save, vt82c686b_load, d);
> +
> +    pci_conf = d->config;
> +    pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_VIA);
> +    pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_VIA_ISA_BRIDGE);
> +    pci_config_set_class(pci_conf, PCI_CLASS_BRIDGE_ISA);
> +    pci_config_set_prog_interface(pci_conf, 0x0);
> +    pci_config_set_revision(pci_conf,0x40); /* Revision 4.0 */
> +    pci_conf[PCI_HEADER_TYPE] =
> +        PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION;
> +
> +    qemu_register_reset(vt82c686b_reset, d);
> +
> +    return 0;
> +}
> +
> +int vt82c686b_init(PCIBus *bus, int devfn)
> +{
> +    PCIDevice *d;
> +    VT82C686BState *vt82c;
> +
> +    d = pci_create_simple(bus, devfn, "VT82C686B");
> +    vt82c = DO_UPCAST(VT82C686BState, dev, d);
> +
> +    /* set super io config */
> +    vt82c->superio_conf = qemu_mallocz(sizeof(SuperIOConfig));
> +
> +    return d->devfn;
> +}
> +
> +static PCIDeviceInfo via_info = {
> +    .qdev.name    = "VT82C686B",
> +    .qdev.desc    = "ISA bridge",
> +    .qdev.size    = sizeof(VT82C686BState),
> +    .qdev.no_user = 1,
> +    .init         = vt82c686b_initfn,
> +    .config_read  = vt82c686b_read_config,
> +    .config_write = vt82c686b_write_config,
> +};
> +
> +static void vt82c686b_register(void)
> +{
> +    pci_qdev_register(&via_info);
> +}
> +device_init(vt82c686b_register);
> -- 
> 1.7.0.4
> 
> 

-- 
yamahata