[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types |
Date: |
Thu, 30 Jan 2014 08:17:04 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Replace them with uint8/32/64.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
Reviewed-by: Eric Blake <address@hidden>
> +++ b/hw/audio/pcspk.c
> @@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error
> **errp)
> }
>
> static Property pcspk_properties[] = {
> - DEFINE_PROP_HEX32("iobase", PCSpkState, iobase, -1),
> + DEFINE_PROP_UINT32("iobase", PCSpkState, iobase, -1),
When there's multiple properties, I can see the value of alignment. But
for this instance, the two spaces before -1 seem spurious; you could fix
them in this patch.
> +++ b/hw/char/parallel.c
> @@ -595,7 +595,7 @@ bool parallel_mm_init(MemoryRegion *address_space,
>
> static Property parallel_isa_properties[] = {
> DEFINE_PROP_UINT32("index", ISAParallelState, index, -1),
> - DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase, -1),
> + DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase, -1),
> DEFINE_PROP_UINT32("irq", ISAParallelState, isairq, 7),
> DEFINE_PROP_CHR("chardev", ISAParallelState, state.chr),
And alignment looks all messed up on this one.
> DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> index 5cb77b3..c9fcb27 100644
> --- a/hw/char/serial-isa.c
> +++ b/hw/char/serial-isa.c
> @@ -88,7 +88,7 @@ static const VMStateDescription vmstate_isa_serial = {
>
> static Property serial_isa_properties[] = {
> DEFINE_PROP_UINT32("index", ISASerialState, index, -1),
> - DEFINE_PROP_HEX32("iobase", ISASerialState, iobase, -1),
> + DEFINE_PROP_UINT32("iobase", ISASerialState, iobase, -1),
Drop a space before ISASerialState to make this one look nice.
> +++ b/hw/display/tcx.c
> @@ -617,11 +617,11 @@ static int tcx_init1(SysBusDevice *dev)
> }
>
> static Property tcx_properties[] = {
> - DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
> + DEFINE_PROP_UINT32("vram_size", TCXState, vram_size, -1),
> DEFINE_PROP_UINT16("width", TCXState, width, -1),
> DEFINE_PROP_UINT16("height", TCXState, height, -1),
> DEFINE_PROP_UINT16("depth", TCXState, depth, -1),
> - DEFINE_PROP_HEX64("prom_addr", TCXState, prom_addr, -1),
> + DEFINE_PROP_UINT64("prom_addr", TCXState, prom_addr, -1),
Another one where alignment is now funky.
> +++ b/hw/ide/isa.c
> @@ -104,8 +104,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int
> iobase2, int isairq,
> }
>
> static Property isa_ide_properties[] = {
> - DEFINE_PROP_HEX32("iobase", ISAIDEState, iobase, 0x1f0),
> - DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6),
> + DEFINE_PROP_UINT32("iobase", ISAIDEState, iobase, 0x1f0),
> + DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
> DEFINE_PROP_UINT32("irq", ISAIDEState, isairq, 14),
And another one.
> DEFINE_PROP_END_OF_LIST(),
> };
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 18c4b7e..6e475e6 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev)
> #define DEFINE_IDE_DEV_PROPERTIES() \
> DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \
> DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \
> - DEFINE_PROP_HEX64("wwn", IDEDrive, dev.wwn, 0), \
> + DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \
Here, the trailing \ lost alignment.
> +++ b/hw/intc/i8259_common.c
> @@ -123,9 +123,9 @@ static const VMStateDescription vmstate_pic_common = {
> };
>
> static Property pic_properties_common[] = {
> - DEFINE_PROP_HEX32("iobase", PICCommonState, iobase, -1),
> - DEFINE_PROP_HEX32("elcr_addr", PICCommonState, elcr_addr, -1),
> - DEFINE_PROP_HEX8("elcr_mask", PICCommonState, elcr_mask, -1),
> + DEFINE_PROP_UINT32("iobase", PICCommonState, iobase, -1),
> + DEFINE_PROP_UINT32("elcr_addr", PICCommonState, elcr_addr, -1),
> + DEFINE_PROP_UINT8("elcr_mask", PICCommonState, elcr_mask, -1),
> DEFINE_PROP_BIT("master", PICCommonState, master, 0, false),
Here there's no alignment, but lots of doubled spaces.
> +++ b/hw/misc/applesmc.c
> @@ -250,7 +250,7 @@ static void applesmc_isa_realize(DeviceState *dev, Error
> **errp)
> }
>
> static Property applesmc_isa_properties[] = {
> - DEFINE_PROP_HEX32("iobase", AppleSMCState, iobase,
> + DEFINE_PROP_UINT32("iobase", AppleSMCState, iobase,
> APPLESMC_DEFAULT_IOBASE),
Indentation is now off.
> +++ b/hw/net/ne2000-isa.c
> @@ -86,7 +86,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Error
> **errp)
> }
>
> static Property ne2000_isa_properties[] = {
> - DEFINE_PROP_HEX32("iobase", ISANE2000State, iobase, 0x300),
> + DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300),
> DEFINE_PROP_UINT32("irq", ISANE2000State, isairq, 9),
> DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c),
Here, the line you didn't touch looks awkward.
> +++ b/hw/sd/sdhci.c
> @@ -1234,9 +1234,9 @@ const VMStateDescription sdhci_vmstate = {
> /* Capabilities registers provide information on supported features of this
> * specific host controller implementation */
> static Property sdhci_properties[] = {
> - DEFINE_PROP_HEX32("capareg", SDHCIState, capareg,
> + DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
> SDHC_CAPAB_REG_DEFAULT),
Might as well fix indentation while touching this.
> +++ b/hw/timer/i8254.c
> @@ -342,7 +342,7 @@ static void pit_realizefn(DeviceState *dev, Error **err)
> }
>
> static Property pit_properties[] = {
> - DEFINE_PROP_HEX32("iobase", PITCommonState, iobase, -1),
> + DEFINE_PROP_UINT32("iobase", PITCommonState, iobase, -1),
> DEFINE_PROP_END_OF_LIST(),
Another instance of unneeded double space.
> +++ b/hw/usb/host-libusb.c
> @@ -1324,8 +1324,8 @@ static Property usb_host_dev_properties[] = {
> DEFINE_PROP_UINT32("hostbus", USBHostDevice, match.bus_num, 0),
> DEFINE_PROP_UINT32("hostaddr", USBHostDevice, match.addr, 0),
> DEFINE_PROP_STRING("hostport", USBHostDevice, match.port),
> - DEFINE_PROP_HEX32("vendorid", USBHostDevice, match.vendor_id, 0),
> - DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0),
> + DEFINE_PROP_UINT32("vendorid", USBHostDevice, match.vendor_id, 0),
> + DEFINE_PROP_UINT32("productid", USBHostDevice, match.product_id, 0),
> DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count, 4),
Alignment looks off.
> +++ b/include/hw/qdev-dma.h
> @@ -7,4 +7,4 @@
> * See the COPYING file in the top-level directory.
> */
> #define DEFINE_PROP_DMAADDR(_n, _s, _f, _d) \
> - DEFINE_PROP_HEX64(_n, _s, _f, _d)
> + DEFINE_PROP_UINT64(_n, _s, _f, _d)
Do we even need this #define, or would it be smarter to just inline the
use of DEFINE_PROP_UINT64 in the rest of the file?
All my comments are cosmetic, so my review still stands; I'm fine
whether you apply this as-is or touch it up per the suggestions.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 06/12] qdev: inline qdev_prop_parse, (continued)
- [Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree", Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types, Paolo Bonzini, 2014/01/30
- Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types,
Eric Blake <=
- [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 12/12] qdev: use QAPI type names for properties, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 13/12] qapi: refine human printing of sizes, Paolo Bonzini, 2014/01/30