Re: [PATCH 05/10] arm: allwinner-h3: add System Control module

2019-12-15 Thread Philippe Mathieu-Daudé

On 12/16/19 12:27 AM, Niek Linnenbank wrote:
On Fri, Dec 13, 2019 at 1:09 AM Philippe Mathieu-Daudé 
mailto:phi...@redhat.com>> wrote:


On 12/2/19 10:09 PM, Niek Linnenbank wrote:

[...]

 > +static const MemoryRegionOps allwinner_h3_syscon_ops = {
 > +    .read = allwinner_h3_syscon_read,
 > +    .write = allwinner_h3_syscon_write,
 > +    .endianness = DEVICE_NATIVE_ENDIAN,
 > +    .valid = {
 > +        .min_access_size = 4,
 > +        .max_access_size = 4,

Can you point me to the datasheet page that says this region is
restricted to 32-bit accesses? Maybe you want .valid -> .impl instead?

Hehe well here I can only give the same answer as for the SD/MMC driver: 
the datasheet
only provides the base address and register offsets, but nothing 
explicitely mentioned about alignment.

I do see that also for this device the registers are 32-bit aligned.

Does that mean I should change MemoryRegionOps to . impl instead?


No, keep them, but add ".impl.min_access_size = 4" (see answer to SD/MMC 
model patch).





Re: [PATCH 05/10] arm: allwinner-h3: add System Control module

2019-12-15 Thread Niek Linnenbank
On Fri, Dec 13, 2019 at 1:09 AM Philippe Mathieu-Daudé 
wrote:

> On 12/2/19 10:09 PM, Niek Linnenbank wrote:
> > The Allwinner H3 System on Chip has an System Control
> > module that provides system wide generic controls and
> > device information. This commit adds support for the
> > Allwinner H3 System Control module.
> >
> > Signed-off-by: Niek Linnenbank 
> > ---
> >   hw/arm/allwinner-h3.c |  11 ++
> >   hw/misc/Makefile.objs |   1 +
> >   hw/misc/allwinner-h3-syscon.c | 139 ++
> >   include/hw/arm/allwinner-h3.h |   2 +
> >   include/hw/misc/allwinner-h3-syscon.h |  43 
> >   5 files changed, 196 insertions(+)
> >   create mode 100644 hw/misc/allwinner-h3-syscon.c
> >   create mode 100644 include/hw/misc/allwinner-h3-syscon.h
> >
> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> > index afeb49c0ac..ebd8fde412 100644
> > --- a/hw/arm/allwinner-h3.c
> > +++ b/hw/arm/allwinner-h3.c
> > @@ -41,6 +41,9 @@ static void aw_h3_init(Object *obj)
> >
> >   sysbus_init_child_obj(obj, "ccu", >ccu, sizeof(s->ccu),
> > TYPE_AW_H3_CLK);
> > +
> > +sysbus_init_child_obj(obj, "syscon", >syscon, sizeof(s->syscon),
> > +  TYPE_AW_H3_SYSCON);
> >   }
> >
> >   static void aw_h3_realize(DeviceState *dev, Error **errp)
> > @@ -184,6 +187,14 @@ static void aw_h3_realize(DeviceState *dev, Error
> **errp)
> >   }
> >   sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, AW_H3_CCU_BASE);
> >
> > +/* System Control */
> > +object_property_set_bool(OBJECT(>syscon), true, "realized",
> );
> > +if (err) {
> > +error_propagate(errp, err);
> > +return;
> > +}
> > +sysbus_mmio_map(SYS_BUS_DEVICE(>syscon), 0, AW_H3_SYSCON_BASE);
> > +
> >   /* Universal Serial Bus */
> >   sysbus_create_simple(TYPE_AW_H3_EHCI, AW_H3_EHCI0_BASE,
> >s->irq[AW_H3_GIC_SPI_EHCI0]);
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 200ed44ce1..b234aefba5 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -29,6 +29,7 @@ common-obj-$(CONFIG_MACIO) += macio/
> >   common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
> >
> >   common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o
> > +common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
> >   common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
> >   common-obj-$(CONFIG_NSERIES) += cbus.o
> >   common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
> > diff --git a/hw/misc/allwinner-h3-syscon.c
> b/hw/misc/allwinner-h3-syscon.c
> > new file mode 100644
> > index 00..66bd518a05
> > --- /dev/null
> > +++ b/hw/misc/allwinner-h3-syscon.c
> > @@ -0,0 +1,139 @@
> > +/*
> > + * Allwinner H3 System Control emulation
> > + *
> > + * Copyright (C) 2019 Niek Linnenbank 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see  >.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/sysbus.h"
> > +#include "migration/vmstate.h"
> > +#include "qemu/log.h"
> > +#include "qemu/module.h"
> > +#include "hw/misc/allwinner-h3-syscon.h"
> > +
> > +/* SYSCON register offsets */
> > +#define REG_VER (0x24)  /* Version */
> > +#define REG_EMAC_PHY_CLK(0x30)  /* EMAC PHY Clock */
> > +#define REG_INDEX(offset)   (offset / sizeof(uint32_t))
> > +
> > +/* SYSCON register reset values */
> > +#define REG_VER_RST (0x0)
> > +#define REG_EMAC_PHY_CLK_RST(0x58000)
> > +
> > +static uint64_t allwinner_h3_syscon_read(void *opaque, hwaddr offset,
> > + unsigned size)
> > +{
> > +const AwH3SysconState *s = (AwH3SysconState *)opaque;
> > +const uint32_t idx = REG_INDEX(offset);
> > +
> > +if (idx >= AW_H3_SYSCON_REGS_NUM) {
> > +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
> > +  __func__, (uint32_t)offset);
> > +return 0;
> > +}
> > +
> > +return s->regs[idx];
> > +}
> > +
> > +static void allwinner_h3_syscon_write(void *opaque, hwaddr offset,
> > +  uint64_t val, unsigned size)
> > +{
> > +AwH3SysconState *s = (AwH3SysconState *)opaque;
> > +const uint32_t idx = REG_INDEX(offset);
> > +
> > +if (idx >= 

Re: [PATCH 05/10] arm: allwinner-h3: add System Control module

2019-12-12 Thread Philippe Mathieu-Daudé

On 12/2/19 10:09 PM, Niek Linnenbank wrote:

The Allwinner H3 System on Chip has an System Control
module that provides system wide generic controls and
device information. This commit adds support for the
Allwinner H3 System Control module.

Signed-off-by: Niek Linnenbank 
---
  hw/arm/allwinner-h3.c |  11 ++
  hw/misc/Makefile.objs |   1 +
  hw/misc/allwinner-h3-syscon.c | 139 ++
  include/hw/arm/allwinner-h3.h |   2 +
  include/hw/misc/allwinner-h3-syscon.h |  43 
  5 files changed, 196 insertions(+)
  create mode 100644 hw/misc/allwinner-h3-syscon.c
  create mode 100644 include/hw/misc/allwinner-h3-syscon.h

diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index afeb49c0ac..ebd8fde412 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -41,6 +41,9 @@ static void aw_h3_init(Object *obj)
  
  sysbus_init_child_obj(obj, "ccu", >ccu, sizeof(s->ccu),

TYPE_AW_H3_CLK);
+
+sysbus_init_child_obj(obj, "syscon", >syscon, sizeof(s->syscon),
+  TYPE_AW_H3_SYSCON);
  }
  
  static void aw_h3_realize(DeviceState *dev, Error **errp)

@@ -184,6 +187,14 @@ static void aw_h3_realize(DeviceState *dev, Error **errp)
  }
  sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, AW_H3_CCU_BASE);
  
