* [PATCH for-5.0 0/2] aspeed: rework inter model link properties
@ 2019-11-18 16:17 Cédric Le Goater
2019-11-18 16:17 ` [PATCH for-5.0 1/2] aspeed: change the "scu" property definition Cédric Le Goater
2019-11-18 16:17 ` [PATCH for-5.0 2/2] aspeed: change the "nic" " Cédric Le Goater
0 siblings, 2 replies; 7+ messages in thread
From: Cédric Le Goater @ 2019-11-18 16:17 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, qemu-devel, Greg Kurz, qemu-arm, Joel Stanley,
Cédric Le Goater
Hello,
Some Aspeed models use links to point to other controllers of the
machine. This series changes the link properties so that they set the
pointer explicitly.
Thanks,
C.
Cédric Le Goater (2):
aspeed: change the "scu" property definition
aspeed: change the "nic" property definition
hw/arm/aspeed_ast2600.c | 13 ++++++-------
hw/arm/aspeed_soc.c | 8 ++++----
hw/net/ftgmac100.c | 19 +++++++++----------
hw/timer/aspeed_timer.c | 17 +++++++++--------
hw/watchdog/wdt_aspeed.c | 17 ++++++++---------
5 files changed, 36 insertions(+), 38 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH for-5.0 1/2] aspeed: change the "scu" property definition
2019-11-18 16:17 [PATCH for-5.0 0/2] aspeed: rework inter model link properties Cédric Le Goater
@ 2019-11-18 16:17 ` Cédric Le Goater
2019-11-18 19:45 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
2019-11-18 16:17 ` [PATCH for-5.0 2/2] aspeed: change the "nic" " Cédric Le Goater
1 sibling, 2 replies; 7+ messages in thread
From: Cédric Le Goater @ 2019-11-18 16:17 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, qemu-devel, Greg Kurz, qemu-arm, Joel Stanley,
Cédric Le Goater
The Aspeed Watchdog and Timer models have a link pointing to the SCU
controller model of the machine.
Change the "scu" property definition so that it explicitly sets the
pointer. The property isn't optional : not being able to set the link
is a bug and QEMU should rather abort than exit in this case.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/aspeed_ast2600.c | 8 ++++----
hw/arm/aspeed_soc.c | 8 ++++----
hw/timer/aspeed_timer.c | 17 +++++++++--------
hw/watchdog/wdt_aspeed.c | 17 ++++++++---------
4 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 0881eb25983e..810fd7de0c06 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -146,8 +146,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
sizeof(s->timerctrl), typename);
- object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
- OBJECT(&s->scu), &error_abort);
snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
@@ -177,8 +175,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
sizeof(s->wdt[i]), typename);
- object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
- OBJECT(&s->scu), &error_abort);
}
for (i = 0; i < sc->macs_num; i++) {
@@ -323,6 +319,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
aspeed_soc_get_irq(s, ASPEED_RTC));
/* Timer */
+ object_property_set_link(OBJECT(&s->timerctrl),
+ OBJECT(&s->scu), "scu", &error_abort);
object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
if (err) {
error_propagate(errp, err);
@@ -415,6 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
for (i = 0; i < sc->wdts_num; i++) {
AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
+ object_property_set_link(OBJECT(&s->wdt[i]),
+ OBJECT(&s->scu), "scu", &error_abort);
object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index b01c97744196..a6237e594017 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -163,8 +163,6 @@ static void aspeed_soc_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
sizeof(s->timerctrl), typename);
- object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
- OBJECT(&s->scu), &error_abort);
snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
@@ -194,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
sizeof(s->wdt[i]), typename);
- object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
- OBJECT(&s->scu), &error_abort);
}
for (i = 0; i < sc->macs_num; i++) {
@@ -291,6 +287,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
aspeed_soc_get_irq(s, ASPEED_RTC));
/* Timer */
+ object_property_set_link(OBJECT(&s->timerctrl),
+ OBJECT(&s->scu), "scu", &error_abort);
object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
if (err) {
error_propagate(errp, err);
@@ -376,6 +374,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
for (i = 0; i < sc->wdts_num; i++) {
AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
+ object_property_set_link(OBJECT(&s->wdt[i]),
+ OBJECT(&s->scu), "scu", &error_abort);
object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index bcce2192a92a..a8c38cc1189b 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -19,6 +19,7 @@
#include "qemu/timer.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "hw/qdev-properties.h"
#include "trace.h"
#define TIMER_NR_REGS 4
@@ -603,15 +604,8 @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp)
int i;
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
AspeedTimerCtrlState *s = ASPEED_TIMER(dev);
- Object *obj;
- Error *err = NULL;
- obj = object_property_get_link(OBJECT(dev), "scu", &err);
- if (!obj) {
- error_propagate_prepend(errp, err, "required link 'scu' not found: ");
- return;
- }
- s->scu = ASPEED_SCU(obj);
+ assert(s->scu);
for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
aspeed_init_one_timer(s, i);
@@ -677,6 +671,12 @@ static const VMStateDescription vmstate_aspeed_timer_state = {
}
};
+static Property aspeed_timer_properties[] = {
+ DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU,
+ AspeedSCUState *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void timer_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -685,6 +685,7 @@ static void timer_class_init(ObjectClass *klass, void *data)
dc->reset = aspeed_timer_reset;
dc->desc = "ASPEED Timer";
dc->vmsd = &vmstate_aspeed_timer_state;
+ dc->props = aspeed_timer_properties;
}
static const TypeInfo aspeed_timer_info = {
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 122aa8daaadf..f50dab922e0f 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -241,16 +241,8 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
{
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
AspeedWDTState *s = ASPEED_WDT(dev);
- Error *err = NULL;
- Object *obj;
- obj = object_property_get_link(OBJECT(dev), "scu", &err);
- if (!obj) {
- error_propagate(errp, err);
- error_prepend(errp, "required link 'scu' not found: ");
- return;
- }
- s->scu = ASPEED_SCU(obj);
+ assert(s->scu);
s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev);
@@ -264,6 +256,12 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(sbd, &s->iomem);
}
+static Property aspeed_wdt_properties[] = {
+ DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU,
+ AspeedSCUState *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -273,6 +271,7 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
dc->reset = aspeed_wdt_reset;
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
dc->vmsd = &vmstate_aspeed_wdt;
+ dc->props = aspeed_wdt_properties;
}
static const TypeInfo aspeed_wdt_info = {
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH for-5.0 2/2] aspeed: change the "nic" property definition
2019-11-18 16:17 [PATCH for-5.0 0/2] aspeed: rework inter model link properties Cédric Le Goater
2019-11-18 16:17 ` [PATCH for-5.0 1/2] aspeed: change the "scu" property definition Cédric Le Goater
@ 2019-11-18 16:17 ` Cédric Le Goater
2019-11-18 19:46 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
1 sibling, 2 replies; 7+ messages in thread
From: Cédric Le Goater @ 2019-11-18 16:17 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, qemu-devel, Greg Kurz, qemu-arm, Joel Stanley,
Cédric Le Goater
The Aspeed MII model has a link pointing to its associated FTGMAC100
NIC in the machine.
Change the "nic" property definition so that it explicitly sets the
pointer. The property isn't optional : not being able to set the link
is a bug and QEMU should rather abort than exit in this case.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/aspeed_ast2600.c | 5 ++---
hw/net/ftgmac100.c | 19 +++++++++----------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 810fd7de0c06..be88005dab8f 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -183,9 +183,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]),
TYPE_ASPEED_MII);
- object_property_add_const_link(OBJECT(&s->mii[i]), "nic",
- OBJECT(&s->ftgmac100[i]),
- &error_abort);
}
sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
@@ -441,6 +438,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
aspeed_soc_get_irq(s, ASPEED_ETH1 + i));
+ object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]),
+ "nic", &error_abort);
object_property_set_bool(OBJECT(&s->mii[i]), true, "realized",
&err);
if (err) {
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index eb8b441461a1..86ac25894a89 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -1204,17 +1204,8 @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp)
{
AspeedMiiState *s = ASPEED_MII(dev);
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
- Object *obj;
- Error *local_err = NULL;
- obj = object_property_get_link(OBJECT(dev), "nic", &local_err);
- if (!obj) {
- error_propagate(errp, local_err);
- error_prepend(errp, "required link 'nic' not found: ");
- return;
- }
-
- s->nic = FTGMAC100(obj);
+ assert(s->nic);
memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s,
TYPE_ASPEED_MII, 0x8);
@@ -1231,6 +1222,13 @@ static const VMStateDescription vmstate_aspeed_mii = {
VMSTATE_END_OF_LIST()
}
};
+
+static Property aspeed_mii_properties[] = {
+ DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100,
+ FTGMAC100State *),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
static void aspeed_mii_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1239,6 +1237,7 @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data)
dc->reset = aspeed_mii_reset;
dc->realize = aspeed_mii_realize;
dc->desc = "Aspeed MII controller";
+ dc->props = aspeed_mii_properties;
}
static const TypeInfo aspeed_mii_info = {
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH for-5.0 1/2] aspeed: change the "scu" property definition
2019-11-18 16:17 ` [PATCH for-5.0 1/2] aspeed: change the "scu" property definition Cédric Le Goater
@ 2019-11-18 19:45 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
1 sibling, 0 replies; 7+ messages in thread
From: Greg Kurz @ 2019-11-18 19:45 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Joel Stanley, qemu-devel
On Mon, 18 Nov 2019 17:17:11 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> The Aspeed Watchdog and Timer models have a link pointing to the SCU
> controller model of the machine.
>
> Change the "scu" property definition so that it explicitly sets the
> pointer. The property isn't optional : not being able to set the link
> is a bug and QEMU should rather abort than exit in this case.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/arm/aspeed_ast2600.c | 8 ++++----
> hw/arm/aspeed_soc.c | 8 ++++----
> hw/timer/aspeed_timer.c | 17 +++++++++--------
> hw/watchdog/wdt_aspeed.c | 17 ++++++++---------
> 4 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 0881eb25983e..810fd7de0c06 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -146,8 +146,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
> sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
> sizeof(s->timerctrl), typename);
> - object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
> - OBJECT(&s->scu), &error_abort);
>
> snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
> @@ -177,8 +175,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
> sizeof(s->wdt[i]), typename);
> - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
> - OBJECT(&s->scu), &error_abort);
> }
>
> for (i = 0; i < sc->macs_num; i++) {
> @@ -323,6 +319,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> aspeed_soc_get_irq(s, ASPEED_RTC));
>
> /* Timer */
> + object_property_set_link(OBJECT(&s->timerctrl),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> @@ -415,6 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> for (i = 0; i < sc->wdts_num; i++) {
> AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
>
> + object_property_set_link(OBJECT(&s->wdt[i]),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index b01c97744196..a6237e594017 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -163,8 +163,6 @@ static void aspeed_soc_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
> sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
> sizeof(s->timerctrl), typename);
> - object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
> - OBJECT(&s->scu), &error_abort);
>
> snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
> @@ -194,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
> sizeof(s->wdt[i]), typename);
> - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
> - OBJECT(&s->scu), &error_abort);
> }
>
> for (i = 0; i < sc->macs_num; i++) {
> @@ -291,6 +287,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> aspeed_soc_get_irq(s, ASPEED_RTC));
>
> /* Timer */
> + object_property_set_link(OBJECT(&s->timerctrl),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> @@ -376,6 +374,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> for (i = 0; i < sc->wdts_num; i++) {
> AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
>
> + object_property_set_link(OBJECT(&s->wdt[i]),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> index bcce2192a92a..a8c38cc1189b 100644
> --- a/hw/timer/aspeed_timer.c
> +++ b/hw/timer/aspeed_timer.c
> @@ -19,6 +19,7 @@
> #include "qemu/timer.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "hw/qdev-properties.h"
> #include "trace.h"
>
> #define TIMER_NR_REGS 4
> @@ -603,15 +604,8 @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp)
> int i;
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedTimerCtrlState *s = ASPEED_TIMER(dev);
> - Object *obj;
> - Error *err = NULL;
>
> - obj = object_property_get_link(OBJECT(dev), "scu", &err);
> - if (!obj) {
> - error_propagate_prepend(errp, err, "required link 'scu' not found: ");
> - return;
> - }
> - s->scu = ASPEED_SCU(obj);
> + assert(s->scu);
>
> for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
> aspeed_init_one_timer(s, i);
> @@ -677,6 +671,12 @@ static const VMStateDescription vmstate_aspeed_timer_state = {
> }
> };
>
> +static Property aspeed_timer_properties[] = {
> + DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU,
> + AspeedSCUState *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void timer_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -685,6 +685,7 @@ static void timer_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_timer_reset;
> dc->desc = "ASPEED Timer";
> dc->vmsd = &vmstate_aspeed_timer_state;
> + dc->props = aspeed_timer_properties;
> }
>
> static const TypeInfo aspeed_timer_info = {
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index 122aa8daaadf..f50dab922e0f 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -241,16 +241,8 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedWDTState *s = ASPEED_WDT(dev);
> - Error *err = NULL;
> - Object *obj;
>
> - obj = object_property_get_link(OBJECT(dev), "scu", &err);
> - if (!obj) {
> - error_propagate(errp, err);
> - error_prepend(errp, "required link 'scu' not found: ");
> - return;
> - }
> - s->scu = ASPEED_SCU(obj);
> + assert(s->scu);
>
> s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev);
>
> @@ -264,6 +256,12 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
> +static Property aspeed_wdt_properties[] = {
> + DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU,
> + AspeedSCUState *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -273,6 +271,7 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_wdt_reset;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> dc->vmsd = &vmstate_aspeed_wdt;
> + dc->props = aspeed_wdt_properties;
> }
>
> static const TypeInfo aspeed_wdt_info = {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-5.0 2/2] aspeed: change the "nic" property definition
2019-11-18 16:17 ` [PATCH for-5.0 2/2] aspeed: change the "nic" " Cédric Le Goater
@ 2019-11-18 19:46 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
1 sibling, 0 replies; 7+ messages in thread
From: Greg Kurz @ 2019-11-18 19:46 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Joel Stanley, qemu-devel
On Mon, 18 Nov 2019 17:17:12 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> The Aspeed MII model has a link pointing to its associated FTGMAC100
> NIC in the machine.
>
> Change the "nic" property definition so that it explicitly sets the
> pointer. The property isn't optional : not being able to set the link
> is a bug and QEMU should rather abort than exit in this case.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/arm/aspeed_ast2600.c | 5 ++---
> hw/net/ftgmac100.c | 19 +++++++++----------
> 2 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 810fd7de0c06..be88005dab8f 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -183,9 +183,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
>
> sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]),
> TYPE_ASPEED_MII);
> - object_property_add_const_link(OBJECT(&s->mii[i]), "nic",
> - OBJECT(&s->ftgmac100[i]),
> - &error_abort);
> }
>
> sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
> @@ -441,6 +438,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
> aspeed_soc_get_irq(s, ASPEED_ETH1 + i));
>
> + object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]),
> + "nic", &error_abort);
> object_property_set_bool(OBJECT(&s->mii[i]), true, "realized",
> &err);
> if (err) {
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index eb8b441461a1..86ac25894a89 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -1204,17 +1204,8 @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp)
> {
> AspeedMiiState *s = ASPEED_MII(dev);
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> - Object *obj;
> - Error *local_err = NULL;
>
> - obj = object_property_get_link(OBJECT(dev), "nic", &local_err);
> - if (!obj) {
> - error_propagate(errp, local_err);
> - error_prepend(errp, "required link 'nic' not found: ");
> - return;
> - }
> -
> - s->nic = FTGMAC100(obj);
> + assert(s->nic);
>
> memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s,
> TYPE_ASPEED_MII, 0x8);
> @@ -1231,6 +1222,13 @@ static const VMStateDescription vmstate_aspeed_mii = {
> VMSTATE_END_OF_LIST()
> }
> };
> +
> +static Property aspeed_mii_properties[] = {
> + DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100,
> + FTGMAC100State *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void aspeed_mii_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1239,6 +1237,7 @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_mii_reset;
> dc->realize = aspeed_mii_realize;
> dc->desc = "Aspeed MII controller";
> + dc->props = aspeed_mii_properties;
> }
>
> static const TypeInfo aspeed_mii_info = {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-5.0 2/2] aspeed: change the "nic" property definition
2019-11-18 16:17 ` [PATCH for-5.0 2/2] aspeed: change the "nic" " Cédric Le Goater
2019-11-18 19:46 ` Greg Kurz
@ 2019-11-18 22:39 ` Joel Stanley
1 sibling, 0 replies; 7+ messages in thread
From: Joel Stanley @ 2019-11-18 22:39 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Greg Kurz, QEMU Developers
On Mon, 18 Nov 2019 at 16:17, Cédric Le Goater <clg@kaod.org> wrote:
>
> The Aspeed MII model has a link pointing to its associated FTGMAC100
> NIC in the machine.
>
> Change the "nic" property definition so that it explicitly sets the
> pointer. The property isn't optional : not being able to set the link
> is a bug and QEMU should rather abort than exit in this case.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> hw/arm/aspeed_ast2600.c | 5 ++---
> hw/net/ftgmac100.c | 19 +++++++++----------
> 2 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 810fd7de0c06..be88005dab8f 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -183,9 +183,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
>
> sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]),
> TYPE_ASPEED_MII);
> - object_property_add_const_link(OBJECT(&s->mii[i]), "nic",
> - OBJECT(&s->ftgmac100[i]),
> - &error_abort);
> }
>
> sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
> @@ -441,6 +438,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
> aspeed_soc_get_irq(s, ASPEED_ETH1 + i));
>
> + object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]),
> + "nic", &error_abort);
> object_property_set_bool(OBJECT(&s->mii[i]), true, "realized",
> &err);
> if (err) {
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index eb8b441461a1..86ac25894a89 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -1204,17 +1204,8 @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp)
> {
> AspeedMiiState *s = ASPEED_MII(dev);
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> - Object *obj;
> - Error *local_err = NULL;
>
> - obj = object_property_get_link(OBJECT(dev), "nic", &local_err);
> - if (!obj) {
> - error_propagate(errp, local_err);
> - error_prepend(errp, "required link 'nic' not found: ");
> - return;
> - }
> -
> - s->nic = FTGMAC100(obj);
> + assert(s->nic);
>
> memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s,
> TYPE_ASPEED_MII, 0x8);
> @@ -1231,6 +1222,13 @@ static const VMStateDescription vmstate_aspeed_mii = {
> VMSTATE_END_OF_LIST()
> }
> };
> +
> +static Property aspeed_mii_properties[] = {
> + DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100,
> + FTGMAC100State *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void aspeed_mii_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -1239,6 +1237,7 @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_mii_reset;
> dc->realize = aspeed_mii_realize;
> dc->desc = "Aspeed MII controller";
> + dc->props = aspeed_mii_properties;
> }
>
> static const TypeInfo aspeed_mii_info = {
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-5.0 1/2] aspeed: change the "scu" property definition
2019-11-18 16:17 ` [PATCH for-5.0 1/2] aspeed: change the "scu" property definition Cédric Le Goater
2019-11-18 19:45 ` Greg Kurz
@ 2019-11-18 22:39 ` Joel Stanley
1 sibling, 0 replies; 7+ messages in thread
From: Joel Stanley @ 2019-11-18 22:39 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Greg Kurz, QEMU Developers
On Mon, 18 Nov 2019 at 16:17, Cédric Le Goater <clg@kaod.org> wrote:
>
> The Aspeed Watchdog and Timer models have a link pointing to the SCU
> controller model of the machine.
>
> Change the "scu" property definition so that it explicitly sets the
> pointer. The property isn't optional : not being able to set the link
> is a bug and QEMU should rather abort than exit in this case.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> hw/arm/aspeed_ast2600.c | 8 ++++----
> hw/arm/aspeed_soc.c | 8 ++++----
> hw/timer/aspeed_timer.c | 17 +++++++++--------
> hw/watchdog/wdt_aspeed.c | 17 ++++++++---------
> 4 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 0881eb25983e..810fd7de0c06 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -146,8 +146,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
> sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
> sizeof(s->timerctrl), typename);
> - object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
> - OBJECT(&s->scu), &error_abort);
>
> snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
> @@ -177,8 +175,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
> sizeof(s->wdt[i]), typename);
> - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
> - OBJECT(&s->scu), &error_abort);
> }
>
> for (i = 0; i < sc->macs_num; i++) {
> @@ -323,6 +319,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> aspeed_soc_get_irq(s, ASPEED_RTC));
>
> /* Timer */
> + object_property_set_link(OBJECT(&s->timerctrl),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> @@ -415,6 +413,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> for (i = 0; i < sc->wdts_num; i++) {
> AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
>
> + object_property_set_link(OBJECT(&s->wdt[i]),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index b01c97744196..a6237e594017 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -163,8 +163,6 @@ static void aspeed_soc_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname);
> sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
> sizeof(s->timerctrl), typename);
> - object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
> - OBJECT(&s->scu), &error_abort);
>
> snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname);
> sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
> @@ -194,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
> sizeof(s->wdt[i]), typename);
> - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu",
> - OBJECT(&s->scu), &error_abort);
> }
>
> for (i = 0; i < sc->macs_num; i++) {
> @@ -291,6 +287,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> aspeed_soc_get_irq(s, ASPEED_RTC));
>
> /* Timer */
> + object_property_set_link(OBJECT(&s->timerctrl),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> @@ -376,6 +374,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> for (i = 0; i < sc->wdts_num; i++) {
> AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]);
>
> + object_property_set_link(OBJECT(&s->wdt[i]),
> + OBJECT(&s->scu), "scu", &error_abort);
> object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
> index bcce2192a92a..a8c38cc1189b 100644
> --- a/hw/timer/aspeed_timer.c
> +++ b/hw/timer/aspeed_timer.c
> @@ -19,6 +19,7 @@
> #include "qemu/timer.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "hw/qdev-properties.h"
> #include "trace.h"
>
> #define TIMER_NR_REGS 4
> @@ -603,15 +604,8 @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp)
> int i;
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedTimerCtrlState *s = ASPEED_TIMER(dev);
> - Object *obj;
> - Error *err = NULL;
>
> - obj = object_property_get_link(OBJECT(dev), "scu", &err);
> - if (!obj) {
> - error_propagate_prepend(errp, err, "required link 'scu' not found: ");
> - return;
> - }
> - s->scu = ASPEED_SCU(obj);
> + assert(s->scu);
>
> for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
> aspeed_init_one_timer(s, i);
> @@ -677,6 +671,12 @@ static const VMStateDescription vmstate_aspeed_timer_state = {
> }
> };
>
> +static Property aspeed_timer_properties[] = {
> + DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU,
> + AspeedSCUState *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void timer_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -685,6 +685,7 @@ static void timer_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_timer_reset;
> dc->desc = "ASPEED Timer";
> dc->vmsd = &vmstate_aspeed_timer_state;
> + dc->props = aspeed_timer_properties;
> }
>
> static const TypeInfo aspeed_timer_info = {
> diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
> index 122aa8daaadf..f50dab922e0f 100644
> --- a/hw/watchdog/wdt_aspeed.c
> +++ b/hw/watchdog/wdt_aspeed.c
> @@ -241,16 +241,8 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> AspeedWDTState *s = ASPEED_WDT(dev);
> - Error *err = NULL;
> - Object *obj;
>
> - obj = object_property_get_link(OBJECT(dev), "scu", &err);
> - if (!obj) {
> - error_propagate(errp, err);
> - error_prepend(errp, "required link 'scu' not found: ");
> - return;
> - }
> - s->scu = ASPEED_SCU(obj);
> + assert(s->scu);
>
> s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev);
>
> @@ -264,6 +256,12 @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp)
> sysbus_init_mmio(sbd, &s->iomem);
> }
>
> +static Property aspeed_wdt_properties[] = {
> + DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU,
> + AspeedSCUState *),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -273,6 +271,7 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
> dc->reset = aspeed_wdt_reset;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> dc->vmsd = &vmstate_aspeed_wdt;
> + dc->props = aspeed_wdt_properties;
> }
>
> static const TypeInfo aspeed_wdt_info = {
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-18 22:46 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-18 16:17 [PATCH for-5.0 0/2] aspeed: rework inter model link properties Cédric Le Goater
2019-11-18 16:17 ` [PATCH for-5.0 1/2] aspeed: change the "scu" property definition Cédric Le Goater
2019-11-18 19:45 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
2019-11-18 16:17 ` [PATCH for-5.0 2/2] aspeed: change the "nic" " Cédric Le Goater
2019-11-18 19:46 ` Greg Kurz
2019-11-18 22:39 ` Joel Stanley
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).