Re: [Qemu-devel] [PATCH v3 1/3] hw/misc: Add a model for the ASPEED System Control Unit

[Andrew Jeffery <andrew@aj.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3329 bytes --]

On Mon, 2016-06-27 at 14:42 +0100, Peter Maydell wrote:
> On 24 June 2016 at 05:58, Andrew Jeffery <andrew@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.
> > 
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > Since v2:
> > 
> > * Fix mixing of offsets and register indexes
> > * Sanity check device property values
> > * Move trace event definition to hw/misc/trace-events
> > 
> > Since v1:
> > 
> > * Move reset values into SCU implementation (also make register defines private)
> > * Expose silicon-rev property which is used to select appropriate reset values
> > * Expose hw-strap1/hw-strap2 properties for board-specific SoC configuration
> > 
> >  hw/misc/Makefile.objs        |   1 +
> >  hw/misc/aspeed_scu.c         | 284 +++++++++++++++++++++++++++++++++++++++++++
> >  hw/misc/trace-events         |   3 +
> >  include/hw/misc/aspeed_scu.h |  34 ++++++
> >  4 files changed, 322 insertions(+)
> >  create mode 100644 hw/misc/aspeed_scu.c
> >  create mode 100644 include/hw/misc/aspeed_scu.h
> > 
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index ffb49c11aca6..54020aa06c00 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -52,3 +52,4 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o
> >  obj-$(CONFIG_EDU) += edu.o
> >  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
> >  obj-$(CONFIG_AUX) += aux.o
> > +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o
> > diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> > new file mode 100644
> > index 000000000000..ff231dbb3c17
> > --- /dev/null
> > +++ b/hw/misc/aspeed_scu.c
> > @@ -0,0 +1,284 @@
> > +/*
> > + * ASPEED System Control Unit
> > + *
> > + * Andrew Jeffery <andrew@aj.id.au>
> > + *
> > + * Copyright 2016 IBM Corp.
> > + *
> > + * This code is licensed under the GPL version 2 or later.  See
> > + * the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include 
> osdep.h provides this, you don't need to include it yourself.
> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> so if this is the only problem with the series I'll fix it up
> when I put it in target-arm.next.
> 

As far as I'm aware that's the only issue aside from the discussion on
patch 3/3.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]