| Message ID | 20180305112732.26471-22-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: use the BYTE-based definitions when useful | expand |
On 03/05/2018, 12:27 PM, Philippe Mathieu-Daudé wrote: > --- a/hw/misc/auxbus.c > +++ b/hw/misc/auxbus.c > @@ -68,7 +68,7 @@ AUXBus *aux_init_bus(DeviceState *parent, const char *name) > > /* Memory related. */ > bus->aux_io = g_malloc(sizeof(*bus->aux_io)); > - memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", (1 << 20)); > + memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 2 * M_BYTE); Only M_BYTE, not "2 *". > address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io"); > return bus; > } > diff --git a/hw/misc/edu.c b/hw/misc/edu.c > index 34eb05d213..ce8235dfc4 100644 > --- a/hw/misc/edu.c > +++ b/hw/misc/edu.c > @@ -357,7 +357,7 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) > edu, QEMU_THREAD_JOINABLE); > > memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, > - "edu-mmio", 1 << 20); > + "edu-mmio", 1 * M_BYTE); "1 * " is superfluous thanks,
Hi Jiri, On 03/05/2018 04:02 PM, Jiri Slaby wrote: > On 03/05/2018, 12:27 PM, Philippe Mathieu-Daudé wrote: >> --- a/hw/misc/auxbus.c >> +++ b/hw/misc/auxbus.c >> @@ -68,7 +68,7 @@ AUXBus *aux_init_bus(DeviceState *parent, const char *name) >> >> /* Memory related. */ >> bus->aux_io = g_malloc(sizeof(*bus->aux_io)); >> - memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", (1 << 20)); >> + memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 2 * M_BYTE); > > Only M_BYTE, not "2 *". Yes! I totally missed this :/ > >> address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io"); >> return bus; >> } >> diff --git a/hw/misc/edu.c b/hw/misc/edu.c >> index 34eb05d213..ce8235dfc4 100644 >> --- a/hw/misc/edu.c >> +++ b/hw/misc/edu.c >> @@ -357,7 +357,7 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) >> edu, QEMU_THREAD_JOINABLE); >> >> memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, >> - "edu-mmio", 1 << 20); >> + "edu-mmio", 1 * M_BYTE); > > "1 * " is superfluous I kept "1 * " to be consistent with the other uses, I find it easier to review (or modify). Thanks for your review! Phil.
diff --git a/hw/misc/auxbus.c b/hw/misc/auxbus.c index b4cacd664b..c78aaebcbe 100644 --- a/hw/misc/auxbus.c +++ b/hw/misc/auxbus.c @@ -68,7 +68,7 @@ AUXBus *aux_init_bus(DeviceState *parent, const char *name) /* Memory related. */ bus->aux_io = g_malloc(sizeof(*bus->aux_io)); - memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", (1 << 20)); + memory_region_init(bus->aux_io, OBJECT(bus), "aux-io", 2 * M_BYTE); address_space_init(&bus->aux_addr_space, bus->aux_io, "aux-io"); return bus; } diff --git a/hw/misc/edu.c b/hw/misc/edu.c index 34eb05d213..ce8235dfc4 100644 --- a/hw/misc/edu.c +++ b/hw/misc/edu.c @@ -357,7 +357,7 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) edu, QEMU_THREAD_JOINABLE); memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, - "edu-mmio", 1 << 20); + "edu-mmio", 1 * M_BYTE); pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &edu->mmio); }
It eases code review, unit is explicit. Patch generated using: $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/ and modified manually. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/misc/auxbus.c | 2 +- hw/misc/edu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)