[Qemu-devel] [PATCH 00/17] onenand, omap_gpmc fixes, features

[Qemu-devel] [PATCH 00/17] onenand, omap_gpmc fixes, features

[Peter Maydell]

This patchset is primarily features and bugfixes for the
omap_gpmc device from the Meego tree.

We start by adding a new sysbus function to get the MemoryRegion*
for a sysbus MMIO region. This was discussed in
 http://www.mail-archive.com/kvm@vger.kernel.org/msg59535.html
and as noted there can also be used to clean up other things,
so it's not just an omap_gpmc thing.

The bulk of the onenand qdevification patch was in an earlier series
I posted; this update makes it work with MemoryRegions.

The patches from "omap_gpmc: Take omap_mpu_state* in omap_gpmc_init"
onwards are omap3 preparation; the features they add aren't used in
the omap2 models currently in master. However I'd rather get things
reviewed in more manageable chunks than save them all up for when I
have omap3.c ready for commit...

Juha Riihimäki (5):
  hw/onenand: Qdevify
  hw/onenand: Minor spacing fixes
  omap_gpmc: Take omap_mpu_state* in omap_gpmc_init
  omap_gpmc: Calculate revision from OMAP model
  omap_gpmc: Accept a zero mask field on omap3630

Peter Maydell (12):
  hw/sysbus: Add sysbus_mmio_get_region()
  omap_gpmc: Clean up omap_gpmc_attach MemoryRegion conversion
  omap_gpmc: Refactor omap_gpmc_cs_map and omap_gpmc_cs_unmap
  omap_gpmc: GPMC_IRQSTATUS is write-one-to-clear
  omap_gpmc: Wire up the GPMC IRQ correctly
  omap_gpmc: Fix handling of FIFOTHRESHOLDSTATUS bit
  omap_gpmc: Reindent misindented switch statements
  omap_gpmc: Support NAND devices
  hw/omap.h: Add OMAP 3630 to omap_mpu_model enumeration
  omap_gpmc: Pull prefetch engine data into sub-struct
  omap: Wire up the DMA request line to the GPMC
  omap_gpmc: Implement prefetch engine

 hw/flash.h     |   10 +-
 hw/nseries.c   |   13 +-
 hw/omap.h      |   14 +-
 hw/omap2.c     |    3 +-
 hw/omap_gpmc.c |  713 +++++++++++++++++++++++++++++++++++++++++++++-----------
 hw/onenand.c   |  179 +++++++++++----
 hw/sysbus.c    |    5 +
 hw/sysbus.h    |    1 +
 8 files changed, 740 insertions(+), 198 deletions(-)

[Qemu-devel] [PATCH 01/17] hw/sysbus: Add sysbus_mmio_get_region()

[Peter Maydell]

Add a sysbus_mmio_get_region() which allows users of sysbus
devices to turn a (SysBusDevice*, mmioidx) tuple into a
MemoryRegion*. This enables some useful simplifications of
devices which pass through another device's mmio region
(either directly or by implementing some kind of memory
controller device).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sysbus.c |    5 +++++
 hw/sysbus.h |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/sysbus.c b/hw/sysbus.c
index 6e89f06..4fab5a4 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -131,6 +131,11 @@ void sysbus_init_mmio_region(SysBusDevice *dev, MemoryRegion *memory)
     dev->mmio[n].memory = memory;
 }
 
+MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n)
+{
+    return dev->mmio[n].memory;
+}
+
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size)
 {
     pio_addr_t i;
diff --git a/hw/sysbus.h b/hw/sysbus.h
index b3e1f99..6c36537 100644
--- a/hw/sysbus.h
+++ b/hw/sysbus.h
@@ -50,6 +50,7 @@ void sysbus_init_mmio(SysBusDevice *dev, target_phys_addr_t size,
 void sysbus_init_mmio_cb2(SysBusDevice *dev,
                           mmio_mapfunc cb, mmio_mapfunc unmap);
 void sysbus_init_mmio_region(SysBusDevice *dev, MemoryRegion *memory);
+MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n);
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p);
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target);
 void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size);
-- 
1.7.1

[Qemu-devel] [PATCH 02/17] hw/onenand: Qdevify

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Qdevify the ONENAND device.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
[Riku Voipio: Fixes and restructuring patchset]
Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
[Peter Maydell: More fixes and cleanups for upstream submission]
Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>
---
 hw/flash.h   |   10 ++--
 hw/nseries.c |    9 ++-
 hw/onenand.c |  165 +++++++++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 138 insertions(+), 46 deletions(-)

diff --git a/hw/flash.h b/hw/flash.h
index 7fb012b..de3c5c3 100644
--- a/hw/flash.h
+++ b/hw/flash.h
@@ -42,12 +42,10 @@ uint32_t nand_getbuswidth(DeviceState *dev);
 #define NAND_MFR_MICRON		0x2c
 
 /* onenand.c */
-void onenand_base_update(void *opaque, target_phys_addr_t new);
-void onenand_base_unmap(void *opaque);
-void *onenand_init(BlockDriverState *bdrv,
-                uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
-                int regshift, qemu_irq irq);
-void *onenand_raw_otp(void *opaque);
+DeviceState *onenand_init(BlockDriverState *bdrv,
+                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
+                          int regshift, qemu_irq irq);
+void *onenand_raw_otp(DeviceState *onenand_device);
 
 /* ecc.c */
 typedef struct {
diff --git a/hw/nseries.c b/hw/nseries.c
index f7aae7a..e61014c 100644
--- a/hw/nseries.c
+++ b/hw/nseries.c
@@ -33,6 +33,7 @@
 #include "loader.h"
 #include "blockdev.h"
 #include "tusb6010.h"
+#include "sysbus.h"
 
 /* Nokia N8x0 support */
 struct n800_s {
@@ -52,7 +53,7 @@ struct n800_s {
     TUSBState *usb;
     void *retu;
     void *tahvo;
-    void *nand;
+    DeviceState *nand;
 };
 
 /* GPIO pins */
@@ -172,8 +173,10 @@ static void n8x0_nand_setup(struct n800_s *s)
     s->nand = onenand_init(dinfo ? dinfo->bdrv : 0,
                     NAND_MFR_SAMSUNG, 0x48, 0, 1,
                     qdev_get_gpio_in(s->cpu->gpio, N8X0_ONENAND_GPIO));
-    omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS, 0, onenand_base_update,
-                    onenand_base_unmap, s->nand);
+    omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS,
+                     sysbus_mmio_get_region(sysbus_from_qdev(s->nand), 0),
+                     NULL, NULL,
+                     s->nand);
     otp_region = onenand_raw_otp(s->nand);
 
     memcpy(otp_region + 0x000, n8x0_cal_wlan_mac, sizeof(n8x0_cal_wlan_mac));
diff --git a/hw/onenand.c b/hw/onenand.c
index 00276a0..15fffc8 100644
--- a/hw/onenand.c
+++ b/hw/onenand.c
@@ -25,6 +25,7 @@
 #include "blockdev.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "sysbus.h"
 
 /* 11 for 2kB-page OneNAND ("2nd generation") and 10 for 1kB-page chips */
 #define PAGE_SHIFT	11
