On Fri, 2016-06-17 at 15:22 +0100, Peter Maydell wrote: On 16 June 2016 at 08:48, Andrew Jeffery <and...@aj.id.au> wrote:
The SCU is a collection of chip-level control registers that manage the various functions supported by the AST2400. Typically the bits control interactions with clocks, external hardware or reset behaviour, and we can largly take a hands-off approach to reads and writes. Firmware makes heavy use of the state to determine how to boot, but the reset values vary from SoC to SoC. qdev properties are exposed so that the integrating SoC model can configure the appropriate reset values. Signed-off-by: Andrew Jeffery <and...@aj.id.au> Reviewed-by: Cédric Le Goater <c...@kaod.org> Reviewed-by: Joel Stanley <j...@jms.id.au> --- +static Property aspeed_scu_properties[] = { + DEFINE_PROP_ARRAY("reset", AspeedSCUState, num_resets, reset, + qdev_prop_uint32, uint32_t), + DEFINE_PROP_END_OF_LIST(), +}; + +#define ASPEED_SCU_NR_REGS (0x1A8 >> 2) This seems like a very unwieldy way of specifying the reset values for this device. Are they really all fully configurable in the hardware? It seems unlikely. I'd much rather see something that looks more like what you might plausibly be configuring when wiring up the SoC, which might be some version/revision numbers and/or some particular tweakable parameters. Right. I left out some context which may clear things up: We are working with two SoCs at the moment, the AST2400 and AST2500 (hopefully the AST2500 patches will be sent to the list soon). I wanted to abstract the configuration to cater for the differences in register values between the SoCs, less so for wiring the one SoC up in a different fashion. For what it's worth, out of 86 registers defined in the IO space between the two SoCs, 37 take the same value and 49 differ. I can rework the commit message to say as much. Separately, the qdev array approach was lifted from your commit 9c7d489379c2 hw/vexpress: Set reset values for daughterboard oscillators. Another approach would be to set the members of the SCU's reset array directly as we know the type. That seems less convoluted but my gut instinct was that wasn't how we should be configuring the devices. Is qdev the right way to go in the SCU's case? Cheers, Andrew
signature.asc
Description: This is a digitally signed message part