[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/3] hw/misc: Add a model for the ASPEED Syst
From: |
Andrew Jeffery |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/3] hw/misc: Add a model for the ASPEED System Control Unit |
Date: |
Fri, 24 Jun 2016 11:34:31 +0930 |
On Thu, 2016-06-23 at 18:42 +0100, Peter Maydell wrote:
> On 23 June 2016 at 03:15, Andrew Jeffery <address@hidden> 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
- Re: [Qemu-devel] [PATCH v2 3/3] palmetto-bmc: Configure the SCU's hardware strapping register, (continued)
[Qemu-devel] [PATCH v2 1/3] hw/misc: Add a model for the ASPEED System Control Unit, Andrew Jeffery, 2016/06/22