Skip to content

Indirection Cleanup

Juan Quintela qemu-devel 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest26 patches 1 replyIn review

v2 from the cover letter

- fix comment missmerge in eepro100 (Stefan Weil)
- add dev.qdev hint instead of double DO_UPCAST on lsi (Kraxel)
- usb-ohci: not everything is pci. Add PCI state only to the pci part (Kraxel)
- ne2000: Don't force pci bits into isa devices (make Kraxel happy)
- ne2000: split isa and pci save/load functions. From now, isa parts are only the common ones.
- cirrus_vga: Don't force pci bits into isa device (make Kraxel happy)
- pcnet: Add it here, split save/load function between pci and generic.
- e1000 cleanup: Add it to the series (was sent alone), just for easy tracking.
- drop es1370 bits: malc already integrated it.

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/26eepro100: convert casts to DO_UPCAST()hw/eepro100.c+4 -40 replies
2/26eepro100: cast a void * makes no sensehw/eepro100.c+3 -30 replies
3/26eepro100: Remove unused indirection of PCIDevicehw/eepro100.c+23 -340 replies
4/26pci: remove casts from void *hw/pci.c+4 -40 replies
5/26rtl8139: Remove unneeded double indirection of PCIRTL8139Statehw/rtl8139.c+13 -250 replies
6/26rtl8139: remove pointless cast from void *hw/rtl8139.c+2 -20 replies
7/26lsi53c895a: remove pointless cast from void *hw/lsi53c895a.c+19 -190 replies
8/26lsi53c895a: use DO_UPCAST to cast from PCIDevicehw/lsi53c895a.c+5 -50 replies
9/26lsi53c895a: rename PCIDevice field from pci_dev to dev (consistence)hw/lsi53c895a.c+12 -120 replies
10/26lsi53c895a: LSIState is a DeviceHosthw/lsi53c895a.c+1 -10 replies
11/26Introduce vga_common_reset() to be able to typcheck vga_reset()hw/cirrus_vga.c hw/vga.c hw/vga_int.h+9 -50 replies
12/26vga: Rename vga_state -> vgahw/vga.c+6 -60 replies
13/26Everything outside of vga.c should use VGACommonStatehw/cirrus_vga.c hw/vga.c hw/vga_int.h +1 more+14 -140 replies
14/26usb-ohci: Change casts to DO_UPCAST() for OHCIPCIStatehw/usb-ohci.c+4 -30 replies
15/26ne2000: remove casts from void *hw/ne2000.c+2 -20 replies
16/26ne2000: pci_dev has this very value with the right typehw/ne2000.c+1 -10 replies
17/26ne2000: Change casts to DO_UPCAST() for PCINE2000Statehw/ne2000.c+2 -20 replies
18/26We need PCINE2000State for save/load functionshw/ne2000.c+5 -50 replies
19/26Add pci_ne2000_{save/load} functions, then remove pci_dev NE2000State fieldhw/ne2000.c+28 -150 replies
20/26cirrus_vga: remove pointless cast from void *hw/cirrus_vga.c+5 -50 replies
21/26cirrus_vga: Change casts to DO_UPCAST() for PCICirrusVGAStatehw/cirrus_vga.c+5 -50 replies
22/26Add pci_cirrus_vga_{save/load} functions, then remove vga.pci_dev usehw/cirrus_vga.c+27 -120 replies
23/26pcnet: Change casts to DO_UPCAST() for PCIPCNetStatehw/pcnet.c+4 -40 replies
24/26pcnet: remove useless casts This are casts to the very type of the functionhw/pcnet.c+6 -60 replies
25/26pcnet: Add pci_pcnet_{save/load} functions, then remove PCNetState pci_dev fieldhw/pcnet.c+26 -130 replies
26/26e1000 cleanuphw/e1000.c+5 -50 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

Cover letter [Qemu-devel] [PATCH 00/26] Indirection Cleanup

Hi

v2:
- fix comment missmerge in eepro100 (Stefan Weil)
- add dev.qdev hint instead of double DO_UPCAST on lsi (Kraxel)
- usb-ohci: not everything is pci. Add PCI state only to the pci part (Kraxel)
- ne2000: Don't force pci bits into isa devices (make Kraxel happy)
- ne2000: split isa and pci save/load functions. From now, isa parts are only the common ones.
- cirrus_vga: Don't force pci bits into isa device (make Kraxel happy)
- pcnet: Add it here, split save/load function between pci and generic.
- e1000 cleanup: Add it to the series (was sent alone), just for easy tracking.
- drop es1370 bits: malc already integrated it.