+/* System Control */

+object_property_set_bool(OBJECT(>syscon), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>syscon), 0, AW_H3_SYSCON_BASE);
+
  /* Universal Serial Bus */
  sysbus_create_simple(TYPE_AW_H3_EHCI, AW_H3_EHCI0_BASE,
   s->irq[AW_H3_GIC_SPI_EHCI0]);
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 200ed44ce1..b234aefba5 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -29,6 +29,7 @@ common-obj-$(CONFIG_MACIO) += macio/
  common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
  
  common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o

+common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
  common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
  common-obj-$(CONFIG_NSERIES) += cbus.o
  common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
diff --git a/hw/misc/allwinner-h3-syscon.c b/hw/misc/allwinner-h3-syscon.c
new file mode 100644
index 00..66bd518a05
--- /dev/null
+++ b/hw/misc/allwinner-h3-syscon.c
@@ -0,0 +1,139 @@
+/*
+ * Allwinner H3 System Control emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/misc/allwinner-h3-syscon.h"
+
+/* SYSCON register offsets */
+#define REG_VER (0x24)  /* Version */
+#define REG_EMAC_PHY_CLK(0x30)  /* EMAC PHY Clock */
+#define REG_INDEX(offset)   (offset / sizeof(uint32_t))
+
+/* SYSCON register reset values */
+#define REG_VER_RST (0x0)
+#define REG_EMAC_PHY_CLK_RST(0x58000)
+
+static uint64_t allwinner_h3_syscon_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+const AwH3SysconState *s = (AwH3SysconState *)opaque;
+const uint32_t idx = REG_INDEX(offset);
+
+if (idx >= AW_H3_SYSCON_REGS_NUM) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
+  __func__, (uint32_t)offset);
+return 0;
+}
+
+return s->regs[idx];
+}
+
+static void allwinner_h3_syscon_write(void *opaque, hwaddr offset,
+  uint64_t val, unsigned size)
+{
+AwH3SysconState *s = (AwH3SysconState *)opaque;
+const uint32_t idx = REG_INDEX(offset);
+
+if (idx >= AW_H3_SYSCON_REGS_NUM) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write offset 0x%04x\n",
+  __func__, (uint32_t)offset);
+return;
+}
+
+switch (offset) {
+case REG_VER:   /* Version */
+break;
+default:
+s->regs[idx] = (uint32_t) val;
+break;
+}
+}
+
+static const MemoryRegionOps allwinner_h3_syscon_ops = {
+.read = allwinner_h3_syscon_read,
+.write = allwinner_h3_syscon_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size 

[PATCH 05/10] arm: allwinner-h3: add System Control module

2019-12-02 Thread Niek Linnenbank
The Allwinner H3 System on Chip has an System Control
module that provides system wide generic controls and
device information. This commit adds support for the
Allwinner H3 System Control module.

Signed-off-by: Niek Linnenbank 
---
 hw/arm/allwinner-h3.c |  11 ++
 hw/misc/Makefile.objs |   1 +
 hw/misc/allwinner-h3-syscon.c | 139 ++
 include/hw/arm/allwinner-h3.h |   2 +
 include/hw/misc/allwinner-h3-syscon.h |  43 
 5 files changed, 196 insertions(+)
 create mode 100644 hw/misc/allwinner-h3-syscon.c
 create mode 100644 include/hw/misc/allwinner-h3-syscon.h

diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index afeb49c0ac..ebd8fde412 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -41,6 +41,9 @@ static void aw_h3_init(Object *obj)
 
 sysbus_init_child_obj(obj, "ccu", >ccu, sizeof(s->ccu),
   TYPE_AW_H3_CLK);
+
+sysbus_init_child_obj(obj, "syscon", >syscon, sizeof(s->syscon),
+  TYPE_AW_H3_SYSCON);
 }
 
 static void aw_h3_realize(DeviceState *dev, Error **errp)
