[PATCH qemu 1/2] hw/misc: Add a model for the ASPEED System Control Unit
Andrew Jeffery
andrew at aj.id.au
Fri Jun 10 11:26:02 AEST 2016
On Thu, 2016-06-09 at 13:29 +0200, Cédric Le Goater wrote:
> On 06/09/2016 10:14 AM, Andrew Jeffery 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. Object properties are exposed so
> > that the integrating SoC model can configure the appropriate reset
> > values.
> >
> > Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> > ---
> > hw/misc/Makefile.objs | 1 +
> > hw/misc/aspeed_scu.c | 295 +++++++++++++++++++++++++++++++++++++++++++
> > include/hw/misc/aspeed_scu.h | 105 +++++++++++++++
> > trace-events | 3 +
> > 4 files changed, 404 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 bc0dd2cc7567..4895e950b377 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -51,3 +51,4 @@ obj-$(CONFIG_MIPS_ITU) += mips_itu.o
> > obj-$(CONFIG_PVPANIC) += pvpanic.o
> > obj-$(CONFIG_EDU) += edu.o
> > obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.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..ae5a4c590bb6
> > --- /dev/null
> > +++ b/hw/misc/aspeed_scu.c
> > @@ -0,0 +1,295 @@
> > +/*
> > + * ASPEED System Control Unit
> > + *
> > + * Andrew Jeffery <andrew at 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
> > +#include "hw/misc/aspeed_scu.h"
> > +#include "hw/qdev-properties.h"
> > +#include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > +#include "qemu/bitops.h"
> > +#include "trace.h"
> > +
> > +#define SCU_KEY 0x1688A8A8
> > +#define SCU_IO_REGION_SIZE 0x20000
> > +
> > +#define TO_REG(o) ((o) >> 2)
> > +#define TO_REG_ID(o) [TO_REG(o)] = stringify(o)
> > +
> > +static const char *aspeed_scu_reg_ids[ASPEED_SCU_NR_REGS] = {
> > + TO_REG_ID(ASPEED_SCU_PROT_KEY),
> > + TO_REG_ID(ASPEED_SCU_SYS_RST_CTRL),
> > + TO_REG_ID(ASPEED_SCU_CLK_SEL),
> > + TO_REG_ID(ASPEED_SCU_CLK_STOP_CTRL),
> > + TO_REG_ID(ASPEED_SCU_FREQ_CNTR_CTRL),
> > + TO_REG_ID(ASPEED_SCU_FREQ_CNTR_EVAL),
> > + TO_REG_ID(ASPEED_SCU_IRQ_CTRL),
> > + TO_REG_ID(ASPEED_SCU_D2PLL_PARAM),
> > + TO_REG_ID(ASPEED_SCU_MPLL_PARAM),
> > + TO_REG_ID(ASPEED_SCU_HPLL_PARAM),
> > + TO_REG_ID(ASPEED_SCU_FREQ_CNTR_RANGE),
> > + TO_REG_ID(ASPEED_SCU_MISC_CTRL1),
> > + TO_REG_ID(ASPEED_SCU_PCI_CTRL1),
> > + TO_REG_ID(ASPEED_SCU_PCI_CTRL2),
> > + TO_REG_ID(ASPEED_SCU_PCI_CTRL3),
> > + TO_REG_ID(ASPEED_SCU_SYS_RST_CTRL),
> > + TO_REG_ID(ASPEED_SCU_SOC_SCRATCH1),
> > + TO_REG_ID(ASPEED_SCU_SOC_SCRATCH2),
> > + TO_REG_ID(ASPEED_SCU_MAC_CLK_DELAY),
> > + TO_REG_ID(ASPEED_SCU_MISC_CTRL2),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH1),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH2),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH3),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH4),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH5),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH6),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH7),
> > + TO_REG_ID(ASPEED_SCU_VGA_SCRATCH8),
> > + TO_REG_ID(ASPEED_SCU_HW_STRAP1),
> > + TO_REG_ID(ASPEED_SCU_RNG_CTRL),
> > + TO_REG_ID(ASPEED_SCU_RNG_DATA),
> > + TO_REG_ID(ASPEED_SCU_REV_ID),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL1),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL2),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL3),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL4),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL5),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL6),
> > + TO_REG_ID(ASPEED_SCU_WDT_RST_CTRL),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL7),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL8),
> > + TO_REG_ID(ASPEED_SCU_PINMUX_CTRL9),
> > + TO_REG_ID(ASPEED_SCU_WAKEUP_EN),
> > + TO_REG_ID(ASPEED_SCU_WAKEUP_CTRL),
> > + TO_REG_ID(ASPEED_SCU_HW_STRAP2),
> > + TO_REG_ID(ASPEED_SCU_FREE_CNTR4),
> > + TO_REG_ID(ASPEED_SCU_FREE_CNTR4_EXT),
> > + TO_REG_ID(ASPEED_SCU_CPU2_CTRL),
> > + TO_REG_ID(ASPEED_SCU_CPU2_BASE_SEG1),
> > + TO_REG_ID(ASPEED_SCU_CPU2_BASE_SEG2),
> > + TO_REG_ID(ASPEED_SCU_CPU2_BASE_SEG3),
> > + TO_REG_ID(ASPEED_SCU_CPU2_BASE_SEG4),
> > + TO_REG_ID(ASPEED_SCU_CPU2_BASE_SEG5),
> > + TO_REG_ID(ASPEED_SCU_CPU2_CACHE_CTRL),
> > + TO_REG_ID(ASPEED_SCU_UART_HPLL_CLK),
> > + TO_REG_ID(ASPEED_SCU_PCIE_CTRL),
> > + TO_REG_ID(ASPEED_SCU_BMC_MMIO_CTRL),
> > + TO_REG_ID(ASPEED_SCU_RELOC_DECODE_BASE1),
> > + TO_REG_ID(ASPEED_SCU_RELOC_DECODE_BASE2),
> > + TO_REG_ID(ASPEED_SCU_MAILBOX_DECODE_BASE),
> > + TO_REG_ID(ASPEED_SCU_SRAM_DECODE_BASE1),
> > + TO_REG_ID(ASPEED_SCU_SRAM_DECODE_BASE2),
> > + TO_REG_ID(ASPEED_SCU_BMC_REV_ID),
> > + TO_REG_ID(ASPEED_SCU_BMC_DEV_ID),
> > +};
> I would start with a smaller set that we know uboot and the kernel use,
> this is minor.
Given that I provided all the defines for register offsets, I figured I
should provide all the property IDs. As it's done now, I don't see any
benefit to removing it. Other than to change the approach, which is
discussed below.
>
> >
> > +void aspeed_scu_configure_reset(AspeedSCUState *scu,
> > + const AspeedSCUResetCfg vals[], int n, Error **errp)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < n; i++) {
> > + const char *name = aspeed_scu_reg_ids[TO_REG(vals[i].offset)];
> > +
> > + if (name) {
> > + object_property_set_int(OBJECT(scu), vals[i].val, name, errp);
> > + if (*errp) {
> > + return;
> > + }
> > + }
> > + }
> > +}
> > +
> > +static void aspeed_scu_get_reset(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + AspeedSCUState *s = ASPEED_SCU(obj);
> > + uint32_t value;
> > + int offset, reg;
> > +
> > + if (sscanf(name, "0x%x", &offset) != 1) {
> > + error_setg(errp, "Error reading %s", name);
> > + return;
> > + }
> I thought the name of the property was "ASPEED_SCU_PROT_KEY" according to the
> object_property_add() below ?
Right. This was the slightly abusive bit. Partly inspired by your
tmp421 patch, the register of interest is derived from the property
name. In fact, here, the property name is simply the string
representation of the hexadecimal offset (via stringify() in the
TO_REG_ID() macro). This is mostly hidden by the aspeed_scu_reg_ids
lookup, which avoids dynamic conversion between integers and strings
(sprintf, asprintf and such).
I'm not at all confident this will be swallowed by upstream, which was
another reason for sending here first :)
So, is there a better approach? object_property_*()s all seemed to
require const char *s. We could just not use properties and assign to
the array member directly, as the AspeedSCUState type isn't opaque in
ast2400.c, but from a quick survey it seemed object_property_*() was
the way to go.
>
> >
> > + reg = TO_REG(offset);
> > +
> > + if (reg > ASPEED_SCU_NR_REGS) {
> > + error_setg(errp, "Invalid register ID: %s", name);
> > + return;
> > + }
> > +
> > + value = s->reset[reg];
> > +
> > + visit_type_uint32(v, name, &value, errp);
> > +}
> > +
> > +static void aspeed_scu_set_reset(Object *obj, Visitor *v, const char *name,
> > + void *opaque, Error **errp)
> > +{
> > + AspeedSCUState *s = ASPEED_SCU(obj);
> > + Error *local_err = NULL;
> > + uint32_t value;
> > + int offset, reg;
> > +
> > + visit_type_uint32(v, name, &value, &local_err);
> > +
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > +
> > + if (sscanf(name, "0x%x", &offset) != 1) {
> > + error_setg(errp, "Error reading %s", name);
> > + }
> same comment.
Same as above
>
> The rest looks fine.
>
> Reviewed-by: Cédric Le Goater <clg at kaod.org>
Cheers,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160610/891bd622/attachment-0001.sig>
More information about the openbmc
mailing list