* [PATCH 1/4] hw/misc/macio: Avoid some QOM casts
2023-01-18 0:32 [PATCH 0/4] Misc macio clean ups BALATON Zoltan
@ 2023-01-18 0:32 ` BALATON Zoltan
2023-01-18 7:16 ` Philippe Mathieu-Daudé
2023-01-22 18:14 ` Mark Cave-Ayland
2023-01-18 0:32 ` [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity BALATON Zoltan
` (3 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-18 0:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
At several places we already have the object pointer with the right
type so we don't need to cast it back and forth. Avoiding these casts
improves readability.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/misc/macio/macio.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 08dbdd7fc0..0dfe372965 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -220,11 +220,11 @@ static void macio_oldworld_init(Object *obj)
DeviceState *dev;
int i;
- object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
+ object_initialize_child(obj, "pic", &os->pic, TYPE_HEATHROW);
- object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
+ object_initialize_child(obj, "cuda", &s->cuda, TYPE_CUDA);
- object_initialize_child(OBJECT(s), "nvram", &os->nvram, TYPE_MACIO_NVRAM);
+ object_initialize_child(obj, "nvram", &os->nvram, TYPE_MACIO_NVRAM);
dev = DEVICE(&os->nvram);
qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
qdev_prop_set_uint32(dev, "it_shift", 4);
@@ -372,9 +372,9 @@ static void macio_newworld_init(Object *obj)
NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
int i;
- object_initialize_child(OBJECT(s), "pic", &ns->pic, TYPE_OPENPIC);
+ object_initialize_child(obj, "pic", &ns->pic, TYPE_OPENPIC);
- object_initialize_child(OBJECT(s), "gpio", &ns->gpio, TYPE_MACIO_GPIO);
+ object_initialize_child(obj, "gpio", &ns->gpio, TYPE_MACIO_GPIO);
for (i = 0; i < 2; i++) {
macio_init_ide(s, &ns->ide[i], i);
@@ -390,9 +390,9 @@ static void macio_instance_init(Object *obj)
qbus_init(&s->macio_bus, sizeof(s->macio_bus), TYPE_MACIO_BUS,
DEVICE(obj), "macio.0");
- object_initialize_child(OBJECT(s), "dbdma", &s->dbdma, TYPE_MAC_DBDMA);
+ object_initialize_child(obj, "dbdma", &s->dbdma, TYPE_MAC_DBDMA);
- object_initialize_child(OBJECT(s), "escc", &s->escc, TYPE_ESCC);
+ object_initialize_child(obj, "escc", &s->escc, TYPE_ESCC);
}
static const VMStateDescription vmstate_macio_oldworld = {
--
2.30.6
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/4] hw/misc/macio: Avoid some QOM casts
2023-01-18 0:32 ` [PATCH 1/4] hw/misc/macio: Avoid some QOM casts BALATON Zoltan
@ 2023-01-18 7:16 ` Philippe Mathieu-Daudé
2023-01-22 18:14 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-18 7:16 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
On 18/1/23 01:32, BALATON Zoltan wrote:
> At several places we already have the object pointer with the right
> type so we don't need to cast it back and forth. Avoiding these casts
> improves readability.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] hw/misc/macio: Avoid some QOM casts
2023-01-18 0:32 ` [PATCH 1/4] hw/misc/macio: Avoid some QOM casts BALATON Zoltan
2023-01-18 7:16 ` Philippe Mathieu-Daudé
@ 2023-01-22 18:14 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Mark Cave-Ayland @ 2023-01-22 18:14 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On 18/01/2023 00:32, BALATON Zoltan wrote:
> At several places we already have the object pointer with the right
> type so we don't need to cast it back and forth. Avoiding these casts
> improves readability.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 08dbdd7fc0..0dfe372965 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -220,11 +220,11 @@ static void macio_oldworld_init(Object *obj)
> DeviceState *dev;
> int i;
>
> - object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
> + object_initialize_child(obj, "pic", &os->pic, TYPE_HEATHROW);
>
> - object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
> + object_initialize_child(obj, "cuda", &s->cuda, TYPE_CUDA);
>
> - object_initialize_child(OBJECT(s), "nvram", &os->nvram, TYPE_MACIO_NVRAM);
> + object_initialize_child(obj, "nvram", &os->nvram, TYPE_MACIO_NVRAM);
> dev = DEVICE(&os->nvram);
> qdev_prop_set_uint32(dev, "size", MACIO_NVRAM_SIZE);
> qdev_prop_set_uint32(dev, "it_shift", 4);
> @@ -372,9 +372,9 @@ static void macio_newworld_init(Object *obj)
> NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
> int i;
>
> - object_initialize_child(OBJECT(s), "pic", &ns->pic, TYPE_OPENPIC);
> + object_initialize_child(obj, "pic", &ns->pic, TYPE_OPENPIC);
>
> - object_initialize_child(OBJECT(s), "gpio", &ns->gpio, TYPE_MACIO_GPIO);
> + object_initialize_child(obj, "gpio", &ns->gpio, TYPE_MACIO_GPIO);
>
> for (i = 0; i < 2; i++) {
> macio_init_ide(s, &ns->ide[i], i);
> @@ -390,9 +390,9 @@ static void macio_instance_init(Object *obj)
> qbus_init(&s->macio_bus, sizeof(s->macio_bus), TYPE_MACIO_BUS,
> DEVICE(obj), "macio.0");
>
> - object_initialize_child(OBJECT(s), "dbdma", &s->dbdma, TYPE_MAC_DBDMA);
> + object_initialize_child(obj, "dbdma", &s->dbdma, TYPE_MAC_DBDMA);
>
> - object_initialize_child(OBJECT(s), "escc", &s->escc, TYPE_ESCC);
> + object_initialize_child(obj, "escc", &s->escc, TYPE_ESCC);
> }
>
> static const VMStateDescription vmstate_macio_oldworld = {
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity
2023-01-18 0:32 [PATCH 0/4] Misc macio clean ups BALATON Zoltan
2023-01-18 0:32 ` [PATCH 1/4] hw/misc/macio: Avoid some QOM casts BALATON Zoltan
@ 2023-01-18 0:32 ` BALATON Zoltan
2023-01-18 7:16 ` Philippe Mathieu-Daudé
2023-01-22 18:15 ` Mark Cave-Ayland
2023-01-18 0:32 ` [PATCH 3/4] hw/misc/macio: Remove some single use local variables BALATON Zoltan
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-18 0:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
Some functions use sysbus_dev while others sbd name for local variable
storing a sysbus device pointer. Standardise on the shorter name to be
consistent and make the code easier to read as short name is less
distracting and needs less line breaks.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/misc/macio/macio.c | 78 +++++++++++++++++++------------------------
1 file changed, 35 insertions(+), 43 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 0dfe372965..4d7223cc85 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -96,14 +96,14 @@ static void macio_bar_setup(MacIOState *s)
static void macio_common_realize(PCIDevice *d, Error **errp)
{
MacIOState *s = MACIO(d);
- SysBusDevice *sysbus_dev;
+ SysBusDevice *sbd;
if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&s->dbdma);
+ sbd = SYS_BUS_DEVICE(&s->dbdma);
memory_region_add_subregion(&s->bar, 0x08000,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
@@ -122,11 +122,10 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
qemu_irq irq0, qemu_irq irq1, int dmaid,
Error **errp)
{
- SysBusDevice *sysbus_dev;
+ SysBusDevice *sbd = SYS_BUS_DEVICE(ide);
- sysbus_dev = SYS_BUS_DEVICE(ide);
- sysbus_connect_irq(sysbus_dev, 0, irq0);
- sysbus_connect_irq(sysbus_dev, 1, irq1);
+ sysbus_connect_irq(sbd, 0, irq0);
+ sysbus_connect_irq(sbd, 1, irq1);
qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma),
&error_abort);
@@ -141,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
OldWorldMacIOState *os = OLDWORLD_MACIO(d);
DeviceState *pic_dev = DEVICE(&os->pic);
Error *err = NULL;
- SysBusDevice *sysbus_dev;
+ SysBusDevice *sbd;
macio_common_realize(d, &err);
if (err) {
@@ -153,33 +152,30 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&os->pic);
+ sbd = SYS_BUS_DEVICE(&os->pic);
memory_region_add_subregion(&s->bar, 0x0,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
s->frequency);
if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&s->cuda);
+ sbd = SYS_BUS_DEVICE(&s->cuda);
memory_region_add_subregion(&s->bar, 0x16000,
- sysbus_mmio_get_region(sysbus_dev, 0));
- sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
- OLDWORLD_CUDA_IRQ));
+ sysbus_mmio_get_region(sbd, 0));
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, OLDWORLD_CUDA_IRQ));
- sysbus_dev = SYS_BUS_DEVICE(&s->escc);
- sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
- OLDWORLD_ESCCB_IRQ));
- sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
- OLDWORLD_ESCCA_IRQ));
+ sbd = SYS_BUS_DEVICE(&s->escc);
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCB_IRQ));
+ sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCA_IRQ));
if (!qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&os->nvram);
+ sbd = SYS_BUS_DEVICE(&os->nvram);
memory_region_add_subregion(&s->bar, 0x60000,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
pmac_format_nvram_partition(&os->nvram, os->nvram.size);
/* IDE buses */
@@ -274,7 +270,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
DeviceState *pic_dev = DEVICE(&ns->pic);
Error *err = NULL;
- SysBusDevice *sysbus_dev;
+ SysBusDevice *sbd;
MemoryRegion *timer_memory = NULL;
macio_common_realize(d, &err);
@@ -285,16 +281,14 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
/* OpenPIC */
qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
- sysbus_dev = SYS_BUS_DEVICE(&ns->pic);
- sysbus_realize_and_unref(sysbus_dev, &error_fatal);
+ sbd = SYS_BUS_DEVICE(&ns->pic);
+ sysbus_realize_and_unref(sbd, &error_fatal);
memory_region_add_subregion(&s->bar, 0x40000,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
- sysbus_dev = SYS_BUS_DEVICE(&s->escc);
- sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
- NEWWORLD_ESCCB_IRQ));
- sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
- NEWWORLD_ESCCA_IRQ));
+ sbd = SYS_BUS_DEVICE(&s->escc);
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCB_IRQ));
+ sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCA_IRQ));
/* IDE buses */
macio_realize_ide(s, &ns->ide[0],
@@ -326,27 +320,26 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
if (!qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
- sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
+ sbd = SYS_BUS_DEVICE(&ns->gpio);
+ sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev,
NEWWORLD_EXTING_GPIO1));
- sysbus_connect_irq(sysbus_dev, 9, qdev_get_gpio_in(pic_dev,
+ sysbus_connect_irq(sbd, 9, qdev_get_gpio_in(pic_dev,
NEWWORLD_EXTING_GPIO9));
memory_region_add_subregion(&s->bar, 0x50,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
/* PMU */
object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU);
- object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sysbus_dev),
+ object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sbd),
&error_abort);
qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&s->pmu);
- sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
- NEWWORLD_PMU_IRQ));
+ sbd = SYS_BUS_DEVICE(&s->pmu);
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_PMU_IRQ));
memory_region_add_subregion(&s->bar, 0x16000,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
} else {
object_unparent(OBJECT(&ns->gpio));
@@ -358,11 +351,10 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
return;
}
- sysbus_dev = SYS_BUS_DEVICE(&s->cuda);
- sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
- NEWWORLD_CUDA_IRQ));
+ sbd = SYS_BUS_DEVICE(&s->cuda);
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_CUDA_IRQ));
memory_region_add_subregion(&s->bar, 0x16000,
- sysbus_mmio_get_region(sysbus_dev, 0));
+ sysbus_mmio_get_region(sbd, 0));
}
}
--
2.30.6
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity
2023-01-18 0:32 ` [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity BALATON Zoltan
@ 2023-01-18 7:16 ` Philippe Mathieu-Daudé
2023-01-22 18:15 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-18 7:16 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
On 18/1/23 01:32, BALATON Zoltan wrote:
> Some functions use sysbus_dev while others sbd name for local variable
> storing a sysbus device pointer. Standardise on the shorter name to be
> consistent and make the code easier to read as short name is less
> distracting and needs less line breaks.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 78 +++++++++++++++++++------------------------
> 1 file changed, 35 insertions(+), 43 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity
2023-01-18 0:32 ` [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity BALATON Zoltan
2023-01-18 7:16 ` Philippe Mathieu-Daudé
@ 2023-01-22 18:15 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Mark Cave-Ayland @ 2023-01-22 18:15 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On 18/01/2023 00:32, BALATON Zoltan wrote:
> Some functions use sysbus_dev while others sbd name for local variable
> storing a sysbus device pointer. Standardise on the shorter name to be
> consistent and make the code easier to read as short name is less
> distracting and needs less line breaks.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 78 +++++++++++++++++++------------------------
> 1 file changed, 35 insertions(+), 43 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 0dfe372965..4d7223cc85 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -96,14 +96,14 @@ static void macio_bar_setup(MacIOState *s)
> static void macio_common_realize(PCIDevice *d, Error **errp)
> {
> MacIOState *s = MACIO(d);
> - SysBusDevice *sysbus_dev;
> + SysBusDevice *sbd;
>
> if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&s->dbdma);
> + sbd = SYS_BUS_DEVICE(&s->dbdma);
> memory_region_add_subregion(&s->bar, 0x08000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
>
> qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
> qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
> @@ -122,11 +122,10 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
> qemu_irq irq0, qemu_irq irq1, int dmaid,
> Error **errp)
> {
> - SysBusDevice *sysbus_dev;
> + SysBusDevice *sbd = SYS_BUS_DEVICE(ide);
>
> - sysbus_dev = SYS_BUS_DEVICE(ide);
> - sysbus_connect_irq(sysbus_dev, 0, irq0);
> - sysbus_connect_irq(sysbus_dev, 1, irq1);
> + sysbus_connect_irq(sbd, 0, irq0);
> + sysbus_connect_irq(sbd, 1, irq1);
> qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
> object_property_set_link(OBJECT(ide), "dbdma", OBJECT(&s->dbdma),
> &error_abort);
> @@ -141,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
> OldWorldMacIOState *os = OLDWORLD_MACIO(d);
> DeviceState *pic_dev = DEVICE(&os->pic);
> Error *err = NULL;
> - SysBusDevice *sysbus_dev;
> + SysBusDevice *sbd;
>
> macio_common_realize(d, &err);
> if (err) {
> @@ -153,33 +152,30 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
> if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&os->pic);
> + sbd = SYS_BUS_DEVICE(&os->pic);
> memory_region_add_subregion(&s->bar, 0x0,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
>
> qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
> s->frequency);
> if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&s->cuda);
> + sbd = SYS_BUS_DEVICE(&s->cuda);
> memory_region_add_subregion(&s->bar, 0x16000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> - sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
> - OLDWORLD_CUDA_IRQ));
> + sysbus_mmio_get_region(sbd, 0));
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, OLDWORLD_CUDA_IRQ));
>
> - sysbus_dev = SYS_BUS_DEVICE(&s->escc);
> - sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
> - OLDWORLD_ESCCB_IRQ));
> - sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
> - OLDWORLD_ESCCA_IRQ));
> + sbd = SYS_BUS_DEVICE(&s->escc);
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCB_IRQ));
> + sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, OLDWORLD_ESCCA_IRQ));
>
> if (!qdev_realize(DEVICE(&os->nvram), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&os->nvram);
> + sbd = SYS_BUS_DEVICE(&os->nvram);
> memory_region_add_subregion(&s->bar, 0x60000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
> pmac_format_nvram_partition(&os->nvram, os->nvram.size);
>
> /* IDE buses */
> @@ -274,7 +270,7 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
> NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
> DeviceState *pic_dev = DEVICE(&ns->pic);
> Error *err = NULL;
> - SysBusDevice *sysbus_dev;
> + SysBusDevice *sbd;
> MemoryRegion *timer_memory = NULL;
>
> macio_common_realize(d, &err);
> @@ -285,16 +281,14 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
>
> /* OpenPIC */
> qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
> - sysbus_dev = SYS_BUS_DEVICE(&ns->pic);
> - sysbus_realize_and_unref(sysbus_dev, &error_fatal);
> + sbd = SYS_BUS_DEVICE(&ns->pic);
> + sysbus_realize_and_unref(sbd, &error_fatal);
> memory_region_add_subregion(&s->bar, 0x40000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
>
> - sysbus_dev = SYS_BUS_DEVICE(&s->escc);
> - sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
> - NEWWORLD_ESCCB_IRQ));
> - sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
> - NEWWORLD_ESCCA_IRQ));
> + sbd = SYS_BUS_DEVICE(&s->escc);
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCB_IRQ));
> + sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCA_IRQ));
>
> /* IDE buses */
> macio_realize_ide(s, &ns->ide[0],
> @@ -326,27 +320,26 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
> if (!qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
> - sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
> + sbd = SYS_BUS_DEVICE(&ns->gpio);
> + sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev,
> NEWWORLD_EXTING_GPIO1));
> - sysbus_connect_irq(sysbus_dev, 9, qdev_get_gpio_in(pic_dev,
> + sysbus_connect_irq(sbd, 9, qdev_get_gpio_in(pic_dev,
> NEWWORLD_EXTING_GPIO9));
> memory_region_add_subregion(&s->bar, 0x50,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
>
> /* PMU */
> object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU);
> - object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sysbus_dev),
> + object_property_set_link(OBJECT(&s->pmu), "gpio", OBJECT(sbd),
> &error_abort);
> qdev_prop_set_bit(DEVICE(&s->pmu), "has-adb", ns->has_adb);
> if (!qdev_realize(DEVICE(&s->pmu), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&s->pmu);
> - sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
> - NEWWORLD_PMU_IRQ));
> + sbd = SYS_BUS_DEVICE(&s->pmu);
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_PMU_IRQ));
> memory_region_add_subregion(&s->bar, 0x16000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
> } else {
> object_unparent(OBJECT(&ns->gpio));
>
> @@ -358,11 +351,10 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
> if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
> return;
> }
> - sysbus_dev = SYS_BUS_DEVICE(&s->cuda);
> - sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
> - NEWWORLD_CUDA_IRQ));
> + sbd = SYS_BUS_DEVICE(&s->cuda);
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(pic_dev, NEWWORLD_CUDA_IRQ));
> memory_region_add_subregion(&s->bar, 0x16000,
> - sysbus_mmio_get_region(sysbus_dev, 0));
> + sysbus_mmio_get_region(sbd, 0));
> }
> }
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] hw/misc/macio: Remove some single use local variables
2023-01-18 0:32 [PATCH 0/4] Misc macio clean ups BALATON Zoltan
2023-01-18 0:32 ` [PATCH 1/4] hw/misc/macio: Avoid some QOM casts BALATON Zoltan
2023-01-18 0:32 ` [PATCH 2/4] hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity BALATON Zoltan
@ 2023-01-18 0:32 ` BALATON Zoltan
2023-01-18 7:17 ` Philippe Mathieu-Daudé
2023-01-22 18:16 ` Mark Cave-Ayland
2023-01-18 0:32 ` [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp BALATON Zoltan
2023-02-01 23:32 ` [PATCH 0/4] Misc macio clean ups Mark Cave-Ayland
4 siblings, 2 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-18 0:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
Drop some local variables that could just be substituted at the single
place they were used. This makes the code shorter and simpler.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/misc/macio/macio.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 4d7223cc85..ae2a9a960d 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -53,10 +53,8 @@
*/
static void macio_escc_legacy_setup(MacIOState *s)
{
- ESCCState *escc = ESCC(&s->escc);
- SysBusDevice *sbd = SYS_BUS_DEVICE(escc);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(&s->escc);
MemoryRegion *escc_legacy = g_new(MemoryRegion, 1);
- MemoryRegion *bar = &s->bar;
int i;
static const int maps[] = {
0x00, 0x00, /* Command B */
@@ -80,16 +78,15 @@ static void macio_escc_legacy_setup(MacIOState *s)
memory_region_add_subregion(escc_legacy, maps[i], port);
}
- memory_region_add_subregion(bar, 0x12000, escc_legacy);
+ memory_region_add_subregion(&s->bar, 0x12000, escc_legacy);
}
static void macio_bar_setup(MacIOState *s)
{
- ESCCState *escc = ESCC(&s->escc);
- SysBusDevice *sbd = SYS_BUS_DEVICE(escc);
- MemoryRegion *bar = &s->bar;
+ SysBusDevice *sbd = SYS_BUS_DEVICE(&s->escc);
+ MemoryRegion *bar = sysbus_mmio_get_region(sbd, 0);
- memory_region_add_subregion(bar, 0x13000, sysbus_mmio_get_region(sbd, 0));
+ memory_region_add_subregion(&s->bar, 0x13000, bar);
macio_escc_legacy_setup(s);
}
--
2.30.6
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/4] hw/misc/macio: Remove some single use local variables
2023-01-18 0:32 ` [PATCH 3/4] hw/misc/macio: Remove some single use local variables BALATON Zoltan
@ 2023-01-18 7:17 ` Philippe Mathieu-Daudé
2023-01-22 18:16 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-18 7:17 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
On 18/1/23 01:32, BALATON Zoltan wrote:
> Drop some local variables that could just be substituted at the single
> place they were used. This makes the code shorter and simpler.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] hw/misc/macio: Remove some single use local variables
2023-01-18 0:32 ` [PATCH 3/4] hw/misc/macio: Remove some single use local variables BALATON Zoltan
2023-01-18 7:17 ` Philippe Mathieu-Daudé
@ 2023-01-22 18:16 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Mark Cave-Ayland @ 2023-01-22 18:16 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On 18/01/2023 00:32, BALATON Zoltan wrote:
> Drop some local variables that could just be substituted at the single
> place they were used. This makes the code shorter and simpler.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 4d7223cc85..ae2a9a960d 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -53,10 +53,8 @@
> */
> static void macio_escc_legacy_setup(MacIOState *s)
> {
> - ESCCState *escc = ESCC(&s->escc);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(escc);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(&s->escc);
> MemoryRegion *escc_legacy = g_new(MemoryRegion, 1);
> - MemoryRegion *bar = &s->bar;
> int i;
> static const int maps[] = {
> 0x00, 0x00, /* Command B */
> @@ -80,16 +78,15 @@ static void macio_escc_legacy_setup(MacIOState *s)
> memory_region_add_subregion(escc_legacy, maps[i], port);
> }
>
> - memory_region_add_subregion(bar, 0x12000, escc_legacy);
> + memory_region_add_subregion(&s->bar, 0x12000, escc_legacy);
> }
>
> static void macio_bar_setup(MacIOState *s)
> {
> - ESCCState *escc = ESCC(&s->escc);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(escc);
> - MemoryRegion *bar = &s->bar;
> + SysBusDevice *sbd = SYS_BUS_DEVICE(&s->escc);
> + MemoryRegion *bar = sysbus_mmio_get_region(sbd, 0);
>
> - memory_region_add_subregion(bar, 0x13000, sysbus_mmio_get_region(sbd, 0));
> + memory_region_add_subregion(&s->bar, 0x13000, bar);
> macio_escc_legacy_setup(s);
> }
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp
2023-01-18 0:32 [PATCH 0/4] Misc macio clean ups BALATON Zoltan
` (2 preceding siblings ...)
2023-01-18 0:32 ` [PATCH 3/4] hw/misc/macio: Remove some single use local variables BALATON Zoltan
@ 2023-01-18 0:32 ` BALATON Zoltan
2023-01-18 7:19 ` Philippe Mathieu-Daudé
2023-02-01 22:44 ` Mark Cave-Ayland
2023-02-01 23:32 ` [PATCH 0/4] Misc macio clean ups Mark Cave-Ayland
4 siblings, 2 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-01-18 0:32 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
Use the convention to return bool from functions which take an error
pointer which allows for callers to pass through their error pointer
without needing a local.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/misc/macio/macio.c | 62 +++++++++++++++++--------------------------
1 file changed, 25 insertions(+), 37 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index ae2a9a960d..265c0bbd8d 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -90,13 +90,13 @@ static void macio_bar_setup(MacIOState *s)
macio_escc_legacy_setup(s);
}
-static void macio_common_realize(PCIDevice *d, Error **errp)
+static bool macio_common_realize(PCIDevice *d, Error **errp)
{
MacIOState *s = MACIO(d);
SysBusDevice *sbd;
if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) {
- return;
+ return false;
}
sbd = SYS_BUS_DEVICE(&s->dbdma);
memory_region_add_subregion(&s->bar, 0x08000,
@@ -108,14 +108,16 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial);
qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial);
if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) {
- return;
+ return false;
}
macio_bar_setup(s);
pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar);
+
+ return true;
}
-static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
+static bool macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
qemu_irq irq0, qemu_irq irq1, int dmaid,
Error **errp)
{
@@ -128,7 +130,7 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
&error_abort);
macio_ide_register_dma(ide);
- qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
+ return qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
}
static void macio_oldworld_realize(PCIDevice *d, Error **errp)
@@ -136,12 +138,9 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
MacIOState *s = MACIO(d);
OldWorldMacIOState *os = OLDWORLD_MACIO(d);
DeviceState *pic_dev = DEVICE(&os->pic);
- Error *err = NULL;
SysBusDevice *sbd;
- macio_common_realize(d, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_common_realize(d, errp)) {
return;
}
@@ -176,21 +175,17 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
pmac_format_nvram_partition(&os->nvram, os->nvram.size);
/* IDE buses */
- macio_realize_ide(s, &os->ide[0],
- qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
- qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_DMA_IRQ),
- 0x16, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_realize_ide(s, &os->ide[0],
+ qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
+ qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_DMA_IRQ),
+ 0x16, errp)) {
return;
}
- macio_realize_ide(s, &os->ide[1],
- qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
- qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_DMA_IRQ),
- 0x1a, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_realize_ide(s, &os->ide[1],
+ qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
+ qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_DMA_IRQ),
+ 0x1a, errp)) {
return;
}
}
@@ -266,13 +261,10 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
MacIOState *s = MACIO(d);
NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
DeviceState *pic_dev = DEVICE(&ns->pic);
- Error *err = NULL;
SysBusDevice *sbd;
MemoryRegion *timer_memory = NULL;
- macio_common_realize(d, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_common_realize(d, errp)) {
return;
}
@@ -288,21 +280,17 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCA_IRQ));
/* IDE buses */
- macio_realize_ide(s, &ns->ide[0],
- qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
- qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
- 0x16, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_realize_ide(s, &ns->ide[0],
+ qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
+ qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
+ 0x16, errp)) {
return;
}
- macio_realize_ide(s, &ns->ide[1],
- qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
- qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
- 0x1a, &err);
- if (err) {
- error_propagate(errp, err);
+ if (!macio_realize_ide(s, &ns->ide[1],
+ qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
+ qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
+ 0x1a, errp)) {
return;
}
--
2.30.6
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp
2023-01-18 0:32 ` [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp BALATON Zoltan
@ 2023-01-18 7:19 ` Philippe Mathieu-Daudé
2023-02-01 22:44 ` Mark Cave-Ayland
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-18 7:19 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
On 18/1/23 01:32, BALATON Zoltan wrote:
> Use the convention to return bool from functions which take an error
> pointer which allows for callers to pass through their error pointer
> without needing a local.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 62 +++++++++++++++++--------------------------
> 1 file changed, 25 insertions(+), 37 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp
2023-01-18 0:32 ` [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp BALATON Zoltan
2023-01-18 7:19 ` Philippe Mathieu-Daudé
@ 2023-02-01 22:44 ` Mark Cave-Ayland
2023-02-01 23:54 ` BALATON Zoltan
1 sibling, 1 reply; 15+ messages in thread
From: Mark Cave-Ayland @ 2023-02-01 22:44 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On 18/01/2023 00:32, BALATON Zoltan wrote:
> Use the convention to return bool from functions which take an error
> pointer which allows for callers to pass through their error pointer
> without needing a local.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/macio.c | 62 +++++++++++++++++--------------------------
> 1 file changed, 25 insertions(+), 37 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index ae2a9a960d..265c0bbd8d 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -90,13 +90,13 @@ static void macio_bar_setup(MacIOState *s)
> macio_escc_legacy_setup(s);
> }
>
> -static void macio_common_realize(PCIDevice *d, Error **errp)
> +static bool macio_common_realize(PCIDevice *d, Error **errp)
> {
> MacIOState *s = MACIO(d);
> SysBusDevice *sbd;
>
> if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) {
> - return;
> + return false;
> }
> sbd = SYS_BUS_DEVICE(&s->dbdma);
> memory_region_add_subregion(&s->bar, 0x08000,
> @@ -108,14 +108,16 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
> qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial);
> qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial);
> if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) {
> - return;
> + return false;
> }
>
> macio_bar_setup(s);
> pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar);
> +
> + return true;
> }
>
> -static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
> +static bool macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
> qemu_irq irq0, qemu_irq irq1, int dmaid,
> Error **errp)
> {
> @@ -128,7 +130,7 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
> &error_abort);
> macio_ide_register_dma(ide);
>
> - qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
> + return qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
> }
>
> static void macio_oldworld_realize(PCIDevice *d, Error **errp)
> @@ -136,12 +138,9 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
> MacIOState *s = MACIO(d);
> OldWorldMacIOState *os = OLDWORLD_MACIO(d);
> DeviceState *pic_dev = DEVICE(&os->pic);
> - Error *err = NULL;
> SysBusDevice *sbd;
>
> - macio_common_realize(d, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_common_realize(d, errp)) {
> return;
> }
>
> @@ -176,21 +175,17 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
> pmac_format_nvram_partition(&os->nvram, os->nvram.size);
>
> /* IDE buses */
> - macio_realize_ide(s, &os->ide[0],
> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_DMA_IRQ),
> - 0x16, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_realize_ide(s, &os->ide[0],
> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_DMA_IRQ),
> + 0x16, errp)) {
> return;
> }
>
> - macio_realize_ide(s, &os->ide[1],
> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_DMA_IRQ),
> - 0x1a, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_realize_ide(s, &os->ide[1],
> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_DMA_IRQ),
> + 0x1a, errp)) {
> return;
> }
> }
> @@ -266,13 +261,10 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
> MacIOState *s = MACIO(d);
> NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
> DeviceState *pic_dev = DEVICE(&ns->pic);
> - Error *err = NULL;
> SysBusDevice *sbd;
> MemoryRegion *timer_memory = NULL;
>
> - macio_common_realize(d, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_common_realize(d, errp)) {
> return;
> }
>
> @@ -288,21 +280,17 @@ static void macio_newworld_realize(PCIDevice *d, Error **errp)
> sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev, NEWWORLD_ESCCA_IRQ));
>
> /* IDE buses */
> - macio_realize_ide(s, &ns->ide[0],
> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
> - 0x16, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_realize_ide(s, &ns->ide[0],
> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
> + 0x16, errp)) {
> return;
> }
>
> - macio_realize_ide(s, &ns->ide[1],
> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
> - 0x1a, &err);
> - if (err) {
> - error_propagate(errp, err);
> + if (!macio_realize_ide(s, &ns->ide[1],
> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
> + 0x1a, errp)) {
> return;
> }
These days you would move macio_common_realize() into TYPE_MACIO, but anyway:
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp
2023-02-01 22:44 ` Mark Cave-Ayland
@ 2023-02-01 23:54 ` BALATON Zoltan
0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-02-01 23:54 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc
On Wed, 1 Feb 2023, Mark Cave-Ayland wrote:
> On 18/01/2023 00:32, BALATON Zoltan wrote:
>> Use the convention to return bool from functions which take an error
>> pointer which allows for callers to pass through their error pointer
>> without needing a local.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/misc/macio/macio.c | 62 +++++++++++++++++--------------------------
>> 1 file changed, 25 insertions(+), 37 deletions(-)
>>
>> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
>> index ae2a9a960d..265c0bbd8d 100644
>> --- a/hw/misc/macio/macio.c
>> +++ b/hw/misc/macio/macio.c
>> @@ -90,13 +90,13 @@ static void macio_bar_setup(MacIOState *s)
>> macio_escc_legacy_setup(s);
>> }
>> -static void macio_common_realize(PCIDevice *d, Error **errp)
>> +static bool macio_common_realize(PCIDevice *d, Error **errp)
>> {
>> MacIOState *s = MACIO(d);
>> SysBusDevice *sbd;
>> if (!qdev_realize(DEVICE(&s->dbdma), BUS(&s->macio_bus), errp)) {
>> - return;
>> + return false;
>> }
>> sbd = SYS_BUS_DEVICE(&s->dbdma);
>> memory_region_add_subregion(&s->bar, 0x08000,
>> @@ -108,14 +108,16 @@ static void macio_common_realize(PCIDevice *d, Error
>> **errp)
>> qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial);
>> qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial);
>> if (!qdev_realize(DEVICE(&s->escc), BUS(&s->macio_bus), errp)) {
>> - return;
>> + return false;
>> }
>> macio_bar_setup(s);
>> pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar);
>> +
>> + return true;
>> }
>> -static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
>> +static bool macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
>> qemu_irq irq0, qemu_irq irq1, int dmaid,
>> Error **errp)
>> {
>> @@ -128,7 +130,7 @@ static void macio_realize_ide(MacIOState *s,
>> MACIOIDEState *ide,
>> &error_abort);
>> macio_ide_register_dma(ide);
>> - qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
>> + return qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
>> }
>> static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>> @@ -136,12 +138,9 @@ static void macio_oldworld_realize(PCIDevice *d, Error
>> **errp)
>> MacIOState *s = MACIO(d);
>> OldWorldMacIOState *os = OLDWORLD_MACIO(d);
>> DeviceState *pic_dev = DEVICE(&os->pic);
>> - Error *err = NULL;
>> SysBusDevice *sbd;
>> - macio_common_realize(d, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_common_realize(d, errp)) {
>> return;
>> }
>> @@ -176,21 +175,17 @@ static void macio_oldworld_realize(PCIDevice *d,
>> Error **errp)
>> pmac_format_nvram_partition(&os->nvram, os->nvram.size);
>> /* IDE buses */
>> - macio_realize_ide(s, &os->ide[0],
>> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
>> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_DMA_IRQ),
>> - 0x16, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_realize_ide(s, &os->ide[0],
>> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
>> + qdev_get_gpio_in(pic_dev,
>> OLDWORLD_IDE0_DMA_IRQ),
>> + 0x16, errp)) {
>> return;
>> }
>> - macio_realize_ide(s, &os->ide[1],
>> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
>> - qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_DMA_IRQ),
>> - 0x1a, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_realize_ide(s, &os->ide[1],
>> + qdev_get_gpio_in(pic_dev, OLDWORLD_IDE1_IRQ),
>> + qdev_get_gpio_in(pic_dev,
>> OLDWORLD_IDE1_DMA_IRQ),
>> + 0x1a, errp)) {
>> return;
>> }
>> }
>> @@ -266,13 +261,10 @@ static void macio_newworld_realize(PCIDevice *d,
>> Error **errp)
>> MacIOState *s = MACIO(d);
>> NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
>> DeviceState *pic_dev = DEVICE(&ns->pic);
>> - Error *err = NULL;
>> SysBusDevice *sbd;
>> MemoryRegion *timer_memory = NULL;
>> - macio_common_realize(d, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_common_realize(d, errp)) {
>> return;
>> }
>> @@ -288,21 +280,17 @@ static void macio_newworld_realize(PCIDevice *d,
>> Error **errp)
>> sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(pic_dev,
>> NEWWORLD_ESCCA_IRQ));
>> /* IDE buses */
>> - macio_realize_ide(s, &ns->ide[0],
>> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
>> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_DMA_IRQ),
>> - 0x16, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_realize_ide(s, &ns->ide[0],
>> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
>> + qdev_get_gpio_in(pic_dev,
>> NEWWORLD_IDE0_DMA_IRQ),
>> + 0x16, errp)) {
>> return;
>> }
>> - macio_realize_ide(s, &ns->ide[1],
>> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
>> - qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_DMA_IRQ),
>> - 0x1a, &err);
>> - if (err) {
>> - error_propagate(errp, err);
>> + if (!macio_realize_ide(s, &ns->ide[1],
>> + qdev_get_gpio_in(pic_dev, NEWWORLD_IDE1_IRQ),
>> + qdev_get_gpio_in(pic_dev,
>> NEWWORLD_IDE1_DMA_IRQ),
>> + 0x1a, errp)) {
>> return;
>> }
>
> These days you would move macio_common_realize() into TYPE_MACIO, but anyway:
Maybe in further patches later as I think we'll need more macio changes in
the future but for now this is enough to simplify it a bit.
Regards,
BALATON Zoltan
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
>
> ATB,
>
> Mark.
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] Misc macio clean ups
2023-01-18 0:32 [PATCH 0/4] Misc macio clean ups BALATON Zoltan
` (3 preceding siblings ...)
2023-01-18 0:32 ` [PATCH 4/4] hw/misc/macio: Return bool from functions taking errp BALATON Zoltan
@ 2023-02-01 23:32 ` Mark Cave-Ayland
4 siblings, 0 replies; 15+ messages in thread
From: Mark Cave-Ayland @ 2023-02-01 23:32 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On 18/01/2023 00:32, BALATON Zoltan wrote:
> Just some small trivial clean ups that I've found while looking at
> hw/misc/macio/macio.c
>
> Regards,
> BALATON Zoltan
>
> BALATON Zoltan (4):
> hw/misc/macio: Avoid some QOM casts
> hw/misc/macio: Rename sysbus_dev to sbd for consistency and brevity
> hw/misc/macio: Remove some single use local variables
> hw/misc/macio: Return bool from functions taking errp
>
> hw/misc/macio/macio.c | 167 ++++++++++++++++++------------------------
> 1 file changed, 72 insertions(+), 95 deletions(-)
Thanks, I've applied these to my qemu-macppc branch.
ATB,
Mark.
^ permalink raw reply [flat|nested] 15+ messages in thread