On 23 June 2016 at 03:15, Andrew Jeffery <and...@aj.id.au> wrote:
> The SCU is a collection of chip-level control registers that manage the
> various functions supported by ASPEED SoCs. 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 (eg AST2400 vs AST2500). A qdev
> property is exposed so that the integrating SoC model can configure the
> silicon revision, which in-turn selects the appropriate reset values.
> Further qdev properties are exposed so the board model can configure the
> board-dependent hardware strapping.
>
> Almost all provided AST2400 reset values are specified by the datasheet.
> The notable exception is SOC_SCRATCH1, where we mark the DRAM as
> successfully initialised to avoid unnecessary dark corners in the SoC's
> u-boot support.

> +static Property aspeed_scu_properties[] = {
> +    DEFINE_PROP_UINT32("silicon-rev", AspeedSCUState, silicon_rev, 0),
> +    DEFINE_PROP_UINT32("hw-strap1", AspeedSCUState, hw_strap1, 0),
> +    DEFINE_PROP_UINT32("hw-strap2", AspeedSCUState, hw_strap1, 0),
> +    DEFINE_PROP_END_OF_LIST(),
> +};

You don't seem to specify in the board layer or the SoC layer
any of these except hw-strap1, so should the default values
for these really all be zero?

I suspect silicon-rev at least should either have a default
value specified here, or have the SoC layer specify it.
(It probably should not be specified at the board level.)

thanks
-- PMM

Reply via email to