@@ -33,6 +34,7 @@
 #define BLOCK_SHIFT	(PAGE_SHIFT + 6)
 
 typedef struct {
+    SysBusDevice busdev;
     struct {
         uint16_t man;
         uint16_t dev;
@@ -49,6 +51,7 @@ typedef struct {
     uint8_t *current;
     MemoryRegion ram;
     MemoryRegion mapped_ram;
+    uint8_t current_direction;
     uint8_t *boot[2];
     uint8_t *data[2][2];
     MemoryRegion iomem;
@@ -120,27 +123,72 @@ static void onenand_mem_setup(OneNANDState *s)
                                         1);
 }
 
-void onenand_base_update(void *opaque, target_phys_addr_t new)
+static void onenand_intr_update(OneNANDState *s)
 {
-    OneNANDState *s = (OneNANDState *) opaque;
-
-    s->base = new;
-
-    memory_region_add_subregion(get_system_memory(), s->base, &s->container);
+    qemu_set_irq(s->intr, ((s->intstatus >> 15) ^ (~s->config[0] >> 6)) & 1);
 }
 
-void onenand_base_unmap(void *opaque)
+static void onenand_pre_save(void *opaque)
 {
-    OneNANDState *s = (OneNANDState *) opaque;
-
-    memory_region_del_subregion(get_system_memory(), &s->container);
+    OneNANDState *s = opaque;
+    if (s->current == s->otp) {
+        s->current_direction = 1;
+    } else if (s->current == s->image) {
+        s->current_direction = 2;
+    } else {
+        s->current_direction = 0;
+    }
 }
 
-static void onenand_intr_update(OneNANDState *s)
+static int onenand_post_load(void *opaque, int version_id)
 {
-    qemu_set_irq(s->intr, ((s->intstatus >> 15) ^ (~s->config[0] >> 6)) & 1);
+    OneNANDState *s = opaque;
+    switch (s->current_direction) {
+    case 0:
+        break;
+    case 1:
+        s->current = s->otp;
+        break;
+    case 2:
+        s->current = s->image;
+        break;
+    default:
+        return -1;
+    }
+    onenand_intr_update(s);
+    return 0;
 }
 
+static const VMStateDescription vmstate_onenand = {
+    .name = "onenand",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .pre_save = onenand_pre_save,
+    .post_load = onenand_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(current_direction, OneNANDState),
+        VMSTATE_INT32(cycle, OneNANDState),
+        VMSTATE_INT32(otpmode, OneNANDState),
+        VMSTATE_UINT16_ARRAY(addr, OneNANDState, 8),
+        VMSTATE_UINT16_ARRAY(unladdr, OneNANDState, 8),
+        VMSTATE_INT32(bufaddr, OneNANDState),
+        VMSTATE_INT32(count, OneNANDState),
+        VMSTATE_UINT16(command, OneNANDState),
+        VMSTATE_UINT16_ARRAY(config, OneNANDState, 2),
+        VMSTATE_UINT16(status, OneNANDState),
+        VMSTATE_UINT16(intstatus, OneNANDState),
+        VMSTATE_UINT16(wpstatus, OneNANDState),
+        VMSTATE_INT32(secs_cur, OneNANDState),
+        VMSTATE_PARTIAL_VBUFFER(blockwp, OneNANDState, blocks),
+        VMSTATE_UINT8(ecc.cp, OneNANDState),
+        VMSTATE_UINT16_ARRAY(ecc.lp, OneNANDState, 2),
+        VMSTATE_UINT16(ecc.count, OneNANDState),
+        VMSTATE_BUFFER_UNSAFE(otp, OneNANDState, 0, ((64 + 2) << PAGE_SHIFT)),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 /* Hot reset (Reset OneNAND command) or warm reset (RP pin low) */
 static void onenand_reset(OneNANDState *s, int cold)
 {
@@ -167,11 +215,17 @@ static void onenand_reset(OneNANDState *s, int cold)
         /* Lock the whole flash */
         memset(s->blockwp, ONEN_LOCK_LOCKED, s->blocks);
 
-        if (s->bdrv && bdrv_read(s->bdrv, 0, s->boot[0], 8) < 0)
-            hw_error("%s: Loading the BootRAM failed.\n", __FUNCTION__);
+        if (s->bdrv_cur && bdrv_read(s->bdrv_cur, 0, s->boot[0], 8) < 0) {
+            hw_error("%s: Loading the BootRAM failed.\n", __func__);
+        }
     }
 }
 
+static void onenand_system_reset(DeviceState *dev)
+{
+    onenand_reset(FROM_SYSBUS(OneNANDState, sysbus_from_qdev(dev)), 1);
+}
+
 static inline int onenand_load_main(OneNANDState *s, int sec, int secn,
                 void *dest)
 {
@@ -326,7 +380,7 @@ fail:
     return 1;
 }
 
-static void onenand_command(OneNANDState *s, int cmd)
+static void onenand_command(OneNANDState *s)
 {
     int b;
     int sec;
@@ -346,7 +400,7 @@ static void onenand_command(OneNANDState *s, int cmd)
             s->data[(s->bufaddr >> 2) & 1][1] : s->boot[1];	\
     buf += (s->bufaddr & 3) << 4;
 
-    switch (cmd) {
+    switch (s->command) {
     case 0x00:	/* Load single/multiple sector data unit into buffer */
         SETADDR(ONEN_BUF_BLOCK, ONEN_BUF_PAGE)
 
@@ -527,7 +581,7 @@ static void onenand_command(OneNANDState *s, int cmd)
         s->status |= ONEN_ERR_CMD;
         s->intstatus |= ONEN_INT;
         fprintf(stderr, "%s: unknown OneNAND command %x\n",
-                        __FUNCTION__, cmd);
+                        __func__, s->command);
     }
 
     onenand_intr_update(s);
@@ -659,7 +713,7 @@ static void onenand_write(void *opaque, target_phys_addr_t addr,
         if (s->intstatus & (1 << 15))
             break;
         s->command = value;
-        onenand_command(s, s->command);
+        onenand_command(s);
         break;
     case 0xf221:	/* System Configuration 1 */
         s->config[0] = value;
@@ -700,30 +754,25 @@ static const MemoryRegionOps onenand_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void *onenand_init(BlockDriverState *bdrv,
-                uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
-                int regshift, qemu_irq irq)
+static int onenand_initfn(SysBusDevice *dev)
 {
-    OneNANDState *s = (OneNANDState *) g_malloc0(sizeof(*s));
-    uint32_t size = 1 << (24 + ((dev_id >> 4) & 7));
+    OneNANDState *s = (OneNANDState *)dev;
+    uint32_t size = 1 << (24 + ((s->id.dev >> 4) & 7));
     void *ram;
-
-    s->shift = regshift;
-    s->intr = irq;
+    s->base = (target_phys_addr_t)-1;
     s->rdy = NULL;
-    s->id.man = man_id;
-    s->id.dev = dev_id;
-    s->id.ver = ver_id;
     s->blocks = size >> BLOCK_SHIFT;
     s->secs = size >> 9;
     s->blockwp = g_malloc(s->blocks);
-    s->density_mask = (dev_id & 0x08) ? (1 << (6 + ((dev_id >> 4) & 7))) : 0;
+    s->density_mask = (s->id.dev & 0x08)
+        ? (1 << (6 + ((s->id.dev >> 4) & 7))) : 0;
     memory_region_init_io(&s->iomem, &onenand_ops, s, "onenand",
                           0x10000 << s->shift);
-    s->bdrv = bdrv;
     if (!s->bdrv) {
         s->image = memset(g_malloc(size + (size >> 5)),
-                        0xff, size + (size >> 5));
+                          0xff, size + (size >> 5));
+    } else {
+        s->bdrv_cur = s->bdrv;
     }
     s->otp = memset(g_malloc((64 + 2) << PAGE_SHIFT),
                     0xff, (64 + 2) << PAGE_SHIFT);
@@ -736,15 +785,57 @@ void *onenand_init(BlockDriverState *bdrv,
     s->data[1][0] = ram + ((0x0200 + (1 << (PAGE_SHIFT - 1))) << s->shift);
     s->data[1][1] = ram + ((0x8010 + (1 << (PAGE_SHIFT - 6))) << s->shift);
     onenand_mem_setup(s);
+    sysbus_init_irq(dev, &s->intr);
+    sysbus_init_mmio_region(dev, &s->container);
+    vmstate_register(&dev->qdev,
+                     ((s->shift & 0x7f) << 24)
+                     | ((s->id.man & 0xff) << 16)
+                     | ((s->id.dev & 0xff) << 8)
+                     | (s->id.ver & 0xff),
+                     &vmstate_onenand, s);
+    return 0;
+}
 
-    onenand_reset(s, 1);
+static SysBusDeviceInfo onenand_info = {
+    .init = onenand_initfn,
+    .qdev.name = "onenand",
+    .qdev.size = sizeof(OneNANDState),
+    .qdev.reset = onenand_system_reset,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_UINT16("manufacturer_id", OneNANDState, id.man, 0),
+        DEFINE_PROP_UINT16("device_id", OneNANDState, id.dev, 0),
+        DEFINE_PROP_UINT16("version_id", OneNANDState, id.ver, 0),
+        DEFINE_PROP_INT32("shift", OneNANDState, shift, 0),
+        DEFINE_PROP_DRIVE("drive", OneNANDState, bdrv),
+        DEFINE_PROP_END_OF_LIST()
+    }
+};
 
-    return s;
+static void onenand_register_device(void)
+{
+    sysbus_register_withprop(&onenand_info);
 }
 
-void *onenand_raw_otp(void *opaque)
+DeviceState *onenand_init(BlockDriverState *bdrv,
+                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
+                          int regshift, qemu_irq irq)
 {
-    OneNANDState *s = (OneNANDState *) opaque;
+    DeviceState *dev = qdev_create(NULL, "onenand");
+    qdev_prop_set_uint16(dev, "manufacturer_id", man_id);
+    qdev_prop_set_uint16(dev, "device_id", dev_id);
+    qdev_prop_set_uint16(dev, "version_id", ver_id);
+    qdev_prop_set_int32(dev, "shift", regshift);
+    if (bdrv) {
+        qdev_prop_set_drive_nofail(dev, "drive", bdrv);
+    }
+    qdev_init_nofail(dev);
+    sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
+    return dev;
+}
 
-    return s->otp;
+void *onenand_raw_otp(DeviceState *onenand_device)
+{
+    return FROM_SYSBUS(OneNANDState, sysbus_from_qdev(onenand_device))->otp;
 }
+
+device_init(onenand_register_device)
-- 
1.7.1

[Qemu-devel] [PATCH 03/17] hw/onenand: Minor spacing fixes

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Minor whitespace-only cleanup (separated out from the qdevifying
patch for clarity).

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
[Riku Voipio: Fixes and restructuring patchset]
Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
[Peter Maydell: More fixes and cleanups for upstream submission]
Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>
---
 hw/onenand.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/onenand.c b/hw/onenand.c
index 15fffc8..1683aeb 100644
--- a/hw/onenand.c
+++ b/hw/onenand.c
@@ -245,8 +245,8 @@ static inline int onenand_prog_main(OneNANDState *s, int sec, int secn,
     int result = 0;
 
     if (secn > 0) {
-        uint32_t size = (uint32_t) secn * 512;
-        const uint8_t *sp = (const uint8_t *) src;
+        uint32_t size = (uint32_t)secn * 512;
+        const uint8_t *sp = (const uint8_t *)src;
         uint8_t *dp = 0;
         if (s->bdrv_cur) {
             dp = g_malloc(size);
@@ -257,7 +257,7 @@ static inline int onenand_prog_main(OneNANDState *s, int sec, int secn,
             if (sec + secn > s->secs_cur) {
                 result = 1;
             } else {
-                dp = (uint8_t *) s->current + (sec << 9);
+                dp = (uint8_t *)s->current + (sec << 9);
             }
         }
         if (!result) {
@@ -299,13 +299,13 @@ static inline int onenand_prog_spare(OneNANDState *s, int sec, int secn,
 {
     int result = 0;
     if (secn > 0) {
-        const uint8_t *sp = (const uint8_t *) src;
+        const uint8_t *sp = (const uint8_t *)src;
         uint8_t *dp = 0, *dpp = 0;
         if (s->bdrv_cur) {
             dp = g_malloc(512);
             if (!dp || bdrv_read(s->bdrv_cur,
-                                s->secs_cur + (sec >> 5),
-                                dp, 1) < 0) {
+                                 s->secs_cur + (sec >> 5),
+                                 dp, 1) < 0) {
                 result = 1;
             } else {
                 dpp = dp + ((sec & 31) << 4);
@@ -324,7 +324,7 @@ static inline int onenand_prog_spare(OneNANDState *s, int sec, int secn,
             }
             if (s->bdrv_cur) {
                 result = bdrv_write(s->bdrv_cur, s->secs_cur + (sec >> 5),
-                                dp, 1) < 0;
+                                    dp, 1) < 0;
             }
         }
         if (dp) {
-- 
1.7.1

[Qemu-devel] [PATCH 04/17] omap_gpmc: Clean up omap_gpmc_attach MemoryRegion conversion

[Peter Maydell]

Now that all callers of omap_gpmc_attach pass in a MemoryRegion*,
we can remove the base_update and unmap function pointer arguments,
and the opaque pointer that was passed into these callbacks.

We can also remove the base and size fields from omap_gpmc_cs_file_s
as these are no longer necessary (you don't need the base/size
to unmap a MemoryRegion the way you did to undo a mapping made
with cpu_register_physical_memory()).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/nseries.c   |   10 ++------
 hw/omap.h      |    4 +--
 hw/omap_gpmc.c |   59 ++++++++++++++++++++-----------------------------------
 3 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/hw/nseries.c b/hw/nseries.c
index e61014c..123bf3b 100644
--- a/hw/nseries.c
+++ b/hw/nseries.c
@@ -174,9 +174,7 @@ static void n8x0_nand_setup(struct n800_s *s)
                     NAND_MFR_SAMSUNG, 0x48, 0, 1,
                     qdev_get_gpio_in(s->cpu->gpio, N8X0_ONENAND_GPIO));
     omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS,
-                     sysbus_mmio_get_region(sysbus_from_qdev(s->nand), 0),
-                     NULL, NULL,
-                     s->nand);
+                     sysbus_mmio_get_region(sysbus_from_qdev(s->nand), 0));
     otp_region = onenand_raw_otp(s->nand);
 
     memcpy(otp_region + 0x000, n8x0_cal_wlan_mac, sizeof(n8x0_cal_wlan_mac));
@@ -773,10 +771,8 @@ static void n8x0_usb_setup(struct n800_s *s)
     TUSBState *tusb = tusb6010_init(tusb_irq);
 
     /* Using the NOR interface */
-    omap_gpmc_attach(s->cpu->gpmc, N8X0_USB_ASYNC_CS,
-                    tusb6010_async_io(tusb), NULL, NULL, tusb);
-    omap_gpmc_attach(s->cpu->gpmc, N8X0_USB_SYNC_CS,
-                    tusb6010_sync_io(tusb), NULL, NULL, tusb);
+    omap_gpmc_attach(s->cpu->gpmc, N8X0_USB_ASYNC_CS, tusb6010_async_io(tusb));
+    omap_gpmc_attach(s->cpu->gpmc, N8X0_USB_SYNC_CS, tusb6010_sync_io(tusb));
 
     s->usb = tusb;
     qdev_connect_gpio_out(s->cpu->gpio, N8X0_TUSB_ENABLE_GPIO, tusb_pwr);
diff --git a/hw/omap.h b/hw/omap.h
index db101c6..47c8629 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -120,9 +120,7 @@ void omap_sdrc_reset(struct omap_sdrc_s *s);
 struct omap_gpmc_s;
 struct omap_gpmc_s *omap_gpmc_init(target_phys_addr_t base, qemu_irq irq);
 void omap_gpmc_reset(struct omap_gpmc_s *s);
-void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem,
-                void (*base_upd)(void *opaque, target_phys_addr_t new),
-                void (*unmap)(void *opaque), void *opaque);
+void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem);
 
 /*
  * Common IRQ numbers for level 1 interrupt handler
diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 673dddd..19f246c 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -40,13 +40,8 @@ struct omap_gpmc_s {
     int prefcount;
     struct omap_gpmc_cs_file_s {
         uint32_t config[7];
-        target_phys_addr_t base;
-        size_t size;
         MemoryRegion *iomem;
         MemoryRegion container;
-        void (*base_update)(void *opaque, target_phys_addr_t new);
-        void (*unmap)(void *opaque);
-        void *opaque;
     } cs_file[8];
     int ecc_cs;
     int ecc_ptr;
@@ -61,6 +56,12 @@ static void omap_gpmc_int_update(struct omap_gpmc_s *s)
 
 static void omap_gpmc_cs_map(struct omap_gpmc_cs_file_s *f, int base, int mask)
 {
+    uint32_t size;
+
+    if (!f->iomem) {
+        return;
+    }
+
     /* TODO: check for overlapping regions and report access errors */
     if ((mask != 0x8 && mask != 0xc && mask != 0xe && mask != 0xf) ||
                     (base < 0 || base >= 0x40) ||
@@ -70,39 +71,27 @@ static void omap_gpmc_cs_map(struct omap_gpmc_cs_file_s *f, int base, int mask)
         return;
     }
 
-    if (!f->opaque)
-        return;
-
-    f->base = base << 24;
-    f->size = (0x0fffffff & ~(mask << 24)) + 1;
+    base <<= 24;
+    size = (0x0fffffff & ~(mask << 24)) + 1;
     /* TODO: rather than setting the size of the mapping (which should be
      * constant), the mask should cause wrapping of the address space, so
      * that the same memory becomes accessible at every <i>size</i> bytes
      * starting from <i>base</i>.  */
-    if (f->iomem) {
-        memory_region_init(&f->container, "omap-gpmc-file", f->size);
-        memory_region_add_subregion(&f->container, 0, f->iomem);
-        memory_region_add_subregion(get_system_memory(), f->base,
-                                    &f->container);
-    }
-
-    if (f->base_update)
-        f->base_update(f->opaque, f->base);
+    memory_region_init(&f->container, "omap-gpmc-file", size);
+    memory_region_add_subregion(&f->container, 0, f->iomem);
+    memory_region_add_subregion(get_system_memory(), base,
+                                &f->container);
 }
 
 static void omap_gpmc_cs_unmap(struct omap_gpmc_cs_file_s *f)
 {
-    if (f->size) {
-        if (f->unmap)
-            f->unmap(f->opaque);
-        if (f->iomem) {
-            memory_region_del_subregion(get_system_memory(), &f->container);
-            memory_region_del_subregion(&f->container, f->iomem);
-            memory_region_destroy(&f->container);
-        }
-        f->base = 0;
-        f->size = 0;
+    if (!f->iomem) {
+        return;
     }
+
+    memory_region_del_subregion(get_system_memory(), &f->container);
+    memory_region_del_subregion(&f->container, f->iomem);
+    memory_region_destroy(&f->container);
 }
 
 void omap_gpmc_reset(struct omap_gpmc_s *s)
@@ -399,19 +388,18 @@ struct omap_gpmc_s *omap_gpmc_init(target_phys_addr_t base, qemu_irq irq)
     struct omap_gpmc_s *s = (struct omap_gpmc_s *)
             g_malloc0(sizeof(struct omap_gpmc_s));
 
-    omap_gpmc_reset(s);
-
     memory_region_init_io(&s->iomem, &omap_gpmc_ops, s, "omap-gpmc", 0x1000);
     memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 
+    omap_gpmc_reset(s);
+
     return s;
 }
 
-void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem,
-                void (*base_upd)(void *opaque, target_phys_addr_t new),
-                void (*unmap)(void *opaque), void *opaque)
+void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem)
 {
     struct omap_gpmc_cs_file_s *f;
+    assert(iomem);
 
     if (cs < 0 || cs >= 8) {
         fprintf(stderr, "%s: bad chip-select %i\n", __FUNCTION__, cs);
@@ -420,9 +408,6 @@ void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem,
     f = &s->cs_file[cs];
 
     f->iomem = iomem;
-    f->base_update = base_upd;
-    f->unmap = unmap;
-    f->opaque = opaque;
 
     if (f->config[6] & (1 << 6))				/* CSVALID */
         omap_gpmc_cs_map(f, f->config[6] & 0x1f,		/* MASKADDR */
-- 
1.7.1

[Qemu-devel] [PATCH 05/17] omap_gpmc: Refactor omap_gpmc_cs_map and omap_gpmc_cs_unmap

[Peter Maydell]

Refactor the omap_gpmc_cs_map/unmap functions:
 * take the omap_gpmc_s* and a chipselect id rather than the
   omap_gpmc_cs_file_s*, so they have access to the general gpmc
   member fields
 * extract the base and mask from the config registers in the functions
   rather than at every callsite
 * check for CSVALID in the functions rather than at every callsite

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |   56 +++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 19f246c..d16b28b 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -54,18 +54,25 @@ static void omap_gpmc_int_update(struct omap_gpmc_s *s)
     qemu_set_irq(s->irq, s->irqen & s->irqst);
 }
 
-static void omap_gpmc_cs_map(struct omap_gpmc_cs_file_s *f, int base, int mask)
+static void omap_gpmc_cs_map(struct omap_gpmc_s *s, int cs)
 {
+    struct omap_gpmc_cs_file_s *f = &s->cs_file[cs];
+    uint32_t mask = (f->config[6] >> 8) & 0xf;
+    uint32_t base = f->config[6] & 0x3f;
     uint32_t size;
 
     if (!f->iomem) {
         return;
     }
 
+    if (!(f->config[6] & (1 << 6))) {
+        /* Do nothing unless CSVALID */
+        return;
+    }
+
     /* TODO: check for overlapping regions and report access errors */
     if ((mask != 0x8 && mask != 0xc && mask != 0xe && mask != 0xf) ||
-                    (base < 0 || base >= 0x40) ||
-                    (base & 0x0f & ~mask)) {
+        (base & 0x0f & ~mask)) {
         fprintf(stderr, "%s: wrong cs address mapping/decoding!\n",
                         __FUNCTION__);
         return;
@@ -83,8 +90,13 @@ static void omap_gpmc_cs_map(struct omap_gpmc_cs_file_s *f, int base, int mask)
                                 &f->container);
 }
 
-static void omap_gpmc_cs_unmap(struct omap_gpmc_cs_file_s *f)
+static void omap_gpmc_cs_unmap(struct omap_gpmc_s *s, int cs)
 {
+    struct omap_gpmc_cs_file_s *f = &s->cs_file[cs];
+    if (!(f->config[6] & (1 << 6))) {
+        /* Do nothing unless CSVALID */
+        return;
+    }
     if (!f->iomem) {
         return;
     }
@@ -110,19 +122,26 @@ void omap_gpmc_reset(struct omap_gpmc_s *s)
     s->preffifo = 0;
     s->prefcount = 0;
     for (i = 0; i < 8; i ++) {
-        if (s->cs_file[i].config[6] & (1 << 6))			/* CSVALID */
-            omap_gpmc_cs_unmap(s->cs_file + i);
-        s->cs_file[i].config[0] = i ? 1 << 12 : 0;
+        omap_gpmc_cs_unmap(s, i);
         s->cs_file[i].config[1] = 0x101001;
         s->cs_file[i].config[2] = 0x020201;
         s->cs_file[i].config[3] = 0x10031003;
         s->cs_file[i].config[4] = 0x10f1111;
         s->cs_file[i].config[5] = 0;
         s->cs_file[i].config[6] = 0xf00 | (i ? 0 : 1 << 6);
-        if (s->cs_file[i].config[6] & (1 << 6))			/* CSVALID */
-            omap_gpmc_cs_map(&s->cs_file[i],
-                            s->cs_file[i].config[6] & 0x1f,	/* MASKADDR */
-                        (s->cs_file[i].config[6] >> 8 & 0xf));	/* BASEADDR */
+
+        s->cs_file[i].config[6] = 0xf00;
+        /* In theory we could probe attached devices for some CFG1
+         * bits here, but we just retain them across resets as they
+         * were set initially by omap_gpmc_attach().
+         */
+        if (i == 0) {
+            s->cs_file[i].config[0] &= 0x00433e00;
+            s->cs_file[i].config[6] |= 1 << 6; /* CSVALID */
+            omap_gpmc_cs_map(s, i);
+        } else {
+            s->cs_file[i].config[0] &= 0x00403c00;
+        }
     }
     s->ecc_cs = 0;
     s->ecc_ptr = 0;
@@ -311,13 +330,10 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
                 break;
             case 0x78:	/* GPMC_CONFIG7 */
                 if ((f->config[6] ^ value) & 0xf7f) {
-                    if (f->config[6] & (1 << 6))		/* CSVALID */
-                        omap_gpmc_cs_unmap(f);
-                    if (value & (1 << 6))			/* CSVALID */
-                        omap_gpmc_cs_map(f, value & 0x1f,	/* MASKADDR */
-                                        (value >> 8 & 0xf));	/* BASEADDR */
+                    omap_gpmc_cs_unmap(s, cs);
+                    f->config[6] = value & 0x00000f7f;
+                    omap_gpmc_cs_map(s, cs);
                 }
-                f->config[6] = value & 0x00000f7f;
                 break;
             case 0x7c:	/* GPMC_NAND_COMMAND */
             case 0x80:	/* GPMC_NAND_ADDRESS */
@@ -407,9 +423,7 @@ void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem)
     }
     f = &s->cs_file[cs];
 
+    omap_gpmc_cs_unmap(s, cs);
     f->iomem = iomem;
-
-    if (f->config[6] & (1 << 6))				/* CSVALID */
-        omap_gpmc_cs_map(f, f->config[6] & 0x1f,		/* MASKADDR */
-                        (f->config[6] >> 8 & 0xf));		/* BASEADDR */
+    omap_gpmc_cs_map(s, cs);
 }
-- 
1.7.1

[Qemu-devel] [PATCH 06/17] omap_gpmc: GPMC_IRQSTATUS is write-one-to-clear

[Peter Maydell]

Fix a bug in the handling of writes to GPMC_IRQSTATUS:
it behaves as "write one to clear, writing zero is ignored".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index d16b28b..ff4d485 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -284,7 +284,7 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
         break;
 
     case 0x018:	/* GPMC_IRQSTATUS */
-        s->irqen = ~value;
+        s->irqen &= ~value;
         omap_gpmc_int_update(s);
         break;
 
-- 
1.7.1

[Qemu-devel] [PATCH 07/17] omap_gpmc: Wire up the GPMC IRQ correctly

[Peter Maydell]

The omap_gpmc wasn't actually wiring up its IRQ, so
anything that provoked an interrupt would be using
uninitialised data for its IRQ number.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index ff4d485..b728397 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -407,6 +407,7 @@ struct omap_gpmc_s *omap_gpmc_init(target_phys_addr_t base, qemu_irq irq)
     memory_region_init_io(&s->iomem, &omap_gpmc_ops, s, "omap-gpmc", 0x1000);
     memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 
+    s->irq = irq;
     omap_gpmc_reset(s);
 
     return s;
-- 
1.7.1

[Qemu-devel] [PATCH 08/17] omap_gpmc: Fix handling of FIFOTHRESHOLDSTATUS bit

[Peter Maydell]

The OMAP3 TRM is inconsistent about whether the GPMC FIFOTHRESHOLDSTATUS
bit should be set when FIFOPOINTER > FIFOTHRESHOLD or when it is >=
FIFOTHRESHOLD. Apparently the underlying functional spec from which
the TRM was created states that the behaviour is ">=", and this also
makes more conceptual sense.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index b728397..9da8491 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -222,7 +222,7 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
         return s->prefcontrol;
     case 0x1f0:	/* GPMC_PREFETCH_STATUS */
         return (s->preffifo << 24) |
-                ((s->preffifo >
+                ((s->preffifo >=
                   ((s->prefconfig[0] >> 8) & 0x7f) ? 1 : 0) << 16) |
                 s->prefcount;
 
-- 
1.7.1

[Qemu-devel] [PATCH 09/17] omap_gpmc: Take omap_mpu_state* in omap_gpmc_init

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Take a pointer to the omap mpu state struct in omap_gpmc_init.
Some details of GPMC behaviour depend on the OMAP version we
are a part of.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
[Riku Voipio: Fixes and restructuring patchset]
Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
[Peter Maydell: More fixes and cleanups for upstream submission]
Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap.h      |    3 ++-
 hw/omap2.c     |    2 +-
 hw/omap_gpmc.c |    3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/omap.h b/hw/omap.h
index 47c8629..8509c82 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -118,7 +118,8 @@ void omap_sdrc_reset(struct omap_sdrc_s *s);
 
 /* OMAP2 general purpose memory controller */
 struct omap_gpmc_s;
-struct omap_gpmc_s *omap_gpmc_init(target_phys_addr_t base, qemu_irq irq);
+struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
+                                   target_phys_addr_t base, qemu_irq irq);
 void omap_gpmc_reset(struct omap_gpmc_s *s);
 void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem);
 
diff --git a/hw/omap2.c b/hw/omap2.c
index 7e5820a..0feb7a5 100644
--- a/hw/omap2.c
+++ b/hw/omap2.c
@@ -2402,7 +2402,7 @@ struct omap_mpu_state_s *omap2420_mpu_init(unsigned long sdram_size,
     sysbus_mmio_map(busdev, 4, omap_l4_region_base(ta, 5));
 
     s->sdrc = omap_sdrc_init(0x68009000);
-    s->gpmc = omap_gpmc_init(0x6800a000, s->irq[0][OMAP_INT_24XX_GPMC_IRQ]);
+    s->gpmc = omap_gpmc_init(s, 0x6800a000, s->irq[0][OMAP_INT_24XX_GPMC_IRQ]);
 
     dinfo = drive_get(IF_SD, 0, 0);
     if (!dinfo) {
diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 9da8491..c86e7ed 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -399,7 +399,8 @@ static const MemoryRegionOps omap_gpmc_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-struct omap_gpmc_s *omap_gpmc_init(target_phys_addr_t base, qemu_irq irq)
+struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
+                                   target_phys_addr_t base, qemu_irq irq)
 {
     struct omap_gpmc_s *s = (struct omap_gpmc_s *)
             g_malloc0(sizeof(struct omap_gpmc_s));
-- 
1.7.1

[Qemu-devel] [PATCH 10/17] omap_gpmc: Calculate revision from OMAP model

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
[Riku Voipio: Fixes and restructuring patchset]
Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
[Peter Maydell: More fixes and cleanups for upstream submission]
Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index c86e7ed..5c1365c 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -29,6 +29,7 @@ struct omap_gpmc_s {
     qemu_irq irq;
     MemoryRegion iomem;
 
+    uint8_t revision;
     uint8_t sysconfig;
     uint16_t irqst;
     uint16_t irqen;
@@ -163,7 +164,7 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
 
     switch (addr) {
     case 0x000:	/* GPMC_REVISION */
-        return 0x20;
+        return s->revision;
 
     case 0x010:	/* GPMC_SYSCONFIG */
         return s->sysconfig;
@@ -409,6 +410,7 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
     memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 
     s->irq = irq;
+    s->revision = cpu_class_omap3(mpu) ? 0x50 : 0x20;
     omap_gpmc_reset(s);
 
     return s;
-- 
1.7.1

[Qemu-devel] [PATCH 11/17] omap_gpmc: Reindent misindented switch statements

[Peter Maydell]

Whitespace-only change fixing indentation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I'm not usually a fan of indentation fix patches but in this case
the following patches touch a lot of the switch statement anyway so
it seemed reasonable...

 hw/omap_gpmc.c |   96 ++++++++++++++++++++++++++++----------------------------
 1 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 5c1365c..c569aa1 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -196,22 +196,22 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
         addr -= cs * 0x30;
         f = s->cs_file + cs;
         switch (addr) {
-            case 0x60:	/* GPMC_CONFIG1 */
-                return f->config[0];
-            case 0x64:	/* GPMC_CONFIG2 */
-                return f->config[1];
-            case 0x68:	/* GPMC_CONFIG3 */
-                return f->config[2];
-            case 0x6c:	/* GPMC_CONFIG4 */
-                return f->config[3];
-            case 0x70:	/* GPMC_CONFIG5 */
-                return f->config[4];
-            case 0x74:	/* GPMC_CONFIG6 */
-                return f->config[5];
-            case 0x78:	/* GPMC_CONFIG7 */
-                return f->config[6];
-            case 0x84:	/* GPMC_NAND_DATA */
-                return 0;
+        case 0x60:      /* GPMC_CONFIG1 */
+            return f->config[0];
+        case 0x64:      /* GPMC_CONFIG2 */
+            return f->config[1];
+        case 0x68:      /* GPMC_CONFIG3 */
+            return f->config[2];
+        case 0x6c:      /* GPMC_CONFIG4 */
+            return f->config[3];
+        case 0x70:      /* GPMC_CONFIG5 */
+            return f->config[4];
+        case 0x74:      /* GPMC_CONFIG6 */
+            return f->config[5];
+        case 0x78:      /* GPMC_CONFIG7 */
+            return f->config[6];
+        case 0x84:      /* GPMC_NAND_DATA */
+            return 0;
         }
         break;
 
@@ -311,38 +311,38 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
         addr -= cs * 0x30;
         f = s->cs_file + cs;
         switch (addr) {
-            case 0x60:	/* GPMC_CONFIG1 */
-                f->config[0] = value & 0xffef3e13;
-                break;
-            case 0x64:	/* GPMC_CONFIG2 */
-                f->config[1] = value & 0x001f1f8f;
-                break;
-            case 0x68:	/* GPMC_CONFIG3 */
-                f->config[2] = value & 0x001f1f8f;
-                break;
-            case 0x6c:	/* GPMC_CONFIG4 */
-                f->config[3] = value & 0x1f8f1f8f;
-                break;
-            case 0x70:	/* GPMC_CONFIG5 */
-                f->config[4] = value & 0x0f1f1f1f;
-                break;
-            case 0x74:	/* GPMC_CONFIG6 */
-                f->config[5] = value & 0x00000fcf;
-                break;
-            case 0x78:	/* GPMC_CONFIG7 */
-                if ((f->config[6] ^ value) & 0xf7f) {
-                    omap_gpmc_cs_unmap(s, cs);
-                    f->config[6] = value & 0x00000f7f;
-                    omap_gpmc_cs_map(s, cs);
-                }
-                break;
-            case 0x7c:	/* GPMC_NAND_COMMAND */
-            case 0x80:	/* GPMC_NAND_ADDRESS */
-            case 0x84:	/* GPMC_NAND_DATA */
-                break;
-
-            default:
-                goto bad_reg;
+        case 0x60:      /* GPMC_CONFIG1 */
+            f->config[0] = value & 0xffef3e13;
+            break;
+        case 0x64:      /* GPMC_CONFIG2 */
+            f->config[1] = value & 0x001f1f8f;
+            break;
+        case 0x68:      /* GPMC_CONFIG3 */
+            f->config[2] = value & 0x001f1f8f;
+            break;
+        case 0x6c:      /* GPMC_CONFIG4 */
+            f->config[3] = value & 0x1f8f1f8f;
+            break;
+        case 0x70:      /* GPMC_CONFIG5 */
+            f->config[4] = value & 0x0f1f1f1f;
+            break;
+        case 0x74:      /* GPMC_CONFIG6 */
+            f->config[5] = value & 0x00000fcf;
+            break;
+        case 0x78:      /* GPMC_CONFIG7 */
+            if ((f->config[6] ^ value) & 0xf7f) {
+                omap_gpmc_cs_unmap(s, cs);
+                f->config[6] = value & 0x00000f7f;
+                omap_gpmc_cs_map(s, cs);
+            }
+            break;
+        case 0x7c:      /* GPMC_NAND_COMMAND */
+        case 0x80:      /* GPMC_NAND_ADDRESS */
+        case 0x84:      /* GPMC_NAND_DATA */
+            break;
+
+        default:
+            goto bad_reg;
         }
         break;
 
-- 
1.7.1

[Qemu-devel] [PATCH 12/17] omap_gpmc: Support NAND devices

[Peter Maydell]

Support accesses to NAND devices, both by mapping them into
the GPMC address space, and via the NAND_COMMAND, NAND_ADDRESS
and NAND_DATA GPMC registers.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap.h      |    1 +
 hw/omap_gpmc.c |  219 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 208 insertions(+), 12 deletions(-)

diff --git a/hw/omap.h b/hw/omap.h
index 8509c82..2018636 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -122,6 +122,7 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
                                    target_phys_addr_t base, qemu_irq irq);
 void omap_gpmc_reset(struct omap_gpmc_s *s);
 void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem);
+void omap_gpmc_attach_nand(struct omap_gpmc_s *s, int cs, DeviceState *nand);
 
 /*
  * Common IRQ numbers for level 1 interrupt handler
diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index c569aa1..d2de72f 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -43,6 +43,8 @@ struct omap_gpmc_s {
         uint32_t config[7];
         MemoryRegion *iomem;
         MemoryRegion container;
+        MemoryRegion nandiomem;
+        DeviceState *dev;
     } cs_file[8];
     int ecc_cs;
     int ecc_ptr;
@@ -50,11 +52,135 @@ struct omap_gpmc_s {
     ECCState ecc[9];
 };
 
+#define OMAP_GPMC_8BIT 0
+#define OMAP_GPMC_16BIT 1
+#define OMAP_GPMC_NOR 0
+#define OMAP_GPMC_NAND 2
+
+static int omap_gpmc_devtype(struct omap_gpmc_cs_file_s *f)
+{
+    return (f->config[0] >> 10) & 3;
+}
+
+static int omap_gpmc_devsize(struct omap_gpmc_cs_file_s *f)
+{
+    /* devsize field is really 2 bits but we ignore the high
+     * bit to ensure consistent behaviour if the guest sets
+     * it (values 2 and 3 are reserved in the TRM)
+     */
+    return (f->config[0] >> 12) & 1;
+}
+
 static void omap_gpmc_int_update(struct omap_gpmc_s *s)
 {
     qemu_set_irq(s->irq, s->irqen & s->irqst);
 }
 
+/* Access functions for when a NAND-like device is mapped into memory:
+ * all addresses in the region behave like accesses to the relevant
+ * GPMC_NAND_DATA_i register (which is actually implemented to call these)
+ */
+static uint64_t omap_nand_read(void *opaque, target_phys_addr_t addr,
+                               unsigned size)
+{
+    struct omap_gpmc_cs_file_s *f = (struct omap_gpmc_cs_file_s *)opaque;
+    uint64_t v;
+    nand_setpins(f->dev, 0, 0, 0, 1, 0);
+    switch (omap_gpmc_devsize(f)) {
+    case OMAP_GPMC_8BIT:
+        v = nand_getio(f->dev);
+        if (size == 1) {
+            return v;
+        }
+        v |= (nand_getio(f->dev) << 8);
+        if (size == 2) {
+            return v;
+        }
+        v |= (nand_getio(f->dev) << 16);
+        v |= (nand_getio(f->dev) << 24);
+        return v;
+    case OMAP_GPMC_16BIT:
+        v = nand_getio(f->dev);
+        if (size == 1) {
+            /* 8 bit read from 16 bit device : probably a guest bug */
+            return v & 0xff;
+        }
+        if (size == 2) {
+            return v;
+        }
+        v |= (nand_getio(f->dev) << 16);
+        return v;
+    default:
+        abort();
+    }
+}
+
+static void omap_nand_setio(DeviceState *dev, uint64_t value,
+                            int nandsize, int size)
+{
+    /* Write the specified value to the NAND device, respecting
+     * both size of the NAND device and size of the write access.
+     */
+    switch (nandsize) {
+    case OMAP_GPMC_8BIT:
+        switch (size) {
+        case 1:
+            nand_setio(dev, value & 0xff);
+            break;
+        case 2:
+            nand_setio(dev, value & 0xff);
+            nand_setio(dev, (value >> 8) & 0xff);
+            break;
+        case 4:
+        default:
+            nand_setio(dev, value & 0xff);
+            nand_setio(dev, (value >> 8) & 0xff);
+            nand_setio(dev, (value >> 16) & 0xff);
+            nand_setio(dev, (value >> 24) & 0xff);
+            break;
+        }
+    case OMAP_GPMC_16BIT:
+        switch (size) {
+        case 1:
+            /* writing to a 16bit device with 8bit access is probably a guest
+             * bug; pass the value through anyway.
+             */
+        case 2:
+            nand_setio(dev, value & 0xffff);
+            break;
+        case 4:
+        default:
+            nand_setio(dev, value & 0xffff);
+            nand_setio(dev, (value >> 16) & 0xffff);
+            break;
+        }
+    }
+}
+
+static void omap_nand_write(void *opaque, target_phys_addr_t addr,
+                            uint64_t value, unsigned size)
+{
+    struct omap_gpmc_cs_file_s *f = (struct omap_gpmc_cs_file_s *)opaque;
+    nand_setpins(f->dev, 0, 0, 0, 1, 0);
+    omap_nand_setio(f->dev, value, omap_gpmc_devsize(f), size);
+}
+
+static const MemoryRegionOps omap_nand_ops = {
+    .read = omap_nand_read,
+    .write = omap_nand_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static MemoryRegion *omap_gpmc_cs_memregion(struct omap_gpmc_s *s, int cs)
+{
+    /* Return the MemoryRegion* to map/unmap for this chipselect */
+    struct omap_gpmc_cs_file_s *f = &s->cs_file[cs];
+    if (omap_gpmc_devtype(f) == OMAP_GPMC_NOR) {
+        return f->iomem;
+    }
+    return &f->nandiomem;
+}
+
 static void omap_gpmc_cs_map(struct omap_gpmc_s *s, int cs)
 {
     struct omap_gpmc_cs_file_s *f = &s->cs_file[cs];
@@ -62,7 +188,7 @@ static void omap_gpmc_cs_map(struct omap_gpmc_s *s, int cs)
     uint32_t base = f->config[6] & 0x3f;
     uint32_t size;
 
-    if (!f->iomem) {
+    if (!f->iomem && !f->dev) {
         return;
     }
 
@@ -86,7 +212,8 @@ static void omap_gpmc_cs_map(struct omap_gpmc_s *s, int cs)
      * that the same memory becomes accessible at every <i>size</i> bytes
      * starting from <i>base</i>.  */
     memory_region_init(&f->container, "omap-gpmc-file", size);
-    memory_region_add_subregion(&f->container, 0, f->iomem);
+    memory_region_add_subregion(&f->container, 0,
+                                omap_gpmc_cs_memregion(s, cs));
     memory_region_add_subregion(get_system_memory(), base,
                                 &f->container);
 }
@@ -98,12 +225,11 @@ static void omap_gpmc_cs_unmap(struct omap_gpmc_s *s, int cs)
         /* Do nothing unless CSVALID */
         return;
     }
-    if (!f->iomem) {
+    if (!f->iomem && !f->dev) {
         return;
     }
-
     memory_region_del_subregion(get_system_memory(), &f->container);
-    memory_region_del_subregion(&f->container, f->iomem);
+    memory_region_del_subregion(&f->container, omap_gpmc_cs_memregion(s, cs));
     memory_region_destroy(&f->container);
 }
 
@@ -151,6 +277,24 @@ void omap_gpmc_reset(struct omap_gpmc_s *s)
         ecc_reset(&s->ecc[i]);
 }
 
+static int gpmc_wordaccess_only(target_phys_addr_t addr)
+{
+    /* Return true if the register offset is to a register that
+     * only permits word width accesses.
+     * Non-word accesses are only OK for GPMC_NAND_DATA/ADDRESS/COMMAND
+     * for any chipselect.
+     */
+    if (addr >= 0x60 && addr <= 0x1d4) {
+        int cs = (addr - 0x60) / 0x30;
+        addr -= cs * 0x30;
+        if (addr >= 0x7c && addr < 0x88) {
+            /* GPMC_NAND_COMMAND, GPMC_NAND_ADDRESS, GPMC_NAND_DATA */
+            return 0;
+        }
+    }
+    return 1;
+}
+
 static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
                                unsigned size)
 {
@@ -158,7 +302,7 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
     int cs;
     struct omap_gpmc_cs_file_s *f;
 
-    if (size != 4) {
+    if (size != 4 && gpmc_wordaccess_only(addr)) {
         return omap_badwidth_read32(opaque, addr);
     }
 
@@ -210,7 +354,10 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
             return f->config[5];
         case 0x78:      /* GPMC_CONFIG7 */
             return f->config[6];
-        case 0x84:      /* GPMC_NAND_DATA */
+        case 0x84 ... 0x87: /* GPMC_NAND_DATA */
+            if (omap_gpmc_devtype(f) == OMAP_GPMC_NAND) {
+                return omap_nand_read(f, 0, size);
+            }
             return 0;
         }
         break;
@@ -260,7 +407,7 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
     int cs;
     struct omap_gpmc_cs_file_s *f;
 
-    if (size != 4) {
+    if (size != 4 && gpmc_wordaccess_only(addr)) {
         return omap_badwidth_write32(opaque, addr, value);
     }
 
@@ -336,11 +483,23 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
                 omap_gpmc_cs_map(s, cs);
             }
             break;
-        case 0x7c:      /* GPMC_NAND_COMMAND */
-        case 0x80:      /* GPMC_NAND_ADDRESS */
-        case 0x84:      /* GPMC_NAND_DATA */
+        case 0x7c ... 0x7f: /* GPMC_NAND_COMMAND */
+            if (omap_gpmc_devtype(f) == OMAP_GPMC_NAND) {
+                nand_setpins(f->dev, 1, 0, 0, 1, 0); /* CLE */
+                omap_nand_setio(f->dev, value, omap_gpmc_devsize(f), size);
+            }
+            break;
+        case 0x80 ... 0x83: /* GPMC_NAND_ADDRESS */
+            if (omap_gpmc_devtype(f) == OMAP_GPMC_NAND) {
+                nand_setpins(f->dev, 0, 1, 0, 1, 0); /* ALE */
+                omap_nand_setio(f->dev, value, omap_gpmc_devsize(f), size);
+            }
+            break;
+        case 0x84 ... 0x87: /* GPMC_NAND_DATA */
+            if (omap_gpmc_devtype(f) == OMAP_GPMC_NAND) {
+                omap_nand_write(f, 0, value, size);
+            }
             break;
-
         default:
             goto bad_reg;
         }
@@ -403,6 +562,7 @@ static const MemoryRegionOps omap_gpmc_ops = {
 struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
                                    target_phys_addr_t base, qemu_irq irq)
 {
+    int cs;
     struct omap_gpmc_s *s = (struct omap_gpmc_s *)
             g_malloc0(sizeof(struct omap_gpmc_s));
 
@@ -413,6 +573,19 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
     s->revision = cpu_class_omap3(mpu) ? 0x50 : 0x20;
     omap_gpmc_reset(s);
 
+    /* We have to register a different IO memory handler for each
+     * chip select region in case a NAND device is mapped there. We
+     * make the region the worst-case size of 256MB and rely on the
+     * container memory region in cs_map to chop it down to the actual
+     * guest-requested size.
+     */
+    for (cs = 0; cs < 8; cs++) {
+        memory_region_init_io(&s->cs_file[cs].nandiomem,
+                              &omap_nand_ops,
+                              &s->cs_file[cs],
+                              "omap-nand",
+                              256 * 1024 * 1024);
+    }
     return s;
 }
 
@@ -428,6 +601,28 @@ void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem)
     f = &s->cs_file[cs];
 
     omap_gpmc_cs_unmap(s, cs);
