On Wed, 2016-07-27 at 18:46 +0200, Cédric Le Goater wrote: > The SCU controler holds the board revision number in its 0x7C > register. Let's use an alias to link a "silicon-rev" property of the > soc to the "silicon-rev" property of the SCU controler. > > The SDMC controler "silicon-rev" property is derived from the SCU one > at realize time. I did not find a better way to handle this part. > Links and aliases being a one-to-one relation, I could not use one of > them. I might wrong though.
Are we trying to over-use the silicon-rev value (it would seem so at
least in the face of the link/alias constraints)? We know which SDMC
revision we need for each SoC and we'll be modelling an explicit SoC
revision, so should we instead set a separate property on the SDMC in
the SoCs' respective initialise functions (and leave silicon-rev to the
SCU)? My thought was the silicon-rev value is reflective of the SoC
design rather than the other way around - but maybe that's splitting
hairs. It would also be trading off a bit of ugliness in this patch for
potential bugs if the properties get out of sync.
Cheers,
Andrew
>
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/arm/ast2400.c | 18 +++++++++++++-----
> hw/arm/palmetto-bmc.c | 2 ++
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/ast2400.c b/hw/arm/ast2400.c
> index 136bf6464e1d..fa535065f765 100644
> --- a/hw/arm/ast2400.c
> +++ b/hw/arm/ast2400.c
> @@ -84,8 +84,8 @@ static void ast2400_init(Object *obj)
> object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
> object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
> qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
> - qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
> - AST2400_A0_SILICON_REV);
> + object_property_add_alias(obj, "silicon-rev", OBJECT(&s->scu),
> + "silicon-rev", &error_abort);
> object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu),
> "hw-strap1", &error_abort);
> object_property_add_alias(obj, "hw-strap2", OBJECT(&s->scu),
> @@ -102,8 +102,6 @@ static void ast2400_init(Object *obj)
> object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
> object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
> qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
> - qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
> - AST2400_A0_SILICON_REV);
> }
>
> static void ast2400_realize(DeviceState *dev, Error **errp)
> @@ -111,6 +109,7 @@ static void ast2400_realize(DeviceState *dev, Error
> **errp)
> int i;
> AST2400State *s = AST2400(dev);
> Error *err = NULL, *local_err = NULL;
> + uint32_t silicon_rev;
>
> /* IO space */
> memory_region_init_io(&s->iomem, NULL, &ast2400_io_ops, NULL,
> @@ -192,7 +191,16 @@ static void ast2400_realize(DeviceState *dev, Error
> **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi), 1, AST2400_SPI_FLASH_BASE);
>
> /* SDMC - SDRAM Memory Controller */
> - object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
> + silicon_rev = (uint32_t)
> + object_property_get_int(OBJECT(&s->scu), "silicon-rev", &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +
> + object_property_set_int(OBJECT(&s->sdmc), silicon_rev, "silicon-rev",
> &err);
> + object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &local_err);
> + error_propagate(&err, local_err);
> if (err) {
> error_propagate(errp, err);
> return;
> diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
> index 54e29a865d88..1ee13d578899 100644
> --- a/hw/arm/palmetto-bmc.c
> +++ b/hw/arm/palmetto-bmc.c
> @@ -74,6 +74,8 @@ static void palmetto_bmc_init(MachineState *machine)
> &error_abort);
> object_property_set_int(OBJECT(&bmc->soc), 0x120CE416, "hw-strap1",
> &error_abort);
> + object_property_set_int(OBJECT(&bmc->soc), AST2400_A0_SILICON_REV,
> + "silicon-rev", &error_abort);
> object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
> &error_abort);
>
signature.asc
Description: This is a digitally signed message part