@@ -184,6 +187,14 @@ static void aw_h3_realize(DeviceState *dev, Error **errp)
 }
 sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, AW_H3_CCU_BASE);
 
+/* System Control */
+object_property_set_bool(OBJECT(>syscon), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>syscon), 0, AW_H3_SYSCON_BASE);
+
 /* Universal Serial Bus */
 sysbus_create_simple(TYPE_AW_H3_EHCI, AW_H3_EHCI0_BASE,
  s->irq[AW_H3_GIC_SPI_EHCI0]);
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 200ed44ce1..b234aefba5 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -29,6 +29,7 @@ common-obj-$(CONFIG_MACIO) += macio/
 common-obj-$(CONFIG_IVSHMEM_DEVICE) += ivshmem.o
 
 common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-clk.o
+common-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3-syscon.o
 common-obj-$(CONFIG_REALVIEW) += arm_sysctl.o
 common-obj-$(CONFIG_NSERIES) += cbus.o
 common-obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
diff --git a/hw/misc/allwinner-h3-syscon.c b/hw/misc/allwinner-h3-syscon.c
new file mode 100644
index 00..66bd518a05
--- /dev/null
+++ b/hw/misc/allwinner-h3-syscon.c
@@ -0,0 +1,139 @@
+/*
+ * Allwinner H3 System Control emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/misc/allwinner-h3-syscon.h"
+
+/* SYSCON register offsets */
+#define REG_VER (0x24)  /* Version */
+#define REG_EMAC_PHY_CLK(0x30)  /* EMAC PHY Clock */
+#define REG_INDEX(offset)   (offset / sizeof(uint32_t))
+
+/* SYSCON register reset values */
+#define REG_VER_RST (0x0)
+#define REG_EMAC_PHY_CLK_RST(0x58000)
+
+static uint64_t allwinner_h3_syscon_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+const AwH3SysconState *s = (AwH3SysconState *)opaque;
+const uint32_t idx = REG_INDEX(offset);
+
+if (idx >= AW_H3_SYSCON_REGS_NUM) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read offset 0x%04x\n",
+  __func__, (uint32_t)offset);
+return 0;
+}
+
+return s->regs[idx];
+}
+
+static void allwinner_h3_syscon_write(void *opaque, hwaddr offset,
+  uint64_t val, unsigned size)
+{
+AwH3SysconState *s = (AwH3SysconState *)opaque;
+const uint32_t idx = REG_INDEX(offset);
+
+if (idx >= AW_H3_SYSCON_REGS_NUM) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write offset 0x%04x\n",
+  __func__, (uint32_t)offset);
+return;
+}
+
+switch (offset) {
+case REG_VER:   /* Version */
+break;
+default:
+s->regs[idx] = (uint32_t) val;
+break;
+}
+}
+
+static const MemoryRegionOps allwinner_h3_syscon_ops = {
+.read = allwinner_h3_syscon_read,
+.write = allwinner_h3_syscon_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+.unaligned = false
+}
+};