* [PATCH 0/6] QOM minor fixes
@ 2020-09-20 8:20 Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
This series started off as a fix for the nd_table misuse in the sparc32-ledma
device as pointed out by Markus, and then I remembered there was similar
issue around the use of serial_hd() in macio. The last patch is one I've had
sitting in a local branch for a while and is a mistake I made during the
original sabre.c split which seems appropriate to include here.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Mark Cave-Ayland (6):
sparc32-dma: use object_initialize_child() for espdma and ledma child
objects
sparc32-ledma: use object_initialize_child() for lance child object
sparc32-espdma: use object_initialize_child() for esp child object
sparc32-ledma: don't reference nd_table directly within the device
macio: don't reference serial_hd() directly within the device
sabre: don't call sysbus_mmio_map() in sabre_realize()
hw/dma/sparc32_dma.c | 49 +++++++++++++++++-----------------
hw/misc/macio/macio.c | 5 +---
hw/pci-host/sabre.c | 8 ------
hw/ppc/mac_newworld.c | 6 +++++
hw/ppc/mac_oldworld.c | 6 +++++
hw/sparc/sun4m.c | 21 +++++++++------
hw/sparc64/sun4u.c | 7 +++++
include/hw/sparc/sparc32_dma.h | 8 +++---
8 files changed, 61 insertions(+), 49 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
Store the child objects directly within the sparc32-dma object rather than using
link properties.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/dma/sparc32_dma.c | 15 +++++++++------
include/hw/sparc/sparc32_dma.h | 4 ++--
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index d20a5bc065..b25a212f7a 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -379,10 +379,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
return;
}
- espdma = qdev_new(TYPE_SPARC32_ESPDMA_DEVICE);
+ espdma = DEVICE(&s->espdma);
object_property_set_link(OBJECT(espdma), "iommu", iommu, &error_abort);
- object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma));
- sysbus_realize_and_unref(SYS_BUS_DEVICE(espdma), &error_fatal);
+ sysbus_realize(SYS_BUS_DEVICE(espdma), &error_fatal);
esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
sbd = SYS_BUS_DEVICE(esp);
@@ -394,10 +393,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(&s->dmamem, 0x0,
sysbus_mmio_get_region(sbd, 0));
- ledma = qdev_new(TYPE_SPARC32_LEDMA_DEVICE);
+ ledma = DEVICE(&s->ledma);
object_property_set_link(OBJECT(ledma), "iommu", iommu, &error_abort);
- object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma));
- sysbus_realize_and_unref(SYS_BUS_DEVICE(ledma), &error_fatal);
+ sysbus_realize(SYS_BUS_DEVICE(ledma), &error_fatal);
lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
sbd = SYS_BUS_DEVICE(lance);
@@ -421,6 +419,11 @@ static void sparc32_dma_init(Object *obj)
memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
sysbus_init_mmio(sbd, &s->dmamem);
+
+ object_initialize_child(obj, "espdma", &s->espdma,
+ TYPE_SPARC32_ESPDMA_DEVICE);
+ object_initialize_child(obj, "ledma", &s->ledma,
+ TYPE_SPARC32_LEDMA_DEVICE);
}
static void sparc32_dma_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index a402665a9c..a7b1dd8418 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -56,8 +56,8 @@ struct SPARC32DMAState {
MemoryRegion dmamem;
MemoryRegion ledma_alias;
- ESPDMADeviceState *espdma;
- LEDMADeviceState *ledma;
+ ESPDMADeviceState espdma;
+ LEDMADeviceState ledma;
};
/* sparc32_dma.c */
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
Store the child object directly within the sparc32-ledma object rather than
using link properties.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/dma/sparc32_dma.c | 14 ++++++++------
include/hw/sparc/sparc32_dma.h | 2 +-
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index b25a212f7a..84196afb95 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -331,24 +331,26 @@ static const TypeInfo sparc32_espdma_device_info = {
static void sparc32_ledma_device_init(Object *obj)
{
DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+ LEDMADeviceState *ls = SPARC32_LEDMA_DEVICE(obj);
memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
"ledma-mmio", DMA_SIZE);
+
+ object_initialize_child(obj, "lance", &ls->lance, TYPE_LANCE);
}
static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
{
- DeviceState *d;
+ LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
+ SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
NICInfo *nd = &nd_table[0];
/* FIXME use qdev NIC properties instead of nd_table[] */
qemu_check_nic_model(nd, TYPE_LANCE);
- d = qdev_new(TYPE_LANCE);
- object_property_add_child(OBJECT(dev), "lance", OBJECT(d));
- qdev_set_nic_properties(d, nd);
- object_property_set_link(OBJECT(d), "dma", OBJECT(dev), &error_abort);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
+ qdev_set_nic_properties(DEVICE(lance), nd);
+ object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
+ sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
}
static void sparc32_ledma_device_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index a7b1dd8418..f2bfe272ba 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -43,7 +43,7 @@ DECLARE_INSTANCE_CHECKER(LEDMADeviceState, SPARC32_LEDMA_DEVICE,
struct LEDMADeviceState {
DMADeviceState parent_obj;
- SysBusPCNetState *lance;
+ SysBusPCNetState lance;
};
#define TYPE_SPARC32_DMA "sparc32-dma"
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp child object
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
Store the child object directly within the sparc32-espdma object rather than
using link properties.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/dma/sparc32_dma.c | 17 ++++++++---------
include/hw/sparc/sparc32_dma.h | 2 +-
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 84196afb95..2cbe331959 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -290,27 +290,26 @@ static const TypeInfo sparc32_dma_device_info = {
static void sparc32_espdma_device_init(Object *obj)
{
DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
+ ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(obj);
memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
"espdma-mmio", DMA_SIZE);
+
+ object_initialize_child(obj, "esp", &es->esp, TYPE_ESP);
}
static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
{
- DeviceState *d;
- SysBusESPState *sysbus;
- ESPState *esp;
-
- d = qdev_new(TYPE_ESP);
- object_property_add_child(OBJECT(dev), "esp", OBJECT(d));
- sysbus = ESP(d);
- esp = &sysbus->esp;
+ ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(dev);
+ SysBusESPState *sysbus = ESP(&es->esp);
+ ESPState *esp = &sysbus->esp;
+
esp->dma_memory_read = espdma_memory_read;
esp->dma_memory_write = espdma_memory_write;
esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
sysbus->it_shift = 2;
esp->dma_enabled = 1;
- sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
+ sysbus_realize(SYS_BUS_DEVICE(sysbus), &error_fatal);
}
static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index f2bfe272ba..d40eca0cc8 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -32,7 +32,7 @@ DECLARE_INSTANCE_CHECKER(ESPDMADeviceState, SPARC32_ESPDMA_DEVICE,
struct ESPDMADeviceState {
DMADeviceState parent_obj;
- SysBusESPState *esp;
+ SysBusESPState esp;
};
#define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
` (2 preceding siblings ...)
2020-09-20 8:20 ` [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-21 9:25 ` Markus Armbruster
2020-09-21 9:57 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 5/6] macio: don't reference serial_hd() " Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
5 siblings, 2 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
Instead use qdev_set_nic_properties() to configure the on-board NIC at the
sun4m machine level.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/dma/sparc32_dma.c | 5 -----
hw/sparc/sun4m.c | 21 +++++++++++++--------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 2cbe331959..b643b413c5 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
{
LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
- NICInfo *nd = &nd_table[0];
- /* FIXME use qdev NIC properties instead of nd_table[] */
- qemu_check_nic_model(nd, TYPE_LANCE);
-
- qdev_set_nic_properties(DEVICE(lance), nd);
object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
}
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 947b69d159..ed5f3ccd9f 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
static void *sparc32_dma_init(hwaddr dma_base,
hwaddr esp_base, qemu_irq espdma_irq,
- hwaddr le_base, qemu_irq ledma_irq)
+ hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
{
DeviceState *dma;
ESPDMADeviceState *espdma;
@@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
SysBusPCNetState *lance;
dma = qdev_new(TYPE_SPARC32_DMA);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
- sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
-
espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
OBJECT(dma), "espdma"));
sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
- sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
- scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
OBJECT(dma), "ledma"));
@@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
lance = SYSBUS_PCNET(object_resolve_path_component(
OBJECT(ledma), "lance"));
+ qdev_set_nic_properties(DEVICE(lance), nd);
+
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
+ sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
+
+ sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
+ scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
+
sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
return dma;
@@ -854,6 +857,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
unsigned int max_cpus = machine->smp.max_cpus;
Object *ram_memdev = object_resolve_path_type(machine->ram_memdev_id,
TYPE_MEMORY_BACKEND, NULL);
+ NICInfo *nd = &nd_table[0];
if (machine->ram_size > hwdef->max_mem) {
error_report("Too much memory for this machine: %" PRId64 ","
@@ -914,9 +918,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
hwdef->iommu_pad_base, hwdef->iommu_pad_len);
}
+ qemu_check_nic_model(nd, TYPE_LANCE);
sparc32_dma_init(hwdef->dma_base,
hwdef->esp_base, slavio_irq[18],
- hwdef->le_base, slavio_irq[16]);
+ hwdef->le_base, slavio_irq[16], nd);
if (graphic_depth != 8 && graphic_depth != 24) {
error_report("Unsupported depth: %d", graphic_depth);
@@ -1047,7 +1052,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
machine->initrd_filename,
machine->ram_size, &initrd_size);
- nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, machine->kernel_cmdline,
+ nvram_init(nvram, (uint8_t *)&nd->macaddr, machine->kernel_cmdline,
machine->boot_order, machine->ram_size, kernel_size,
graphic_width, graphic_height, graphic_depth,
hwdef->nvram_machine_id, "Sun4m");
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/6] macio: don't reference serial_hd() directly within the device
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
` (3 preceding siblings ...)
2020-09-20 8:20 ` [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-20 10:52 ` BALATON Zoltan via
2020-09-20 8:20 ` [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
5 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
Mac Old World and New World machine level.
Also remove the now obsolete comment referring to the use of serial_hd() and
change user_createable to true accordingly.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/misc/macio/macio.c | 5 +----
hw/ppc/mac_newworld.c | 6 ++++++
hw/ppc/mac_oldworld.c | 6 ++++++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 679722628e..ce641d41fd 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4);
- qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hd(0));
- qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1));
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)) {
@@ -458,8 +456,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_OTHERS << 8;
device_class_set_props(dc, macio_properties);
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
- /* Reason: Uses serial_hds in macio_instance_init */
- dc->user_creatable = false;
+ dc->user_creatable = true;
}
static const TypeInfo macio_bus_info = {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index e42bd7a626..e59b30e0a7 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -123,6 +123,7 @@ static void ppc_core99_init(MachineState *machine)
UNINHostState *uninorth_pci;
PCIBus *pci_bus;
PCIDevice *macio;
+ ESCCState *escc;
bool has_pmu, has_adb;
MACIOIDEState *macio_ide;
BusState *adb_bus;
@@ -382,6 +383,11 @@ static void ppc_core99_init(MachineState *machine)
qdev_prop_set_bit(dev, "has-adb", has_adb);
object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
&error_abort);
+
+ escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+ qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+ qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
pci_realize_and_unref(macio, pci_bus, &error_fatal);
/* We only emulate 2 out of 3 IDE controllers for now */
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 7aba040f1b..25ade63ba3 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -96,6 +96,7 @@ static void ppc_heathrow_init(MachineState *machine)
PCIBus *pci_bus;
PCIDevice *macio;
MACIOIDEState *macio_ide;
+ ESCCState *escc;
SysBusDevice *s;
DeviceState *dev, *pic_dev;
BusState *adb_bus;
@@ -283,6 +284,11 @@ static void ppc_heathrow_init(MachineState *machine)
qdev_prop_set_uint64(dev, "frequency", tbfreq);
object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
&error_abort);
+
+ escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+ qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+ qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
pci_realize_and_unref(macio, pci_bus, &error_fatal);
macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize()
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
` (4 preceding siblings ...)
2020-09-20 8:20 ` [PATCH 5/6] macio: don't reference serial_hd() " Mark Cave-Ayland
@ 2020-09-20 8:20 ` Mark Cave-Ayland
2020-09-20 10:48 ` Philippe Mathieu-Daudé
5 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 8:20 UTC (permalink / raw)
To: armbru, david, atar4qemu, qemu-devel, qemu-ppc
The device should not map itself but instead should be mapped to sysbus by the
sun4u machine.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/pci-host/sabre.c | 8 --------
hw/sparc64/sun4u.c | 7 +++++++
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index 5ac6283623..5394ad5cd0 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -378,16 +378,8 @@ static void sabre_realize(DeviceState *dev, Error **errp)
{
SabreState *s = SABRE(dev);
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
- SysBusDevice *sbd = SYS_BUS_DEVICE(s);
PCIDevice *pci_dev;
- /* sabre_config */
- sysbus_mmio_map(sbd, 0, s->special_base);
- /* PCI configuration space */
- sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
- /* pci_ioport */
- sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
-
memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
memory_region_add_subregion(get_system_memory(), s->mem_base,
&s->pci_mmio);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index b4aabfc076..f80ddde5dc 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -592,6 +592,13 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
&error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
+ /* sabre_config */
+ sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 0, PBM_SPECIAL_BASE);
+ /* PCI configuration space */
+ sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 1, PBM_SPECIAL_BASE + 0x1000000ULL);
+ /* pci_ioport */
+ sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 2, PBM_SPECIAL_BASE + 0x2000000ULL);
+
/* Wire up PCI interrupts to CPU */
for (i = 0; i < IVEC_MAX; i++) {
qdev_connect_gpio_out_named(DEVICE(sabre), "ivec-irq", i,
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize()
2020-09-20 8:20 ` [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
@ 2020-09-20 10:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-20 10:48 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
> The device should not map itself but instead should be mapped to sysbus by the
> sun4u machine.
Yeah good cleanup.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/pci-host/sabre.c | 8 --------
> hw/sparc64/sun4u.c | 7 +++++++
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
> index 5ac6283623..5394ad5cd0 100644
> --- a/hw/pci-host/sabre.c
> +++ b/hw/pci-host/sabre.c
> @@ -378,16 +378,8 @@ static void sabre_realize(DeviceState *dev, Error **errp)
> {
> SabreState *s = SABRE(dev);
> PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> - SysBusDevice *sbd = SYS_BUS_DEVICE(s);
> PCIDevice *pci_dev;
>
> - /* sabre_config */
> - sysbus_mmio_map(sbd, 0, s->special_base);
> - /* PCI configuration space */
> - sysbus_mmio_map(sbd, 1, s->special_base + 0x1000000ULL);
> - /* pci_ioport */
> - sysbus_mmio_map(sbd, 2, s->special_base + 0x2000000ULL);
> -
> memory_region_init(&s->pci_mmio, OBJECT(s), "pci-mmio", 0x100000000ULL);
> memory_region_add_subregion(get_system_memory(), s->mem_base,
> &s->pci_mmio);
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index b4aabfc076..f80ddde5dc 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -592,6 +592,13 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> &error_abort);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(sabre), &error_fatal);
>
> + /* sabre_config */
> + sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 0, PBM_SPECIAL_BASE);
> + /* PCI configuration space */
> + sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 1, PBM_SPECIAL_BASE + 0x1000000ULL);
> + /* pci_ioport */
> + sysbus_mmio_map(SYS_BUS_DEVICE(sabre), 2, PBM_SPECIAL_BASE + 0x2000000ULL);
> +
> /* Wire up PCI interrupts to CPU */
> for (i = 0; i < IVEC_MAX; i++) {
> qdev_connect_gpio_out_named(DEVICE(sabre), "ivec-irq", i,
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects
2020-09-20 8:20 ` [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
@ 2020-09-20 10:49 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-20 10:49 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
> Store the child objects directly within the sparc32-dma object rather than using
> link properties.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/dma/sparc32_dma.c | 15 +++++++++------
> include/hw/sparc/sparc32_dma.h | 4 ++--
Consider using scripts/git.orderfile to avoid reviewer scrolling :)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index d20a5bc065..b25a212f7a 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -379,10 +379,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - espdma = qdev_new(TYPE_SPARC32_ESPDMA_DEVICE);
> + espdma = DEVICE(&s->espdma);
> object_property_set_link(OBJECT(espdma), "iommu", iommu, &error_abort);
> - object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma));
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(espdma), &error_fatal);
> + sysbus_realize(SYS_BUS_DEVICE(espdma), &error_fatal);
>
> esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
> sbd = SYS_BUS_DEVICE(esp);
> @@ -394,10 +393,9 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
> memory_region_add_subregion(&s->dmamem, 0x0,
> sysbus_mmio_get_region(sbd, 0));
>
> - ledma = qdev_new(TYPE_SPARC32_LEDMA_DEVICE);
> + ledma = DEVICE(&s->ledma);
> object_property_set_link(OBJECT(ledma), "iommu", iommu, &error_abort);
> - object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma));
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(ledma), &error_fatal);
> + sysbus_realize(SYS_BUS_DEVICE(ledma), &error_fatal);
>
> lance = DEVICE(object_resolve_path_component(OBJECT(ledma), "lance"));
> sbd = SYS_BUS_DEVICE(lance);
> @@ -421,6 +419,11 @@ static void sparc32_dma_init(Object *obj)
>
> memory_region_init(&s->dmamem, OBJECT(s), "dma", DMA_SIZE + DMA_ETH_SIZE);
> sysbus_init_mmio(sbd, &s->dmamem);
> +
> + object_initialize_child(obj, "espdma", &s->espdma,
> + TYPE_SPARC32_ESPDMA_DEVICE);
> + object_initialize_child(obj, "ledma", &s->ledma,
> + TYPE_SPARC32_LEDMA_DEVICE);
> }
>
> static void sparc32_dma_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index a402665a9c..a7b1dd8418 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -56,8 +56,8 @@ struct SPARC32DMAState {
>
> MemoryRegion dmamem;
> MemoryRegion ledma_alias;
> - ESPDMADeviceState *espdma;
> - LEDMADeviceState *ledma;
> + ESPDMADeviceState espdma;
> + LEDMADeviceState ledma;
> };
>
> /* sparc32_dma.c */
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object
2020-09-20 8:20 ` [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
@ 2020-09-20 10:49 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-20 10:49 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
> Store the child object directly within the sparc32-ledma object rather than
> using link properties.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/dma/sparc32_dma.c | 14 ++++++++------
> include/hw/sparc/sparc32_dma.h | 2 +-
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index b25a212f7a..84196afb95 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -331,24 +331,26 @@ static const TypeInfo sparc32_espdma_device_info = {
> static void sparc32_ledma_device_init(Object *obj)
> {
> DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
> + LEDMADeviceState *ls = SPARC32_LEDMA_DEVICE(obj);
>
> memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
> "ledma-mmio", DMA_SIZE);
> +
> + object_initialize_child(obj, "lance", &ls->lance, TYPE_LANCE);
> }
>
> static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
> {
> - DeviceState *d;
> + LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
> + SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
> NICInfo *nd = &nd_table[0];
>
> /* FIXME use qdev NIC properties instead of nd_table[] */
> qemu_check_nic_model(nd, TYPE_LANCE);
>
> - d = qdev_new(TYPE_LANCE);
> - object_property_add_child(OBJECT(dev), "lance", OBJECT(d));
> - qdev_set_nic_properties(d, nd);
> - object_property_set_link(OBJECT(d), "dma", OBJECT(dev), &error_abort);
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
> + qdev_set_nic_properties(DEVICE(lance), nd);
> + object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
> + sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
> }
>
> static void sparc32_ledma_device_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index a7b1dd8418..f2bfe272ba 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -43,7 +43,7 @@ DECLARE_INSTANCE_CHECKER(LEDMADeviceState, SPARC32_LEDMA_DEVICE,
> struct LEDMADeviceState {
> DMADeviceState parent_obj;
>
> - SysBusPCNetState *lance;
> + SysBusPCNetState lance;
> };
>
> #define TYPE_SPARC32_DMA "sparc32-dma"
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp child object
2020-09-20 8:20 ` [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
@ 2020-09-20 10:49 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-20 10:49 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
> Store the child object directly within the sparc32-espdma object rather than
> using link properties.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/dma/sparc32_dma.c | 17 ++++++++---------
> include/hw/sparc/sparc32_dma.h | 2 +-
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 84196afb95..2cbe331959 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -290,27 +290,26 @@ static const TypeInfo sparc32_dma_device_info = {
> static void sparc32_espdma_device_init(Object *obj)
> {
> DMADeviceState *s = SPARC32_DMA_DEVICE(obj);
> + ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(obj);
>
> memory_region_init_io(&s->iomem, OBJECT(s), &dma_mem_ops, s,
> "espdma-mmio", DMA_SIZE);
> +
> + object_initialize_child(obj, "esp", &es->esp, TYPE_ESP);
> }
>
> static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
> {
> - DeviceState *d;
> - SysBusESPState *sysbus;
> - ESPState *esp;
> -
> - d = qdev_new(TYPE_ESP);
> - object_property_add_child(OBJECT(dev), "esp", OBJECT(d));
> - sysbus = ESP(d);
> - esp = &sysbus->esp;
> + ESPDMADeviceState *es = SPARC32_ESPDMA_DEVICE(dev);
> + SysBusESPState *sysbus = ESP(&es->esp);
> + ESPState *esp = &sysbus->esp;
> +
> esp->dma_memory_read = espdma_memory_read;
> esp->dma_memory_write = espdma_memory_write;
> esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
> sysbus->it_shift = 2;
> esp->dma_enabled = 1;
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
> + sysbus_realize(SYS_BUS_DEVICE(sysbus), &error_fatal);
> }
>
> static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index f2bfe272ba..d40eca0cc8 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -32,7 +32,7 @@ DECLARE_INSTANCE_CHECKER(ESPDMADeviceState, SPARC32_ESPDMA_DEVICE,
> struct ESPDMADeviceState {
> DMADeviceState parent_obj;
>
> - SysBusESPState *esp;
> + SysBusESPState esp;
> };
>
> #define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device
2020-09-20 8:20 ` [PATCH 5/6] macio: don't reference serial_hd() " Mark Cave-Ayland
@ 2020-09-20 10:52 ` BALATON Zoltan via
2020-09-20 17:59 ` Mark Cave-Ayland
0 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan via @ 2020-09-20 10:52 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: qemu-devel, qemu-ppc, armbru, atar4qemu, david
On Sun, 20 Sep 2020, Mark Cave-Ayland wrote:
> Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
> Mac Old World and New World machine level.
>
> Also remove the now obsolete comment referring to the use of serial_hd() and
> change user_createable to true accordingly.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/misc/macio/macio.c | 5 +----
> hw/ppc/mac_newworld.c | 6 ++++++
> hw/ppc/mac_oldworld.c | 6 ++++++
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 679722628e..ce641d41fd 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
> qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
> qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
> qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4);
> - qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hd(0));
> - qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1));
> 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)) {
> @@ -458,8 +456,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
> k->class_id = PCI_CLASS_OTHERS << 8;
> device_class_set_props(dc, macio_properties);
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> - /* Reason: Uses serial_hds in macio_instance_init */
> - dc->user_creatable = false;
> + dc->user_creatable = true;
According to a comment in
https://git.qemu.org/?p=qemu.git;a=blob;f=hw/core/qdev.c;h=96772a15bd5b76d3ebe27d45ed1f2c1beb7f5386;hb=HEAD#l1135
user_creatable = true is the default and most devices don't set it
explicitely so I think you can just remove the line setting it here.
Regards,
BALATON Zoltan
> }
>
> static const TypeInfo macio_bus_info = {
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index e42bd7a626..e59b30e0a7 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -123,6 +123,7 @@ static void ppc_core99_init(MachineState *machine)
> UNINHostState *uninorth_pci;
> PCIBus *pci_bus;
> PCIDevice *macio;
> + ESCCState *escc;
> bool has_pmu, has_adb;
> MACIOIDEState *macio_ide;
> BusState *adb_bus;
> @@ -382,6 +383,11 @@ static void ppc_core99_init(MachineState *machine)
> qdev_prop_set_bit(dev, "has-adb", has_adb);
> object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
> &error_abort);
> +
> + escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> + qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> + qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
> pci_realize_and_unref(macio, pci_bus, &error_fatal);
>
> /* We only emulate 2 out of 3 IDE controllers for now */
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 7aba040f1b..25ade63ba3 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -96,6 +96,7 @@ static void ppc_heathrow_init(MachineState *machine)
> PCIBus *pci_bus;
> PCIDevice *macio;
> MACIOIDEState *macio_ide;
> + ESCCState *escc;
> SysBusDevice *s;
> DeviceState *dev, *pic_dev;
> BusState *adb_bus;
> @@ -283,6 +284,11 @@ static void ppc_heathrow_init(MachineState *machine)
> qdev_prop_set_uint64(dev, "frequency", tbfreq);
> object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
> &error_abort);
> +
> + escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> + qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> + qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
> pci_realize_and_unref(macio, pci_bus, &error_fatal);
>
> macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device
2020-09-20 10:52 ` BALATON Zoltan via
@ 2020-09-20 17:59 ` Mark Cave-Ayland
0 siblings, 0 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-20 17:59 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: david, qemu-ppc, qemu-devel, atar4qemu, armbru
On 20/09/2020 11:52, BALATON Zoltan via wrote:
> On Sun, 20 Sep 2020, Mark Cave-Ayland wrote:
>> Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
>> Mac Old World and New World machine level.
>>
>> Also remove the now obsolete comment referring to the use of serial_hd() and
>> change user_createable to true accordingly.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/misc/macio/macio.c | 5 +----
>> hw/ppc/mac_newworld.c | 6 ++++++
>> hw/ppc/mac_oldworld.c | 6 ++++++
>> 3 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
>> index 679722628e..ce641d41fd 100644
>> --- a/hw/misc/macio/macio.c
>> +++ b/hw/misc/macio/macio.c
>> @@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
>> qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0);
>> qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK);
>> qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4);
>> - qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hd(0));
>> - qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hd(1));
>> 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)) {
>> @@ -458,8 +456,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
>> k->class_id = PCI_CLASS_OTHERS << 8;
>> device_class_set_props(dc, macio_properties);
>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>> - /* Reason: Uses serial_hds in macio_instance_init */
>> - dc->user_creatable = false;
>> + dc->user_creatable = true;
>
> According to a comment in
>
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/core/qdev.c;h=96772a15bd5b76d3ebe27d45ed1f2c1beb7f5386;hb=HEAD#l1135
>
> user_creatable = true is the default and most devices don't set it explicitely so I
> think you can just remove the line setting it here.
>
> Regards,
> BALATON Zoltan
Ah yes indeed, thanks for the reference. I'll see if anyone else has any further
comments on the series before posting an updated v2 with this line removed.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-20 8:20 ` [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
@ 2020-09-21 9:25 ` Markus Armbruster
2020-09-21 17:03 ` Mark Cave-Ayland
2020-09-21 9:57 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 21+ messages in thread
From: Markus Armbruster @ 2020-09-21 9:25 UTC (permalink / raw)
To: Mark Cave-Ayland; +Cc: qemu-ppc, qemu-devel, atar4qemu, david
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
> sun4m machine level.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/dma/sparc32_dma.c | 5 -----
> hw/sparc/sun4m.c | 21 +++++++++++++--------
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 2cbe331959..b643b413c5 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
> {
> LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
> SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
> - NICInfo *nd = &nd_table[0];
>
> - /* FIXME use qdev NIC properties instead of nd_table[] */
> - qemu_check_nic_model(nd, TYPE_LANCE);
> -
> - qdev_set_nic_properties(DEVICE(lance), nd);
> object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
> sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
> }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 947b69d159..ed5f3ccd9f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>
> static void *sparc32_dma_init(hwaddr dma_base,
> hwaddr esp_base, qemu_irq espdma_irq,
> - hwaddr le_base, qemu_irq ledma_irq)
> + hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
> {
> DeviceState *dma;
> ESPDMADeviceState *espdma;
> @@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
> SysBusPCNetState *lance;
>
> dma = qdev_new(TYPE_SPARC32_DMA);
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> - sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> -
> espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
> OBJECT(dma), "espdma"));
> sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>
> esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
> - sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> - scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
>
> ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
> OBJECT(dma), "ledma"));
> @@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
>
> lance = SYSBUS_PCNET(object_resolve_path_component(
> OBJECT(ledma), "lance"));
> + qdev_set_nic_properties(DEVICE(lance), nd);
> +
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> + scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
> +
> sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>
> return dma;
You delay a bit of work on devices @dma and @esp. Can you explain why?
> @@ -854,6 +857,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> unsigned int max_cpus = machine->smp.max_cpus;
> Object *ram_memdev = object_resolve_path_type(machine->ram_memdev_id,
> TYPE_MEMORY_BACKEND, NULL);
> + NICInfo *nd = &nd_table[0];
>
> if (machine->ram_size > hwdef->max_mem) {
> error_report("Too much memory for this machine: %" PRId64 ","
> @@ -914,9 +918,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> hwdef->iommu_pad_base, hwdef->iommu_pad_len);
> }
>
> + qemu_check_nic_model(nd, TYPE_LANCE);
> sparc32_dma_init(hwdef->dma_base,
> hwdef->esp_base, slavio_irq[18],
> - hwdef->le_base, slavio_irq[16]);
> + hwdef->le_base, slavio_irq[16], nd);
>
> if (graphic_depth != 8 && graphic_depth != 24) {
> error_report("Unsupported depth: %d", graphic_depth);
> @@ -1047,7 +1052,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> machine->initrd_filename,
> machine->ram_size, &initrd_size);
>
> - nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, machine->kernel_cmdline,
> + nvram_init(nvram, (uint8_t *)&nd->macaddr, machine->kernel_cmdline,
> machine->boot_order, machine->ram_size, kernel_size,
> graphic_width, graphic_height, graphic_depth,
> hwdef->nvram_machine_id, "Sun4m");
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-20 8:20 ` [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
2020-09-21 9:25 ` Markus Armbruster
@ 2020-09-21 9:57 ` Philippe Mathieu-Daudé
2020-09-21 17:08 ` Mark Cave-Ayland
1 sibling, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 9:57 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
Hi Mark,
On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
> sun4m machine level.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/dma/sparc32_dma.c | 5 -----
> hw/sparc/sun4m.c | 21 +++++++++++++--------
> 2 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 2cbe331959..b643b413c5 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
> {
> LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
> SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
> - NICInfo *nd = &nd_table[0];
>
> - /* FIXME use qdev NIC properties instead of nd_table[] */
> - qemu_check_nic_model(nd, TYPE_LANCE);
> -
> - qdev_set_nic_properties(DEVICE(lance), nd);
> object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
> sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
> }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 947b69d159..ed5f3ccd9f 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>
> static void *sparc32_dma_init(hwaddr dma_base,
> hwaddr esp_base, qemu_irq espdma_irq,
> - hwaddr le_base, qemu_irq ledma_irq)
> + hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
Instead of passing NICInfo to sparc32_dma_init,
shouldn't you extract the lance code from it?
> {
> DeviceState *dma;
> ESPDMADeviceState *espdma;
> @@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
> SysBusPCNetState *lance;
>
> dma = qdev_new(TYPE_SPARC32_DMA);
> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> - sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> -
> espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
> OBJECT(dma), "espdma"));
> sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>
> esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
> - sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> - scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
>
> ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
> OBJECT(dma), "ledma"));
> @@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
>
> lance = SYSBUS_PCNET(object_resolve_path_component(
> OBJECT(ledma), "lance"));
> + qdev_set_nic_properties(DEVICE(lance), nd);
> +
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
> + sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
> +
> + sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
> + scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
> +
> sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>
> return dma;
> @@ -854,6 +857,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> unsigned int max_cpus = machine->smp.max_cpus;
> Object *ram_memdev = object_resolve_path_type(machine->ram_memdev_id,
> TYPE_MEMORY_BACKEND, NULL);
> + NICInfo *nd = &nd_table[0];
>
> if (machine->ram_size > hwdef->max_mem) {
> error_report("Too much memory for this machine: %" PRId64 ","
> @@ -914,9 +918,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> hwdef->iommu_pad_base, hwdef->iommu_pad_len);
> }
>
> + qemu_check_nic_model(nd, TYPE_LANCE);
> sparc32_dma_init(hwdef->dma_base,
> hwdef->esp_base, slavio_irq[18],
> - hwdef->le_base, slavio_irq[16]);
> + hwdef->le_base, slavio_irq[16], nd);
>
> if (graphic_depth != 8 && graphic_depth != 24) {
> error_report("Unsupported depth: %d", graphic_depth);
> @@ -1047,7 +1052,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> machine->initrd_filename,
> machine->ram_size, &initrd_size);
>
> - nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, machine->kernel_cmdline,
> + nvram_init(nvram, (uint8_t *)&nd->macaddr, machine->kernel_cmdline,
> machine->boot_order, machine->ram_size, kernel_size,
> graphic_width, graphic_height, graphic_depth,
> hwdef->nvram_machine_id, "Sun4m");
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 9:25 ` Markus Armbruster
@ 2020-09-21 17:03 ` Mark Cave-Ayland
2020-09-21 17:14 ` Mark Cave-Ayland
0 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-21 17:03 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-ppc, qemu-devel, atar4qemu, david
On 21/09/2020 10:25, Markus Armbruster wrote:
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>
>> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
>> sun4m machine level.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/dma/sparc32_dma.c | 5 -----
>> hw/sparc/sun4m.c | 21 +++++++++++++--------
>> 2 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>> index 2cbe331959..b643b413c5 100644
>> --- a/hw/dma/sparc32_dma.c
>> +++ b/hw/dma/sparc32_dma.c
>> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
>> {
>> LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
>> SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
>> - NICInfo *nd = &nd_table[0];
>>
>> - /* FIXME use qdev NIC properties instead of nd_table[] */
>> - qemu_check_nic_model(nd, TYPE_LANCE);
>> -
>> - qdev_set_nic_properties(DEVICE(lance), nd);
>> object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
>> sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
>> }
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 947b69d159..ed5f3ccd9f 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>>
>> static void *sparc32_dma_init(hwaddr dma_base,
>> hwaddr esp_base, qemu_irq espdma_irq,
>> - hwaddr le_base, qemu_irq ledma_irq)
>> + hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
>> {
>> DeviceState *dma;
>> ESPDMADeviceState *espdma;
>> @@ -328,16 +328,11 @@ static void *sparc32_dma_init(hwaddr dma_base,
>> SysBusPCNetState *lance;
>>
>> dma = qdev_new(TYPE_SPARC32_DMA);
>> - sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
>> - sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
>> -
>> espdma = SPARC32_ESPDMA_DEVICE(object_resolve_path_component(
>> OBJECT(dma), "espdma"));
>> sysbus_connect_irq(SYS_BUS_DEVICE(espdma), 0, espdma_irq);
>>
>> esp = ESP(object_resolve_path_component(OBJECT(espdma), "esp"));
>> - sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
>> - scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
>>
>> ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
>> OBJECT(dma), "ledma"));
>> @@ -345,6 +340,14 @@ static void *sparc32_dma_init(hwaddr dma_base,
>>
>> lance = SYSBUS_PCNET(object_resolve_path_component(
>> OBJECT(ledma), "lance"));
>> + qdev_set_nic_properties(DEVICE(lance), nd);
>> +
>> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal);
>> + sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, dma_base);
>> +
>> + sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
>> + scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
>> +
>> sysbus_mmio_map(SYS_BUS_DEVICE(lance), 0, le_base);
>>
>> return dma;
>
> You delay a bit of work on devices @dma and @esp. Can you explain why?
This is where it starts to get a little hazy: the sysbus_mmio_map() for the dma
device should be fine, since that's the container device for ledma and espdma devices
which is realised the line above.
The call to scsi_bus_legacy_handle_cmdline() is interesting - AFAICT if the esp
device within the dma device hasn't been realised then I get a crash, and that's why
I moved the legacy command line handling to after realise.
The lance and esp devices are embedded within ledma and espdma devices respectively,
but are actually sysbus devices because they can be used by other machines. I'm not
sure if lance is used anywhere else, but esp certainly is. Hence they are mapped
after the dma device is realised as it feels odd to attach devices to sysbus outside
of a machine init function.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 9:57 ` Philippe Mathieu-Daudé
@ 2020-09-21 17:08 ` Mark Cave-Ayland
2020-09-21 17:58 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-21 17:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 21/09/2020 10:57, Philippe Mathieu-Daudé wrote:
> Hi Mark,
>
> On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
>> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
>> sun4m machine level.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/dma/sparc32_dma.c | 5 -----
>> hw/sparc/sun4m.c | 21 +++++++++++++--------
>> 2 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>> index 2cbe331959..b643b413c5 100644
>> --- a/hw/dma/sparc32_dma.c
>> +++ b/hw/dma/sparc32_dma.c
>> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
>> {
>> LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
>> SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
>> - NICInfo *nd = &nd_table[0];
>>
>> - /* FIXME use qdev NIC properties instead of nd_table[] */
>> - qemu_check_nic_model(nd, TYPE_LANCE);
>> -
>> - qdev_set_nic_properties(DEVICE(lance), nd);
>> object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
>> sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
>> }
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 947b69d159..ed5f3ccd9f 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>>
>> static void *sparc32_dma_init(hwaddr dma_base,
>> hwaddr esp_base, qemu_irq espdma_irq,
>> - hwaddr le_base, qemu_irq ledma_irq)
>> + hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
>
> Instead of passing NICInfo to sparc32_dma_init,
> shouldn't you extract the lance code from it?
Hi Philippe,
I'm not sure I understand what you mean here? The sparc32-dma device is realised
within the sparc32_dma_init() function and qdev_set_nic_properties() must be called
before realise happens.
If you can explain a bit more about how you think it can be separated out then I can
take a look.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 17:03 ` Mark Cave-Ayland
@ 2020-09-21 17:14 ` Mark Cave-Ayland
2020-09-26 10:23 ` Mark Cave-Ayland
0 siblings, 1 reply; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-21 17:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-ppc, qemu-devel, atar4qemu, david
On 21/09/2020 18:03, Mark Cave-Ayland wrote:
> The lance and esp devices are embedded within ledma and espdma devices respectively,
> but are actually sysbus devices because they can be used by other machines. I'm not
> sure if lance is used anywhere else, but esp certainly is. Hence they are mapped
> after the dma device is realised as it feels odd to attach devices to sysbus outside
> of a machine init function.
Actually I have a better idea for this: use sysbus_mmio_get_region() within the
sparc32-dma device to attach the lance and esp memory regions to its own container
memory region: then the single sysbus_mmio_map() for the sparc32-dma device will just
work on its own.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 17:08 ` Mark Cave-Ayland
@ 2020-09-21 17:58 ` Philippe Mathieu-Daudé
2020-09-26 10:29 ` Mark Cave-Ayland
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 17:58 UTC (permalink / raw)
To: Mark Cave-Ayland, armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 9/21/20 7:08 PM, Mark Cave-Ayland wrote:
> On 21/09/2020 10:57, Philippe Mathieu-Daudé wrote:
>
>> Hi Mark,
>>
>> On 9/20/20 10:20 AM, Mark Cave-Ayland wrote:
>>> Instead use qdev_set_nic_properties() to configure the on-board NIC at the
>>> sun4m machine level.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> hw/dma/sparc32_dma.c | 5 -----
>>> hw/sparc/sun4m.c | 21 +++++++++++++--------
>>> 2 files changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
>>> index 2cbe331959..b643b413c5 100644
>>> --- a/hw/dma/sparc32_dma.c
>>> +++ b/hw/dma/sparc32_dma.c
>>> @@ -342,12 +342,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
>>> {
>>> LEDMADeviceState *s = SPARC32_LEDMA_DEVICE(dev);
>>> SysBusPCNetState *lance = SYSBUS_PCNET(&s->lance);
>>> - NICInfo *nd = &nd_table[0];
>>>
>>> - /* FIXME use qdev NIC properties instead of nd_table[] */
>>> - qemu_check_nic_model(nd, TYPE_LANCE);
>>> -
>>> - qdev_set_nic_properties(DEVICE(lance), nd);
>>> object_property_set_link(OBJECT(lance), "dma", OBJECT(dev), &error_abort);
>>> sysbus_realize(SYS_BUS_DEVICE(lance), &error_fatal);
>>> }
>>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>>> index 947b69d159..ed5f3ccd9f 100644
>>> --- a/hw/sparc/sun4m.c
>>> +++ b/hw/sparc/sun4m.c
>>> @@ -319,7 +319,7 @@ static void *iommu_init(hwaddr addr, uint32_t version, qemu_irq irq)
>>>
>>> static void *sparc32_dma_init(hwaddr dma_base,
>>> hwaddr esp_base, qemu_irq espdma_irq,
>>> - hwaddr le_base, qemu_irq ledma_irq)
>>> + hwaddr le_base, qemu_irq ledma_irq, NICInfo *nd)
>>
>> Instead of passing NICInfo to sparc32_dma_init,
>> shouldn't you extract the lance code from it?
>
> Hi Philippe,
>
> I'm not sure I understand what you mean here? The sparc32-dma device is realised
> within the sparc32_dma_init() function and qdev_set_nic_properties() must be called
> before realise happens.
>
> If you can explain a bit more about how you think it can be separated out then I can
> take a look.
Sorry I guess I got confused by the 2 different sparc32_dma_init()
functions.
Since ledma always expose lance, maybe you can use:
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 2cbe331959a..9a907a30373 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -336,18 +336,14 @@ static void sparc32_ledma_device_init(Object *obj)
"ledma-mmio", DMA_SIZE);
object_initialize_child(obj, "lance", &ls->lance, TYPE_LANCE);
+ qdev_alias_all_properties(DEVICE(&ls->lance), obj);
}
This way you set the properties directly on the ledma and only
have to sysbus_map lance.
>
>
> ATB,
>
> Mark.
>
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 17:14 ` Mark Cave-Ayland
@ 2020-09-26 10:23 ` Mark Cave-Ayland
0 siblings, 0 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-26 10:23 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-ppc, qemu-devel, atar4qemu, david
On 21/09/2020 18:14, Mark Cave-Ayland wrote:
> On 21/09/2020 18:03, Mark Cave-Ayland wrote:
>
>> The lance and esp devices are embedded within ledma and espdma devices respectively,
>> but are actually sysbus devices because they can be used by other machines. I'm not
>> sure if lance is used anywhere else, but esp certainly is. Hence they are mapped
>> after the dma device is realised as it feels odd to attach devices to sysbus outside
>> of a machine init function.
>
> Actually I have a better idea for this: use sysbus_mmio_get_region() within the
> sparc32-dma device to attach the lance and esp memory regions to its own container
> memory region: then the single sysbus_mmio_map() for the sparc32-dma device will just
> work on its own.
(goes and looks)
Ah now I remember - the DMA control registers and the lance/ESP devices are mapped to
2 different memory locations. So I think the current solution is the best one here.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device
2020-09-21 17:58 ` Philippe Mathieu-Daudé
@ 2020-09-26 10:29 ` Mark Cave-Ayland
0 siblings, 0 replies; 21+ messages in thread
From: Mark Cave-Ayland @ 2020-09-26 10:29 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
armbru, david, atar4qemu, qemu-devel, qemu-ppc
On 21/09/2020 18:58, Philippe Mathieu-Daudé wrote:
> Sorry I guess I got confused by the 2 different sparc32_dma_init()
> functions.
>
> Since ledma always expose lance, maybe you can use:
>
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index 2cbe331959a..9a907a30373 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -336,18 +336,14 @@ static void sparc32_ledma_device_init(Object *obj)
> "ledma-mmio", DMA_SIZE);
>
> object_initialize_child(obj, "lance", &ls->lance, TYPE_LANCE);
> + qdev_alias_all_properties(DEVICE(&ls->lance), obj);
> }
>
> This way you set the properties directly on the ledma and only
> have to sysbus_map lance.
Thanks for the hint. I've had a look at qdev_alias_all_properties() and for the
moment I'd prefer to get the reference to the internal lance/esp devices via
object_resolve_path_component(), since to me it makes it clearer on which device the
properties really belong from just looking at the code.
ATB,
Mark.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-09-26 10:30 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-20 8:20 [PATCH 0/6] QOM minor fixes Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 1/6] sparc32-dma: use object_initialize_child() for espdma and ledma child objects Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 2/6] sparc32-ledma: use object_initialize_child() for lance child object Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 3/6] sparc32-espdma: use object_initialize_child() for esp " Mark Cave-Ayland
2020-09-20 10:49 ` Philippe Mathieu-Daudé
2020-09-20 8:20 ` [PATCH 4/6] sparc32-ledma: don't reference nd_table directly within the device Mark Cave-Ayland
2020-09-21 9:25 ` Markus Armbruster
2020-09-21 17:03 ` Mark Cave-Ayland
2020-09-21 17:14 ` Mark Cave-Ayland
2020-09-26 10:23 ` Mark Cave-Ayland
2020-09-21 9:57 ` Philippe Mathieu-Daudé
2020-09-21 17:08 ` Mark Cave-Ayland
2020-09-21 17:58 ` Philippe Mathieu-Daudé
2020-09-26 10:29 ` Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 5/6] macio: don't reference serial_hd() " Mark Cave-Ayland
2020-09-20 10:52 ` BALATON Zoltan via
2020-09-20 17:59 ` Mark Cave-Ayland
2020-09-20 8:20 ` [PATCH 6/6] sabre: don't call sysbus_mmio_map() in sabre_realize() Mark Cave-Ayland
2020-09-20 10:48 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).