+    f->config[0] &= ~(0xf << 10);
     f->iomem = iomem;
     omap_gpmc_cs_map(s, cs);
 }
+
+void omap_gpmc_attach_nand(struct omap_gpmc_s *s, int cs, DeviceState *nand)
+{
+    struct omap_gpmc_cs_file_s *f;
+    assert(nand);
+
+    if (cs < 0 || cs >= 8) {
+        fprintf(stderr, "%s: bad chip-select %i\n", __func__, cs);
+        exit(-1);
+    }
+    f = &s->cs_file[cs];
+
+    omap_gpmc_cs_unmap(s, cs);
+    f->config[0] &= ~(0xf << 10);
+    f->config[0] |= (OMAP_GPMC_NAND << 10);
+    f->dev = nand;
+    if (nand_getbuswidth(f->dev) == 16) {
+        f->config[0] |= OMAP_GPMC_16BIT << 12;
+    }
+    omap_gpmc_cs_map(s, cs);
+}
-- 
1.7.1

[Qemu-devel] [PATCH 13/17] hw/omap.h: Add OMAP 3630 to omap_mpu_model enumeration

[Peter Maydell]

Add the OMAP 3630 to the omap_mpu_model enumeration, and add the
corresponding cpu_is_omap3630() function.

(OMAP3 isn't supported yet but this is useful in upgrading common
components to be "OMAP3 ready". We already have this for OMAP3430.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap.h |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/omap.h b/hw/omap.h
index 2018636..81f5544 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -788,6 +788,7 @@ i2c_bus *omap_i2c_bus(struct omap_i2c_s *s);
 # define cpu_is_omap2420(cpu)		(cpu->mpu_model == omap2420)
 # define cpu_is_omap2430(cpu)		(cpu->mpu_model == omap2430)
 # define cpu_is_omap3430(cpu)		(cpu->mpu_model == omap3430)
+# define cpu_is_omap3630(cpu)           (cpu->mpu_model == omap3630)
 
 # define cpu_is_omap15xx(cpu)		\
         (cpu_is_omap310(cpu) || cpu_is_omap1510(cpu))
@@ -799,7 +800,8 @@ i2c_bus *omap_i2c_bus(struct omap_i2c_s *s);
 # define cpu_class_omap1(cpu)		\
         (cpu_is_omap15xx(cpu) || cpu_is_omap16xx(cpu))
 # define cpu_class_omap2(cpu)		cpu_is_omap24xx(cpu)
-# define cpu_class_omap3(cpu)		cpu_is_omap3430(cpu)
+# define cpu_class_omap3(cpu) \
+        (cpu_is_omap3430(cpu) || cpu_is_omap3630(cpu))
 
 struct omap_mpu_state_s {
     enum omap_mpu_model {
@@ -813,6 +815,7 @@ struct omap_mpu_state_s {
         omap2423,
         omap2430,
         omap3430,
+        omap3630,
     } mpu_model;
 
     CPUState *env;
-- 
1.7.1

[Qemu-devel] [PATCH 14/17] omap_gpmc: Accept a zero mask field on omap3630

[Peter Maydell]

From: Juha Riihimäki <juha.riihimaki@nokia.com>

OMAP3630 adds an extra bit of address masking, so a mask of
0xb1111 is valid. Unfortunately the GPMC_REVISION is the same as
on the OMAP3430 which only has three bits of address masking, so
we have to derive this feature directly from the OMAP revision
rather than from the GPMC revision.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
[Riku Voipio: Fixes and restructuring patchset]
Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
[Peter Maydell: More fixes and cleanups for upstream submission]
Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index d2de72f..0326d49 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -28,6 +28,7 @@
 struct omap_gpmc_s {
     qemu_irq irq;
     MemoryRegion iomem;
+    int accept_256;
 
     uint8_t revision;
     uint8_t sysconfig;
@@ -198,11 +199,10 @@ static void omap_gpmc_cs_map(struct omap_gpmc_s *s, int cs)
     }
 
     /* TODO: check for overlapping regions and report access errors */
-    if ((mask != 0x8 && mask != 0xc && mask != 0xe && mask != 0xf) ||
-        (base & 0x0f & ~mask)) {
-        fprintf(stderr, "%s: wrong cs address mapping/decoding!\n",
-                        __FUNCTION__);
-        return;
+    if (mask != 0x8 && mask != 0xc && mask != 0xe && mask != 0xf
+         && !(s->accept_256 && !mask)) {
+        fprintf(stderr, "%s: invalid chip-select mask address (0x%x)\n",
+                 __func__, mask);
     }
 
     base <<= 24;
@@ -570,6 +570,7 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
     memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 
     s->irq = irq;
+    s->accept_256 = cpu_is_omap3630(mpu);
     s->revision = cpu_class_omap3(mpu) ? 0x50 : 0x20;
     omap_gpmc_reset(s);
 
-- 
1.7.1

[Qemu-devel] [PATCH 15/17] omap_gpmc: Pull prefetch engine data into sub-struct

[Peter Maydell]

Refactor the gpmc state structure so items relating to
the prefetch engine are in their own sub-struct and have
more useful names.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |   52 ++++++++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 0326d49..158c097 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -36,10 +36,6 @@ struct omap_gpmc_s {
     uint16_t irqen;
     uint16_t timeout;
     uint16_t config;
-    uint32_t prefconfig[2];
-    int prefcontrol;
-    int preffifo;
-    int prefcount;
     struct omap_gpmc_cs_file_s {
         uint32_t config[7];
         MemoryRegion *iomem;
@@ -51,6 +47,13 @@ struct omap_gpmc_s {
     int ecc_ptr;
     uint32_t ecc_cfg;
     ECCState ecc[9];
+    struct prefetch {
+        uint32_t config1; /* GPMC_PREFETCH_CONFIG1 */
+        uint32_t transfercount; /* GPMC_PREFETCH_CONFIG2:TRANSFERCOUNT */
+        int startengine; /* GPMC_PREFETCH_CONTROL:STARTENGINE */
+        int fifopointer; /* GPMC_PREFETCH_STATUS:FIFOPOINTER */
+        int count; /* GPMC_PREFETCH_STATUS:COUNTVALUE */
+    } prefetch;
 };
 
 #define OMAP_GPMC_8BIT 0
@@ -243,11 +246,11 @@ void omap_gpmc_reset(struct omap_gpmc_s *s)
     omap_gpmc_int_update(s);
     s->timeout = 0;
     s->config = 0xa00;
-    s->prefconfig[0] = 0x00004000;
-    s->prefconfig[1] = 0x00000000;
-    s->prefcontrol = 0;
-    s->preffifo = 0;
-    s->prefcount = 0;
+    s->prefetch.config1 = 0x00004000;
+    s->prefetch.transfercount = 0x00000000;
+    s->prefetch.startengine = 0;
+    s->prefetch.fifopointer = 0;
+    s->prefetch.count = 0;
     for (i = 0; i < 8; i ++) {
         omap_gpmc_cs_unmap(s, i);
         s->cs_file[i].config[1] = 0x101001;
@@ -363,16 +366,16 @@ static uint64_t omap_gpmc_read(void *opaque, target_phys_addr_t addr,
         break;
 
     case 0x1e0:	/* GPMC_PREFETCH_CONFIG1 */
-        return s->prefconfig[0];
+        return s->prefetch.config1;
     case 0x1e4:	/* GPMC_PREFETCH_CONFIG2 */
-        return s->prefconfig[1];
+        return s->prefetch.transfercount;
     case 0x1ec:	/* GPMC_PREFETCH_CONTROL */
-        return s->prefcontrol;
+        return s->prefetch.startengine;
     case 0x1f0:	/* GPMC_PREFETCH_STATUS */
-        return (s->preffifo << 24) |
-                ((s->preffifo >=
-                  ((s->prefconfig[0] >> 8) & 0x7f) ? 1 : 0) << 16) |
-                s->prefcount;
+        return (s->prefetch.fifopointer << 24) |
+                ((s->prefetch.fifopointer >=
+                  ((s->prefetch.config1 >> 8) & 0x7f) ? 1 : 0) << 16) |
+                s->prefetch.count;
 
     case 0x1f4:	/* GPMC_ECC_CONFIG */
         return s->ecc_cs;
@@ -506,21 +509,22 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
         break;
 
     case 0x1e0:	/* GPMC_PREFETCH_CONFIG1 */
-        s->prefconfig[0] = value & 0x7f8f7fbf;
+        s->prefetch.config1 = value & 0x7f8f7fbf;
         /* TODO: update interrupts, fifos, dmas */
         break;
 
     case 0x1e4:	/* GPMC_PREFETCH_CONFIG2 */
-        s->prefconfig[1] = value & 0x3fff;
+        s->prefetch.transfercount = value & 0x3fff;
         break;
 
     case 0x1ec:	/* GPMC_PREFETCH_CONTROL */
-        s->prefcontrol = value & 1;
-        if (s->prefcontrol) {
-            if (s->prefconfig[0] & 1)
-                s->preffifo = 0x40;
-            else
-                s->preffifo = 0x00;
+        s->prefetch.startengine = value & 1;
+        if (s->prefetch.startengine) {
+            if (s->prefetch.config1 & 1) {
+                s->prefetch.fifopointer = 0x40;
+            } else {
+                s->prefetch.fifopointer = 0x00;
+            }
         }
         /* TODO: start */
         break;
-- 
1.7.1

[Qemu-devel] [PATCH 16/17] omap: Wire up the DMA request line to the GPMC

[Peter Maydell]

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap.h      |    3 ++-
 hw/omap2.c     |    3 ++-
 hw/omap_gpmc.c |    5 ++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/omap.h b/hw/omap.h
index 81f5544..d9ab006 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -119,7 +119,8 @@ void omap_sdrc_reset(struct omap_sdrc_s *s);
 /* OMAP2 general purpose memory controller */
 struct omap_gpmc_s;
 struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
-                                   target_phys_addr_t base, qemu_irq irq);
+                                   target_phys_addr_t base,
+                                   qemu_irq irq, qemu_irq drq);
 void omap_gpmc_reset(struct omap_gpmc_s *s);
 void omap_gpmc_attach(struct omap_gpmc_s *s, int cs, MemoryRegion *iomem);
 void omap_gpmc_attach_nand(struct omap_gpmc_s *s, int cs, DeviceState *nand);
diff --git a/hw/omap2.c b/hw/omap2.c
index 0feb7a5..ca088d9 100644
--- a/hw/omap2.c
+++ b/hw/omap2.c
@@ -2402,7 +2402,8 @@ struct omap_mpu_state_s *omap2420_mpu_init(unsigned long sdram_size,
     sysbus_mmio_map(busdev, 4, omap_l4_region_base(ta, 5));
 
     s->sdrc = omap_sdrc_init(0x68009000);
-    s->gpmc = omap_gpmc_init(s, 0x6800a000, s->irq[0][OMAP_INT_24XX_GPMC_IRQ]);
+    s->gpmc = omap_gpmc_init(s, 0x6800a000, s->irq[0][OMAP_INT_24XX_GPMC_IRQ],
+                             s->drq[OMAP24XX_DMA_GPMC]);
 
     dinfo = drive_get(IF_SD, 0, 0);
     if (!dinfo) {
diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index 158c097..be309fe 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -27,6 +27,7 @@
 /* General-Purpose Memory Controller */
 struct omap_gpmc_s {
     qemu_irq irq;
+    qemu_irq drq;
     MemoryRegion iomem;
     int accept_256;
 
@@ -564,7 +565,8 @@ static const MemoryRegionOps omap_gpmc_ops = {
 };
 
 struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
-                                   target_phys_addr_t base, qemu_irq irq)
+                                   target_phys_addr_t base,
+                                   qemu_irq irq, qemu_irq drq)
 {
     int cs;
     struct omap_gpmc_s *s = (struct omap_gpmc_s *)
@@ -574,6 +576,7 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
     memory_region_add_subregion(get_system_memory(), base, &s->iomem);
 
     s->irq = irq;
+    s->drq = drq;
     s->accept_256 = cpu_is_omap3630(mpu);
     s->revision = cpu_class_omap3(mpu) ? 0x50 : 0x20;
     omap_gpmc_reset(s);
-- 
1.7.1

[Qemu-devel] [PATCH 17/17] omap_gpmc: Implement prefetch engine

[Peter Maydell]

This commit implements the prefetch engine feature of the GPMC
which can be used for NAND devices. This includes both interrupt
driven and DMA-filling modes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/omap_gpmc.c |  257 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 247 insertions(+), 10 deletions(-)

diff --git a/hw/omap_gpmc.c b/hw/omap_gpmc.c
index be309fe..02f0c52 100644
--- a/hw/omap_gpmc.c
+++ b/hw/omap_gpmc.c
@@ -35,6 +35,7 @@ struct omap_gpmc_s {
     uint8_t sysconfig;
     uint16_t irqst;
     uint16_t irqen;
+    uint16_t lastirq;
     uint16_t timeout;
     uint16_t config;
     struct omap_gpmc_cs_file_s {
@@ -54,6 +55,8 @@ struct omap_gpmc_s {
         int startengine; /* GPMC_PREFETCH_CONTROL:STARTENGINE */
         int fifopointer; /* GPMC_PREFETCH_STATUS:FIFOPOINTER */
         int count; /* GPMC_PREFETCH_STATUS:COUNTVALUE */
+        MemoryRegion iomem;
+        uint8_t fifo[64];
     } prefetch;
 };
 
@@ -76,9 +79,42 @@ static int omap_gpmc_devsize(struct omap_gpmc_cs_file_s *f)
     return (f->config[0] >> 12) & 1;
 }
 
+/* Extract the chip-select value from the prefetch config1 register */
+static int prefetch_cs(uint32_t config1)
+{
+    return (config1 >> 24) & 7;
+}
+
+static int prefetch_threshold(uint32_t config1)
+{
+    return (config1 >> 8) & 0x7f;
+}
+
 static void omap_gpmc_int_update(struct omap_gpmc_s *s)
 {
-    qemu_set_irq(s->irq, s->irqen & s->irqst);
+    /* The TRM is a bit unclear, but it seems to say that
+     * the TERMINALCOUNTSTATUS bit is set only on the
+     * transition when the prefetch engine goes from
+     * active to inactive, whereas the FIFOEVENTSTATUS
+     * bit is held high as long as the fifo has at
+     * least THRESHOLD bytes available.
+     * So we do the latter here, but TERMINALCOUNTSTATUS
+     * is set elsewhere.
+     */
+    if (s->prefetch.fifopointer >= prefetch_threshold(s->prefetch.config1)) {
+        s->irqst |= 1;
+    }
+    if ((s->irqen & s->irqst) != s->lastirq) {
+        s->lastirq = s->irqen & s->irqst;
+        qemu_set_irq(s->irq, s->lastirq);
+    }
+}
+
+static void omap_gpmc_dma_update(struct omap_gpmc_s *s, int value)
+{
+    if (s->prefetch.config1 & 4) {
+        qemu_set_irq(s->drq, value);
+    }
 }
 
 /* Access functions for when a NAND-like device is mapped into memory:
@@ -176,6 +212,161 @@ static const MemoryRegionOps omap_nand_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void fill_prefetch_fifo(struct omap_gpmc_s *s)
+{
+    /* Fill the prefetch FIFO by reading data from NAND.
+     * We do this synchronously, unlike the hardware which
+     * will do this asynchronously. We refill when the
+     * FIFO has THRESHOLD bytes free, and we always refill
+     * as much data as possible starting at the top end
+     * of the FIFO.
+     * (We have to refill at THRESHOLD rather than waiting
+     * for the FIFO to empty to allow for the case where
+     * the FIFO size isn't an exact multiple of THRESHOLD
+     * and we're doing DMA transfers.)
+     * This means we never need to handle wrap-around in
+     * the fifo-reading code, and the next byte of data
+     * to read is always fifo[63 - fifopointer].
+     */
+    int fptr;
+    int cs = prefetch_cs(s->prefetch.config1);
+    int is16bit = (((s->cs_file[cs].config[0] >> 12) & 3) != 0);
+    int bytes;
+    /* Don't believe the bit of the OMAP TRM that says that COUNTVALUE
+     * and TRANSFERCOUNT are in units of 16 bit words for 16 bit NAND.
+     * Instead believe the bit that says it is always a byte count.
+     */
+    bytes = 64 - s->prefetch.fifopointer;
+    if (bytes > s->prefetch.count) {
+        bytes = s->prefetch.count;
+    }
+    s->prefetch.count -= bytes;
+    s->prefetch.fifopointer += bytes;
+    fptr = 64 - s->prefetch.fifopointer;
+    /* Move the existing data in the FIFO so it sits just
+     * before what we're about to read in
+     */
+    while (fptr < (64 - bytes)) {
+        s->prefetch.fifo[fptr] = s->prefetch.fifo[fptr + bytes];
+        fptr++;
+    }
+    while (fptr < 64) {
+        if (is16bit) {
+            uint32_t v = omap_nand_read(&s->cs_file[cs], 0, 2);
+            s->prefetch.fifo[fptr++] = v & 0xff;
+            s->prefetch.fifo[fptr++] = (v >> 8) & 0xff;
+        } else {
+            s->prefetch.fifo[fptr++] = omap_nand_read(&s->cs_file[cs], 0, 1);
+        }
+    }
+    if (s->prefetch.startengine && (s->prefetch.count == 0)) {
+        /* This was the final transfer: raise TERMINALCOUNTSTATUS */
+        s->irqst |= 2;
+        s->prefetch.startengine = 0;
+    }
+    /* If there are any bytes in the FIFO at this point then
+     * we must raise a DMA request (either this is a final part
+     * transfer, or we filled the FIFO in which case we certainly
+     * have THRESHOLD bytes available)
+     */
+    if (s->prefetch.fifopointer != 0) {
+        omap_gpmc_dma_update(s, 1);
+    }
+    omap_gpmc_int_update(s);
+}
+
+/* Access functions for a NAND-like device when the prefetch/postwrite
+ * engine is enabled -- all addresses in the region behave alike:
+ * data is read or written to the FIFO.
+ */
+static uint64_t omap_gpmc_prefetch_read(void *opaque, target_phys_addr_t addr,
+                                        unsigned size)
+{
+    struct omap_gpmc_s *s = (struct omap_gpmc_s *) opaque;
+    uint32_t data;
+    if (s->prefetch.config1 & 1) {
+        /* The TRM doesn't define the behaviour if you read from the
+         * FIFO when the prefetch engine is in write mode. We choose
+         * to always return zero.
+         */
+        return 0;
+    }
+    /* Note that trying to read an empty fifo repeats the last byte */
+    if (s->prefetch.fifopointer) {
+        s->prefetch.fifopointer--;
+    }
+    data = s->prefetch.fifo[63 - s->prefetch.fifopointer];
+    if (s->prefetch.fifopointer ==
+        (64 - prefetch_threshold(s->prefetch.config1))) {
+        /* We've drained THRESHOLD bytes now. So deassert the
+         * DMA request, then refill the FIFO (which will probably
+         * assert it again.)
+         */
+        omap_gpmc_dma_update(s, 0);
+        fill_prefetch_fifo(s);
+    }
+    omap_gpmc_int_update(s);
+    return data;
+}
+
+static void omap_gpmc_prefetch_write(void *opaque, target_phys_addr_t addr,
+                                     uint64_t value, unsigned size)
+{
+    struct omap_gpmc_s *s = (struct omap_gpmc_s *) opaque;
+    int cs = prefetch_cs(s->prefetch.config1);
+    if ((s->prefetch.config1 & 1) == 0) {
+        /* The TRM doesn't define the behaviour of writing to the
+         * FIFO when the prefetch engine is in read mode. We
+         * choose to ignore the write.
+         */
+        return;
+    }
+    if (s->prefetch.count == 0) {
+        /* The TRM doesn't define the behaviour of writing to the
+         * FIFO if the transfer is complete. We choose to ignore.
+         */
+        return;
+    }
+    /* The only reason we do any data buffering in postwrite
+     * mode is if we are talking to a 16 bit NAND device, in
+     * which case we need to buffer the first byte of the
+     * 16 bit word until the other byte arrives.
+     */
+    int is16bit = (((s->cs_file[cs].config[0] >> 12) & 3) != 0);
+    if (is16bit) {
+        /* fifopointer alternates between 64 (waiting for first
+         * byte of word) and 63 (waiting for second byte)
+         */
+        if (s->prefetch.fifopointer == 64) {
+            s->prefetch.fifo[0] = value;
+            s->prefetch.fifopointer--;
+        } else {
+            value = (value << 8) | s->prefetch.fifo[0];
+            omap_nand_write(&s->cs_file[cs], 0, value, 2);
+            s->prefetch.count--;
+            s->prefetch.fifopointer = 64;
+        }
+    } else {
+        /* Just write the byte : fifopointer remains 64 at all times */
+        omap_nand_write(&s->cs_file[cs], 0, value, 1);
+        s->prefetch.count--;
+    }
+    if (s->prefetch.count == 0) {
+        /* Final transfer: raise TERMINALCOUNTSTATUS */
+        s->irqst |= 2;
+        s->prefetch.startengine = 0;
+    }
+    omap_gpmc_int_update(s);
+}
+
+static const MemoryRegionOps omap_prefetch_ops = {
+    .read = omap_gpmc_prefetch_read,
+    .write = omap_gpmc_prefetch_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 1,
+};
+
 static MemoryRegion *omap_gpmc_cs_memregion(struct omap_gpmc_s *s, int cs)
 {
     /* Return the MemoryRegion* to map/unmap for this chipselect */
@@ -183,6 +374,11 @@ static MemoryRegion *omap_gpmc_cs_memregion(struct omap_gpmc_s *s, int cs)
     if (omap_gpmc_devtype(f) == OMAP_GPMC_NOR) {
         return f->iomem;
     }
+    if ((s->prefetch.config1 & 0x80) &&
+        (prefetch_cs(s->prefetch.config1) == cs)) {
+        /* The prefetch engine is enabled for this CS: map the FIFO */
+        return &s->prefetch.iomem;
+    }
     return &f->nandiomem;
 }
 
@@ -510,24 +706,61 @@ static void omap_gpmc_write(void *opaque, target_phys_addr_t addr,
         break;
 
     case 0x1e0:	/* GPMC_PREFETCH_CONFIG1 */
-        s->prefetch.config1 = value & 0x7f8f7fbf;
-        /* TODO: update interrupts, fifos, dmas */
+        if (!s->prefetch.startengine) {
+            uint32_t oldconfig1 = s->prefetch.config1;
+            uint32_t changed;
+            s->prefetch.config1 = value & 0x7f8f7fbf;
+            changed = oldconfig1 ^ s->prefetch.config1;
+            if (changed & (0x80 | 0x7000000)) {
+                /* Turning the engine on or off, or mapping it somewhere else.
+                 * cs_map() and cs_unmap() check the prefetch config and
+                 * overall CSVALID bits, so it is sufficient to unmap-and-map
+                 * both the old cs and the new one.
+                 */
+                int oldcs = prefetch_cs(oldconfig1);
+                int newcs = prefetch_cs(s->prefetch.config1);
+                omap_gpmc_cs_unmap(s, oldcs);
+                omap_gpmc_cs_map(s, oldcs);
+                if (newcs != oldcs) {
+                    omap_gpmc_cs_unmap(s, newcs);
+                    omap_gpmc_cs_map(s, newcs);
+                }
+            }
+        }
         break;
 
     case 0x1e4:	/* GPMC_PREFETCH_CONFIG2 */
-        s->prefetch.transfercount = value & 0x3fff;
+        if (!s->prefetch.startengine) {
+            s->prefetch.transfercount = value & 0x3fff;
+        }
         break;
 
     case 0x1ec:	/* GPMC_PREFETCH_CONTROL */
-        s->prefetch.startengine = value & 1;
-        if (s->prefetch.startengine) {
-            if (s->prefetch.config1 & 1) {
-                s->prefetch.fifopointer = 0x40;
+        if (s->prefetch.startengine != (value & 1)) {
+            s->prefetch.startengine = value & 1;
+            if (s->prefetch.startengine) {
+                /* Prefetch engine start */
+                s->prefetch.count = s->prefetch.transfercount;
+                if (s->prefetch.config1 & 1) {
+                    /* Write */
+                    s->prefetch.fifopointer = 64;
+                } else {
+                    /* Read */
+                    s->prefetch.fifopointer = 0;
+                    fill_prefetch_fifo(s);
+                }
             } else {
-                s->prefetch.fifopointer = 0x00;
+                /* Prefetch engine forcibly stopped. The TRM
+                 * doesn't define the behaviour if you do this.
+                 * We clear the prefetch count, which means that
+                 * we permit no more writes, and don't read any
+                 * more data from NAND. The CPU can still drain
+                 * the FIFO of unread data.
+                 */
+                s->prefetch.count = 0;
             }
+            omap_gpmc_int_update(s);
         }
-        /* TODO: start */
         break;
 
     case 0x1f4:	/* GPMC_ECC_CONFIG */
@@ -579,6 +812,7 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
     s->drq = drq;
     s->accept_256 = cpu_is_omap3630(mpu);
     s->revision = cpu_class_omap3(mpu) ? 0x50 : 0x20;
+    s->lastirq = 0;
     omap_gpmc_reset(s);
 
     /* We have to register a different IO memory handler for each
@@ -594,6 +828,9 @@ struct omap_gpmc_s *omap_gpmc_init(struct omap_mpu_state_s *mpu,
                               "omap-nand",
                               256 * 1024 * 1024);
     }
+
+    memory_region_init_io(&s->prefetch.iomem, &omap_prefetch_ops, s,
+                          "omap-gpmc-prefetch", 256 * 1024 * 1024);
     return s;
 }
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH 00/17] onenand, omap_gpmc fixes, features

[Edgar E. Iglesias]

On Thu, Aug 25, 2011 at 09:04:54PM +0100, Peter Maydell wrote:
> This patchset is primarily features and bugfixes for the
> omap_gpmc device from the Meego tree.
> 
> We start by adding a new sysbus function to get the MemoryRegion*
> for a sysbus MMIO region. This was discussed in
>  http://www.mail-archive.com/kvm@vger.kernel.org/msg59535.html
> and as noted there can also be used to clean up other things,
> so it's not just an omap_gpmc thing.
> 
> The bulk of the onenand qdevification patch was in an earlier series
> I posted; this update makes it work with MemoryRegions.
> 
> The patches from "omap_gpmc: Take omap_mpu_state* in omap_gpmc_init"
> onwards are omap3 preparation; the features they add aren't used in
> the omap2 models currently in master. However I'd rather get things
> reviewed in more manageable chunks than save them all up for when I
> have omap3.c ready for commit...


Thanks Peter, do you have a tree to pull from?

Cheers


> 
> Juha Riihimäki (5):
>   hw/onenand: Qdevify
>   hw/onenand: Minor spacing fixes
>   omap_gpmc: Take omap_mpu_state* in omap_gpmc_init
>   omap_gpmc: Calculate revision from OMAP model
>   omap_gpmc: Accept a zero mask field on omap3630
> 
> Peter Maydell (12):
>   hw/sysbus: Add sysbus_mmio_get_region()
>   omap_gpmc: Clean up omap_gpmc_attach MemoryRegion conversion
>   omap_gpmc: Refactor omap_gpmc_cs_map and omap_gpmc_cs_unmap
>   omap_gpmc: GPMC_IRQSTATUS is write-one-to-clear
>   omap_gpmc: Wire up the GPMC IRQ correctly
>   omap_gpmc: Fix handling of FIFOTHRESHOLDSTATUS bit
>   omap_gpmc: Reindent misindented switch statements
>   omap_gpmc: Support NAND devices
>   hw/omap.h: Add OMAP 3630 to omap_mpu_model enumeration
>   omap_gpmc: Pull prefetch engine data into sub-struct
>   omap: Wire up the DMA request line to the GPMC
>   omap_gpmc: Implement prefetch engine
> 
>  hw/flash.h     |   10 +-
>  hw/nseries.c   |   13 +-
>  hw/omap.h      |   14 +-
>  hw/omap2.c     |    3 +-
>  hw/omap_gpmc.c |  713 +++++++++++++++++++++++++++++++++++++++++++++-----------
>  hw/onenand.c   |  179 +++++++++++----
>  hw/sysbus.c    |    5 +
>  hw/sysbus.h    |    1 +
>  8 files changed, 740 insertions(+), 198 deletions(-)
> 
> 

Re: [Qemu-devel] [PATCH 00/17] onenand, omap_gpmc fixes, features

[Peter Maydell]

On 27 August 2011 03:30, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Aug 25, 2011 at 09:04:54PM +0100, Peter Maydell wrote:
>> This patchset is primarily features and bugfixes for the
>> omap_gpmc device from the Meego tree.

> Thanks Peter, do you have a tree to pull from?

git://git.linaro.org/people/pmaydell/qemu-arm.git omap-for-upstream

-- PMM

Re: [Qemu-devel] [PATCH 02/17] hw/onenand: Qdevify

[Edgar E. Iglesias]

On Thu, Aug 25, 2011 at 09:04:56PM +0100, Peter Maydell wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
> 
> Qdevify the ONENAND device.
> 
> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
> [Riku Voipio: Fixes and restructuring patchset]
> Signed-off-by: Riku Voipio <riku.voipio@iki.fi>
> [Peter Maydell: More fixes and cleanups for upstream submission]
> Signed-off-by:  Peter Maydell <peter.maydell@linaro.org>


Hello



> ---
>  hw/flash.h   |   10 ++--
>  hw/nseries.c |    9 ++-
>  hw/onenand.c |  165 +++++++++++++++++++++++++++++++++++++++++++++-------------
>  3 files changed, 138 insertions(+), 46 deletions(-)
> 
> diff --git a/hw/flash.h b/hw/flash.h
> index 7fb012b..de3c5c3 100644
> --- a/hw/flash.h
> +++ b/hw/flash.h
> @@ -42,12 +42,10 @@ uint32_t nand_getbuswidth(DeviceState *dev);
>  #define NAND_MFR_MICRON		0x2c
>  
>  /* onenand.c */
> -void onenand_base_update(void *opaque, target_phys_addr_t new);
> -void onenand_base_unmap(void *opaque);
> -void *onenand_init(BlockDriverState *bdrv,
> -                uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
> -                int regshift, qemu_irq irq);
> -void *onenand_raw_otp(void *opaque);
> +DeviceState *onenand_init(BlockDriverState *bdrv,
> +                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
> +                          int regshift, qemu_irq irq);
> +void *onenand_raw_otp(DeviceState *onenand_device);
>  
>  /* ecc.c */
>  typedef struct {
> diff --git a/hw/nseries.c b/hw/nseries.c
> index f7aae7a..e61014c 100644
> --- a/hw/nseries.c
> +++ b/hw/nseries.c
> @@ -33,6 +33,7 @@
>  #include "loader.h"
>  #include "blockdev.h"
>  #include "tusb6010.h"
> +#include "sysbus.h"
>  
>  /* Nokia N8x0 support */
>  struct n800_s {
> @@ -52,7 +53,7 @@ struct n800_s {
>      TUSBState *usb;
>      void *retu;
>      void *tahvo;
> -    void *nand;
> +    DeviceState *nand;
>  };
>  
>  /* GPIO pins */
> @@ -172,8 +173,10 @@ static void n8x0_nand_setup(struct n800_s *s)
>      s->nand = onenand_init(dinfo ? dinfo->bdrv : 0,
>                      NAND_MFR_SAMSUNG, 0x48, 0, 1,
>                      qdev_get_gpio_in(s->cpu->gpio, N8X0_ONENAND_GPIO));
> -    omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS, 0, onenand_base_update,
> -                    onenand_base_unmap, s->nand);
> +    omap_gpmc_attach(s->cpu->gpmc, N8X0_ONENAND_CS,
> +                     sysbus_mmio_get_region(sysbus_from_qdev(s->nand), 0),
> +                     NULL, NULL,
> +                     s->nand);
>      otp_region = onenand_raw_otp(s->nand);
>  
>      memcpy(otp_region + 0x000, n8x0_cal_wlan_mac, sizeof(n8x0_cal_wlan_mac));
> diff --git a/hw/onenand.c b/hw/onenand.c
> index 00276a0..15fffc8 100644
> --- a/hw/onenand.c
> +++ b/hw/onenand.c
> @@ -25,6 +25,7 @@
>  #include "blockdev.h"
>  #include "memory.h"
>  #include "exec-memory.h"
> +#include "sysbus.h"
>  
>  /* 11 for 2kB-page OneNAND ("2nd generation") and 10 for 1kB-page chips */
>  #define PAGE_SHIFT	11
> @@ -33,6 +34,7 @@
>  #define BLOCK_SHIFT	(PAGE_SHIFT + 6)
>  
>  typedef struct {
> +    SysBusDevice busdev;
>      struct {
>          uint16_t man;
>          uint16_t dev;
> @@ -49,6 +51,7 @@ typedef struct {
>      uint8_t *current;
>      MemoryRegion ram;
>      MemoryRegion mapped_ram;
> +    uint8_t current_direction;
>      uint8_t *boot[2];
>      uint8_t *data[2][2];
>      MemoryRegion iomem;
> @@ -120,27 +123,72 @@ static void onenand_mem_setup(OneNANDState *s)
>                                          1);
>  }
>  
> -void onenand_base_update(void *opaque, target_phys_addr_t new)
> +static void onenand_intr_update(OneNANDState *s)
>  {
> -    OneNANDState *s = (OneNANDState *) opaque;
> -
> -    s->base = new;
> -
> -    memory_region_add_subregion(get_system_memory(), s->base, &s->container);
> +    qemu_set_irq(s->intr, ((s->intstatus >> 15) ^ (~s->config[0] >> 6)) & 1);
>  }
>  
> -void onenand_base_unmap(void *opaque)
> +static void onenand_pre_save(void *opaque)
>  {
> -    OneNANDState *s = (OneNANDState *) opaque;
> -
> -    memory_region_del_subregion(get_system_memory(), &s->container);
> +    OneNANDState *s = opaque;
> +    if (s->current == s->otp) {
> +        s->current_direction = 1;
> +    } else if (s->current == s->image) {
> +        s->current_direction = 2;
> +    } else {
> +        s->current_direction = 0;
> +    }
>  }
>  
> -static void onenand_intr_update(OneNANDState *s)
> +static int onenand_post_load(void *opaque, int version_id)
>  {
> -    qemu_set_irq(s->intr, ((s->intstatus >> 15) ^ (~s->config[0] >> 6)) & 1);
> +    OneNANDState *s = opaque;
> +    switch (s->current_direction) {
> +    case 0:
> +        break;
> +    case 1:
> +        s->current = s->otp;
> +        break;
> +    case 2:
> +        s->current = s->image;
> +        break;
> +    default:
> +        return -1;
> +    }
> +    onenand_intr_update(s);
> +    return 0;
>  }
>  
> +static const VMStateDescription vmstate_onenand = {
> +    .name = "onenand",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .pre_save = onenand_pre_save,
> +    .post_load = onenand_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8(current_direction, OneNANDState),
> +        VMSTATE_INT32(cycle, OneNANDState),
> +        VMSTATE_INT32(otpmode, OneNANDState),
> +        VMSTATE_UINT16_ARRAY(addr, OneNANDState, 8),
> +        VMSTATE_UINT16_ARRAY(unladdr, OneNANDState, 8),
> +        VMSTATE_INT32(bufaddr, OneNANDState),
> +        VMSTATE_INT32(count, OneNANDState),
> +        VMSTATE_UINT16(command, OneNANDState),
> +        VMSTATE_UINT16_ARRAY(config, OneNANDState, 2),
> +        VMSTATE_UINT16(status, OneNANDState),
> +        VMSTATE_UINT16(intstatus, OneNANDState),
> +        VMSTATE_UINT16(wpstatus, OneNANDState),
> +        VMSTATE_INT32(secs_cur, OneNANDState),
> +        VMSTATE_PARTIAL_VBUFFER(blockwp, OneNANDState, blocks),
> +        VMSTATE_UINT8(ecc.cp, OneNANDState),
> +        VMSTATE_UINT16_ARRAY(ecc.lp, OneNANDState, 2),
> +        VMSTATE_UINT16(ecc.count, OneNANDState),
> +        VMSTATE_BUFFER_UNSAFE(otp, OneNANDState, 0, ((64 + 2) << PAGE_SHIFT)),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  /* Hot reset (Reset OneNAND command) or warm reset (RP pin low) */
>  static void onenand_reset(OneNANDState *s, int cold)
>  {
> @@ -167,11 +215,17 @@ static void onenand_reset(OneNANDState *s, int cold)
>          /* Lock the whole flash */
>          memset(s->blockwp, ONEN_LOCK_LOCKED, s->blocks);
>  
> -        if (s->bdrv && bdrv_read(s->bdrv, 0, s->boot[0], 8) < 0)
> -            hw_error("%s: Loading the BootRAM failed.\n", __FUNCTION__);
> +        if (s->bdrv_cur && bdrv_read(s->bdrv_cur, 0, s->boot[0], 8) < 0) {
> +            hw_error("%s: Loading the BootRAM failed.\n", __func__);
> +        }
>      }
>  }
>  
> +static void onenand_system_reset(DeviceState *dev)
> +{
> +    onenand_reset(FROM_SYSBUS(OneNANDState, sysbus_from_qdev(dev)), 1);
> +}
> +
>  static inline int onenand_load_main(OneNANDState *s, int sec, int secn,
>                  void *dest)
>  {
> @@ -326,7 +380,7 @@ fail:
>      return 1;
>  }
>  
> -static void onenand_command(OneNANDState *s, int cmd)
> +static void onenand_command(OneNANDState *s)
>  {
>      int b;
>      int sec;
> @@ -346,7 +400,7 @@ static void onenand_command(OneNANDState *s, int cmd)
>              s->data[(s->bufaddr >> 2) & 1][1] : s->boot[1];	\
>      buf += (s->bufaddr & 3) << 4;
>  
> -    switch (cmd) {
> +    switch (s->command) {
>      case 0x00:	/* Load single/multiple sector data unit into buffer */
>          SETADDR(ONEN_BUF_BLOCK, ONEN_BUF_PAGE)
>  
> @@ -527,7 +581,7 @@ static void onenand_command(OneNANDState *s, int cmd)
>          s->status |= ONEN_ERR_CMD;
>          s->intstatus |= ONEN_INT;
>          fprintf(stderr, "%s: unknown OneNAND command %x\n",
> -                        __FUNCTION__, cmd);
> +                        __func__, s->command);
>      }
>  
>      onenand_intr_update(s);
> @@ -659,7 +713,7 @@ static void onenand_write(void *opaque, target_phys_addr_t addr,
>          if (s->intstatus & (1 << 15))
>              break;
>          s->command = value;
> -        onenand_command(s, s->command);
> +        onenand_command(s);
>          break;


This s->command change doesnt seem related, is there a reason for it that
I'm missing?



>      case 0xf221:	/* System Configuration 1 */
>          s->config[0] = value;
> @@ -700,30 +754,25 @@ static const MemoryRegionOps onenand_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -void *onenand_init(BlockDriverState *bdrv,
> -                uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
> -                int regshift, qemu_irq irq)
> +static int onenand_initfn(SysBusDevice *dev)
>  {
> -    OneNANDState *s = (OneNANDState *) g_malloc0(sizeof(*s));
> -    uint32_t size = 1 << (24 + ((dev_id >> 4) & 7));
> +    OneNANDState *s = (OneNANDState *)dev;
> +    uint32_t size = 1 << (24 + ((s->id.dev >> 4) & 7));
>      void *ram;
> -
> -    s->shift = regshift;
> -    s->intr = irq;
> +    s->base = (target_phys_addr_t)-1;
>      s->rdy = NULL;
> -    s->id.man = man_id;
> -    s->id.dev = dev_id;
> -    s->id.ver = ver_id;
>      s->blocks = size >> BLOCK_SHIFT;
>      s->secs = size >> 9;
>      s->blockwp = g_malloc(s->blocks);
> -    s->density_mask = (dev_id & 0x08) ? (1 << (6 + ((dev_id >> 4) & 7))) : 0;
> +    s->density_mask = (s->id.dev & 0x08)
> +        ? (1 << (6 + ((s->id.dev >> 4) & 7))) : 0;
>      memory_region_init_io(&s->iomem, &onenand_ops, s, "onenand",
>                            0x10000 << s->shift);
> -    s->bdrv = bdrv;
>      if (!s->bdrv) {
>          s->image = memset(g_malloc(size + (size >> 5)),
> -                        0xff, size + (size >> 5));
> +                          0xff, size + (size >> 5));
> +    } else {
> +        s->bdrv_cur = s->bdrv;
>      }
>      s->otp = memset(g_malloc((64 + 2) << PAGE_SHIFT),
>                      0xff, (64 + 2) << PAGE_SHIFT);
> @@ -736,15 +785,57 @@ void *onenand_init(BlockDriverState *bdrv,
>      s->data[1][0] = ram + ((0x0200 + (1 << (PAGE_SHIFT - 1))) << s->shift);
>      s->data[1][1] = ram + ((0x8010 + (1 << (PAGE_SHIFT - 6))) << s->shift);
>      onenand_mem_setup(s);
> +    sysbus_init_irq(dev, &s->intr);
> +    sysbus_init_mmio_region(dev, &s->container);
> +    vmstate_register(&dev->qdev,
> +                     ((s->shift & 0x7f) << 24)
> +                     | ((s->id.man & 0xff) << 16)
> +                     | ((s->id.dev & 0xff) << 8)
> +                     | (s->id.ver & 0xff),
> +                     &vmstate_onenand, s);
> +    return 0;
> +}
>  
> -    onenand_reset(s, 1);
> +static SysBusDeviceInfo onenand_info = {
> +    .init = onenand_initfn,
> +    .qdev.name = "onenand",
> +    .qdev.size = sizeof(OneNANDState),
> +    .qdev.reset = onenand_system_reset,
> +    .qdev.props = (Property[]) {
> +        DEFINE_PROP_UINT16("manufacturer_id", OneNANDState, id.man, 0),
> +        DEFINE_PROP_UINT16("device_id", OneNANDState, id.dev, 0),
> +        DEFINE_PROP_UINT16("version_id", OneNANDState, id.ver, 0),
> +        DEFINE_PROP_INT32("shift", OneNANDState, shift, 0),
> +        DEFINE_PROP_DRIVE("drive", OneNANDState, bdrv),
> +        DEFINE_PROP_END_OF_LIST()
> +    }
> +};
>  
> -    return s;
> +static void onenand_register_device(void)
> +{
> +    sysbus_register_withprop(&onenand_info);
>  }
>  
> -void *onenand_raw_otp(void *opaque)
> +DeviceState *onenand_init(BlockDriverState *bdrv,
> +                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
> +                          int regshift, qemu_irq irq)
>  {
> -    OneNANDState *s = (OneNANDState *) opaque;
> +    DeviceState *dev = qdev_create(NULL, "onenand");
> +    qdev_prop_set_uint16(dev, "manufacturer_id", man_id);
> +    qdev_prop_set_uint16(dev, "device_id", dev_id);
> +    qdev_prop_set_uint16(dev, "version_id", ver_id);
> +    qdev_prop_set_int32(dev, "shift", regshift);
> +    if (bdrv) {
> +        qdev_prop_set_drive_nofail(dev, "drive", bdrv);
> +    }
> +    qdev_init_nofail(dev);
> +    sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
> +    return dev;
> +}

Personally Im not a fan of having code that conceptually runs above Qdev,
embedded in qdev models. But there seems to be a lack of agreement on this
and its commonly done elsewere. I'm not NAKing but if you agree, and would
like to move it out, I'd appreciate it.

This looks good otherwise.

Cheers

Re: [Qemu-devel] [PATCH 00/17] onenand, omap_gpmc fixes, features

[Edgar E. Iglesias]

On Thu, Aug 25, 2011 at 09:04:54PM +0100, Peter Maydell wrote:
> This patchset is primarily features and bugfixes for the
> omap_gpmc device from the Meego tree.
> 
> We start by adding a new sysbus function to get the MemoryRegion*
> for a sysbus MMIO region. This was discussed in
>  http://www.mail-archive.com/kvm@vger.kernel.org/msg59535.html
> and as noted there can also be used to clean up other things,
> so it's not just an omap_gpmc thing.
> 
> The bulk of the onenand qdevification patch was in an earlier series
> I posted; this update makes it work with MemoryRegions.
> 
> The patches from "omap_gpmc: Take omap_mpu_state* in omap_gpmc_init"
> onwards are omap3 preparation; the features they add aren't used in
> the omap2 models currently in master. However I'd rather get things
> reviewed in more manageable chunks than save them all up for when I
> have omap3.c ready for commit...


This looks good to me except for some minor questions on:
[PATCH 02/17] hw/onenand: Qdevify

Cheers

Re: [Qemu-devel] [PATCH 02/17] hw/onenand: Qdevify

[Peter Maydell]

On 27 August 2011 18:38, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Aug 25, 2011 at 09:04:56PM +0100, Peter Maydell wrote:
>> @@ -659,7 +713,7 @@ static void onenand_write(void *opaque, target_phys_addr_t addr,
>>          if (s->intstatus & (1 << 15))
>>              break;
>>          s->command = value;
>> -        onenand_command(s, s->command);
>> +        onenand_command(s);
>>          break;
>
>
> This s->command change doesnt seem related, is there a reason for it that
> I'm missing?

Are you objecting to the change itself or the fact it's in this
patch rather than its own patch? (I'm happy to split it out into
a separate patch if you prefer.)

I think the change itself is right -- in a qdev device these
functions are basically methods on the qdev object, and it
doesn't make sense to pass a method one of the object's own
fields as a method argument. So either we should have
   onenand_command(s, value);
and make onenand_command do the setting of s->command;
or we do what this patch does and let onenand_command()
read the member field.

>> +DeviceState *onenand_init(BlockDriverState *bdrv,
>> +                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
>> +                          int regshift, qemu_irq irq)
>>  {
>> -    OneNANDState *s = (OneNANDState *) opaque;
>> +    DeviceState *dev = qdev_create(NULL, "onenand");
>> +    qdev_prop_set_uint16(dev, "manufacturer_id", man_id);
>> +    qdev_prop_set_uint16(dev, "device_id", dev_id);
>> +    qdev_prop_set_uint16(dev, "version_id", ver_id);
>> +    qdev_prop_set_int32(dev, "shift", regshift);
>> +    if (bdrv) {
>> +        qdev_prop_set_drive_nofail(dev, "drive", bdrv);
>> +    }
>> +    qdev_init_nofail(dev);
>> +    sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
>> +    return dev;
>> +}
>
> Personally Im not a fan of having code that conceptually runs above Qdev,
> embedded in qdev models. But there seems to be a lack of agreement on this
> and its commonly done elsewere. I'm not NAKing but if you agree, and would
> like to move it out, I'd appreciate it.

I think they're really just utility functions which are working around
the problem qdev has that instantiating, configuring and wiring up
a qdev model is so verbose. But I'm happy to lose this one, especially
since it's only used once. (well, twice once I get the n900 model in
a submittable state...)

Thanks for the rapid review of this patchset.

-- PMM

Re: [Qemu-devel] [PATCH 02/17] hw/onenand: Qdevify

[Edgar E. Iglesias]

On Sun, Aug 28, 2011 at 02:21:29PM +0100, Peter Maydell wrote:
> On 27 August 2011 18:38, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > On Thu, Aug 25, 2011 at 09:04:56PM +0100, Peter Maydell wrote:
> >> @@ -659,7 +713,7 @@ static void onenand_write(void *opaque, target_phys_addr_t addr,
> >>          if (s->intstatus & (1 << 15))
> >>              break;
> >>          s->command = value;
> >> -        onenand_command(s, s->command);
> >> +        onenand_command(s);
> >>          break;
> >
> >
> > This s->command change doesnt seem related, is there a reason for it that
> > I'm missing?
> 
> Are you objecting to the change itself or the fact it's in this
> patch rather than its own patch? (I'm happy to split it out into
> a separate patch if you prefer.)
> 
> I think the change itself is right -- in a qdev device these
> functions are basically methods on the qdev object, and it
> doesn't make sense to pass a method one of the object's own
> fields as a method argument. So either we should have
>    onenand_command(s, value);
> and make onenand_command do the setting of s->command;
> or we do what this patch does and let onenand_command()
> read the member field.

OK thanks, I see them more like stylistic changes and would have
probably left them out for the sake of reviewability. But it doesn't
matter, my main concern was that I was missing something more important
here. No need to respin just for this..


> >> +DeviceState *onenand_init(BlockDriverState *bdrv,
> >> +                          uint16_t man_id, uint16_t dev_id, uint16_t ver_id,
> >> +                          int regshift, qemu_irq irq)
> >>  {
> >> -    OneNANDState *s = (OneNANDState *) opaque;
> >> +    DeviceState *dev = qdev_create(NULL, "onenand");
> >> +    qdev_prop_set_uint16(dev, "manufacturer_id", man_id);
> >> +    qdev_prop_set_uint16(dev, "device_id", dev_id);
> >> +    qdev_prop_set_uint16(dev, "version_id", ver_id);
> >> +    qdev_prop_set_int32(dev, "shift", regshift);
> >> +    if (bdrv) {
> >> +        qdev_prop_set_drive_nofail(dev, "drive", bdrv);
> >> +    }
> >> +    qdev_init_nofail(dev);
> >> +    sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
> >> +    return dev;
> >> +}
> >
> > Personally Im not a fan of having code that conceptually runs above Qdev,
> > embedded in qdev models. But there seems to be a lack of agreement on this
> > and its commonly done elsewere. I'm not NAKing but if you agree, and would
> > like to move it out, I'd appreciate it.
> 
> I think they're really just utility functions which are working around
> the problem qdev has that instantiating, configuring and wiring up
> a qdev model is so verbose. But I'm happy to lose this one, especially
> since it's only used once. (well, twice once I get the n900 model in
> a submittable state...)

Thanks. Please note that I'm not opposed to the helpers per se, but more
with the placement of them. It was shortly discussed here (a long while ago):
http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00843.html

Cheers