On Thu, 2016-06-23 at 18:42 +0100, Peter Maydell wrote:
> 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?Both strap register default values are 0 according to the datasheet. > > 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.) I intended to set silicon-rev in the SoC layer, so I'll fix patch 2/3. With the addition of sanity checking in the SCU's realise() we'll catch the case where it's an invalid value (eg 0). I don't think it's right to plow ahead with an unexpected configuration if a chosen default value doesn't match the SoC at hand. Maybe I shouldn't send patches with a heavy head cold :/ Cheers, Andrew
signature.asc
Description: This is a digitally signed message part