ToDo:
- vga: It needs to have a separate VGAState and VGACommonState. And make clear what is shared and what no (convince cirrus_vga, vmware_vga and blizzard to use same/similar subset).

Later, Juan.

v1:

This patch series clean up "half" converted qemu drivers that had changed from:

struct FOOState

to

typedef PCIFOOState {
    PCIDevice dev;
    FOOState foo;
} PCIFOOState;

It just moves PCIDevice to be the 1st field of FOOState.

Once there, other cleanups were done:
a - pci_dev pointer from FOOState to PCIFOOState is removed, jsut use s->dev
The field is leave only in the drivers that also emulate isa.
b- Once there, transformo
PCIFOOState *s = (PCIFOOState *)pci_dev
to
PCIFOOState *s = DO_UPCAST(PCIFOOState, dev, pci_dev)
where pci_dev is a PCI_DEVICE.
c- again, once there, remove all the casts from void * (they are not
needed since '89)
PCIFOOState *s = (PCIFOOState *)opaque;
to
PCIFOOState *s = opaque;
d- Start of vga.c cleanup. It is not trivial, as just now
VGAState == VGACommonState, functions need to be changed to use the right value.

ToDo:
- pcnet: It needs a different approach, because it can be both a PCIDevice or a SysBus device.
- vga: It have to separate the common part for the not common part, problems now is that VGAState is used for both the common state and the standard vga. To make things more interesting, different bits are "inherited" from vga in different devices:
- cirrus_vga
- vmware_vga
- blizzard (vga that is not pci)
- vga common state has a pci_dev pointer, that is only needed by std vga, as cirrus_vga stores it (with this patches) otherplace, blizzard is not pci .... you get the idea
- I guess some other driver is missing, but my fast grep didn't found it.

Later, Juan.

*** BLURB HERE ***

Juan Quintela (26):
eepro100: convert casts to DO_UPCAST()
eepro100: cast a void * makes no sense
eepro100: Remove unused indirection of PCIDevice
pci: remove casts from void *
rtl8139: Remove unneeded double indirection of PCIRTL8139State
rtl8139: remove pointless cast from void *
lsi53c895a: remove pointless cast from void *
lsi53c895a: use DO_UPCAST to cast from PCIDevice
lsi53c895a: rename PCIDevice field from pci_dev to dev (consistence)
lsi53c895a: LSIState is a PCIDevice is a DeviceHost Introduce vga_common_reset() to be able to typcheck vga_reset()
vga: Rename vga_state -> vga
Everything outside of vga.c should use VGACommonState
usb-ohci: Change casts to DO_UPCAST() for OHCIPCIState
ne2000: remove casts from void *
ne2000: pci_dev has this very value with the right type
ne2000: Change casts to DO_UPCAST() for PCINE2000State
We need PCINE2000State for save/load functions
Add pci_ne2000_{save/load} functions, then remove pci_dev NE2000State
field
cirrus_vga: remove pointless cast from void *
cirrus_vga: Change casts to DO_UPCAST() for PCICirrusVGAState Add pci_cirrus_vga_{save/load} functions, then remove vga.pci_dev use
pcnet: Change casts to DO_UPCAST() for PCIPCNetState
pcnet: remove useless casts This are casts to the very type of the
function
pcnet: Add pci_pcnet_{save/load} functions, then remove PCNetState
pci_dev field
e1000 cleanup

 hw/cirrus_vga.c |   69 +++++++++++++++++++++++++++++++++---------------------
 hw/e1000.c      |   10 ++++----
 hw/eepro100.c   |   63 ++++++++++++++++++++-----------------------------
 hw/lsi53c895a.c |   64 +++++++++++++++++++++++++-------------------------
 hw/ne2000.c     |   61 +++++++++++++++++++++++++++++-------------------
 hw/pci.c        |    8 +++---
 hw/pcnet.c      |   59 ++++++++++++++++++++++++++++------------------
 hw/rtl8139.c    |   42 ++++++++++++---------------------
 hw/usb-ohci.c   |    7 +++--
 hw/vga.c        |   26 ++++++++++++--------
 hw/vga_int.h    |   12 ++++-----
 hw/vmware_vga.c |    4 +-
 12 files changed, 224 insertions(+), 201 deletions(-)