* [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups
@ 2024-01-30 19:03 Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface Philippe Mathieu-Daudé
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-30 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Manos Pitsidianakis, Markus Armbruster, Gustavo Romero, qemu-arm,
Peter Maydell, Philippe Mathieu-Daudé
Gustavo wants to access the QOM path of an input IRQ line
from the NVIC, but since the device is orphan he ends up
with this nasty path [*]:
-device ivshmem-flat,chardev=ivshmem_flat,x-irq-qompath='/machine/unattached/device[1]/nvic/unnamed-gpio-in[0]',x-bus-qompath='/sysbus'
Add the missing parent so the tree is now:
(qemu) info qom-tree
/machine (lm3s6965evb-machine)
/gamepad (stellaris-gamepad)
/oled (ssd0323)
/peripheral (container)
/peripheral-anon (container)
/soc (container)
/v7m (armv7m)
/cpu (cortex-m3-arm-cpu)
/unnamed-gpio-in[0] (irq)
/unnamed-gpio-in[1] (irq)
/unnamed-gpio-in[2] (irq)
/unnamed-gpio-in[3] (irq)
/cpuclk (clock)
/nvic (armv7m_nvic)
/NMI[0] (irq)
/nvic_sysregs[0] (memory-region)
/systick-trigger[0] (irq)
/systick-trigger[1] (irq)
/unnamed-gpio-in[0] (irq)
...
[*] https://lore.kernel.org/qemu-devel/20231127052024.435743-1-gustavo.romero@linaro.org/
Philippe Mathieu-Daudé (4):
hw/arm/stellaris: Convert ADC controller to Resettable interface
hw/arm/stellaris: Convert I2C controller to Resettable interface
hw/arm/stellaris: Add missing QOM 'machine' parent
hw/arm/stellaris: Add missing QOM 'SoC' parent
hw/arm/stellaris.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface
2024-01-30 19:03 [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups Philippe Mathieu-Daudé
@ 2024-01-30 19:03 ` Philippe Mathieu-Daudé
2024-02-01 16:18 ` Peter Maydell
2024-01-30 19:03 ` [PATCH 2/4] hw/arm/stellaris: Convert I2C " Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-30 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Manos Pitsidianakis, Markus Armbruster, Gustavo Romero, qemu-arm,
Peter Maydell, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/stellaris.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index d18b1144af..afbc83f1e6 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -773,8 +773,9 @@ static void stellaris_adc_trigger(void *opaque, int irq, int level)
}
}
-static void stellaris_adc_reset(StellarisADCState *s)
+static void stellaris_adc_reset_hold(Object *obj)
{
+ StellarisADCState *s = STELLARIS_ADC(obj);
int n;
for (n = 0; n < 4; n++) {
@@ -946,7 +947,6 @@ static void stellaris_adc_init(Object *obj)
memory_region_init_io(&s->iomem, obj, &stellaris_adc_ops, s,
"adc", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
- stellaris_adc_reset(s);
qdev_init_gpio_in(dev, stellaris_adc_trigger, 1);
}
@@ -1397,7 +1397,9 @@ static const TypeInfo stellaris_i2c_info = {
static void stellaris_adc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ rc->phases.hold = stellaris_adc_reset_hold;
dc->vmsd = &vmstate_stellaris_adc;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] hw/arm/stellaris: Convert I2C controller to Resettable interface
2024-01-30 19:03 [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface Philippe Mathieu-Daudé
@ 2024-01-30 19:03 ` Philippe Mathieu-Daudé
2024-02-01 16:24 ` Peter Maydell
2024-01-30 19:03 ` [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent Philippe Mathieu-Daudé
3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-30 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Manos Pitsidianakis, Markus Armbruster, Gustavo Romero, qemu-arm,
Peter Maydell, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/stellaris.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index afbc83f1e6..284b95005f 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -607,8 +607,11 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset,
stellaris_i2c_update(s);
}
-static void stellaris_i2c_reset(stellaris_i2c_state *s)
+static void stellaris_i2c_reset_exit(Object *obj)
{
+ stellaris_i2c_state *s = STELLARIS_I2C(obj);
+
+ /* ??? For now we only implement the master interface. */
if (s->mcs & STELLARIS_I2C_MCS_BUSBSY)
i2c_end_transfer(s->bus);
@@ -658,8 +661,6 @@ static void stellaris_i2c_init(Object *obj)
memory_region_init_io(&s->iomem, obj, &stellaris_i2c_ops, s,
"i2c", 0x1000);
sysbus_init_mmio(sbd, &s->iomem);
- /* ??? For now we only implement the master interface. */
- stellaris_i2c_reset(s);
}
/* Analogue to Digital Converter. This is only partially implemented,
@@ -1382,7 +1383,9 @@ type_init(stellaris_machine_init)
static void stellaris_i2c_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ rc->phases.exit = stellaris_i2c_reset_exit;
dc->vmsd = &vmstate_stellaris_i2c;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent
2024-01-30 19:03 [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 2/4] hw/arm/stellaris: Convert I2C " Philippe Mathieu-Daudé
@ 2024-01-30 19:03 ` Philippe Mathieu-Daudé
2024-02-01 16:28 ` Peter Maydell
2024-02-01 16:47 ` Peter Maydell
2024-01-30 19:03 ` [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent Philippe Mathieu-Daudé
3 siblings, 2 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-30 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Manos Pitsidianakis, Markus Armbruster, Gustavo Romero, qemu-arm,
Peter Maydell, Philippe Mathieu-Daudé
QDev objects created with qdev_new() need to manually add
their parent relationship with object_property_add_child().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/stellaris.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 284b95005f..bb88b3ebde 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1247,10 +1247,13 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
&error_fatal);
ssddev = qdev_new("ssd0323");
+ object_property_add_child(OBJECT(ms), "oled", OBJECT(ssddev));
qdev_prop_set_uint8(ssddev, "cs", 1);
qdev_realize_and_unref(ssddev, bus, &error_fatal);
gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
+ object_property_add_child(OBJECT(ms), "splitter",
+ OBJECT(gpio_d_splitter));
qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
qdev_realize_and_unref(gpio_d_splitter, NULL, &error_fatal);
qdev_connect_gpio_out(
@@ -1287,6 +1290,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
DeviceState *gpad;
gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
+ object_property_add_child(OBJECT(ms), "gamepad", OBJECT(gpad));
for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent
2024-01-30 19:03 [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2024-01-30 19:03 ` [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent Philippe Mathieu-Daudé
@ 2024-01-30 19:03 ` Philippe Mathieu-Daudé
2024-02-01 16:46 ` Peter Maydell
3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-01-30 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Manos Pitsidianakis, Markus Armbruster, Gustavo Romero, qemu-arm,
Peter Maydell, Philippe Mathieu-Daudé
QDev objects created with qdev_new() need to manually add
their parent relationship with object_property_add_child().
Since we don't model the SoC, just use a QOM container.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/arm/stellaris.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index bb88b3ebde..e349981308 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1018,6 +1018,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
* 400fe000 system control
*/
+ Object *soc_container;
DeviceState *gpio_dev[7], *nvic;
qemu_irq gpio_in[7][8];
qemu_irq gpio_out[7][8];
@@ -1038,6 +1039,9 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
flash_size = (((board->dc0 & 0xffff) + 1) << 1) * 1024;
sram_size = ((board->dc0 >> 18) + 1) * 1024;
+ soc_container = object_new("container");
+ object_property_add_child(OBJECT(ms), "soc", soc_container);
+
/* Flash programming is done via the SCU, so pretend it is ROM. */
memory_region_init_rom(flash, NULL, "stellaris.flash", flash_size,
&error_fatal);
@@ -1052,6 +1056,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
* need its sysclk output.
*/
ssys_dev = qdev_new(TYPE_STELLARIS_SYS);
+ object_property_add_child(soc_container, "sys", OBJECT(ssys_dev));
/* Most devices come preprogrammed with a MAC address in the user data. */
macaddr = nd_table[0].macaddr.a;
qdev_prop_set_uint32(ssys_dev, "user0",
@@ -1068,6 +1073,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
sysbus_realize_and_unref(SYS_BUS_DEVICE(ssys_dev), &error_fatal);
nvic = qdev_new(TYPE_ARMV7M);
+ object_property_add_child(soc_container, "v7m", OBJECT(nvic));
qdev_prop_set_uint32(nvic, "num-irq", NUM_IRQ_LINES);
qdev_prop_set_uint8(nvic, "num-prio-bits", NUM_PRIO_BITS);
qdev_prop_set_string(nvic, "cpu-type", ms->cpu_type);
@@ -1101,6 +1107,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
dev = qdev_new(TYPE_STELLARIS_GPTM);
sbd = SYS_BUS_DEVICE(dev);
+ object_property_add_child(soc_container, "gptm[*]", OBJECT(dev));
qdev_connect_clock_in(dev, "clk",
qdev_get_clock_out(ssys_dev, "SYSCLK"));
sysbus_realize_and_unref(sbd, &error_fatal);
@@ -1114,7 +1121,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
if (board->dc1 & (1 << 3)) { /* watchdog present */
dev = qdev_new(TYPE_LUMINARY_WATCHDOG);
-
+ object_property_add_child(soc_container, "wdg", OBJECT(dev));
qdev_connect_clock_in(dev, "WDOGCLK",
qdev_get_clock_out(ssys_dev, "SYSCLK"));
@@ -1154,6 +1161,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
SysBusDevice *sbd;
dev = qdev_new("pl011_luminary");
+ object_property_add_child(soc_container, "uart[*]", OBJECT(dev));
sbd = SYS_BUS_DEVICE(dev);
qdev_prop_set_chr(dev, "chardev", serial_hd(i));
sysbus_realize_and_unref(sbd, &error_fatal);
@@ -1276,6 +1284,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
qemu_check_nic_model(&nd_table[0], "stellaris");
enet = qdev_new("stellaris_enet");
+ object_property_add_child(soc_container, "enet", OBJECT(enet));
qdev_set_nic_properties(enet, &nd_table[0]);
sysbus_realize_and_unref(SYS_BUS_DEVICE(enet), &error_fatal);
sysbus_mmio_map(SYS_BUS_DEVICE(enet), 0, 0x40048000);
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface
2024-01-30 19:03 ` [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface Philippe Mathieu-Daudé
@ 2024-02-01 16:18 ` Peter Maydell
0 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-02-01 16:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/stellaris.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] hw/arm/stellaris: Convert I2C controller to Resettable interface
2024-01-30 19:03 ` [PATCH 2/4] hw/arm/stellaris: Convert I2C " Philippe Mathieu-Daudé
@ 2024-02-01 16:24 ` Peter Maydell
2024-02-13 15:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2024-02-01 16:24 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/stellaris.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index afbc83f1e6..284b95005f 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -607,8 +607,11 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset,
> stellaris_i2c_update(s);
> }
>
> -static void stellaris_i2c_reset(stellaris_i2c_state *s)
> +static void stellaris_i2c_reset_exit(Object *obj)
> {
> + stellaris_i2c_state *s = STELLARIS_I2C(obj);
> +
> + /* ??? For now we only implement the master interface. */
> if (s->mcs & STELLARIS_I2C_MCS_BUSBSY)
> i2c_end_transfer(s->bus);
>
> @@ -658,8 +661,6 @@ static void stellaris_i2c_init(Object *obj)
> memory_region_init_io(&s->iomem, obj, &stellaris_i2c_ops, s,
> "i2c", 0x1000);
> sysbus_init_mmio(sbd, &s->iomem);
> - /* ??? For now we only implement the master interface. */
I'm not 100%, but I think this comment is a general one,
not reset specific, so it should stay in the init function.
> - stellaris_i2c_reset(s);
> }
I think that the i2c_end_transfer() should be in the
"enter" phase, and the clearing of the state fields in
"hold", and then the stellaris_i2c_update() call in "exit".
Though usually we don't bother to do an update in reset
if it's just "device has reset and now its outbound IRQ
line is not set", so we could alternatively just delete that.
> /* Analogue to Digital Converter. This is only partially implemented,
> @@ -1382,7 +1383,9 @@ type_init(stellaris_machine_init)
> static void stellaris_i2c_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> + rc->phases.exit = stellaris_i2c_reset_exit;
> dc->vmsd = &vmstate_stellaris_i2c;
> }
Also on the subject of resetting i2c controllers and buses:
https://patchew.org/QEMU/20240126005541.1839038-1-komlodi@google.com/
That patchset proposes that the bus should basically drop
any pending transactions on the floor at reset; it would
also be possible for the bus to do the "end any transfer
in progress", so the individual controller model doesn't
have to. No idea what the overall Right Thing is, so
let's not block this tidyup on figuring out general i2c reset
cleanups.
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent
2024-01-30 19:03 ` [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent Philippe Mathieu-Daudé
@ 2024-02-01 16:28 ` Peter Maydell
2024-02-13 15:40 ` Philippe Mathieu-Daudé
2024-02-01 16:47 ` Peter Maydell
1 sibling, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2024-02-01 16:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> QDev objects created with qdev_new() need to manually add
> their parent relationship with object_property_add_child().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/stellaris.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 284b95005f..bb88b3ebde 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -1247,10 +1247,13 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> &error_fatal);
>
> ssddev = qdev_new("ssd0323");
> + object_property_add_child(OBJECT(ms), "oled", OBJECT(ssddev));
> qdev_prop_set_uint8(ssddev, "cs", 1);
> qdev_realize_and_unref(ssddev, bus, &error_fatal);
>
> gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
> + object_property_add_child(OBJECT(ms), "splitter",
> + OBJECT(gpio_d_splitter));
> qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
> qdev_realize_and_unref(gpio_d_splitter, NULL, &error_fatal);
> qdev_connect_gpio_out(
> @@ -1287,6 +1290,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> DeviceState *gpad;
>
> gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
> + object_property_add_child(OBJECT(ms), "gamepad", OBJECT(gpad));
> for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
> qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
> }
> --
We create almost all the devices in this board with
qdev_new(), and we don't use object_property_add_child()
on any of them. What is it about these three devices in
particular that means we need to call that function?
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent
2024-01-30 19:03 ` [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent Philippe Mathieu-Daudé
@ 2024-02-01 16:46 ` Peter Maydell
2024-02-13 15:36 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: Peter Maydell @ 2024-02-01 16:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> QDev objects created with qdev_new() need to manually add
> their parent relationship with object_property_add_child().
>
> Since we don't model the SoC, just use a QOM container.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
Ah, this is where the other qdev_new() calls are sorted.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I wonder if we should add a variant on qdev_new() that
you can pass in the parent object to?
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent
2024-01-30 19:03 ` [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent Philippe Mathieu-Daudé
2024-02-01 16:28 ` Peter Maydell
@ 2024-02-01 16:47 ` Peter Maydell
1 sibling, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2024-02-01 16:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> QDev objects created with qdev_new() need to manually add
> their parent relationship with object_property_add_child().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/arm/stellaris.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent
2024-02-01 16:46 ` Peter Maydell
@ 2024-02-13 15:36 ` Philippe Mathieu-Daudé
2024-02-14 7:08 ` Cédric Le Goater
0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-13 15:36 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm, Cédric Le Goater
On 1/2/24 17:46, Peter Maydell wrote:
> On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> QDev objects created with qdev_new() need to manually add
>> their parent relationship with object_property_add_child().
>>
>> Since we don't model the SoC, just use a QOM container.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>
> Ah, this is where the other qdev_new() calls are sorted.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I wonder if we should add a variant on qdev_new() that
> you can pass in the parent object to?
Yes, this is what we discussed with Markus. In order to
stop using the "/unattached" container from pre-QOM,
qdev_new() must take a QOM parent. I tried to do it but hit
some problem with some odd use in PPC or S390 (discussed
with Cédric so likely PPC, I need to go back to it).
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent
2024-02-01 16:28 ` Peter Maydell
@ 2024-02-13 15:40 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-13 15:40 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On 1/2/24 17:28, Peter Maydell wrote:
> On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> QDev objects created with qdev_new() need to manually add
>> their parent relationship with object_property_add_child().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/arm/stellaris.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>> index 284b95005f..bb88b3ebde 100644
>> --- a/hw/arm/stellaris.c
>> +++ b/hw/arm/stellaris.c
>> @@ -1247,10 +1247,13 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>> &error_fatal);
>>
>> ssddev = qdev_new("ssd0323");
>> + object_property_add_child(OBJECT(ms), "oled", OBJECT(ssddev));
>> qdev_prop_set_uint8(ssddev, "cs", 1);
>> qdev_realize_and_unref(ssddev, bus, &error_fatal);
>>
>> gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
>> + object_property_add_child(OBJECT(ms), "splitter",
>> + OBJECT(gpio_d_splitter));
>> qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
>> qdev_realize_and_unref(gpio_d_splitter, NULL, &error_fatal);
>> qdev_connect_gpio_out(
>> @@ -1287,6 +1290,7 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>> DeviceState *gpad;
>>
>> gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
>> + object_property_add_child(OBJECT(ms), "gamepad", OBJECT(gpad));
>> for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
>> qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
>> }
>> --
>
> We create almost all the devices in this board with
> qdev_new(), and we don't use object_property_add_child()
> on any of them. What is it about these three devices in
> particular that means we need to call that function?
In v2 I added to the description:
This commit plug the devices which aren't part of the SoC;
they will be plugged into a SoC container in the next one.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] hw/arm/stellaris: Convert I2C controller to Resettable interface
2024-02-01 16:24 ` Peter Maydell
@ 2024-02-13 15:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-13 15:50 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On 1/2/24 17:24, Peter Maydell wrote:
> On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/arm/stellaris.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>> @@ -658,8 +661,6 @@ static void stellaris_i2c_init(Object *obj)
>> memory_region_init_io(&s->iomem, obj, &stellaris_i2c_ops, s,
>> "i2c", 0x1000);
>> sysbus_init_mmio(sbd, &s->iomem);
>> - /* ??? For now we only implement the master interface. */
>
> I'm not 100%, but I think this comment is a general one,
> not reset specific, so it should stay in the init function.
>
>> - stellaris_i2c_reset(s);
>> }
>
> I think that the i2c_end_transfer() should be in the
> "enter" phase, and the clearing of the state fields in
> "hold", and then the stellaris_i2c_update() call in "exit".
Indeed.
> Though usually we don't bother to do an update in reset
> if it's just "device has reset and now its outbound IRQ
> line is not set", so we could alternatively just delete that.
I'll let that as an possible cleanup on top.
Thanks for the review,
Phil.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent
2024-02-13 15:36 ` Philippe Mathieu-Daudé
@ 2024-02-14 7:08 ` Cédric Le Goater
0 siblings, 0 replies; 14+ messages in thread
From: Cédric Le Goater @ 2024-02-14 7:08 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: qemu-devel, Manos Pitsidianakis, Markus Armbruster,
Gustavo Romero, qemu-arm
On 2/13/24 16:36, Philippe Mathieu-Daudé wrote:
> On 1/2/24 17:46, Peter Maydell wrote:
>> On Tue, 30 Jan 2024 at 19:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>
>>> QDev objects created with qdev_new() need to manually add
>>> their parent relationship with object_property_add_child().
>>>
>>> Since we don't model the SoC, just use a QOM container.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>
>> Ah, this is where the other qdev_new() calls are sorted.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>> I wonder if we should add a variant on qdev_new() that
>> you can pass in the parent object to?
>
> Yes, this is what we discussed with Markus. In order to
> stop using the "/unattached" container from pre-QOM,
> qdev_new() must take a QOM parent. I tried to do it but hit
> some problem with some odd use in PPC or S390 (discussed
> with Cédric so likely PPC, I need to go back to it).
Can you remind what this was about ?
Thanks,
C.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-02-14 7:09 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-30 19:03 [PATCH 0/4] hw/arm/stellaris: QOM/QDev cleanups Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 1/4] hw/arm/stellaris: Convert ADC controller to Resettable interface Philippe Mathieu-Daudé
2024-02-01 16:18 ` Peter Maydell
2024-01-30 19:03 ` [PATCH 2/4] hw/arm/stellaris: Convert I2C " Philippe Mathieu-Daudé
2024-02-01 16:24 ` Peter Maydell
2024-02-13 15:50 ` Philippe Mathieu-Daudé
2024-01-30 19:03 ` [PATCH 3/4] hw/arm/stellaris: Add missing QOM 'machine' parent Philippe Mathieu-Daudé
2024-02-01 16:28 ` Peter Maydell
2024-02-13 15:40 ` Philippe Mathieu-Daudé
2024-02-01 16:47 ` Peter Maydell
2024-01-30 19:03 ` [PATCH 4/4] hw/arm/stellaris: Add missing QOM 'SoC' parent Philippe Mathieu-Daudé
2024-02-01 16:46 ` Peter Maydell
2024-02-13 15:36 ` Philippe Mathieu-Daudé
2024-02-14 7:08 ` Cédric Le Goater
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).