Huacai Chen <zltjiang...@gmail.com> wrote: > Signed-off-by: Huacai Chen <zltjiang...@gmail.com>
> +static void superio_ioport_writeb(void *opaque, uint32_t addr, uint32_t data) > +{ > + int can_write; > + SuperIOConfig *superio_conf = (SuperIOConfig *)opaque; Useless cast from void *. > +static uint32_t superio_ioport_readb(void *opaque, uint32_t addr) > +{ > + SuperIOConfig *superio_conf = (SuperIOConfig *)opaque; > the same. > +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); > +} You use vmstate in the rest of devices, why use old style here? > +typedef struct VT686AC97State { > + PCIDevice dev; > + int unused; not needed really. > +} VT686AC97State; > + > +typedef struct VT686MC97State { > + PCIDevice dev; > + int unused; also not needed. > +} 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) not used in the rest of the code. Suspicions to be the same bit that previous one. > + > + s = (VT686AC97State *)pci_register_device(bus, > + "vt82c686b_AC97", > sizeof(VT686AC97State), > + devfn, NULL, NULL); use DO_UPCAST like rest of driver instead of cast. > +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); Same than previous comment. > + /* set super io config */ > + vt686->superio_conf = qemu_mallocz(sizeof(SuperIOConfig)); Why do you use a pointer instead of changing it to a embeded estruct inside struct VT82C686BState? Later, Juan.