All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller
@ 2012-08-06  2:16 Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 01/15] ssi: Support for multiple attached devices Peter A. G. Crosthwaite
                   ` (14 more replies)
  0 siblings, 15 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

This series reworks the SSI bus framework for SPI and add some new SPI controllers and devices:

Patches 1-5 reworks SSI to add chip-select support to SPI devices and allow for multiple SPI devices attach
ed to the same bus.

Patches 6-7 fix the SPI setup in the stellaris machine model.

Patch 8 is a trivial cleanup I did along the way.

Patch 9 is a general FIFO helper API used by the upcomming patches.

Patch 10 is a device model for the m25p80 SPI flash family.

Patches 11 & 13 are the Xilinx SPI flash controller devices

Patches 12 & 14 add SPI controllers to the ML605 and Zynq machine models.

Patch 15 is Maintainerships.

CHANGELOG:
changed from v4 (Major changes):
Completely reworked SPI refactor. Please re-review from scratch.
Added Zynq SPI flash.
Factored out FIFO functionality from SPI flash controller.
changed from v3:
addressed reviewer comments from P Maydell and S Hajnoczi
added patch 5 (re Paul Brooks request)
changed from v2:
folded former SPI bus functionality into existing SSI infrastructure (suggested - Paul Brook) (all patches)
made m25p80 use async io (suggested - Stefan Hajnoczi) (2/4)
instantiated two spi flashes instead of one in ml605 ref design (4/4)
changed from v1:
minor sylistic changes (1/4)
converted spi api to modified txrx style (1-3/4)
heavily refactored m25p80 model (2/4)

Peter A. G. Crosthwaite (15):
  ssi: Support for multiple attached devices
  ssi: Added VMSD stub
  ssi: Implemented CS behaviour
  ssi: Added create_slave_no_init()
  qdev: allow multiple qdev_init_gpio_in() calls
  hw/stellaris: Removed gpio_out init array.
  stellaris: Removed SSI mux
  ssd0323: abort() instead of exit(1) on error.
  hw: Added generic FIFO API.
  m25p80: Initial implementation of SPI flash device
  xilinx_spi: Initial impl. of Xilinx SPI controller
  petalogix-ml605: added SPI controller with n25q128
  xilinx_spips: Xilinx Zynq SPI cntrlr device model
  xilinx_zynq: Added SPI controllers + flashes
  MAINTAINERS: Added maintainerships for SSI

 MAINTAINERS                              |    8 +
 default-configs/arm-softmmu.mak          |    1 +
 default-configs/microblaze-softmmu.mak   |    2 +
 default-configs/microblazeel-softmmu.mak |    2 +
 hw/Makefile.objs                         |    2 +
 hw/ads7846.c                             |    1 +
 hw/arm/Makefile.objs                     |    1 +
 hw/fifo.c                                |   79 ++++
 hw/fifo.h                                |   47 +++
 hw/m25p80.c                              |  572 ++++++++++++++++++++++++++++++
 hw/max111x.c                             |    1 +
 hw/microblaze/Makefile.objs              |    1 +
 hw/petalogix_ml605_mmu.c                 |   28 ++-
 hw/qdev.c                                |   16 +-
 hw/spitz.c                               |    2 +
 hw/ssd0323.c                             |   11 +-
 hw/ssi-sd.c                              |    7 +
 hw/ssi.c                                 |   76 ++++-
 hw/ssi.h                                 |   38 ++
 hw/stellaris.c                           |  111 ++-----
 hw/xilinx_spi.c                          |  390 ++++++++++++++++++++
 hw/xilinx_spips.c                        |  352 ++++++++++++++++++
 hw/xilinx_zynq.c                         |   34 ++
 hw/z2.c                                  |    1 +
 24 files changed, 1675 insertions(+), 108 deletions(-)
 create mode 100644 hw/fifo.c
 create mode 100644 hw/fifo.h
 create mode 100644 hw/m25p80.c
 create mode 100644 hw/xilinx_spi.c
 create mode 100644 hw/xilinx_spips.c

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 01/15] ssi: Support for multiple attached devices
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub Peter A. G. Crosthwaite
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Removed assertion that only one device is attached to the SSI bus.

When multiple devices are attached, all slaves have their transfer function
called for transfers. Each device is responsible for knowing whether or not its
CS is active, and if not returning 0. The returned data is the logical or of
all responses from the (mulitple) devices.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ssi.c |   24 +++++++++---------------
 1 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/ssi.c b/hw/ssi.c
index e5f14a0..35d0a04 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -2,6 +2,8 @@
  * QEMU Synchronous Serial Interface support
  *
  * Copyright (c) 2009 CodeSourcery.
+ * Copyright (c) 2012 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.com)
+ * Copyright (c) 2012 PetaLogix Pty Ltd.
  * Written by Paul Brook
  *
  * This code is licensed under the GNU GPL v2.
@@ -29,14 +31,6 @@ static int ssi_slave_init(DeviceState *dev)
 {
     SSISlave *s = SSI_SLAVE(dev);
     SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
-    SSIBus *bus;
-    BusChild *kid;
-
-    bus = FROM_QBUS(SSIBus, qdev_get_parent_bus(dev));
-    kid = QTAILQ_FIRST(&bus->qbus.children);
-    if (kid->child != dev || QTAILQ_NEXT(kid, sibling) != NULL) {
-        hw_error("Too many devices on SSI bus");
-    }
 
     return ssc->init(s);
 }
@@ -74,16 +68,16 @@ SSIBus *ssi_create_bus(DeviceState *parent, const char *name)
 uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
 {
     BusChild *kid;
-    SSISlave *slave;
     SSISlaveClass *ssc;
+    uint32_t r = 0;
 
-    kid = QTAILQ_FIRST(&bus->qbus.children);
-    if (!kid) {
-        return 0;
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+        SSISlave *slave = SSI_SLAVE(kid->child);
+        ssc = SSI_SLAVE_GET_CLASS(slave);
+        r |= ssc->transfer(slave, val);
     }
-    slave = SSI_SLAVE(kid->child);
-    ssc = SSI_SLAVE_GET_CLASS(slave);
-    return ssc->transfer(slave, val);
+
+    return r;
 }
 
 static void ssi_slave_register_types(void)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 01/15] ssi: Support for multiple attached devices Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:13   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour Peter A. G. Crosthwaite
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
SSI slave state (e.g. the CS line state).

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ads7846.c   |    1 +
 hw/max111x.c   |    1 +
 hw/spitz.c     |    2 ++
 hw/ssi.c       |   10 ++++++++++
 hw/ssi.h       |   10 ++++++++++
 hw/stellaris.c |    1 +
 hw/z2.c        |    1 +
 7 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/ads7846.c b/hw/ads7846.c
index 41c7f10..6a523f6 100644
--- a/hw/ads7846.c
+++ b/hw/ads7846.c
@@ -124,6 +124,7 @@ static const VMStateDescription vmstate_ads7846 = {
     .minimum_version_id_old = 0,
     .post_load = ads7856_post_load,
     .fields      = (VMStateField[]) {
+        VMSTATE_SSI_SLAVE(ssidev, ADS7846State),
         VMSTATE_INT32_ARRAY(input, ADS7846State, 8),
         VMSTATE_INT32(noise, ADS7846State),
         VMSTATE_INT32(cycle, ADS7846State),
diff --git a/hw/max111x.c b/hw/max111x.c
index 706d89f..948fd97 100644
--- a/hw/max111x.c
+++ b/hw/max111x.c
@@ -103,6 +103,7 @@ static const VMStateDescription vmstate_max111x = {
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .fields      = (VMStateField[]) {
+        VMSTATE_SSI_SLAVE(ssidev, MAX111xState),
         VMSTATE_UINT8(tb1, MAX111xState),
         VMSTATE_UINT8(rb2, MAX111xState),
         VMSTATE_UINT8(rb3, MAX111xState),
diff --git a/hw/spitz.c b/hw/spitz.c
index 20e7835..f5d502d 100644
--- a/hw/spitz.c
+++ b/hw/spitz.c
@@ -1087,6 +1087,7 @@ static const VMStateDescription vmstate_corgi_ssp_regs = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields = (VMStateField []) {
+        VMSTATE_SSI_SLAVE(ssidev, CorgiSSPState),
         VMSTATE_UINT32_ARRAY(enable, CorgiSSPState, 3),
         VMSTATE_END_OF_LIST(),
     }
@@ -1115,6 +1116,7 @@ static const VMStateDescription vmstate_spitz_lcdtg_regs = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields = (VMStateField []) {
+        VMSTATE_SSI_SLAVE(ssidev, SpitzLCDTG),
         VMSTATE_UINT32(bl_intensity, SpitzLCDTG),
         VMSTATE_UINT32(bl_power, SpitzLCDTG),
         VMSTATE_END_OF_LIST(),
diff --git a/hw/ssi.c b/hw/ssi.c
index 35d0a04..2db88fc 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -80,6 +80,16 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
     return r;
 }
 
+const VMStateDescription vmstate_ssi_slave = {
+    .name = "SSISlave",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void ssi_slave_register_types(void)
 {
     type_register_static(&ssi_bus_info);
diff --git a/hw/ssi.h b/hw/ssi.h
index 06657d7..975f9fb 100644
--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -38,6 +38,16 @@ struct SSISlave {
 #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
 #define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
 
+extern const VMStateDescription vmstate_ssi_slave;
+
+#define VMSTATE_SSI_SLAVE(_field, _state) {                          \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(SSISlave),                                  \
+    .vmsd       = &vmstate_ssi_slave,                                \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, SSISlave),    \
+}
+
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
 
 /* Master interface.  */
diff --git a/hw/stellaris.c b/hw/stellaris.c
index 562fbbf..4d26857 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1188,6 +1188,7 @@ static const VMStateDescription vmstate_stellaris_ssi_bus = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields      = (VMStateField[]) {
+        VMSTATE_SSI_SLAVE(ssidev, stellaris_ssi_bus_state),
         VMSTATE_INT32(current_dev, stellaris_ssi_bus_state),
         VMSTATE_END_OF_LIST()
     }
diff --git a/hw/z2.c b/hw/z2.c
index 289cee9..721aaf1 100644
--- a/hw/z2.c
+++ b/hw/z2.c
@@ -165,6 +165,7 @@ static VMStateDescription vmstate_zipit_lcd_state = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields = (VMStateField[]) {
+        VMSTATE_SSI_SLAVE(ssidev, ZipitLCD),
         VMSTATE_INT32(selected, ZipitLCD),
         VMSTATE_INT32(enabled, ZipitLCD),
         VMSTATE_BUFFER(buf, ZipitLCD),
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 01/15] ssi: Support for multiple attached devices Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:25   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init() Peter A. G. Crosthwaite
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added default CS behaviour for SSI slaves. SSI devices can set a property
to enable CS behaviour which will create a GPIO on the device which is the
CS. Tristating of the bus on SSI transfers is implemented.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ssd0323.c |    6 ++++++
 hw/ssi-sd.c  |    6 ++++++
 hw/ssi.c     |   39 ++++++++++++++++++++++++++++++++++++++-
 hw/ssi.h     |   27 +++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/hw/ssd0323.c b/hw/ssd0323.c
index b0b2e94..db16d20 100644
--- a/hw/ssd0323.c
+++ b/hw/ssd0323.c
@@ -277,6 +277,7 @@ static void ssd0323_cd(void *opaque, int n, int level)
 
 static void ssd0323_save(QEMUFile *f, void *opaque)
 {
+    SSISlave *ss = SSI_SLAVE(opaque);
     ssd0323_state *s = (ssd0323_state *)opaque;
     int i;
 
@@ -294,10 +295,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->remap);
     qemu_put_be32(f, s->mode);
     qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
+
+    qemu_put_be32(f, ss->cs ? 1 : 0);
 }
 
 static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
 {
+    SSISlave *ss = SSI_SLAVE(opaque);
     ssd0323_state *s = (ssd0323_state *)opaque;
     int i;
 
@@ -319,6 +323,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
     s->mode = qemu_get_be32(f);
     qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
 
+    ss->cs = !!qemu_get_be32(f);
+
     return 0;
 }
 
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index b519bdb..6fd9ab9 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -197,6 +197,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
 
 static void ssi_sd_save(QEMUFile *f, void *opaque)
 {
+    SSISlave *ss = SSI_SLAVE(opaque);
     ssi_sd_state *s = (ssi_sd_state *)opaque;
     int i;
 
@@ -209,10 +210,13 @@ static void ssi_sd_save(QEMUFile *f, void *opaque)
     qemu_put_be32(f, s->arglen);
     qemu_put_be32(f, s->response_pos);
     qemu_put_be32(f, s->stopping);
+
+    qemu_put_be32(f, ss->cs ? 1 : 0);
 }
 
 static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
 {
+    SSISlave *ss = SSI_SLAVE(opaque);
     ssi_sd_state *s = (ssi_sd_state *)opaque;
     int i;
 
@@ -229,6 +233,8 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
     s->response_pos = qemu_get_be32(f);
     s->stopping = qemu_get_be32(f);
 
+    ss->cs = !!qemu_get_be32(f);
+
     return 0;
 }
 
diff --git a/hw/ssi.c b/hw/ssi.c
index 2db88fc..2e4f2fe 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -27,19 +27,55 @@ static const TypeInfo ssi_bus_info = {
     .instance_size = sizeof(SSIBus),
 };
 
+static void ssi_cs_default(void *opaque, int n, int level)
+{
+    SSISlave *s = SSI_SLAVE(opaque);
+    bool cs = !!level;
+    assert(n == 0);
+    if (s->cs != cs) {
+        SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
+        if (ssc->set_cs) {
+            ssc->set_cs(s, cs);
+        }
+    }
+    s->cs = cs;
+}
+
+static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
+{
+    SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(dev);
+
+    if ((dev->cs && ssc->cs_polarity == SSI_CS_HIGH) ||
+            (!dev->cs && ssc->cs_polarity == SSI_CS_LOW) ||
+            ssc->cs_polarity == SSI_CS_NONE) {
+        return ssc->transfer(dev, val);
+    }
+    return 0;
+}
+
 static int ssi_slave_init(DeviceState *dev)
 {
     SSISlave *s = SSI_SLAVE(dev);
     SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
 
+    if (ssc->transfer_raw == ssi_transfer_raw_default &&
+            ssc->cs_polarity != SSI_CS_NONE) {
+        qdev_init_gpio_in(&s->qdev, ssi_cs_default, 1);
+    }
+
     return ssc->init(s);
 }
 
 static void ssi_slave_class_init(ObjectClass *klass, void *data)
 {
+    SSISlaveClass *ssc = SSI_SLAVE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
+
     dc->init = ssi_slave_init;
     dc->bus_type = TYPE_SSI_BUS;
+    if (!ssc->transfer_raw) {
+        ssc->transfer_raw = ssi_transfer_raw_default;
+    }
 }
 
 static TypeInfo ssi_slave_info = {
@@ -74,7 +110,7 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
     QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
         SSISlave *slave = SSI_SLAVE(kid->child);
         ssc = SSI_SLAVE_GET_CLASS(slave);
-        r |= ssc->transfer(slave, val);
+        r |= ssc->transfer_raw(slave, val);
     }
 
     return r;
@@ -86,6 +122,7 @@ const VMStateDescription vmstate_ssi_slave = {
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
     .fields      = (VMStateField[]) {
+        VMSTATE_BOOL(cs, SSISlave),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/ssi.h b/hw/ssi.h
index 975f9fb..5b69a3b 100644
--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -23,16 +23,43 @@ typedef struct SSISlave SSISlave;
 #define SSI_SLAVE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE)
 
+typedef enum {
+    SSI_CS_NONE = 0,
+    SSI_CS_LOW,
+    SSI_CS_HIGH,
+} SSICSMode;
+
 /* Slave devices.  */
 typedef struct SSISlaveClass {
     DeviceClass parent_class;
 
     int (*init)(SSISlave *dev);
+
+    /* if you have standard or no CS behaviour, just override transfer.
+     * This is called when the device cs is active (true by default).
+     */
     uint32_t (*transfer)(SSISlave *dev, uint32_t val);
+    /* called when the CS line changes. Optional, devices only need to implement
+     * this if they have side effects assoicated with the cs line (beyond
+     * tristating the txrx lines).
+     */
+    int (*set_cs)(SSISlave *dev, bool select);
+    /* define whether or not CS exists and is active low/high */
+    SSICSMode cs_polarity;
+
+    /* if you have non-standard CS behvaiour override this to take control
+     * of the CS behvaiour at the device level. transfer, set_cs, and
+     * cs_polarity are unused if this is overwritten. Transfer_raw, will
+     * always be called for the device for every txrx access the to parent bus
+     */
+    uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
 } SSISlaveClass;
 
 struct SSISlave {
     DeviceState qdev;
+
+    /* Chip select state */
+    bool cs;
 };
 
 #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init()
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (2 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:29   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls Peter A. G. Crosthwaite
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Slave creation function that can be used to create an SSI slave without
qdev_init() being called. This give machine models a change to set properties.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ssi.c |    9 +++++++--
 hw/ssi.h |    1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/ssi.c b/hw/ssi.c
index 2e4f2fe..c47419d 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -86,10 +86,15 @@ static TypeInfo ssi_slave_info = {
     .abstract = true,
 };
 
+DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name)
+{
+    return qdev_create(&bus->qbus, name);
+}
+
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
 {
-    DeviceState *dev;
-    dev = qdev_create(&bus->qbus, name);
+    DeviceState *dev = ssi_create_slave_no_init(bus, name);
+
     qdev_init_nofail(dev);
     return dev;
 }
diff --git a/hw/ssi.h b/hw/ssi.h
index 5b69a3b..80b9664 100644
--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -76,6 +76,7 @@ extern const VMStateDescription vmstate_ssi_slave;
 }
 
 DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
+DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
 
 /* Master interface.  */
 SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (3 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init() Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:38   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 06/15] hw/stellaris: Removed gpio_out init array Peter A. G. Crosthwaite
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Allow multiple qdev_init_gpio_in() calls for the one device. The first call will
define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled
with different handlers. Needed when two levels of the QOM class heirachy both
define GPIO functionality, as a single GPIO handler with an index selecter is
not possible.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/qdev.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index b5b74b9..ce91a72 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -293,9 +293,19 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
 
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
 {
-    assert(dev->num_gpio_in == 0);
-    dev->num_gpio_in = n;
-    dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
+    qemu_irq *new_irqs = qemu_allocate_irqs(handler, dev, n);
+
+    if (dev->num_gpio_in == 0) {
+        dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
+    } else {
+        qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
+        memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * dev->num_gpio_in);
+        g_free(dev->gpio_in);
+        memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * n),
+        g_free(new_irqs);
+        dev->gpio_in = all_irqs;
+    }
+    dev->num_gpio_in += n;
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 06/15] hw/stellaris: Removed gpio_out init array.
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (4 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 07/15] stellaris: Removed SSI mux Peter A. G. Crosthwaite
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

stellaris_init() defines arrays of qemu_irq to decides what each of the GPIO
pins are connected to. This is ok for inputs (as an input can only have one
source) but is flawed for outputs as an output can connect to any number of
sinks. Removed the gpio_out array completely and just replaced its setters with
direct calls to qdev_connect_gpio_out().

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/stellaris.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/stellaris.c b/hw/stellaris.c
index 4d26857..ec55c0e 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1244,7 +1244,6 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
     qemu_irq *pic;
     DeviceState *gpio_dev[7];
     qemu_irq gpio_in[7][8];
-    qemu_irq gpio_out[7][8];
     qemu_irq adc;
     int sram_size;
     int flash_size;
@@ -1284,8 +1283,9 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
                                                pic[gpio_irq[i]]);
             for (j = 0; j < 8; j++) {
                 gpio_in[i][j] = qdev_get_gpio_in(gpio_dev[i], j);
-                gpio_out[i][j] = NULL;
             }
+        } else {
+            gpio_dev[i] = NULL;
         }
     }
 
@@ -1308,20 +1308,27 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
         if (board->peripherals & BP_OLED_SSI) {
             DeviceState *mux;
             void *bus;
+            qemu_irq select_pin;
 
             bus = qdev_get_child_bus(dev, "ssi");
             mux = ssi_create_slave(bus, "evb6965-ssi");
-            gpio_out[GPIO_D][0] = qdev_get_gpio_in(mux, 0);
+            select_pin = qdev_get_gpio_in(mux, 0);
+            if (gpio_dev[GPIO_D]) {
+                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0, select_pin);
+            }
 
             bus = qdev_get_child_bus(mux, "ssi0");
             ssi_create_slave(bus, "ssi-sd");
 
             bus = qdev_get_child_bus(mux, "ssi1");
             dev = ssi_create_slave(bus, "ssd0323");
-            gpio_out[GPIO_C][7] = qdev_get_gpio_in(dev, 0);
+            if (gpio_dev[GPIO_C]) {
+                qdev_connect_gpio_out(gpio_dev[GPIO_C], 7,
+                                        qdev_get_gpio_in(dev, 0));
+            }
 
             /* Make sure the select pin is high.  */
-            qemu_irq_raise(gpio_out[GPIO_D][0]);
+            qemu_irq_raise(select_pin);
         }
     }
     if (board->dc4 & (1 << 28)) {
@@ -1347,15 +1354,6 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
 
         stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
     }
-    for (i = 0; i < 7; i++) {
-        if (board->dc4 & (1 << i)) {
-            for (j = 0; j < 8; j++) {
-                if (gpio_out[i][j]) {
-                    qdev_connect_gpio_out(gpio_dev[i], j, gpio_out[i][j]);
-                }
-            }
-        }
-    }
 }
 
 /* FIXME: Figure out how to generate these from stellaris_boards.  */
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 07/15] stellaris: Removed SSI mux
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (5 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 06/15] hw/stellaris: Removed gpio_out init array Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error Peter A. G. Crosthwaite
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Removed the explicit SSI mux and wired the CS line directly up to the SSI
devices.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ssd0323.c   |    1 +
 hw/ssi-sd.c    |    1 +
 hw/stellaris.c |   98 ++++++++++----------------------------------------------
 3 files changed, 19 insertions(+), 81 deletions(-)

diff --git a/hw/ssd0323.c b/hw/ssd0323.c
index db16d20..d8a0c14 100644
--- a/hw/ssd0323.c
+++ b/hw/ssd0323.c
@@ -352,6 +352,7 @@ static void ssd0323_class_init(ObjectClass *klass, void *data)
 
     k->init = ssd0323_init;
     k->transfer = ssd0323_transfer;
+    k->cs_polarity = SSI_CS_HIGH;
 }
 
 static TypeInfo ssd0323_info = {
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index 6fd9ab9..1b1f625 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -256,6 +256,7 @@ static void ssi_sd_class_init(ObjectClass *klass, void *data)
 
     k->init = ssi_sd_init;
     k->transfer = ssi_sd_transfer;
+    k->cs_polarity = SSI_CS_LOW;
 }
 
 static TypeInfo ssi_sd_info = {
diff --git a/hw/stellaris.c b/hw/stellaris.c
index ec55c0e..acb297b 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1154,58 +1154,6 @@ static int stellaris_adc_init(SysBusDevice *dev)
     return 0;
 }
 
-/* Some boards have both an OLED controller and SD card connected to
-   the same SSI port, with the SD card chip select connected to a
-   GPIO pin.  Technically the OLED chip select is connected to the SSI
-   Fss pin.  We do not bother emulating that as both devices should
-   never be selected simultaneously, and our OLED controller ignores stray
-   0xff commands that occur when deselecting the SD card.  */
-
-typedef struct {
-    SSISlave ssidev;
-    qemu_irq irq;
-    int current_dev;
-    SSIBus *bus[2];
-} stellaris_ssi_bus_state;
-
-static void stellaris_ssi_bus_select(void *opaque, int irq, int level)
-{
-    stellaris_ssi_bus_state *s = (stellaris_ssi_bus_state *)opaque;
-
-    s->current_dev = level;
-}
-
-static uint32_t stellaris_ssi_bus_transfer(SSISlave *dev, uint32_t val)
-{
-    stellaris_ssi_bus_state *s = FROM_SSI_SLAVE(stellaris_ssi_bus_state, dev);
-
-    return ssi_transfer(s->bus[s->current_dev], val);
-}
-
-static const VMStateDescription vmstate_stellaris_ssi_bus = {
-    .name = "stellaris_ssi_bus",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields      = (VMStateField[]) {
-        VMSTATE_SSI_SLAVE(ssidev, stellaris_ssi_bus_state),
-        VMSTATE_INT32(current_dev, stellaris_ssi_bus_state),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static int stellaris_ssi_bus_init(SSISlave *dev)
-{
-    stellaris_ssi_bus_state *s = FROM_SSI_SLAVE(stellaris_ssi_bus_state, dev);
-
-    s->bus[0] = ssi_create_bus(&dev->qdev, "ssi0");
-    s->bus[1] = ssi_create_bus(&dev->qdev, "ssi1");
-    qdev_init_gpio_in(&dev->qdev, stellaris_ssi_bus_select, 1);
-
-    vmstate_register(&dev->qdev, -1, &vmstate_stellaris_ssi_bus, s);
-    return 0;
-}
-
 /* Board init.  */
 static stellaris_board_info stellaris_boards[] = {
   { "LM3S811EVB",
@@ -1306,29 +1254,33 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
     if (board->dc2 & (1 << 4)) {
         dev = sysbus_create_simple("pl022", 0x40008000, pic[7]);
         if (board->peripherals & BP_OLED_SSI) {
-            DeviceState *mux;
             void *bus;
-            qemu_irq select_pin;
 
+            /* Some boards have both an OLED controller and SD card connected to
+             * the same SSI port, with the SD card chip select connected to a
+             * GPIO pin.  Technically the OLED chip select is connected to the
+             * SSI Fss pin.  We do not bother emulating that as both devices
+             * should never be selected simultaneously, and our OLED controller
+             * ignores stray 0xff commands that occur when deselecting the SD
+             * card.
+             */
             bus = qdev_get_child_bus(dev, "ssi");
-            mux = ssi_create_slave(bus, "evb6965-ssi");
-            select_pin = qdev_get_gpio_in(mux, 0);
+
+            dev = ssi_create_slave(bus, "ssi-sd");
             if (gpio_dev[GPIO_D]) {
-                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0, select_pin);
+                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0,
+                                        qdev_get_gpio_in(dev, 0));
             }
 
-            bus = qdev_get_child_bus(mux, "ssi0");
-            ssi_create_slave(bus, "ssi-sd");
-
-            bus = qdev_get_child_bus(mux, "ssi1");
             dev = ssi_create_slave(bus, "ssd0323");
+            if (gpio_dev[GPIO_D]) {
+                qdev_connect_gpio_out(gpio_dev[GPIO_D], 0,
+                                        qdev_get_gpio_in(dev, 0));
+            }
             if (gpio_dev[GPIO_C]) {
                 qdev_connect_gpio_out(gpio_dev[GPIO_C], 7,
-                                        qdev_get_gpio_in(dev, 0));
+                                        qdev_get_gpio_in(dev, 1));
             }
-
-            /* Make sure the select pin is high.  */
-            qemu_irq_raise(select_pin);
         }
     }
     if (board->dc4 & (1 << 28)) {
@@ -1393,21 +1345,6 @@ static void stellaris_machine_init(void)
 
 machine_init(stellaris_machine_init);
 
-static void stellaris_ssi_bus_class_init(ObjectClass *klass, void *data)
-{
-    SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
-
-    k->init = stellaris_ssi_bus_init;
-    k->transfer = stellaris_ssi_bus_transfer;
-}
-
-static TypeInfo stellaris_ssi_bus_info = {
-    .name          = "evb6965-ssi",
-    .parent        = TYPE_SSI_SLAVE,
-    .instance_size = sizeof(stellaris_ssi_bus_state),
-    .class_init    = stellaris_ssi_bus_class_init,
-};
-
 static void stellaris_i2c_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
@@ -1455,7 +1392,6 @@ static void stellaris_register_types(void)
     type_register_static(&stellaris_i2c_info);
     type_register_static(&stellaris_gptm_info);
     type_register_static(&stellaris_adc_info);
-    type_register_static(&stellaris_ssi_bus_info);
 }
 
 type_init(stellaris_register_types)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (6 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 07/15] stellaris: Removed SSI mux Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:41   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

To be more consistent with the newer ways of error signalling. That and SIGABT
is easier to debug with than exit(1).

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/ssd0323.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/ssd0323.c b/hw/ssd0323.c
index d8a0c14..0800866 100644
--- a/hw/ssd0323.c
+++ b/hw/ssd0323.c
@@ -19,7 +19,9 @@
 #define DPRINTF(fmt, ...) \
 do { printf("ssd0323: " fmt , ## __VA_ARGS__); } while (0)
 #define BADF(fmt, ...) \
-do { fprintf(stderr, "ssd0323: error: " fmt , ## __VA_ARGS__); exit(1);} while (0)
+do { \
+    fprintf(stderr, "ssd0323: error: " fmt , ## __VA_ARGS__); abort(); \
+} while (0)
 #else
 #define DPRINTF(fmt, ...) do {} while(0)
 #define BADF(fmt, ...) \
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (7 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:40   ` Igor Mitsyanko
                     ` (2 more replies)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 10/15] m25p80: Initial implementation of SPI flash device Peter A. G. Crosthwaite
                   ` (5 subsequent siblings)
  14 siblings, 3 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added a FIFO API that can be used to create and operate byte FIFOs.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/Makefile.objs |    1 +
 hw/fifo.c        |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/fifo.h        |   47 ++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 hw/fifo.c
 create mode 100644 hw/fifo.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 8327e55..6ba570e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -15,6 +15,7 @@ hw-obj-$(CONFIG_ECC) += ecc.o
 hw-obj-$(CONFIG_NAND) += nand.o
 hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
 hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
+hw-obj-y += fifo.o
 
 hw-obj-$(CONFIG_M48T59) += m48t59.o
 hw-obj-$(CONFIG_ESCC) += escc.o
diff --git a/hw/fifo.c b/hw/fifo.c
new file mode 100644
index 0000000..5e14e1e
--- /dev/null
+++ b/hw/fifo.c
@@ -0,0 +1,79 @@
+/*
+ * Generic FIFO component, implemented as a circular buffer.
+ *
+ * Copyright (c) 2012 Peter A. G. Crosthwaite
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "fifo.h"
+
+void fifo8_create(Fifo8 *fifo, uint32_t capacity)
+{
+    fifo->data = g_new(uint8_t, capacity);
+    fifo->capacity = capacity;
+    fifo->head = 0;
+    fifo->num = 0;
+}
+
+void fifo8_destroy(Fifo8 *fifo)
+{
+    g_free(fifo->data);
+}
+
+void fifo8_push(Fifo8 *fifo, uint8_t data)
+{
+    if (fifo->num == fifo->capacity) {
+        abort();
+    }
+    fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
+    fifo->num++;
+}
+
+uint8_t fifo8_pop(Fifo8 *fifo)
+{
+    uint8_t ret;
+
+    if (fifo->num == 0) {
+        abort();
+    }
+    ret = fifo->data[fifo->head++];
+    fifo->head %= fifo->capacity;
+    fifo->num--;
+    return ret;
+}
+
+void fifo8_reset(Fifo8 *fifo)
+{
+    fifo->num = 0;
+}
+
+bool fifo8_is_empty(Fifo8 *fifo)
+{
+    return (fifo->num == 0);
+}
+
+bool fifo8_is_full(Fifo8 *fifo)
+{
+    return (fifo->num == fifo->capacity);
+}
+
+const VMStateDescription vmstate_fifo8 = {
+    .name = "SSISlave",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
+        VMSTATE_UINT32(head, Fifo8),
+        VMSTATE_UINT32(num, Fifo8),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
diff --git a/hw/fifo.h b/hw/fifo.h
new file mode 100644
index 0000000..3fb09ff
--- /dev/null
+++ b/hw/fifo.h
@@ -0,0 +1,47 @@
+#ifndef FIFO_H
+#define FIFO_H
+
+#include "hw.h"
+
+typedef struct {
+    /* All fields are private */
+    uint8_t *data;
+    uint32_t capacity;
+    uint32_t head;
+    uint32_t num;
+} Fifo8;
+
+/* create a fifo of the specified size */
+
+void fifo8_create(Fifo8 *, uint32_t);
+
+/* cleanup a fifo */
+
+void fifo8_destroy(Fifo8 *);
+
+/* push a data byte to the fifo. Behaviour is undefined if the fifo is full */
+
+void fifo8_push(Fifo8 *, uint8_t);
+
+/* pop a data byte from the fifo. Behviour is undefined if the fifo is empty */
+
+uint8_t fifo8_pop(Fifo8 *);
+
+/* reset (empty) the fifo */
+
+void fifo8_reset(Fifo8 *);
+
+bool fifo8_is_empty(Fifo8 *);
+bool fifo8_is_full(Fifo8 *);
+
+extern const VMStateDescription vmstate_fifo8;
+
+#define VMSTATE_FIFO8(_field, _state) {                              \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(Fifo8),                                     \
+    .vmsd       = &vmstate_fifo8,                                    \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
+}
+
+#endif /* FIFO_H */
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 10/15] m25p80: Initial implementation of SPI flash device
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (8 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 11/15] xilinx_spi: Initial impl. of Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added device model for m25p80 style SPI flash family.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
changed from v4:
Added write-1 flag (EEPROM mode).
n25q128 table entry indentation fix.
updated for new SSI interface.
various debug messages cleaned up and added.
changed from v3:
changed licence to v2 or later (PMM review)
generalised device model - rather than being fixed to the fl064k, it can handle a wide range of m25p80 devices
refactored erase commands (previously they were fl064k specific and used spansions broken terminology)
typdef'd strcuts and enums
fixed some camel casing
added comment to explain why bdrv_sync_complete is a nop (PMM review)
removed hardcoded "512" for BDRV_SECTOR_SIZE
flash_sync_area: use bdrv_aio_writev instead of bdrv_write 
flash_chip_erase/flash_block_erase32k/flash_sector_erase: consolidated to one function
decode_new_cmd: fixed multi-statement lines (PMM review)
CHIP_ERASE->BULK_ERASE
init: drive_get -> drive_get_next (PMM review)
changed from v2:
updated for SSI slave interface
used async io (suggested - Stefan Hajnoczi)
changed from v1:
converted spi api to modified txrx style
factored out lots of common code and inlined overly short single call functions.
undated for txrx style spi interface

 default-configs/arm-softmmu.mak          |    1 +
 default-configs/microblaze-softmmu.mak   |    2 +
 default-configs/microblazeel-softmmu.mak |    2 +
 hw/Makefile.objs                         |    1 +
 hw/m25p80.c                              |  572 ++++++++++++++++++++++++++++++
 5 files changed, 578 insertions(+), 0 deletions(-)
 create mode 100644 hw/m25p80.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index e542b4f..ce5ff25 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -22,6 +22,7 @@ CONFIG_ADS7846=y
 CONFIG_MAX111X=y
 CONFIG_SSI=y
 CONFIG_SSI_SD=y
+CONFIG_SSI_M25P80=y
 CONFIG_LAN9118=y
 CONFIG_SMC91C111=y
 CONFIG_DS1338=y
diff --git a/default-configs/microblaze-softmmu.mak b/default-configs/microblaze-softmmu.mak
index 64c9485..2f442e5 100644
--- a/default-configs/microblaze-softmmu.mak
+++ b/default-configs/microblaze-softmmu.mak
@@ -5,3 +5,5 @@ CONFIG_PFLASH_CFI01=y
 CONFIG_SERIAL=y
 CONFIG_XILINX=y
 CONFIG_XILINX_AXI=y
+CONFIG_SSI=y
+CONFIG_SSI_M25P80=y
diff --git a/default-configs/microblazeel-softmmu.mak b/default-configs/microblazeel-softmmu.mak
index a962276..af9a3cd 100644
--- a/default-configs/microblazeel-softmmu.mak
+++ b/default-configs/microblazeel-softmmu.mak
@@ -5,3 +5,5 @@ CONFIG_PFLASH_CFI01=y
 CONFIG_SERIAL=y
 CONFIG_XILINX=y
 CONFIG_XILINX_AXI=y
+CONFIG_SSI=y
+CONFIG_SSI_M25P80=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 6ba570e..e6a94d6 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -143,6 +143,7 @@ common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o
 common-obj-y += scsi-generic.o scsi-bus.o
 common-obj-y += hid.o
 common-obj-$(CONFIG_SSI) += ssi.o
+common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
 common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
 common-obj-$(CONFIG_SD) += sd.o
 common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o
diff --git a/hw/m25p80.c b/hw/m25p80.c
new file mode 100644
index 0000000..389adf6
--- /dev/null
+++ b/hw/m25p80.c
@@ -0,0 +1,572 @@
+/*
+ * ST M25P80 emulator. Emulate all SPI flash devices based on the m25p80 command
+ * set. Known devices table current as of Jun/2012 and taked from linux.
+ * See drivers/mtd/devices/m25p80.c.
+ *
+ * Copyright (C) 2011 Edgar E. Iglesias <edgar.iglesias@gmail.com>
+ * Copyright (C) 2012 Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
+ * Copyright (C) 2012 PetaLogix
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) a later version of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw.h"
+#include "blockdev.h"
+#include "ssi.h"
+#include "devices.h"
+
+#ifdef M25P80_ERR_DEBUG
+#define DB_PRINT(...) do { \
+    fprintf(stderr,  ": %s: ", __func__); \
+    fprintf(stderr, ## __VA_ARGS__); \
+    } while (0);
+#else
+    #define DB_PRINT(...)
+#endif
+
+typedef struct FlashPartInfo {
+    const char *part_name;
+    /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd etc */
+    uint32_t jedec;
+    /* extended jedec code */
+    uint16_t ext_jedec;
+    /* there is confusion between manufacturers as to what a sector is. In this
+     * device model, a "sector" is the size that is erased by the ERASE_SECTOR
+     * command (opcode 0xd8).
+     */
+    uint32_t sector_size;
+    uint32_t n_sectors;
+    uint32_t page_size;
+    uint8_t flags;
+    /* erase capabilities */
+#define ER_4K 1
+#define ER_32K 2
+    /* set to allow the page program command to write 0s back to 1. Useful for
+     * modelling EEPROM with SPI flash command set
+     */
+#define WR_1 0x100
+} FlashPartInfo;
+
+/* adapted from linux */
+
+#define INFO(_part_name, _jedec, _ext_jedec, _sector_size, _n_sectors, _flags)\
+    .part_name = (_part_name),\
+    .jedec = (_jedec),\
+    .ext_jedec = (_ext_jedec),\
+    .sector_size = (_sector_size),\
+    .n_sectors = (_n_sectors),\
+    .page_size = 256,\
+    .flags = (_flags),\
+
+static const FlashPartInfo known_devices[] = {
+    /* Atmel -- some are (confusingly) marketed as "DataFlash" */
+    { INFO("at25fs010",   0x1f6601,      0,  32 << 10,   4, ER_4K) },
+    { INFO("at25fs040",   0x1f6604,      0,  64 << 10,   8, ER_4K) },
+
+    { INFO("at25df041a",  0x1f4401,      0,  64 << 10,   8, ER_4K) },
+    { INFO("at25df321a",  0x1f4701,      0,  64 << 10,  64, ER_4K) },
+    { INFO("at25df641",   0x1f4800,      0,  64 << 10, 128, ER_4K) },
+
+    { INFO("at26f004",    0x1f0400,      0,  64 << 10,   8, ER_4K) },
+    { INFO("at26df081a",  0x1f4501,      0,  64 << 10,  16, ER_4K) },
+    { INFO("at26df161a",  0x1f4601,      0,  64 << 10,  32, ER_4K) },
+    { INFO("at26df321",   0x1f4700,      0,  64 << 10,  64, ER_4K) },
+
+    /* EON -- en25xxx */
+    { INFO("en25f32",     0x1c3116,      0,  64 << 10,  64, ER_4K) },
+    { INFO("en25p32",     0x1c2016,      0,  64 << 10,  64, 0) },
+    { INFO("en25q32b",    0x1c3016,      0,  64 << 10,  64, 0) },
+    { INFO("en25p64",     0x1c2017,      0,  64 << 10, 128, 0) },
+
+    /* Intel/Numonyx -- xxxs33b */
+    { INFO("160s33b",     0x898911,      0,  64 << 10,  32, 0) },
+    { INFO("320s33b",     0x898912,      0,  64 << 10,  64, 0) },
+    { INFO("640s33b",     0x898913,      0,  64 << 10, 128, 0) },
+
+    /* Macronix */
+    { INFO("mx25l4005a",  0xc22013,      0,  64 << 10,   8, ER_4K) },
+    { INFO("mx25l8005",   0xc22014,      0,  64 << 10,  16, 0) },
+    { INFO("mx25l1606e",  0xc22015,      0,  64 << 10,  32, ER_4K) },
+    { INFO("mx25l3205d",  0xc22016,      0,  64 << 10,  64, 0) },
+    { INFO("mx25l6405d",  0xc22017,      0,  64 << 10, 128, 0) },
+    { INFO("mx25l12805d", 0xc22018,      0,  64 << 10, 256, 0) },
+    { INFO("mx25l12855e", 0xc22618,      0,  64 << 10, 256, 0) },
+    { INFO("mx25l25635e", 0xc22019,      0,  64 << 10, 512, 0) },
+    { INFO("mx25l25655e", 0xc22619,      0,  64 << 10, 512, 0) },
+
+    /* Spansion -- single (large) sector size only, at least
+     * for the chips listed here (without boot sectors).
+     */
+    { INFO("s25sl004a",   0x010212,      0,  64 << 10,   8, 0) },
+    { INFO("s25sl008a",   0x010213,      0,  64 << 10,  16, 0) },
+    { INFO("s25sl016a",   0x010214,      0,  64 << 10,  32, 0) },
+    { INFO("s25sl032a",   0x010215,      0,  64 << 10,  64, 0) },
+    { INFO("s25sl032p",   0x010215, 0x4d00,  64 << 10,  64, ER_4K) },
+    { INFO("s25sl064a",   0x010216,      0,  64 << 10, 128, 0) },
+    { INFO("s25fl256s0",  0x010219, 0x4d00, 256 << 10, 128, 0) },
+    { INFO("s25fl256s1",  0x010219, 0x4d01,  64 << 10, 512, 0) },
+    { INFO("s25fl512s",   0x010220, 0x4d00, 256 << 10, 256, 0) },
+    { INFO("s70fl01gs",   0x010221, 0x4d00, 256 << 10, 256, 0) },
+    { INFO("s25sl12800",  0x012018, 0x0300, 256 << 10,  64, 0) },
+    { INFO("s25sl12801",  0x012018, 0x0301,  64 << 10, 256, 0) },
+    { INFO("s25fl129p0",  0x012018, 0x4d00, 256 << 10,  64, 0) },
+    { INFO("s25fl129p1",  0x012018, 0x4d01,  64 << 10, 256, 0) },
+    { INFO("s25fl016k",   0xef4015,      0,  64 << 10,  32, ER_4K | ER_32K) },
+    { INFO("s25fl064k",   0xef4017,      0,  64 << 10, 128, ER_4K | ER_32K) },
+
+    /* SST -- large erase sizes are "overlays", "sectors" are 4<< 10 */
+    { INFO("sst25vf040b", 0xbf258d,      0,  64 << 10,   8, ER_4K) },
+    { INFO("sst25vf080b", 0xbf258e,      0,  64 << 10,  16, ER_4K) },
+    { INFO("sst25vf016b", 0xbf2541,      0,  64 << 10,  32, ER_4K) },
+    { INFO("sst25vf032b", 0xbf254a,      0,  64 << 10,  64, ER_4K) },
+    { INFO("sst25wf512",  0xbf2501,      0,  64 << 10,   1, ER_4K) },
+    { INFO("sst25wf010",  0xbf2502,      0,  64 << 10,   2, ER_4K) },
+    { INFO("sst25wf020",  0xbf2503,      0,  64 << 10,   4, ER_4K) },
+    { INFO("sst25wf040",  0xbf2504,      0,  64 << 10,   8, ER_4K) },
+
+    /* ST Microelectronics -- newer production may have feature updates */
+    { INFO("m25p05",      0x202010,      0,  32 << 10,   2, 0) },
+    { INFO("m25p10",      0x202011,      0,  32 << 10,   4, 0) },
+    { INFO("m25p20",      0x202012,      0,  64 << 10,   4, 0) },
+    { INFO("m25p40",      0x202013,      0,  64 << 10,   8, 0) },
+    { INFO("m25p80",      0x202014,      0,  64 << 10,  16, 0) },
+    { INFO("m25p16",      0x202015,      0,  64 << 10,  32, 0) },
+    { INFO("m25p32",      0x202016,      0,  64 << 10,  64, 0) },
+    { INFO("m25p64",      0x202017,      0,  64 << 10, 128, 0) },
+    { INFO("m25p128",     0x202018,      0, 256 << 10,  64, 0) },
+
+    { INFO("m45pe10",     0x204011,      0,  64 << 10,   2, 0) },
+    { INFO("m45pe80",     0x204014,      0,  64 << 10,  16, 0) },
+    { INFO("m45pe16",     0x204015,      0,  64 << 10,  32, 0) },
+
+    { INFO("m25pe80",     0x208014,      0,  64 << 10,  16, 0) },
+    { INFO("m25pe16",     0x208015,      0,  64 << 10,  32, ER_4K) },
+
+    { INFO("m25px32",     0x207116,      0,  64 << 10,  64, ER_4K) },
+    { INFO("m25px32-s0",  0x207316,      0,  64 << 10,  64, ER_4K) },
+    { INFO("m25px32-s1",  0x206316,      0,  64 << 10,  64, ER_4K) },
+    { INFO("m25px64",     0x207117,      0,  64 << 10, 128, 0) },
+
+    /* Winbond -- w25x "blocks" are 64k, "sectors" are 4KiB */
+    { INFO("w25x10",      0xef3011,      0,  64 << 10,   2, ER_4K) },
+    { INFO("w25x20",      0xef3012,      0,  64 << 10,   4, ER_4K) },
+    { INFO("w25x40",      0xef3013,      0,  64 << 10,   8, ER_4K) },
+    { INFO("w25x80",      0xef3014,      0,  64 << 10,  16, ER_4K) },
+    { INFO("w25x16",      0xef3015,      0,  64 << 10,  32, ER_4K) },
+    { INFO("w25x32",      0xef3016,      0,  64 << 10,  64, ER_4K) },
+    { INFO("w25q32",      0xef4016,      0,  64 << 10,  64, ER_4K) },
+    { INFO("w25x64",      0xef3017,      0,  64 << 10, 128, ER_4K) },
+    { INFO("w25q64",      0xef4017,      0,  64 << 10, 128, ER_4K) },
+
+    /* Numonyx -- n25q128 */
+    { INFO("n25q128",      0x20ba18,      0,  64 << 10, 256, 0) },
+
+    { },
+};
+
+typedef enum {
+    NOP = 0,
+    PP = 0x2,
+    READ = 0x3,
+    WRDI = 0x4,
+    RDSR = 0x5,
+    WREN = 0x6,
+    FAST_READ = 0xb,
+    ERASE_4K = 0x20,
+    ERASE_32K = 0x52,
+    ERASE_SECTOR = 0xd8,
+    JEDEC_READ = 0x9f,
+    BULK_ERASE = 0xc7,
+} FlashCMD;
+
+typedef enum {
+    STATE_IDLE,
+    STATE_PAGE_PROGRAM,
+    STATE_READ,
+    STATE_COLLECTING_DATA,
+    STATE_READING_DATA,
+} CMDState;
+
+typedef struct Flash {
+    SSISlave ssidev;
+    uint32_t r;
+
+    BlockDriverState *bdrv;
+    CMDState state;
+
+    uint8_t *storage;
+    uint32_t size;
+    int page_size;
+
+    uint8_t data[16];
+    uint32_t len;
+    uint32_t pos;
+    uint8_t needed_bytes;
+    FlashCMD cmd_in_progress;
+
+    int64_t dirty_page;
+
+    uint64_t waddr;
+    int write_enable;
+
+    char *part_name;
+    const FlashPartInfo *pi;
+
+} Flash;
+
+static void bdrv_sync_complete(void *opaque, int ret)
+{
+    /* do nothing. Masters do not directly interact with the backing store,
+     * only the working copy so no mutexing required.
+     */
+}
+
+static void flash_sync_page(Flash *s, int page)
+{
+    if (s->bdrv) {
+        int bdrv_sector, nb_sectors;
+        QEMUIOVector iov;
+
+        bdrv_sector = (page * s->pi->page_size) / BDRV_SECTOR_SIZE;
+        nb_sectors = DIV_ROUND_UP(s->pi->page_size, BDRV_SECTOR_SIZE);
+        qemu_iovec_init(&iov, 1);
+        qemu_iovec_add(&iov, s->storage + bdrv_sector * BDRV_SECTOR_SIZE,
+                                                nb_sectors * BDRV_SECTOR_SIZE);
+        bdrv_aio_writev(s->bdrv, bdrv_sector, &iov, nb_sectors,
+                                                bdrv_sync_complete, NULL);
+    }
+}
+
+static inline void flash_sync_area(Flash *s, int64_t off, int64_t len)
+{
+    int64_t start, end;
+    int nb_sectors;
+    QEMUIOVector iov;
+
+    if (!s->bdrv) {
+        return;
+    }
+
+    assert(!(len % BDRV_SECTOR_SIZE));
+    start = off / BDRV_SECTOR_SIZE;
+    end = (off + len) / BDRV_SECTOR_SIZE;
+    nb_sectors = end - start;
+    qemu_iovec_init(&iov, 1);
+    qemu_iovec_add(&iov, s->storage + (start * BDRV_SECTOR_SIZE),
+                                        nb_sectors * BDRV_SECTOR_SIZE);
+    bdrv_aio_writev(s->bdrv, start, &iov, nb_sectors, bdrv_sync_complete, NULL);
+}
+
+static void flash_erase(Flash *s, int offset, FlashCMD cmd)
+{
+    uint32_t len;
+    uint8_t capa_to_assert = 0;
+
+    switch (cmd) {
+    case ERASE_4K:
+        len = 4 << 10;
+        capa_to_assert = ER_4K;
+        break;
+    case ERASE_32K:
+        len = 32 << 10;
+        capa_to_assert = ER_32K;
+        break;
+    case ERASE_SECTOR:
+        len = s->pi->sector_size;
+        break;
+    case BULK_ERASE:
+        len = s->size;
+    default:
+        assert(false);
+    }
+
+    DB_PRINT("offset = %#x, len = %d\n", offset, len);
+    if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
+        hw_error("m25p80: %dk erase size not supported by device\n", len);
+    }
+
+    if (!s->write_enable) {
+        DB_PRINT("erase with write protect!\n");
+        return;
+    }
+    memset(s->storage + offset, 0xff, len);
+    flash_sync_area(s, offset, len);
+}
+
+static inline void flash_sync_dirty(Flash *s, int64_t newpage)
+{
+    if (s->dirty_page >= 0 && s->dirty_page != newpage) {
+        flash_sync_page(s, s->dirty_page);
+        s->dirty_page = newpage;
+    }
+}
+
+static inline
+void flash_write8(Flash *s, uint64_t addr, uint8_t data)
+{
+    int64_t page = addr / s->pi->page_size;
+    uint8_t prev = s->storage[s->waddr];
+
+    if (!s->write_enable) {
+        DB_PRINT("write with write protect!\n");
+    }
+
+    if ((prev ^ data) & data) {
+        DB_PRINT("programming zero to one! addr=%lx  %x -> %x\n",
+                  addr, prev, data);
+    }
+
+    if (s->pi->flags & WR_1) {
+        s->storage[s->waddr] = data;
+    } else {
+        s->storage[s->waddr] &= ~data;
+    }
+
+    flash_sync_dirty(s, page);
+    s->dirty_page = page;
+}
+
+static void complete_collecting_data(Flash *s)
+{
+    s->waddr = s->data[0] << 16;
+    s->waddr |= s->data[1] << 8;
+    s->waddr |= s->data[2];
+
+    switch (s->cmd_in_progress) {
+    case PP:
+        s->state = STATE_PAGE_PROGRAM;
+        break;
+    case READ:
+    case FAST_READ:
+        s->state = STATE_READ;
+        break;
+    case ERASE_4K:
+    case ERASE_32K:
+    case ERASE_SECTOR:
+        flash_erase(s, s->waddr, s->cmd_in_progress);
+        break;
+    default:
+        break;
+    }
+}
+
+static void decode_new_cmd(Flash *s, uint32_t value)
+{
+    s->cmd_in_progress = value;
+    DB_PRINT("decoded new command:%x\n", value);
+
+    switch (value) {
+
+    case ERASE_4K:
+    case ERASE_32K:
+    case ERASE_SECTOR:
+    case READ:
+    case PP:
+        s->needed_bytes = 3;
+        s->pos = 0;
+        s->len = 0;
+        s->state = STATE_COLLECTING_DATA;
+        break;
+
+    case FAST_READ:
+        s->needed_bytes = 4;
+        s->pos = 0;
+        s->len = 0;
+        s->state = STATE_COLLECTING_DATA;
+        break;
+
+    case WRDI:
+        s->write_enable = 0;
+        break;
+    case WREN:
+        s->write_enable = 1;
+        break;
+
+    case RDSR:
+        s->data[0] = (!!s->write_enable) << 1;
+        s->pos = 0;
+        s->len = 1;;
+        s->state = STATE_READING_DATA;
+        break;
+
+    case JEDEC_READ:
+        DB_PRINT("populated jedec code\n");
+        s->data[0] = (s->pi->jedec >> 16) & 0xff;
+        s->data[1] = (s->pi->jedec >> 8) & 0xff;
+        s->data[2] = s->pi->jedec & 0xff;
+        if (s->pi->ext_jedec) {
+            s->data[3] = (s->pi->ext_jedec >> 8) & 0xff;
+            s->data[4] = s->pi->ext_jedec & 0xff;
+            s->len = 5;
+        } else {
+            s->len = 3;
+        }
+        s->pos = 0;
+        s->state = STATE_READING_DATA;
+        break;
+
+    case BULK_ERASE:
+        if (s->write_enable) {
+            DB_PRINT("chip erase\n");
+            flash_erase(s, 0, BULK_ERASE);
+        } else {
+            DB_PRINT("chip erase with write protect!\n");
+        }
+        break;
+    case NOP:
+        break;
+    default:
+        DB_PRINT("Unknown cmd %x\n", value);
+        break;
+    }
+}
+
+static int m25p80_cs(SSISlave *ss, bool select)
+{
+    Flash *s = FROM_SSI_SLAVE(Flash, ss);
+
+    if (select) {
+        s->len = 0;
+        s->pos = 0;
+        s->state = STATE_IDLE;
+        flash_sync_dirty(s, -1);
+        DB_PRINT("deselect\n");
+    }
+
+    return 0;
+}
+
+static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t tx)
+{
+    Flash *s = FROM_SSI_SLAVE(Flash, ss);
+    uint32_t r = 0;
+
+    switch (s->state) {
+
+    case STATE_PAGE_PROGRAM:
+        DB_PRINT("page program waddr=%lx data=%x\n", s->waddr, (uint8_t)tx);
+        flash_write8(s, s->waddr, (uint8_t)tx);
+        s->waddr++;
+        break;
+
+    case STATE_READ:
+        r = s->storage[s->waddr];
+        DB_PRINT("READ 0x%lx=%x\n", s->waddr, r);
+        s->waddr = (s->waddr + 1) % s->size;
+        break;
+
+    case STATE_COLLECTING_DATA:
+        s->data[s->len] = (uint8_t)tx;
+        s->len++;
+
+        if (s->len == s->needed_bytes) {
+            complete_collecting_data(s);
+        }
+        break;
+
+    case STATE_READING_DATA:
+        r = s->data[s->pos];
+        s->pos++;
+        if (s->pos == s->len) {
+            s->pos = 0;
+            s->state = STATE_IDLE;
+        }
+        break;
+
+    default:
+    case STATE_IDLE:
+        decode_new_cmd(s, (uint8_t)tx);
+        break;
+    }
+
+    return r;
+}
+
+static int m25p80_init(SSISlave *ss)
+{
+    DriveInfo *dinfo;
+    Flash *s = FROM_SSI_SLAVE(Flash, ss);
+    const FlashPartInfo *i;
+
+    if (!s->part_name) { /* default to actual m25p80 if no partname given */
+        s->part_name = (char *)"m25p80";
+    }
+
+    i = known_devices;
+    for (i = known_devices;; i++) {
+        assert(i);
+        if (!i->part_name) {
+            fprintf(stderr, "Unknown SPI flash part: \"%s\"\n", s->part_name);
+            return 1;
+        } else if (!strcmp(i->part_name, s->part_name)) {
+            s->pi = i;
+            break;
+        }
+    }
+
+    s->size = s->pi->sector_size * s->pi->n_sectors;
+    s->dirty_page = -1;
+    s->storage = qemu_blockalign(s->bdrv, s->size);
+
+    dinfo = drive_get_next(IF_MTD);
+
+    if (dinfo && dinfo->bdrv) {
+        int rsize;
+
+        DB_PRINT("Binding to IF_MTD drive\n");
+        s->bdrv = dinfo->bdrv;
+        rsize = MIN(bdrv_getlength(s->bdrv), s->size);
+        /* FIXME: Move to late init */
+        if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size,
+                                                    BDRV_SECTOR_SIZE))) {
+            fprintf(stderr, "Failed to initialize SPI flash!\n");
+            return 1;
+        }
+    } else {
+        memset(s->storage, 0xFF, s->size);
+    }
+
+    return 0;
+}
+
+static Property m25p80_properties[] = {
+    DEFINE_PROP_STRING("partname", Flash, part_name),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void m25p80_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
+
+    k->init = m25p80_init;
+    k->transfer = m25p80_transfer8;
+    k->set_cs = m25p80_cs;
+    k->cs_polarity = SSI_CS_LOW;
+    dc->props = m25p80_properties;
+}
+
+static TypeInfo m25p80_info = {
+    .name           = "m25p80",
+    .parent         = TYPE_SSI_SLAVE,
+    .instance_size  = sizeof(Flash),
+    .class_init     = m25p80_class_init,
+};
+
+static void m25p80_register_types(void)
+{
+    type_register_static(&m25p80_info);
+}
+
+type_init(m25p80_register_types)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 11/15] xilinx_spi: Initial impl. of Xilinx SPI controller
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (9 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 10/15] m25p80: Initial implementation of SPI flash device Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128 Peter A. G. Crosthwaite
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Device model for xilinx XPS SPI controller (v2.0)

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
changed from v4 (Near total rewrite):
removed timer delay. This was innacturate anyways removed for simlicity.
updated for new SSI interface.
factored out txrx fifos using fifo.h
changed from v3:
typedef'd struct XilinxSPI
changed unsigned int -> uin32_t
removed unused vars (c_fifo_exist and cmd_ongoing)
txfifo_reset removed duplicate s->regs[R_SPISR] &= ~SR_TX_FULL (PMM review)
reset: changed to Device Class style reset
reset: stope the ptimer (pmm review)
xlx_spi_update_irq: dont -> don't (PMM review)
init: set irq_line to 1 (force refresh on vmsd load)
init: dropped call to reset
implemetned vmsd
changed from v2:
converted spi api to ssi api
changed from v1:
converted spi api to modified txrx style
 hw/microblaze/Makefile.objs |    1 +
 hw/xilinx_spi.c             |  390 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 391 insertions(+), 0 deletions(-)
 create mode 100644 hw/xilinx_spi.c

diff --git a/hw/microblaze/Makefile.objs b/hw/microblaze/Makefile.objs
index 274d2c5..3028e65 100644
--- a/hw/microblaze/Makefile.objs
+++ b/hw/microblaze/Makefile.objs
@@ -1,6 +1,7 @@
 obj-y = petalogix_s3adsp1800_mmu.o
 obj-y += petalogix_ml605_mmu.o
 obj-y += microblaze_boot.o
+obj-y += xilinx_spi.o
 
 obj-y += microblaze_pic_cpu.o
 obj-y += xilinx_ethlite.o
diff --git a/hw/xilinx_spi.c b/hw/xilinx_spi.c
new file mode 100644
index 0000000..933858f
--- /dev/null
+++ b/hw/xilinx_spi.c
@@ -0,0 +1,390 @@
+/*
+ * QEMU model of the Xilinx SPI Controller
+ *
+ * Copyright (C) 2010 Edgar E. Iglesias.
+ * Copyright (C) 2012 Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
+ * Copyright (C) 2012 PetaLogix
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "sysbus.h"
+#include "sysemu.h"
+#include "qemu-log.h"
+#include "fifo.h"
+
+#include "ssi.h"
+
+#ifdef XILINX_SPI_ERR_DEBUG
+#define DB_PRINT(...) do { \
+    fprintf(stderr,  ": %s: ", __func__); \
+    fprintf(stderr, ## __VA_ARGS__); \
+    } while (0);
+#else
+    #define DB_PRINT(...)
+#endif
+
+#define R_DGIER     (0x1c / 4)
+#define R_DGIER_IE  (1 << 31)
+
+#define R_IPISR     (0x20 / 4)
+#define IRQ_DRR_NOT_EMPTY    (1 << (31 - 23))
+#define IRQ_DRR_OVERRUN      (1 << (31 - 26))
+#define IRQ_DRR_FULL         (1 << (31 - 27))
+#define IRQ_TX_FF_HALF_EMPTY (1 << 6)
+#define IRQ_DTR_UNDERRUN     (1 << 3)
+#define IRQ_DTR_EMPTY        (1 << (31 - 29))
+
+#define R_IPIER     (0x28 / 4)
+#define R_SRR       (0x40 / 4)
+#define R_SPICR     (0x60 / 4)
+#define R_SPICR_TXFF_RST     (1 << 5)
+#define R_SPICR_RXFF_RST     (1 << 6)
+#define R_SPICR_MTI          (1 << 8)
+
+#define R_SPISR     (0x64 / 4)
+#define SR_TX_FULL    (1 << 3)
+#define SR_TX_EMPTY   (1 << 2)
+#define SR_RX_FULL    (1 << 1)
+#define SR_RX_EMPTY   (1 << 0)
+
+#define R_SPIDTR    (0x68 / 4)
+#define R_SPIDRR    (0x6C / 4)
+#define R_SPISSR    (0x70 / 4)
+#define R_TX_FF_OCY (0x74 / 4)
+#define R_RX_FF_OCY (0x78 / 4)
+#define R_MAX       (0x7C / 4)
+
+#define FIFO_CAPACITY 256
+
+typedef struct XilinxSPI {
+    SysBusDevice busdev;
+    MemoryRegion mmio;
+
+    qemu_irq irq;
+    int irqline;
+
+    uint8_t num_cs;
+    qemu_irq *cs_lines;
+
+    SSIBus *spi;
+
+    Fifo8 rx_fifo;
+    Fifo8 tx_fifo;
+
+    uint32_t regs[R_MAX];
+} XilinxSPI;
+
+static void txfifo_reset(XilinxSPI *s)
+{
+    fifo8_reset(&s->tx_fifo);
+
+    s->regs[R_SPISR] &= ~SR_TX_FULL;
+    s->regs[R_SPISR] |= SR_TX_EMPTY;
+}
+
+static void rxfifo_reset(XilinxSPI *s)
+{
+    fifo8_reset(&s->rx_fifo);
+
+    s->regs[R_SPISR] |= SR_RX_EMPTY;
+    s->regs[R_SPISR] &= ~SR_RX_FULL;
+}
+
+static void xlx_spi_update_cs(XilinxSPI *s)
+{
+   int i;
+
+    for (i = 0; i < s->num_cs; ++i) {
+        qemu_set_irq(s->cs_lines[i], !(~s->regs[R_SPISSR] & 1 << i));
+    }
+}
+
+static void xlx_spi_update_irq(XilinxSPI *s)
+{
+    uint32_t pending;
+
+    s->regs[R_IPISR] |=
+            (!fifo8_is_empty(&s->rx_fifo) ? IRQ_DRR_NOT_EMPTY : 0) |
+            (fifo8_is_full(&s->rx_fifo) ? IRQ_DRR_FULL : 0);
+
+    pending = s->regs[R_IPISR] & s->regs[R_IPIER];
+
+    pending = pending && (s->regs[R_DGIER] & R_DGIER_IE);
+    pending = !!pending;
+
+    /* This call lies right in the data paths so don't call the
+       irq chain unless things really changed.  */
+    if (pending != s->irqline) {
+        s->irqline = pending;
+        DB_PRINT("irq_change of state %d ISR:%x IER:%X\n",
+                    pending, s->regs[R_IPISR], s->regs[R_IPIER]);
+        qemu_set_irq(s->irq, pending);
+    }
+
+}
+
+static void xlx_spi_do_reset(XilinxSPI *s)
+{
+    memset(s->regs, 0, sizeof s->regs);
+
+    rxfifo_reset(s);
+    txfifo_reset(s);
+
+    s->regs[R_SPISSR] = ~0;
+    xlx_spi_update_irq(s);
+    xlx_spi_update_cs(s);
+}
+
+static void xlx_spi_reset(DeviceState *d)
+{
+    xlx_spi_do_reset(DO_UPCAST(XilinxSPI, busdev.qdev, d));
+}
+
+static inline int spi_master_enabled(XilinxSPI *s)
+{
+    return !(s->regs[R_SPICR] & R_SPICR_MTI);
+}
+
+static void spi_flush_txfifo(XilinxSPI *s)
+{
+    uint32_t tx;
+    uint32_t rx;
+
+    while (!fifo8_is_empty(&s->tx_fifo)) {
+        tx = (uint32_t)fifo8_pop(&s->tx_fifo);
+        DB_PRINT("data tx:%x\n", tx);
+        rx = ssi_transfer(s->spi, tx);
+        DB_PRINT("data rx:%x\n", rx);
+        if (fifo8_is_full(&s->rx_fifo)) {
+            s->regs[R_IPISR] |= IRQ_DRR_OVERRUN;
+        } else {
+            fifo8_push(&s->rx_fifo, (uint8_t)rx);
+            if (fifo8_is_full(&s->rx_fifo)) {
+                s->regs[R_SPISR] |= SR_RX_FULL;
+                s->regs[R_IPISR] |= IRQ_DRR_FULL;
+            }
+        }
+
+        s->regs[R_SPISR] &= ~SR_RX_EMPTY;
+        s->regs[R_SPISR] &= ~SR_TX_FULL;
+        s->regs[R_SPISR] |= SR_TX_EMPTY;
+
+        s->regs[R_IPISR] |= IRQ_DTR_EMPTY;
+        s->regs[R_IPISR] |= IRQ_DRR_NOT_EMPTY;
+    }
+
+}
+
+static uint64_t
+spi_read(void *opaque, target_phys_addr_t addr, unsigned int size)
+{
+    XilinxSPI *s = opaque;
+    uint32_t r = 0;
+
+    addr >>= 2;
+    switch (addr) {
+    case R_SPIDRR:
+        if (fifo8_is_empty(&s->rx_fifo)) {
+            DB_PRINT("Read from empty FIFO!\n");
+            return 0xdeadbeef;
+        }
+
+        s->regs[R_SPISR] &= ~SR_RX_FULL;
+        r = fifo8_pop(&s->rx_fifo);
+        if (fifo8_is_empty(&s->rx_fifo)) {
+            s->regs[R_SPISR] |= SR_RX_EMPTY;
+        }
+        break;
+
+    case R_SPISR:
+        r = s->regs[addr];
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            r = s->regs[addr];
+        }
+        break;
+
+    }
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr * 4, r);
+    xlx_spi_update_irq(s);
+    return r;
+}
+
+static void
+spi_write(void *opaque, target_phys_addr_t addr,
+            uint64_t val64, unsigned int size)
+{
+    XilinxSPI *s = opaque;
+    uint32_t value = val64;
+
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, value);
+    addr >>= 2;
+    switch (addr) {
+    case R_SRR:
+        if (value != 0xa) {
+            DB_PRINT("Invalid write to SRR %x\n", value);
+        } else {
+            xlx_spi_do_reset(s);
+        }
+        break;
+
+    case R_SPIDTR:
+        s->regs[R_SPISR] &= ~SR_TX_EMPTY;
+        fifo8_push(&s->tx_fifo, (uint8_t)value);
+        if (fifo8_is_full(&s->tx_fifo)) {
+            s->regs[R_SPISR] |= SR_TX_FULL;
+        }
+        if (!spi_master_enabled(s)) {
+            goto done;
+        } else {
+            DB_PRINT("DTR and master enabled\n");
+        }
+        spi_flush_txfifo(s);
+        break;
+
+    case R_SPISR:
+        DB_PRINT("Invalid write to SPISR %x\n", value);
+        break;
+
+    case R_IPISR:
+        /* Toggle the bits.  */
+        s->regs[addr] ^= value;
+        break;
+
+    /* Slave Select Register.  */
+    case R_SPISSR:
+        s->regs[addr] = value;
+        xlx_spi_update_cs(s);
+        break;
+
+    case R_SPICR:
+        /* FIXME: reset irq and sr state to empty queues.  */
+        if (value & R_SPICR_RXFF_RST) {
+            rxfifo_reset(s);
+        }
+
+        if (value & R_SPICR_TXFF_RST) {
+            txfifo_reset(s);
+        }
+        value &= ~(R_SPICR_RXFF_RST | R_SPICR_TXFF_RST);
+        s->regs[addr] = value;
+
+        if (!(value & R_SPICR_MTI)) {
+            spi_flush_txfifo(s);
+        }
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            s->regs[addr] = value;
+        }
+        break;
+    }
+
+done:
+    xlx_spi_update_irq(s);
+}
+
+static const MemoryRegionOps spi_ops = {
+    .read = spi_read,
+    .write = spi_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static int xilinx_spi_init(SysBusDevice *dev)
+{
+    int i;
+    XilinxSPI *s = FROM_SYSBUS(typeof(*s), dev);
+
+    DB_PRINT("\n");
+    sysbus_init_irq(dev, &s->irq);
+    s->cs_lines = g_new(qemu_irq, s->num_cs);
+    for (i = 0; i < s->num_cs; ++i) {
+        sysbus_init_irq(dev, &s->cs_lines[i]);
+    }
+
+    memory_region_init_io(&s->mmio, &spi_ops, s, "xilinx-spi", R_MAX * 4);
+    sysbus_init_mmio(dev, &s->mmio);
+
+    s->irqline = -1;
+
+    s->spi = ssi_create_bus(&dev->qdev, "spi");
+
+    fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
+    fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
+
+    return 0;
+}
+
+static int xilinx_spi_post_load(void *opaque, int version_id)
+{
+    xlx_spi_update_irq((XilinxSPI *)opaque);
+    return 0;
+}
+
+static const VMStateDescription vmstate_xilinx_spi = {
+    .name = "xilinx_spi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = xilinx_spi_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_FIFO8(tx_fifo, XilinxSPI),
+        VMSTATE_FIFO8(rx_fifo, XilinxSPI),
+        VMSTATE_UINT32_ARRAY(regs, XilinxSPI, R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property xilinx_spi_properties[] = {
+    DEFINE_PROP_UINT8("num-cs", XilinxSPI, num_cs, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void xilinx_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = xilinx_spi_init;
+    dc->reset = xlx_spi_reset;
+    dc->props = xilinx_spi_properties;
+    dc->vmsd = &vmstate_xilinx_spi;
+}
+
+static TypeInfo xilinx_spi_info = {
+    .name           = "xilinx,spi",
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(XilinxSPI),
+    .class_init     = xilinx_spi_class_init,
+};
+
+static void xilinx_spi_register_types(void)
+{
+    type_register_static(&xilinx_spi_info);
+}
+
+type_init(xilinx_spi_register_types)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (10 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 11/15] xilinx_spi: Initial impl. of Xilinx SPI controller Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  9:50   ` Peter Maydell
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 13/15] xilinx_spips: Xilinx Zynq SPI cntrlr device model Peter A. G. Crosthwaite
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added SPI controller to the reference design, with two n25q128 spi-flashes
connected.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/petalogix_ml605_mmu.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index 6a7d0c0..f0ecc1f 100644
--- a/hw/petalogix_ml605_mmu.c
+++ b/hw/petalogix_ml605_mmu.c
@@ -36,6 +36,7 @@
 #include "blockdev.h"
 #include "pc.h"
 #include "exec-memory.h"
+#include "ssi.h"
 
 #include "microblaze_boot.h"
 #include "microblaze_pic_cpu.h"
@@ -54,6 +55,8 @@
 #define AXIENET_BASEADDR 0x82780000
 #define AXIDMA_BASEADDR 0x84600000
 
+#define NUM_SPI_FLASHES 2
+
 static void machine_cpu_reset(MicroBlazeCPU *cpu)
 {
     CPUMBState *env = &cpu->env;
@@ -78,6 +81,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
     MemoryRegion *address_space_mem = get_system_memory();
     DeviceState *dev;
     MicroBlazeCPU *cpu;
+    SysBusDevice *busdev;
     CPUMBState *env;
     DriveInfo *dinfo;
     int i;
@@ -135,9 +139,31 @@ petalogix_ml605_init(ram_addr_t ram_size,
                                      irq[1], irq[0], 100 * 1000000);
     }
 
+    {
+        SSIBus *spi;
+
+        dev = qdev_create(NULL, "xilinx,spi");
+        qdev_prop_set_uint8(dev, "num-cs", NUM_SPI_FLASHES);
+        qdev_init_nofail(dev);
+        busdev = sysbus_from_qdev(dev);
+        sysbus_mmio_map(busdev, 0, 0x40a00000);
+        sysbus_connect_irq(busdev, 0, irq[4]);
+
+        spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
+
+        for (i = 0; i < NUM_SPI_FLASHES; i++) {
+            qemu_irq cs_line;
+
+            dev = ssi_create_slave_no_init(spi, "m25p80");
+            qdev_prop_set_string(dev, "partname", (char *)"n25q128");
+            qdev_init_nofail(dev);
+            cs_line = qdev_get_gpio_in(dev, 0);
+            sysbus_connect_irq(busdev, i+1, cs_line);
+        }
+    }
+
     microblaze_load_kernel(cpu, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
                                                             machine_cpu_reset);
-
 }
 
 static QEMUMachine petalogix_ml605_machine = {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 13/15] xilinx_spips: Xilinx Zynq SPI cntrlr device model
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (11 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128 Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 14/15] xilinx_zynq: Added SPI controllers + flashes Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 15/15] MAINTAINERS: Added maintainerships for SSI Peter A. G. Crosthwaite
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added device model for the Xilinx Zynq SPI controller (SPIPS).

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/arm/Makefile.objs |    1 +
 hw/xilinx_spips.c    |  352 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 353 insertions(+), 0 deletions(-)
 create mode 100644 hw/xilinx_spips.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index c413780..bb5a5ba 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -6,6 +6,7 @@ obj-y += cadence_uart.o
 obj-y += cadence_ttc.o
 obj-y += cadence_gem.o
 obj-y += xilinx_zynq.o zynq_slcr.o
+obj-y += xilinx_spips.o
 obj-y += arm_gic.o arm_gic_common.o
 obj-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
 obj-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o
diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
new file mode 100644
index 0000000..7aa8e4a
--- /dev/null
+++ b/hw/xilinx_spips.c
@@ -0,0 +1,352 @@
+/*
+ * QEMU model of the Xilinx Zynq SPI controller
+ *
+ * Copyright (c) 2012 Peter A. G. Crosthwaite
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "sysbus.h"
+#include "sysemu.h"
+#include "ptimer.h"
+#include "qemu-log.h"
+#include "fifo.h"
+#include "ssi.h"
+
+#ifdef XILINX_SPIPS_ERR_DEBUG
+#define DB_PRINT(...) do { \
+    fprintf(stderr,  ": %s: ", __func__); \
+    fprintf(stderr, ## __VA_ARGS__); \
+    } while (0);
+#else
+    #define DB_PRINT(...)
+#endif
+
+/* config register */
+#define R_CONFIG            (0x00 / 4)
+#define MODEFAIL_GEN_EN     (1 << 17)
+#define MAN_START_COM       (1 << 16)
+#define MAN_START_EN        (1 << 15)
+#define MANUAL_CS           (1 << 14)
+#define CS                  (0xF << 10)
+#define CS_SHIFT            (10)
+#define PERI_SEL            (1 << 9)
+#define REF_CLK             (1 << 8)
+#define FIFO_WIDTH          (3 << 6)
+#define BAUD_RATE_DIV       (7 << 3)
+#define CLK_PH              (1 << 2)
+#define CLK_POL             (1 << 1)
+#define MODE_SEL            (1 << 1)
+
+/* interrupt mechanism */
+#define R_INTR_STATUS       (0x04 / 4)
+#define R_INTR_EN           (0x08 / 4)
+#define R_INTR_DIS          (0x0C / 4)
+#define R_INTR_MASK         (0x10 / 4)
+#define IXR_TX_FIFO_UNDERFLOW   (1 << 6)
+#define IXR_RX_FIFO_FULL        (1 << 5)
+#define IXR_RX_FIFO_NOT_EMPTY   (1 << 4)
+#define IXR_TX_FIFO_FULL        (1 << 3)
+#define IXR_TX_FIFO_NOT_FULL    (1 << 2)
+#define IXR_TX_FIFO_MODE_FAIL   (1 << 1)
+#define IXR_RX_FIFO_OVERFLOW    (1 << 0)
+#define IXR_ALL                 ((IXR_TX_FIFO_UNDERFLOW<<1)-1)
+
+#define R_EN                (0x14 / 4)
+#define R_DELAY             (0x18 / 4)
+#define R_TX_DATA           (0x1C / 4)
+#define R_RX_DATA           (0x20 / 4)
+#define R_SLAVE_IDLE_COUNT  (0x24 / 4)
+#define R_TX_THRES          (0x28 / 4)
+#define R_RX_THRES          (0x2C / 4)
+#define R_MOD_ID            (0xFC / 4)
+
+#define R_MAX (R_MOD_ID+1)
+
+/* size of TXRX FIFOs */
+#define NUM_CS_LINES    4
+#define RXFF_A          32
+#define TXFF_A          32
+
+typedef struct {
+    SysBusDevice busdev;
+    MemoryRegion iomem;
+    qemu_irq irq;
+    int irqline;
+
+    qemu_irq cs_lines[NUM_CS_LINES];
+    SSIBus *spi;
+
+    Fifo8 rx_fifo;
+    Fifo8 tx_fifo;
+
+    uint32_t regs[R_MAX];
+} XilinxSPIPS;
+
+static void xilinx_spips_update_cs_lines(XilinxSPIPS *s)
+{
+    int i;
+    bool found = false;
+    int field = s->regs[R_CONFIG] >> CS_SHIFT;
+
+    for (i = 0; i < NUM_CS_LINES; i++) {
+        if (~field & (1 << i) & !found) {
+            found = true;
+            DB_PRINT("selecting slave %d\n", i);
+            qemu_set_irq(s->cs_lines[i], 0);
+        } else {
+            qemu_set_irq(s->cs_lines[i], 1);
+        }
+     }
+}
+
+static void xilinx_spips_update_ixr(XilinxSPIPS *s)
+{
+    /* These are set/cleared as they occur */
+    s->regs[R_INTR_STATUS] &= (IXR_TX_FIFO_UNDERFLOW | IXR_RX_FIFO_OVERFLOW |
+                                IXR_TX_FIFO_MODE_FAIL);
+    /* these are pure functions of fifo state, set them here */
+    s->regs[R_INTR_STATUS] |=
+        (fifo8_is_full(&s->rx_fifo) ? IXR_RX_FIFO_FULL : 0) |
+        (s->rx_fifo.num >= s->regs[R_RX_THRES] ? IXR_RX_FIFO_NOT_EMPTY : 0) |
+        (fifo8_is_full(&s->tx_fifo) ? IXR_TX_FIFO_FULL : 0) |
+        (s->tx_fifo.num < s->regs[R_TX_THRES] ? IXR_TX_FIFO_NOT_FULL : 0);
+    /* drive external interupt pin */
+    int new_irqline = !!(s->regs[R_INTR_MASK] & s->regs[R_INTR_STATUS] &
+                                                                IXR_ALL);
+    if (new_irqline != s->irqline) {
+        s->irqline = new_irqline;
+        qemu_set_irq(s->irq, s->irqline);
+    }
+}
+
+static void xilinx_spips_reset(DeviceState *d)
+{
+    XilinxSPIPS *s = DO_UPCAST(XilinxSPIPS, busdev.qdev, d);
+
+    int i;
+    for (i = 0; i < R_MAX; i++) {
+        s->regs[i] = 0;
+    }
+
+    fifo8_reset(&s->rx_fifo);
+    fifo8_reset(&s->rx_fifo);
+    /* non zero resets */
+    s->regs[R_CONFIG] |= MODEFAIL_GEN_EN;
+    s->regs[R_SLAVE_IDLE_COUNT] = 0xFF;
+    s->regs[R_TX_THRES] = 1;
+    s->regs[R_RX_THRES] = 1;
+    /* FIXME: move magic number defintion somewhere sensible */
+    s->regs[R_MOD_ID] = 0x01090106;
+    xilinx_spips_update_ixr(s);
+    xilinx_spips_update_cs_lines(s);
+}
+
+static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
+{
+    for (;;) {
+        uint32_t r;
+        uint8_t value;
+
+        if (fifo8_is_empty(&s->tx_fifo)) {
+            s->regs[R_INTR_STATUS] |= IXR_TX_FIFO_UNDERFLOW;
+            break;
+        } else {
+            value = fifo8_pop(&s->tx_fifo);
+        }
+
+        r = ssi_transfer(s->spi, (uint32_t)value);
+        DB_PRINT("tx = %02x rx = %02x\n", value, r);
+        if (fifo8_is_full(&s->rx_fifo)) {
+            s->regs[R_INTR_STATUS] |= IXR_RX_FIFO_OVERFLOW;
+            DB_PRINT("rx FIFO overflow");
+        } else {
+            fifo8_push(&s->rx_fifo, (uint8_t)r);
+        }
+    };
+    xilinx_spips_update_ixr(s);
+}
+
+static uint64_t xilinx_spips_read(void *opaque, target_phys_addr_t addr,
+                                                        unsigned size)
+{
+    XilinxSPIPS *s = opaque;
+    uint32_t mask = ~0;
+    uint32_t ret;
+
+    addr >>= 2;
+    switch (addr) {
+    case R_CONFIG:
+        mask = 0x0002FFFF;
+        break;
+    case R_INTR_STATUS:
+    case R_INTR_MASK:
+        mask = IXR_ALL;
+        break;
+    case  R_EN:
+        mask = 0x1;
+        break;
+    case R_SLAVE_IDLE_COUNT:
+        mask = 0xFF;
+        break;
+    case R_MOD_ID:
+        mask = 0x01FFFFFF;
+        break;
+    case R_INTR_EN:
+    case R_INTR_DIS:
+    case R_TX_DATA:
+        mask = 0;
+        break;
+    case R_RX_DATA:
+        ret = (uint32_t)fifo8_pop(&s->rx_fifo);
+        DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr * 4, ret);
+        xilinx_spips_update_ixr(s);
+        return ret;
+    }
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr * 4, s->regs[addr] & mask);
+    return s->regs[addr] & mask;
+
+}
+
+static void xilinx_spips_write(void *opaque, target_phys_addr_t addr,
+                                        uint64_t value, unsigned size)
+{
+    int mask = ~0;
+    int man_start_com = 0;
+    XilinxSPIPS *s = opaque;
+
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, (unsigned)value);
+    addr >>= 2;
+    switch (addr) {
+    case R_CONFIG:
+        mask = 0x0002FFFF;
+        if (value & MAN_START_COM) {
+            man_start_com = 1;
+        }
+        break;
+    case R_INTR_STATUS:
+        mask = IXR_ALL;
+        s->regs[R_INTR_STATUS] &= ~(mask & value);
+        goto no_reg_update;
+    case R_INTR_DIS:
+        mask = IXR_ALL;
+        s->regs[R_INTR_MASK] &= ~(mask & value);
+        goto no_reg_update;
+    case R_INTR_EN:
+        mask = IXR_ALL;
+        s->regs[R_INTR_MASK] |= mask & value;
+        goto no_reg_update;
+    case R_EN:
+        mask = 0x1;
+        break;
+    case R_SLAVE_IDLE_COUNT:
+        mask = 0xFF;
+        break;
+    case R_RX_DATA:
+    case R_INTR_MASK:
+    case R_MOD_ID:
+        mask = 0;
+        break;
+    case R_TX_DATA:
+        fifo8_push(&s->tx_fifo, (uint8_t)value);
+        goto no_reg_update;
+    }
+    s->regs[addr] = (s->regs[addr] & ~mask) | (value & mask);
+no_reg_update:
+    if (man_start_com) {
+        xilinx_spips_flush_txfifo(s);
+    }
+    xilinx_spips_update_ixr(s);
+    xilinx_spips_update_cs_lines(s);
+}
+
+static const MemoryRegionOps spips_ops = {
+    .read = xilinx_spips_read,
+    .write = xilinx_spips_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static int xilinx_spips_init(SysBusDevice *dev)
+{
+    XilinxSPIPS *s = FROM_SYSBUS(typeof(*s), dev);
+    int i;
+
+    DB_PRINT("inited device model\n");
+
+    sysbus_init_irq(dev, &s->irq);
+    for (i = 0; i < NUM_CS_LINES; ++i) {
+        sysbus_init_irq(dev, &s->cs_lines[i]);
+    }
+
+    memory_region_init_io(&s->iomem, &spips_ops, s, "spi", R_MAX*4);
+    sysbus_init_mmio(dev, &s->iomem);
+
+    s->irqline = -1;
+    s->spi = ssi_create_bus(&dev->qdev, "spi");
+
+    fifo8_create(&s->rx_fifo, RXFF_A);
+    fifo8_create(&s->tx_fifo, TXFF_A);
+
+    return 0;
+}
+
+static int xilinx_spips_post_load(void *opaque, int version_id)
+{
+    xilinx_spips_update_ixr((XilinxSPIPS *)opaque);
+    xilinx_spips_update_cs_lines((XilinxSPIPS *)opaque);
+    return 0;
+}
+
+static const VMStateDescription vmstate_xilinx_spips = {
+    .name = "xilinx_spips",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = xilinx_spips_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_FIFO8(tx_fifo, XilinxSPIPS),
+        VMSTATE_FIFO8(rx_fifo, XilinxSPIPS),
+        VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void xilinx_spips_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+
+    sdc->init = xilinx_spips_init;
+    dc->reset = xilinx_spips_reset;
+    dc->vmsd = &vmstate_xilinx_spips;
+}
+
+static TypeInfo xilinx_spips_info = {
+    .name  = "xilinx,spips",
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(XilinxSPIPS),
+    .class_init = xilinx_spips_class_init,
+};
+
+static void xilinx_spips_register_types(void)
+{
+    type_register_static(&xilinx_spips_info);
+}
+
+type_init(xilinx_spips_register_types)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 14/15] xilinx_zynq: Added SPI controllers + flashes
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (12 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 13/15] xilinx_spips: Xilinx Zynq SPI cntrlr device model Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 15/15] MAINTAINERS: Added maintainerships for SSI Peter A. G. Crosthwaite
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added the two SPI controllers to the zynq machine model. Attached two SPI flash
devices to each controller.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/xilinx_zynq.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c
index 7e6c273..e273711 100644
--- a/hw/xilinx_zynq.c
+++ b/hw/xilinx_zynq.c
@@ -24,6 +24,9 @@
 #include "flash.h"
 #include "blockdev.h"
 #include "loader.h"
+#include "ssi.h"
+
+#define NUM_SPI_FLASHES 2
 
 #define FLASH_SIZE (64 * 1024 * 1024)
 #define FLASH_SECTOR_SIZE (128 * 1024)
@@ -46,6 +49,34 @@ static void gem_init(NICInfo *nd, uint32_t base, qemu_irq irq)
     sysbus_connect_irq(s, 0, irq);
 }
 
+static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq)
+{
+    DeviceState *dev;
+    SysBusDevice *busdev;
+    SSIBus *spi;
+    int i;
+
+    dev = qdev_create(NULL, "xilinx,spips");
+    qdev_init_nofail(dev);
+    busdev = sysbus_from_qdev(dev);
+    sysbus_mmio_map(busdev, 0, base_addr);
+    sysbus_connect_irq(busdev, 0, irq);
+
+    spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
+
+    for (i = 0; i < NUM_SPI_FLASHES; ++i) {
+        qemu_irq cs_line;
+
+        dev = ssi_create_slave_no_init(spi, "m25p80");
+        qdev_prop_set_string(dev, "partname", (char *)"n25q128");
+        qdev_init_nofail(dev);
+
+        cs_line = qdev_get_gpio_in(dev, 0);
+        sysbus_connect_irq(busdev, i+1, cs_line);
+    }
+
+}
+
 static void zynq_init(ram_addr_t ram_size, const char *boot_device,
                         const char *kernel_filename, const char *kernel_cmdline,
                         const char *initrd_filename, const char *cpu_model)
@@ -113,6 +144,9 @@ static void zynq_init(ram_addr_t ram_size, const char *boot_device,
         pic[n] = qdev_get_gpio_in(dev, n);
     }
 
+    zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET]);
+    zynq_init_spi_flashes(0xE0007000, pic[81-IRQ_OFFSET]);
+
     sysbus_create_simple("cadence_uart", 0xE0000000, pic[59-IRQ_OFFSET]);
     sysbus_create_simple("cadence_uart", 0xE0001000, pic[82-IRQ_OFFSET]);
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [Qemu-devel] [PATCH v5 15/15] MAINTAINERS: Added maintainerships for SSI
  2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (13 preceding siblings ...)
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 14/15] xilinx_zynq: Added SPI controllers + flashes Peter A. G. Crosthwaite
@ 2012-08-06  2:16 ` Peter A. G. Crosthwaite
  14 siblings, 0 replies; 39+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-08-06  2:16 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias, peter.maydell, stefanha
  Cc: peter.crosthwaite, i.mitsyanko, john.williams

Added maintainership for SSI, M25P80 and the Xilinx SPI controllers.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 MAINTAINERS |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d219d2..0f28f19 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -268,6 +268,7 @@ S: Maintained
 F: hw/xilinx_zynq.c
 F: hw/zynq_slcr.c
 F: hw/cadence_*
+F: hw/xilinx_spips.c
 
 CRIS Machines
 -------------
@@ -455,6 +456,12 @@ M: Paul Brook <paul@codesourcery.com>
 S: Odd Fixes
 F: hw/lsi53c895a.c
 
+SSI
+M: Peter Crosthwaite <peter.crosthwaite@petalogix.com>
+S: Maintained
+F: hw/ssi.*
+F: hw/m25p80.c
+
 USB
 M: Gerd Hoffmann <kraxel@redhat.com>
 S: Maintained
@@ -498,6 +505,7 @@ F: hw/xilinx_intc.c
 F: hw/xilinx_ethlite.c
 F: hw/xilinx_timer.c
 F: hw/xilinx.h
+F: hw/xilinx_spi.c
 
 Subsystems
 ----------
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub Peter A. G. Crosthwaite
@ 2012-08-06  9:13   ` Peter Maydell
  2012-08-06  9:15     ` Peter Maydell
  2012-08-10  4:19     ` Peter Crosthwaite
  0 siblings, 2 replies; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:13 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: Anthony Liguori, i.mitsyanko, Juan Quintela, stefanha,
	qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
> SSI slave state (e.g. the CS line state).

This is more me being confused about how this should work than a
review comment, but it seems a bit odd that we have a hierarchy
Device->SSI->ADS7846[etc], where the VMState for the ADS7846
includes a field for the SSI VMState, but the SSI VMState doesn't
include a field for the Device VMState.

What you've done here matches how i2c works currently, but I've
just cc'd Anthony and Juan to check whether there's a better way
of doing it.


> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/ads7846.c   |    1 +
>  hw/max111x.c   |    1 +
>  hw/spitz.c     |    2 ++
>  hw/ssi.c       |   10 ++++++++++
>  hw/ssi.h       |   10 ++++++++++
>  hw/stellaris.c |    1 +
>  hw/z2.c        |    1 +
>  7 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ads7846.c b/hw/ads7846.c
> index 41c7f10..6a523f6 100644
> --- a/hw/ads7846.c
> +++ b/hw/ads7846.c
> @@ -124,6 +124,7 @@ static const VMStateDescription vmstate_ads7846 = {
>      .minimum_version_id_old = 0,
>      .post_load = ads7856_post_load,
>      .fields      = (VMStateField[]) {
> +        VMSTATE_SSI_SLAVE(ssidev, ADS7846State),
>          VMSTATE_INT32_ARRAY(input, ADS7846State, 8),
>          VMSTATE_INT32(noise, ADS7846State),
>          VMSTATE_INT32(cycle, ADS7846State),
> diff --git a/hw/max111x.c b/hw/max111x.c
> index 706d89f..948fd97 100644
> --- a/hw/max111x.c
> +++ b/hw/max111x.c
> @@ -103,6 +103,7 @@ static const VMStateDescription vmstate_max111x = {
>      .minimum_version_id = 0,
>      .minimum_version_id_old = 0,
>      .fields      = (VMStateField[]) {
> +        VMSTATE_SSI_SLAVE(ssidev, MAX111xState),
>          VMSTATE_UINT8(tb1, MAX111xState),
>          VMSTATE_UINT8(rb2, MAX111xState),
>          VMSTATE_UINT8(rb3, MAX111xState),
> diff --git a/hw/spitz.c b/hw/spitz.c
> index 20e7835..f5d502d 100644
> --- a/hw/spitz.c
> +++ b/hw/spitz.c
> @@ -1087,6 +1087,7 @@ static const VMStateDescription vmstate_corgi_ssp_regs = {
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields = (VMStateField []) {
> +        VMSTATE_SSI_SLAVE(ssidev, CorgiSSPState),
>          VMSTATE_UINT32_ARRAY(enable, CorgiSSPState, 3),
>          VMSTATE_END_OF_LIST(),
>      }
> @@ -1115,6 +1116,7 @@ static const VMStateDescription vmstate_spitz_lcdtg_regs = {
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields = (VMStateField []) {
> +        VMSTATE_SSI_SLAVE(ssidev, SpitzLCDTG),
>          VMSTATE_UINT32(bl_intensity, SpitzLCDTG),
>          VMSTATE_UINT32(bl_power, SpitzLCDTG),
>          VMSTATE_END_OF_LIST(),
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 35d0a04..2db88fc 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -80,6 +80,16 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
>      return r;
>  }
>
> +const VMStateDescription vmstate_ssi_slave = {
> +    .name = "SSISlave",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static void ssi_slave_register_types(void)
>  {
>      type_register_static(&ssi_bus_info);
> diff --git a/hw/ssi.h b/hw/ssi.h
> index 06657d7..975f9fb 100644
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -38,6 +38,16 @@ struct SSISlave {
>  #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
>  #define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
>
> +extern const VMStateDescription vmstate_ssi_slave;
> +
> +#define VMSTATE_SSI_SLAVE(_field, _state) {                          \
> +    .name       = (stringify(_field)),                               \
> +    .size       = sizeof(SSISlave),                                  \
> +    .vmsd       = &vmstate_ssi_slave,                                \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, SSISlave),    \
> +}
> +
>  DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>
>  /* Master interface.  */
> diff --git a/hw/stellaris.c b/hw/stellaris.c
> index 562fbbf..4d26857 100644
> --- a/hw/stellaris.c
> +++ b/hw/stellaris.c
> @@ -1188,6 +1188,7 @@ static const VMStateDescription vmstate_stellaris_ssi_bus = {
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields      = (VMStateField[]) {
> +        VMSTATE_SSI_SLAVE(ssidev, stellaris_ssi_bus_state),
>          VMSTATE_INT32(current_dev, stellaris_ssi_bus_state),
>          VMSTATE_END_OF_LIST()
>      }
> diff --git a/hw/z2.c b/hw/z2.c
> index 289cee9..721aaf1 100644
> --- a/hw/z2.c
> +++ b/hw/z2.c
> @@ -165,6 +165,7 @@ static VMStateDescription vmstate_zipit_lcd_state = {
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields = (VMStateField[]) {
> +        VMSTATE_SSI_SLAVE(ssidev, ZipitLCD),
>          VMSTATE_INT32(selected, ZipitLCD),
>          VMSTATE_INT32(enabled, ZipitLCD),
>          VMSTATE_BUFFER(buf, ZipitLCD),
> --
> 1.7.0.4
>


-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
  2012-08-06  9:13   ` Peter Maydell
@ 2012-08-06  9:15     ` Peter Maydell
  2012-08-10  4:19     ` Peter Crosthwaite
  1 sibling, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:15 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: Anthony Liguori, i.mitsyanko, Juan Quintela, stefanha,
	qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 10:13, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
>> SSI slave state (e.g. the CS line state).
>
> This is more me being confused about how this should work than a
> review comment, but it seems a bit odd that we have a hierarchy
> Device->SSI->ADS7846[etc], where the VMState for the ADS7846
> includes a field for the SSI VMState, but the SSI VMState doesn't
> include a field for the Device VMState.
>
> What you've done here matches how i2c works currently, but I've
> just cc'd Anthony and Juan to check whether there's a better way
> of doing it.

Oh, and just to mention the obvious, if we add fields to these
vmstates we need to bump the version numbers.

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour Peter A. G. Crosthwaite
@ 2012-08-06  9:25   ` Peter Maydell
  2012-08-07  5:17     ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:25 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Added default CS behaviour for SSI slaves. SSI devices can set a property
> to enable CS behaviour which will create a GPIO on the device which is the
> CS. Tristating of the bus on SSI transfers is implemented.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/ssd0323.c |    6 ++++++
>  hw/ssi-sd.c  |    6 ++++++
>  hw/ssi.c     |   39 ++++++++++++++++++++++++++++++++++++++-
>  hw/ssi.h     |   27 +++++++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 1 deletions(-)
>
> diff --git a/hw/ssd0323.c b/hw/ssd0323.c
> index b0b2e94..db16d20 100644
> --- a/hw/ssd0323.c
> +++ b/hw/ssd0323.c
> @@ -277,6 +277,7 @@ static void ssd0323_cd(void *opaque, int n, int level)
>
>  static void ssd0323_save(QEMUFile *f, void *opaque)
>  {
> +    SSISlave *ss = SSI_SLAVE(opaque);
>      ssd0323_state *s = (ssd0323_state *)opaque;
>      int i;
>
> @@ -294,10 +295,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->remap);
>      qemu_put_be32(f, s->mode);
>      qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
> +
> +    qemu_put_be32(f, ss->cs ? 1 : 0);

You don't need this ?: -- the standard bool-to-integer casting
rules will do it for you. Also, stray blank line, and you're
changing save/load format so you need to update a version number
somewhere.

>  }
>
>  static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>  {
> +    SSISlave *ss = SSI_SLAVE(opaque);
>      ssd0323_state *s = (ssd0323_state *)opaque;
>      int i;
>
> @@ -319,6 +323,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>      s->mode = qemu_get_be32(f);
>      qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>
> +    ss->cs = !!qemu_get_be32(f);

!! unnecessary here.

Same comments as here and the one above apply to the other devices
below.

> +
>      return 0;
>  }
>
> diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
> index b519bdb..6fd9ab9 100644
> --- a/hw/ssi-sd.c
> +++ b/hw/ssi-sd.c
> @@ -197,6 +197,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
>
>  static void ssi_sd_save(QEMUFile *f, void *opaque)
>  {
> +    SSISlave *ss = SSI_SLAVE(opaque);
>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>      int i;
>
> @@ -209,10 +210,13 @@ static void ssi_sd_save(QEMUFile *f, void *opaque)
>      qemu_put_be32(f, s->arglen);
>      qemu_put_be32(f, s->response_pos);
>      qemu_put_be32(f, s->stopping);
> +
> +    qemu_put_be32(f, ss->cs ? 1 : 0);
>  }
>
>  static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
>  {
> +    SSISlave *ss = SSI_SLAVE(opaque);
>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>      int i;
>
> @@ -229,6 +233,8 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
>      s->response_pos = qemu_get_be32(f);
>      s->stopping = qemu_get_be32(f);
>
> +    ss->cs = !!qemu_get_be32(f);
> +
>      return 0;
>  }
>
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 2db88fc..2e4f2fe 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -27,19 +27,55 @@ static const TypeInfo ssi_bus_info = {
>      .instance_size = sizeof(SSIBus),
>  };
>
> +static void ssi_cs_default(void *opaque, int n, int level)
> +{
> +    SSISlave *s = SSI_SLAVE(opaque);
> +    bool cs = !!level;
> +    assert(n == 0);
> +    if (s->cs != cs) {
> +        SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
> +        if (ssc->set_cs) {
> +            ssc->set_cs(s, cs);
> +        }
> +    }
> +    s->cs = cs;
> +}
> +
> +static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
> +{
> +    SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(dev);
> +
> +    if ((dev->cs && ssc->cs_polarity == SSI_CS_HIGH) ||
> +            (!dev->cs && ssc->cs_polarity == SSI_CS_LOW) ||
> +            ssc->cs_polarity == SSI_CS_NONE) {
> +        return ssc->transfer(dev, val);
> +    }
> +    return 0;
> +}
> +
>  static int ssi_slave_init(DeviceState *dev)
>  {
>      SSISlave *s = SSI_SLAVE(dev);
>      SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
>
> +    if (ssc->transfer_raw == ssi_transfer_raw_default &&
> +            ssc->cs_polarity != SSI_CS_NONE) {
> +        qdev_init_gpio_in(&s->qdev, ssi_cs_default, 1);
> +    }
> +
>      return ssc->init(s);
>  }
>
>  static void ssi_slave_class_init(ObjectClass *klass, void *data)
>  {
> +    SSISlaveClass *ssc = SSI_SLAVE_CLASS(klass);
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +
>      dc->init = ssi_slave_init;
>      dc->bus_type = TYPE_SSI_BUS;
> +    if (!ssc->transfer_raw) {
> +        ssc->transfer_raw = ssi_transfer_raw_default;
> +    }
>  }
>
>  static TypeInfo ssi_slave_info = {
> @@ -74,7 +110,7 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
>      QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
>          SSISlave *slave = SSI_SLAVE(kid->child);
>          ssc = SSI_SLAVE_GET_CLASS(slave);
> -        r |= ssc->transfer(slave, val);
> +        r |= ssc->transfer_raw(slave, val);
>      }
>
>      return r;
> @@ -86,6 +122,7 @@ const VMStateDescription vmstate_ssi_slave = {
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields      = (VMStateField[]) {
> +        VMSTATE_BOOL(cs, SSISlave),

New field -> must bump version. You could probably avoid this by
rearranging things so this field is in the vmstate from the start
rather than added in a subsequent patch.

>          VMSTATE_END_OF_LIST()
>      }
>  };
> diff --git a/hw/ssi.h b/hw/ssi.h
> index 975f9fb..5b69a3b 100644
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -23,16 +23,43 @@ typedef struct SSISlave SSISlave;
>  #define SSI_SLAVE_GET_CLASS(obj) \
>       OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE)
>
> +typedef enum {
> +    SSI_CS_NONE = 0,
> +    SSI_CS_LOW,
> +    SSI_CS_HIGH,
> +} SSICSMode;
> +
>  /* Slave devices.  */
>  typedef struct SSISlaveClass {
>      DeviceClass parent_class;
>
>      int (*init)(SSISlave *dev);
> +
> +    /* if you have standard or no CS behaviour, just override transfer.
> +     * This is called when the device cs is active (true by default).
> +     */
>      uint32_t (*transfer)(SSISlave *dev, uint32_t val);
> +    /* called when the CS line changes. Optional, devices only need to implement
> +     * this if they have side effects assoicated with the cs line (beyond

"associated"

> +     * tristating the txrx lines).
> +     */
> +    int (*set_cs)(SSISlave *dev, bool select);
> +    /* define whether or not CS exists and is active low/high */
> +    SSICSMode cs_polarity;
> +
> +    /* if you have non-standard CS behvaiour override this to take control

"behaviour" (and below)

> +     * of the CS behvaiour at the device level. transfer, set_cs, and
> +     * cs_polarity are unused if this is overwritten. Transfer_raw, will

spurious comma

> +     * always be called for the device for every txrx access the to parent bus

"to the"

> +     */
> +    uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
>  } SSISlaveClass;
>
>  struct SSISlave {
>      DeviceState qdev;
> +
> +    /* Chip select state */
> +    bool cs;
>  };
>
>  #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
> --
> 1.7.0.4
>


-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init()
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init() Peter A. G. Crosthwaite
@ 2012-08-06  9:29   ` Peter Maydell
  2012-08-07  0:04     ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:29 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Slave creation function that can be used to create an SSI slave without
> qdev_init() being called. This give machine models a change to set properties.

Not convinced about this one -- I think that if machine models need to
do more complicated things with the qdev device then they should just
call qdev_create/set properties/qdev_init themselves.

-- PMM

> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/ssi.c |    9 +++++++--
>  hw/ssi.h |    1 +
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi.c b/hw/ssi.c
> index 2e4f2fe..c47419d 100644
> --- a/hw/ssi.c
> +++ b/hw/ssi.c
> @@ -86,10 +86,15 @@ static TypeInfo ssi_slave_info = {
>      .abstract = true,
>  };
>
> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name)
> +{
> +    return qdev_create(&bus->qbus, name);
> +}
> +
>  DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
>  {
> -    DeviceState *dev;
> -    dev = qdev_create(&bus->qbus, name);
> +    DeviceState *dev = ssi_create_slave_no_init(bus, name);
> +
>      qdev_init_nofail(dev);
>      return dev;
>  }
> diff --git a/hw/ssi.h b/hw/ssi.h
> index 5b69a3b..80b9664 100644
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -76,6 +76,7 @@ extern const VMStateDescription vmstate_ssi_slave;
>  }
>
>  DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>
>  /* Master interface.  */
>  SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
> --
> 1.7.0.4
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls Peter A. G. Crosthwaite
@ 2012-08-06  9:38   ` Peter Maydell
  2012-08-07  0:12     ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:38 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Allow multiple qdev_init_gpio_in() calls for the one device. The first call will
> define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled
> with different handlers. Needed when two levels of the QOM class heirachy both
> define GPIO functionality, as a single GPIO handler with an index selecter is
> not possible.

I generally like this idea and I think there are a few devices in the
existing codebase that could profitably use this rather than trying
to sort things out in the handler function. (Long term we would ideally
replace the single gpio array with a set of named arrays of Pins but
this is useful in the meantime.)

> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/qdev.c |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index b5b74b9..ce91a72 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -293,9 +293,19 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
>
>  void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
>  {
> -    assert(dev->num_gpio_in == 0);
> -    dev->num_gpio_in = n;
> -    dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
> +    qemu_irq *new_irqs = qemu_allocate_irqs(handler, dev, n);
> +
> +    if (dev->num_gpio_in == 0) {
> +        dev->gpio_in = qemu_allocate_irqs(handler, dev, n);

In this case we've just called qemu_allocate_irqs() twice and leaked
the copy in new_irqs.

> +    } else {
> +        qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
> +        memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * dev->num_gpio_in);
> +        g_free(dev->gpio_in);
> +        memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * n),
> +        g_free(new_irqs);

I think this is rather looking into private details of what qemu_allocate_irqs
does, and it would be better to define a new qemu_reallocate_irqs() in irq.c
so we can use it here. (when doing so, consider whether g_renew() might help
in producing nice looking code)


> +        dev->gpio_in = all_irqs;
> +    }
> +    dev->num_gpio_in += n;
>  }
>
>  void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
> --
> 1.7.0.4
>


-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
@ 2012-08-06  9:40   ` Igor Mitsyanko
  2012-08-06  9:48   ` Peter Maydell
  2012-08-06  9:52   ` Igor Mitsyanko
  2 siblings, 0 replies; 39+ messages in thread
From: Igor Mitsyanko @ 2012-08-06  9:40 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: peter.maydell, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 08/06/2012 06:16 AM, Peter A. G. Crosthwaite wrote:
> Added a FIFO API that can be used to create and operate byte FIFOs.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>   hw/Makefile.objs |    1 +
>   hw/fifo.c        |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/fifo.h        |   47 ++++++++++++++++++++++++++++++++
>   3 files changed, 127 insertions(+), 0 deletions(-)
>   create mode 100644 hw/fifo.c
>   create mode 100644 hw/fifo.h
>
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 8327e55..6ba570e 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -15,6 +15,7 @@ hw-obj-$(CONFIG_ECC) += ecc.o
>   hw-obj-$(CONFIG_NAND) += nand.o
>   hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
>   hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
> +hw-obj-y += fifo.o
Perhaps it'd be better to make it common object and put it into root 
directory, like its done for bitops.c and bitmap.c
>   
>   hw-obj-$(CONFIG_M48T59) += m48t59.o
>   hw-obj-$(CONFIG_ESCC) += escc.o
> diff --git a/hw/fifo.c b/hw/fifo.c
> new file mode 100644
> index 0000000..5e14e1e
> --- /dev/null
> +++ b/hw/fifo.c
> @@ -0,0 +1,79 @@
> +/*
> + * Generic FIFO component, implemented as a circular buffer.
> + *
> + * Copyright (c) 2012 Peter A. G. Crosthwaite
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "fifo.h"
> +
> +void fifo8_create(Fifo8 *fifo, uint32_t capacity)
> +{
> +    fifo->data = g_new(uint8_t, capacity);
> +    fifo->capacity = capacity;
> +    fifo->head = 0;
> +    fifo->num = 0;
> +}
> +
> +void fifo8_destroy(Fifo8 *fifo)
> +{
> +    g_free(fifo->data);
> +}
> +
> +void fifo8_push(Fifo8 *fifo, uint8_t data)
> +{
> +    if (fifo->num == fifo->capacity) {
> +        abort();
> +    }
I think its too harsh to abort here (and in pop() too), fifo 
overrun/underrun condition is absolutely normal for most of the devices, 
usually it would just trigger an  interrupt. I suggest return a error 
code instead and let a caller decide what should happen in this situation.
> +    fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
> +    fifo->num++;
> +}
> +
> +uint8_t fifo8_pop(Fifo8 *fifo)
> +{
> +    uint8_t ret;
> +
> +    if (fifo->num == 0) {
> +        abort();
> +    }
> +    ret = fifo->data[fifo->head++];
> +    fifo->head %= fifo->capacity;
> +    fifo->num--;
> +    return ret;
> +}
> +
> +void fifo8_reset(Fifo8 *fifo)
> +{
> +    fifo->num = 0;
> +}
> +
> +bool fifo8_is_empty(Fifo8 *fifo)
> +{
> +    return (fifo->num == 0);
> +}
> +
> +bool fifo8_is_full(Fifo8 *fifo)
> +{
> +    return (fifo->num == fifo->capacity);
> +}
> +
> +const VMStateDescription vmstate_fifo8 = {
> +    .name = "SSISlave",
thats not a good name for a generic fifo)
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
too much spaces here
> +        VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
> +        VMSTATE_UINT32(head, Fifo8),
> +        VMSTATE_UINT32(num, Fifo8),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> diff --git a/hw/fifo.h b/hw/fifo.h
> new file mode 100644
> index 0000000..3fb09ff
> --- /dev/null
> +++ b/hw/fifo.h
> @@ -0,0 +1,47 @@
> +#ifndef FIFO_H
> +#define FIFO_H
> +
> +#include "hw.h"
> +
> +typedef struct {
> +    /* All fields are private */
> +    uint8_t *data;
> +    uint32_t capacity;
> +    uint32_t head;
> +    uint32_t num;
> +} Fifo8;
> +
> +/* create a fifo of the specified size */
> +
> +void fifo8_create(Fifo8 *, uint32_t);
> +
> +/* cleanup a fifo */
> +
> +void fifo8_destroy(Fifo8 *);
> +
> +/* push a data byte to the fifo. Behaviour is undefined if the fifo is full */
> +
> +void fifo8_push(Fifo8 *, uint8_t);
> +
> +/* pop a data byte from the fifo. Behviour is undefined if the fifo is empty */
> +
> +uint8_t fifo8_pop(Fifo8 *);
> +
> +/* reset (empty) the fifo */
> +
> +void fifo8_reset(Fifo8 *);
> +
> +bool fifo8_is_empty(Fifo8 *);
> +bool fifo8_is_full(Fifo8 *);
> +
> +extern const VMStateDescription vmstate_fifo8;
> +
> +#define VMSTATE_FIFO8(_field, _state) {                              \
> +    .name       = (stringify(_field)),                               \
> +    .size       = sizeof(Fifo8),                                     \
> +    .vmsd       = &vmstate_fifo8,                                    \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
> +}
> +
> +#endif /* FIFO_H */

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error Peter A. G. Crosthwaite
@ 2012-08-06  9:41   ` Peter Maydell
  2012-08-10 23:31     ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:41 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> To be more consistent with the newer ways of error signalling. That and SIGABT
> is easier to debug with than exit(1).
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
  2012-08-06  9:40   ` Igor Mitsyanko
@ 2012-08-06  9:48   ` Peter Maydell
  2012-08-06 12:42     ` Igor Mitsyanko
  2012-08-07  6:05     ` Peter Crosthwaite
  2012-08-06  9:52   ` Igor Mitsyanko
  2 siblings, 2 replies; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:48 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Added a FIFO API that can be used to create and operate byte FIFOs.

I'm not asking for actual conversions, but it would be nice to see a
list of some devices that could in principle be moved to using this FIFO,
as an indication of its general utility.

Would it make sense for the FIFO to be a QOM object, or is that a
silly idea?

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128 Peter A. G. Crosthwaite
@ 2012-08-06  9:50   ` Peter Maydell
  2012-08-07  5:24     ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-08-06  9:50 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 6 August 2012 03:16, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Added SPI controller to the reference design, with two n25q128 spi-flashes
> connected.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  hw/petalogix_ml605_mmu.c |   28 +++++++++++++++++++++++++++-
>  1 files changed, 27 insertions(+), 1 deletions(-)
>
> diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
> index 6a7d0c0..f0ecc1f 100644
> --- a/hw/petalogix_ml605_mmu.c
> +++ b/hw/petalogix_ml605_mmu.c
> @@ -36,6 +36,7 @@
>  #include "blockdev.h"
>  #include "pc.h"
>  #include "exec-memory.h"
> +#include "ssi.h"
>
>  #include "microblaze_boot.h"
>  #include "microblaze_pic_cpu.h"
> @@ -54,6 +55,8 @@
>  #define AXIENET_BASEADDR 0x82780000
>  #define AXIDMA_BASEADDR 0x84600000
>
> +#define NUM_SPI_FLASHES 2
> +
>  static void machine_cpu_reset(MicroBlazeCPU *cpu)
>  {
>      CPUMBState *env = &cpu->env;
> @@ -78,6 +81,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
>      MemoryRegion *address_space_mem = get_system_memory();
>      DeviceState *dev;
>      MicroBlazeCPU *cpu;
> +    SysBusDevice *busdev;
>      CPUMBState *env;
>      DriveInfo *dinfo;
>      int i;
> @@ -135,9 +139,31 @@ petalogix_ml605_init(ram_addr_t ram_size,
>                                       irq[1], irq[0], 100 * 1000000);
>      }
>
> +    {
> +        SSIBus *spi;
> +
> +        dev = qdev_create(NULL, "xilinx,spi");
> +        qdev_prop_set_uint8(dev, "num-cs", NUM_SPI_FLASHES);
> +        qdev_init_nofail(dev);
> +        busdev = sysbus_from_qdev(dev);
> +        sysbus_mmio_map(busdev, 0, 0x40a00000);
> +        sysbus_connect_irq(busdev, 0, irq[4]);
> +
> +        spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
> +
> +        for (i = 0; i < NUM_SPI_FLASHES; i++) {
> +            qemu_irq cs_line;
> +
> +            dev = ssi_create_slave_no_init(spi, "m25p80");
> +            qdev_prop_set_string(dev, "partname", (char *)"n25q128");

Why the cast?

> +            qdev_init_nofail(dev);
> +            cs_line = qdev_get_gpio_in(dev, 0);
> +            sysbus_connect_irq(busdev, i+1, cs_line);
> +        }
> +    }
> +
>      microblaze_load_kernel(cpu, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
>                                                              machine_cpu_reset);
> -
>  }
>
>  static QEMUMachine petalogix_ml605_machine = {
> --
> 1.7.0.4
>


-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
  2012-08-06  9:40   ` Igor Mitsyanko
  2012-08-06  9:48   ` Peter Maydell
@ 2012-08-06  9:52   ` Igor Mitsyanko
  2012-08-07  6:10     ` Peter Crosthwaite
  2 siblings, 1 reply; 39+ messages in thread
From: Igor Mitsyanko @ 2012-08-06  9:52 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite
  Cc: peter.maydell, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 08/06/2012 06:16 AM, Peter A. G. Crosthwaite wrote:
> Added a FIFO API that can be used to create and operate byte FIFOs.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>   hw/Makefile.objs |    1 +
>   hw/fifo.c        |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/fifo.h        |   47 ++++++++++++++++++++++++++++++++
>   3 files changed, 127 insertions(+), 0 deletions(-)
>   create mode 100644 hw/fifo.c
>   create mode 100644 hw/fifo.h
>
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 8327e55..6ba570e 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -15,6 +15,7 @@ hw-obj-$(CONFIG_ECC) += ecc.o
>   hw-obj-$(CONFIG_NAND) += nand.o
>   hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
>   hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
> +hw-obj-y += fifo.o
>   
>   hw-obj-$(CONFIG_M48T59) += m48t59.o
>   hw-obj-$(CONFIG_ESCC) += escc.o
> diff --git a/hw/fifo.c b/hw/fifo.c
> new file mode 100644
> index 0000000..5e14e1e
> --- /dev/null
> +++ b/hw/fifo.c
> @@ -0,0 +1,79 @@
> +/*
> + * Generic FIFO component, implemented as a circular buffer.
> + *
> + * Copyright (c) 2012 Peter A. G. Crosthwaite
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "fifo.h"
> +
> +void fifo8_create(Fifo8 *fifo, uint32_t capacity)
> +{
> +    fifo->data = g_new(uint8_t, capacity);
> +    fifo->capacity = capacity;
> +    fifo->head = 0;
> +    fifo->num = 0;
> +}
> +
> +void fifo8_destroy(Fifo8 *fifo)
> +{
> +    g_free(fifo->data);
> +}
> +
> +void fifo8_push(Fifo8 *fifo, uint8_t data)
> +{
> +    if (fifo->num == fifo->capacity) {
> +        abort();
> +    }
> +    fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
> +    fifo->num++;
> +}
> +
> +uint8_t fifo8_pop(Fifo8 *fifo)
> +{
> +    uint8_t ret;
> +
> +    if (fifo->num == 0) {
> +        abort();
> +    }
> +    ret = fifo->data[fifo->head++];
> +    fifo->head %= fifo->capacity;
> +    fifo->num--;
> +    return ret;
> +}
> +
> +void fifo8_reset(Fifo8 *fifo)
> +{
> +    fifo->num = 0;
> +}
> +
> +bool fifo8_is_empty(Fifo8 *fifo)
> +{
> +    return (fifo->num == 0);
> +}
> +
> +bool fifo8_is_full(Fifo8 *fifo)
> +{
> +    return (fifo->num == fifo->capacity);
> +}
> +
> +const VMStateDescription vmstate_fifo8 = {
> +    .name = "SSISlave",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
> +        VMSTATE_UINT32(head, Fifo8),
> +        VMSTATE_UINT32(num, Fifo8),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> diff --git a/hw/fifo.h b/hw/fifo.h
> new file mode 100644
> index 0000000..3fb09ff
> --- /dev/null
> +++ b/hw/fifo.h
> @@ -0,0 +1,47 @@
> +#ifndef FIFO_H
> +#define FIFO_H
> +
> +#include "hw.h"
> +
> +typedef struct {
> +    /* All fields are private */
> +    uint8_t *data;
> +    uint32_t capacity;
> +    uint32_t head;
> +    uint32_t num;
> +} Fifo8;
> +
> +/* create a fifo of the specified size */
> +
> +void fifo8_create(Fifo8 *, uint32_t);
> +
> +/* cleanup a fifo */
> +
> +void fifo8_destroy(Fifo8 *);
> +
> +/* push a data byte to the fifo. Behaviour is undefined if the fifo is full */
> +
> +void fifo8_push(Fifo8 *, uint8_t);
> +
> +/* pop a data byte from the fifo. Behviour is undefined if the fifo is empty */
> +
> +uint8_t fifo8_pop(Fifo8 *);
> +
> +/* reset (empty) the fifo */
> +
> +void fifo8_reset(Fifo8 *);
> +
> +bool fifo8_is_empty(Fifo8 *);
> +bool fifo8_is_full(Fifo8 *);
> +
> +extern const VMStateDescription vmstate_fifo8;
> +
> +#define VMSTATE_FIFO8(_field, _state) {                              \
> +    .name       = (stringify(_field)),                               \
> +    .size       = sizeof(Fifo8),                                     \
> +    .vmsd       = &vmstate_fifo8,                                    \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
> +}

how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro 
instead? And maybe this should go to vmstate.h header

> +
> +#endif /* FIFO_H */

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  9:48   ` Peter Maydell
@ 2012-08-06 12:42     ` Igor Mitsyanko
  2012-08-07  6:05     ` Peter Crosthwaite
  1 sibling, 0 replies; 39+ messages in thread
From: Igor Mitsyanko @ 2012-08-06 12:42 UTC (permalink / raw)
  To: Peter Maydell
  Cc: stefanha, qemu-devel, Peter A. G. Crosthwaite, paul,
	edgar.iglesias, john.williams

On 08/06/2012 01:48 PM, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Added a FIFO API that can be used to create and operate byte FIFOs.
> I'm not asking for actual conversions, but it would be nice to see a
> list of some devices that could in principle be moved to using this FIFO,
> as an indication of its general utility.
>
> Would it make sense for the FIFO to be a QOM object, or is that a
> silly idea?
>
> -- PMM
>
FIFO introspection capability could be useful I think, and we could 
implement device-specific fifo "mutants" then (for example, PL330 fifo 
could be a general FIFO object + "tag" variable).

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init()
  2012-08-06  9:29   ` Peter Maydell
@ 2012-08-07  0:04     ` Peter Crosthwaite
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  0:04 UTC (permalink / raw)
  To: Peter Maydell
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Mon, Aug 6, 2012 at 7:29 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Slave creation function that can be used to create an SSI slave without
>> qdev_init() being called. This give machine models a change to set properties.
>
> Not convinced about this one -- I think that if machine models need to
> do more complicated things with the qdev device then they should just
> call qdev_create/set properties/qdev_init themselves.
>

Yeh I tried that didnt work. See comment below.

> -- PMM
>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  hw/ssi.c |    9 +++++++--
>>  hw/ssi.h |    1 +
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ssi.c b/hw/ssi.c
>> index 2e4f2fe..c47419d 100644
>> --- a/hw/ssi.c
>> +++ b/hw/ssi.c
>> @@ -86,10 +86,15 @@ static TypeInfo ssi_slave_info = {
>>      .abstract = true,
>>  };
>>
>> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name)
>> +{
>> +    return qdev_create(&bus->qbus, name);

bus->qbus accesses requires the definition of struct SSIBus which is
private to ssi.c. Machine models cant access the qbus field which they
need for qdev_create().

Regards,
Peter

>> +}
>> +
>>  DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
>>  {
>> -    DeviceState *dev;
>> -    dev = qdev_create(&bus->qbus, name);
>> +    DeviceState *dev = ssi_create_slave_no_init(bus, name);
>> +
>>      qdev_init_nofail(dev);
>>      return dev;
>>  }
>> diff --git a/hw/ssi.h b/hw/ssi.h
>> index 5b69a3b..80b9664 100644
>> --- a/hw/ssi.h
>> +++ b/hw/ssi.h
>> @@ -76,6 +76,7 @@ extern const VMStateDescription vmstate_ssi_slave;
>>  }
>>
>>  DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
>> +DeviceState *ssi_create_slave_no_init(SSIBus *bus, const char *name);
>>
>>  /* Master interface.  */
>>  SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
>> --
>> 1.7.0.4
>>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
  2012-08-06  9:38   ` Peter Maydell
@ 2012-08-07  0:12     ` Peter Crosthwaite
  2012-08-07  8:03       ` Peter Maydell
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  0:12 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Anthony Liguori, i.mitsyanko, stefanha, qemu-devel, paul,
	edgar.iglesias, Andreas Färber, john.williams

On Mon, Aug 6, 2012 at 7:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Allow multiple qdev_init_gpio_in() calls for the one device. The first call will
>> define GPIOs 0-N-1, the next GPIOs N- ... . Allows different GPIOs to be handled
>> with different handlers. Needed when two levels of the QOM class heirachy both
>> define GPIO functionality, as a single GPIO handler with an index selecter is
>> not possible.
>
> I generally like this idea and I think there are a few devices in the
> existing codebase that could profitably use this rather than trying
> to sort things out in the handler function. (Long term we would ideally
> replace the single gpio array with a set of named arrays of Pins but
> this is useful in the meantime.)
>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  hw/qdev.c |   16 +++++++++++++---
>>  1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index b5b74b9..ce91a72 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -293,9 +293,19 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
>>
>>  void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
>>  {
>> -    assert(dev->num_gpio_in == 0);
>> -    dev->num_gpio_in = n;
>> -    dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
>> +    qemu_irq *new_irqs = qemu_allocate_irqs(handler, dev, n);

RE comment below, this should be in the else condition, ill move it.

>> +
>> +    if (dev->num_gpio_in == 0) {
>> +        dev->gpio_in = qemu_allocate_irqs(handler, dev, n);
>
> In this case we've just called qemu_allocate_irqs() twice and leaked
> the copy in new_irqs.
>
>> +    } else {
>> +        qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
>> +        memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * dev->num_gpio_in);
>> +        g_free(dev->gpio_in);
>> +        memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * n),
>> +        g_free(new_irqs);
>
> I think this is rather looking into private details of what qemu_allocate_irqs
> does, and it would be better to define a new qemu_reallocate_irqs() in irq.c
> so we can use it here. (when doing so, consider whether g_renew() might help
> in producing nice looking code)

My understanding is Anthony is doing major refactoring in the GPIO
area anyways. Long term, will this code in qdev.c/irq.c even going to
exist? In this patch, I took something of a path of least resistance
to just make this series work, as I suspect this patch in its current
form will be short lived due to continued QOM development.

cc Andreas and Anthony.

Regards,
Peter

>
>
>> +        dev->gpio_in = all_irqs;
>> +    }
>> +    dev->num_gpio_in += n;
>>  }
>>
>>  void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
>> --
>> 1.7.0.4
>>
>
>
> -- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour
  2012-08-06  9:25   ` Peter Maydell
@ 2012-08-07  5:17     ` Peter Crosthwaite
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  5:17 UTC (permalink / raw)
  To: Peter Maydell
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Mon, Aug 6, 2012 at 7:25 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Added default CS behaviour for SSI slaves. SSI devices can set a property
>> to enable CS behaviour which will create a GPIO on the device which is the
>> CS. Tristating of the bus on SSI transfers is implemented.
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  hw/ssd0323.c |    6 ++++++
>>  hw/ssi-sd.c  |    6 ++++++
>>  hw/ssi.c     |   39 ++++++++++++++++++++++++++++++++++++++-
>>  hw/ssi.h     |   27 +++++++++++++++++++++++++++
>>  4 files changed, 77 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/ssd0323.c b/hw/ssd0323.c
>> index b0b2e94..db16d20 100644
>> --- a/hw/ssd0323.c
>> +++ b/hw/ssd0323.c
>> @@ -277,6 +277,7 @@ static void ssd0323_cd(void *opaque, int n, int level)
>>
>>  static void ssd0323_save(QEMUFile *f, void *opaque)
>>  {
>> +    SSISlave *ss = SSI_SLAVE(opaque);
>>      ssd0323_state *s = (ssd0323_state *)opaque;
>>      int i;
>>
>> @@ -294,10 +295,13 @@ static void ssd0323_save(QEMUFile *f, void *opaque)
>>      qemu_put_be32(f, s->remap);
>>      qemu_put_be32(f, s->mode);
>>      qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>> +
>> +    qemu_put_be32(f, ss->cs ? 1 : 0);
>
> You don't need this ?: -- the standard bool-to-integer casting
> rules will do it for you. Also, stray blank line, and you're
> changing save/load format so you need to update a version number
> somewhere.
>

Ok, any quick guidelines for where to find information on how to do this?

>>  }
>>
>>  static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>>  {
>> +    SSISlave *ss = SSI_SLAVE(opaque);
>>      ssd0323_state *s = (ssd0323_state *)opaque;
>>      int i;
>>
>> @@ -319,6 +323,8 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
>>      s->mode = qemu_get_be32(f);
>>      qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
>>
>> +    ss->cs = !!qemu_get_be32(f);
>
> !! unnecessary here.
>
> Same comments as here and the one above apply to the other devices
> below.
>
>> +
>>      return 0;
>>  }
>>
>> diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
>> index b519bdb..6fd9ab9 100644
>> --- a/hw/ssi-sd.c
>> +++ b/hw/ssi-sd.c
>> @@ -197,6 +197,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
>>
>>  static void ssi_sd_save(QEMUFile *f, void *opaque)
>>  {
>> +    SSISlave *ss = SSI_SLAVE(opaque);
>>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>>      int i;
>>
>> @@ -209,10 +210,13 @@ static void ssi_sd_save(QEMUFile *f, void *opaque)
>>      qemu_put_be32(f, s->arglen);
>>      qemu_put_be32(f, s->response_pos);
>>      qemu_put_be32(f, s->stopping);
>> +
>> +    qemu_put_be32(f, ss->cs ? 1 : 0);
>>  }
>>
>>  static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
>>  {
>> +    SSISlave *ss = SSI_SLAVE(opaque);
>>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>>      int i;
>>
>> @@ -229,6 +233,8 @@ static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
>>      s->response_pos = qemu_get_be32(f);
>>      s->stopping = qemu_get_be32(f);
>>
>> +    ss->cs = !!qemu_get_be32(f);
>> +
>>      return 0;
>>  }
>>
>> diff --git a/hw/ssi.c b/hw/ssi.c
>> index 2db88fc..2e4f2fe 100644
>> --- a/hw/ssi.c
>> +++ b/hw/ssi.c
>> @@ -27,19 +27,55 @@ static const TypeInfo ssi_bus_info = {
>>      .instance_size = sizeof(SSIBus),
>>  };
>>
>> +static void ssi_cs_default(void *opaque, int n, int level)
>> +{
>> +    SSISlave *s = SSI_SLAVE(opaque);
>> +    bool cs = !!level;
>> +    assert(n == 0);
>> +    if (s->cs != cs) {
>> +        SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
>> +        if (ssc->set_cs) {
>> +            ssc->set_cs(s, cs);
>> +        }
>> +    }
>> +    s->cs = cs;
>> +}
>> +
>> +static uint32_t ssi_transfer_raw_default(SSISlave *dev, uint32_t val)
>> +{
>> +    SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(dev);
>> +
>> +    if ((dev->cs && ssc->cs_polarity == SSI_CS_HIGH) ||
>> +            (!dev->cs && ssc->cs_polarity == SSI_CS_LOW) ||
>> +            ssc->cs_polarity == SSI_CS_NONE) {
>> +        return ssc->transfer(dev, val);
>> +    }
>> +    return 0;
>> +}
>> +
>>  static int ssi_slave_init(DeviceState *dev)
>>  {
>>      SSISlave *s = SSI_SLAVE(dev);
>>      SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s);
>>
>> +    if (ssc->transfer_raw == ssi_transfer_raw_default &&
>> +            ssc->cs_polarity != SSI_CS_NONE) {
>> +        qdev_init_gpio_in(&s->qdev, ssi_cs_default, 1);
>> +    }
>> +
>>      return ssc->init(s);
>>  }
>>
>>  static void ssi_slave_class_init(ObjectClass *klass, void *data)
>>  {
>> +    SSISlaveClass *ssc = SSI_SLAVE_CLASS(klass);
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>>      dc->init = ssi_slave_init;
>>      dc->bus_type = TYPE_SSI_BUS;
>> +    if (!ssc->transfer_raw) {
>> +        ssc->transfer_raw = ssi_transfer_raw_default;
>> +    }
>>  }
>>
>>  static TypeInfo ssi_slave_info = {
>> @@ -74,7 +110,7 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
>>      QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
>>          SSISlave *slave = SSI_SLAVE(kid->child);
>>          ssc = SSI_SLAVE_GET_CLASS(slave);
>> -        r |= ssc->transfer(slave, val);
>> +        r |= ssc->transfer_raw(slave, val);
>>      }
>>
>>      return r;
>> @@ -86,6 +122,7 @@ const VMStateDescription vmstate_ssi_slave = {
>>      .minimum_version_id = 1,
>>      .minimum_version_id_old = 1,
>>      .fields      = (VMStateField[]) {
>> +        VMSTATE_BOOL(cs, SSISlave),
>
> New field -> must bump version. You could probably avoid this by
> rearranging things so this field is in the vmstate from the start
> rather than added in a subsequent patch.
>

So is this a bisect-ability issue between this and the previous patch?
If so Ill just fold the two together, they are related enough to do
so. Better to not change functionality for the sake of revision
control.

>>          VMSTATE_END_OF_LIST()
>>      }
>>  };
>> diff --git a/hw/ssi.h b/hw/ssi.h
>> index 975f9fb..5b69a3b 100644
>> --- a/hw/ssi.h
>> +++ b/hw/ssi.h
>> @@ -23,16 +23,43 @@ typedef struct SSISlave SSISlave;
>>  #define SSI_SLAVE_GET_CLASS(obj) \
>>       OBJECT_GET_CLASS(SSISlaveClass, (obj), TYPE_SSI_SLAVE)
>>
>> +typedef enum {
>> +    SSI_CS_NONE = 0,
>> +    SSI_CS_LOW,
>> +    SSI_CS_HIGH,
>> +} SSICSMode;
>> +
>>  /* Slave devices.  */
>>  typedef struct SSISlaveClass {
>>      DeviceClass parent_class;
>>
>>      int (*init)(SSISlave *dev);
>> +
>> +    /* if you have standard or no CS behaviour, just override transfer.
>> +     * This is called when the device cs is active (true by default).
>> +     */
>>      uint32_t (*transfer)(SSISlave *dev, uint32_t val);
>> +    /* called when the CS line changes. Optional, devices only need to implement
>> +     * this if they have side effects assoicated with the cs line (beyond
>
> "associated"
>
>> +     * tristating the txrx lines).
>> +     */
>> +    int (*set_cs)(SSISlave *dev, bool select);
>> +    /* define whether or not CS exists and is active low/high */
>> +    SSICSMode cs_polarity;
>> +
>> +    /* if you have non-standard CS behvaiour override this to take control
>
> "behaviour" (and below)
>
>> +     * of the CS behvaiour at the device level. transfer, set_cs, and
>> +     * cs_polarity are unused if this is overwritten. Transfer_raw, will
>
> spurious comma
>
>> +     * always be called for the device for every txrx access the to parent bus
>
> "to the"
>
>> +     */
>> +    uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
>>  } SSISlaveClass;
>>
>>  struct SSISlave {
>>      DeviceState qdev;
>> +
>> +    /* Chip select state */
>> +    bool cs;
>>  };
>>
>>  #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
>> --
>> 1.7.0.4
>>
>
>
> -- PMM

Regards,
Peter

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128
  2012-08-06  9:50   ` Peter Maydell
@ 2012-08-07  5:24     ` Peter Crosthwaite
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  5:24 UTC (permalink / raw)
  To: Peter Maydell
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Mon, Aug 6, 2012 at 7:50 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Added SPI controller to the reference design, with two n25q128 spi-flashes
>> connected.
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  hw/petalogix_ml605_mmu.c |   28 +++++++++++++++++++++++++++-
>>  1 files changed, 27 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
>> index 6a7d0c0..f0ecc1f 100644
>> --- a/hw/petalogix_ml605_mmu.c
>> +++ b/hw/petalogix_ml605_mmu.c
>> @@ -36,6 +36,7 @@
>>  #include "blockdev.h"
>>  #include "pc.h"
>>  #include "exec-memory.h"
>> +#include "ssi.h"
>>
>>  #include "microblaze_boot.h"
>>  #include "microblaze_pic_cpu.h"
>> @@ -54,6 +55,8 @@
>>  #define AXIENET_BASEADDR 0x82780000
>>  #define AXIDMA_BASEADDR 0x84600000
>>
>> +#define NUM_SPI_FLASHES 2
>> +
>>  static void machine_cpu_reset(MicroBlazeCPU *cpu)
>>  {
>>      CPUMBState *env = &cpu->env;
>> @@ -78,6 +81,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
>>      MemoryRegion *address_space_mem = get_system_memory();
>>      DeviceState *dev;
>>      MicroBlazeCPU *cpu;
>> +    SysBusDevice *busdev;
>>      CPUMBState *env;
>>      DriveInfo *dinfo;
>>      int i;
>> @@ -135,9 +139,31 @@ petalogix_ml605_init(ram_addr_t ram_size,
>>                                       irq[1], irq[0], 100 * 1000000);
>>      }
>>
>> +    {
>> +        SSIBus *spi;
>> +
>> +        dev = qdev_create(NULL, "xilinx,spi");
>> +        qdev_prop_set_uint8(dev, "num-cs", NUM_SPI_FLASHES);
>> +        qdev_init_nofail(dev);
>> +        busdev = sysbus_from_qdev(dev);
>> +        sysbus_mmio_map(busdev, 0, 0x40a00000);
>> +        sysbus_connect_irq(busdev, 0, irq[4]);
>> +
>> +        spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
>> +
>> +        for (i = 0; i < NUM_SPI_FLASHES; i++) {
>> +            qemu_irq cs_line;
>> +
>> +            dev = ssi_create_slave_no_init(spi, "m25p80");
>> +            qdev_prop_set_string(dev, "partname", (char *)"n25q128");
>
> Why the cast?
>

Used to issue a warning, but just checked and this was fixed recently
(3b25597bcf7fa8c92ba2107fbdb260ce0eccd64b). Can remove it now. Will do
so in the next revision.

Regards,
Peter

>> +            qdev_init_nofail(dev);
>> +            cs_line = qdev_get_gpio_in(dev, 0);
>> +            sysbus_connect_irq(busdev, i+1, cs_line);
>> +        }
>> +    }
>> +
>>      microblaze_load_kernel(cpu, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
>>                                                              machine_cpu_reset);
>> -
>>  }
>>
>>  static QEMUMachine petalogix_ml605_machine = {
>> --
>> 1.7.0.4
>>
>
>
> -- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  9:48   ` Peter Maydell
  2012-08-06 12:42     ` Igor Mitsyanko
@ 2012-08-07  6:05     ` Peter Crosthwaite
  1 sibling, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  6:05 UTC (permalink / raw)
  To: Peter Maydell
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Mon, Aug 6, 2012 at 7:48 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Added a FIFO API that can be used to create and operate byte FIFOs.
>
> I'm not asking for actual conversions, but it would be nice to see a
> list of some devices that could in principle be moved to using this FIFO,
> as an indication of its general utility.
>

The two device models in this series. mcf_uart.c, strongarm.c
(StrongARMUARTState), xilinx_uartlite, cadence_uart.c, sh_serial.c,
tsc210x.c to name the ones Ive just had a brief look over. There are
more. grep -i fifo hw | grep uint8_t gives you a short list of files
to look for potential clients.

Dominated by uarts as they are the ones that need 8-bit fifos.
Usability is greatly expanded if the fifo can be an arbitrary data
type (uint32_t, uint16_t, struct foo), but that is a much harder
problem to solve that id like to say is out of scope of this series. I
think this is what Igor is getting at RE his PL330 comments on his
continuation of this thread. Can we get the ball rolling with uint8_t
then talk about generalisation strategy?

> Would it make sense for the FIFO to be a QOM object, or is that a
> silly idea?

There could be merit in making the data entries some form of QOM
object. It would hurt performance, but could solve the arbitrary data
problem.

Regards,
Peter

>
> -- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-06  9:52   ` Igor Mitsyanko
@ 2012-08-07  6:10     ` Peter Crosthwaite
  2012-08-07  6:28       ` Igor Mitsyanko
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  6:10 UTC (permalink / raw)
  To: Igor Mitsyanko
  Cc: peter.maydell, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

>> +
>> +extern const VMStateDescription vmstate_fifo8;
>> +
>> +#define VMSTATE_FIFO8(_field, _state) {                              \
>> +    .name       = (stringify(_field)),                               \
>> +    .size       = sizeof(Fifo8),                                     \
>> +    .vmsd       = &vmstate_fifo8,                                    \
>> +    .flags      = VMS_STRUCT,                                        \
>> +    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
>> +}
>
>
> how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro
> instead?

This has no existing precedent in QEMU so I am unsure of what you mean?

And maybe this should go to vmstate.h header

I disagree. All other clients of VMS_STRUCT are out in their repective
device specific headers (pci.h, i2c.h) etc. Unless this is new
established policy, I dont really want to change the current adopted
approach.

Regards,
Peter

>
>
>> +
>> +#endif /* FIFO_H */
>
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-07  6:10     ` Peter Crosthwaite
@ 2012-08-07  6:28       ` Igor Mitsyanko
  2012-08-07  6:42         ` Peter Crosthwaite
  0 siblings, 1 reply; 39+ messages in thread
From: Igor Mitsyanko @ 2012-08-07  6:28 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: peter.maydell, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 08/07/2012 10:10 AM, Peter Crosthwaite wrote:
>>> +
>>> +extern const VMStateDescription vmstate_fifo8;
>>> +
>>> +#define VMSTATE_FIFO8(_field, _state) {                              \
>>> +    .name       = (stringify(_field)),                               \
>>> +    .size       = sizeof(Fifo8),                                     \
>>> +    .vmsd       = &vmstate_fifo8,                                    \
>>> +    .flags      = VMS_STRUCT,                                        \
>>> +    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
>>> +}
>>
>> how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro
>> instead?
> This has no existing precedent in QEMU so I am unsure of what you mean?

I meant VMSTATE_TIMER_TEST() in vmstate.h as an example, which is a 
wrapper to VMSTATE_POINTER_TEST(). With this approach, fifo macro could be

#define VMSTATE_FIFO8(_field, _state) \
VMSTATE_STRUCT(_field, _state, 0, vmstate_fifo8, Fifo8)


>
> And maybe this should go to vmstate.h header
>
> I disagree. All other clients of VMS_STRUCT are out in their repective
> device specific headers (pci.h, i2c.h) etc. Unless this is new
> established policy, I dont really want to change the current adopted
> approach.

Yeah, looks like you're right.

> Regards,
> Peter
>
>>
>>> +
>>> +#endif /* FIFO_H */
>>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API.
  2012-08-07  6:28       ` Igor Mitsyanko
@ 2012-08-07  6:42         ` Peter Crosthwaite
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-07  6:42 UTC (permalink / raw)
  To: Igor Mitsyanko
  Cc: peter.maydell, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Tue, Aug 7, 2012 at 4:28 PM, Igor Mitsyanko <i.mitsyanko@samsung.com> wrote:
> On 08/07/2012 10:10 AM, Peter Crosthwaite wrote:
>>>>
>>>> +
>>>> +extern const VMStateDescription vmstate_fifo8;
>>>> +
>>>> +#define VMSTATE_FIFO8(_field, _state) {                              \
>>>> +    .name       = (stringify(_field)),                               \
>>>> +    .size       = sizeof(Fifo8),                                     \
>>>> +    .vmsd       = &vmstate_fifo8,                                    \
>>>> +    .flags      = VMS_STRUCT,                                        \
>>>> +    .offset     = vmstate_offset_value(_state, _field, Fifo8),       \
>>>> +}
>>>
>>>
>>> how about implementing this as a wrapper to VMSTATE_STRUCT_TEST() macro
>>> instead?
>>
>> This has no existing precedent in QEMU so I am unsure of what you mean?
>
>
> I meant VMSTATE_TIMER_TEST() in vmstate.h as an example, which is a wrapper
> to VMSTATE_POINTER_TEST(). With this approach, fifo macro could be
>
> #define VMSTATE_FIFO8(_field, _state) \
> VMSTATE_STRUCT(_field, _state, 0, vmstate_fifo8, Fifo8)
>

Yeh just greppin around it looks like this is functionally equivalent.
VMSTATE_IDE_BUS looks like a reasonable precedent for that. Any
opinions one way or the other RE with my original approach (based on
I2C) vs Igors shortened version?

Regards,
Peter

>
>
>>
>> And maybe this should go to vmstate.h header
>>
>> I disagree. All other clients of VMS_STRUCT are out in their repective
>> device specific headers (pci.h, i2c.h) etc. Unless this is new
>> established policy, I dont really want to change the current adopted
>> approach.
>
>
> Yeah, looks like you're right.
>
>
>> Regards,
>> Peter
>>
>>>
>>>> +
>>>> +#endif /* FIFO_H */
>>>
>>>
>

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls
  2012-08-07  0:12     ` Peter Crosthwaite
@ 2012-08-07  8:03       ` Peter Maydell
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2012-08-07  8:03 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Anthony Liguori, i.mitsyanko, stefanha, qemu-devel, paul,
	edgar.iglesias, Andreas Färber, john.williams

On 7 August 2012 01:12, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> On Mon, Aug 6, 2012 at 7:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 6 August 2012 03:16, Peter A. G. Crosthwaite
>> <peter.crosthwaite@petalogix.com> wrote:
>>> +        qemu_irq *all_irqs = g_new(qemu_irq, n + dev->num_gpio_in);
>>> +        memcpy(all_irqs, dev->gpio_in, sizeof(*all_irqs) * dev->num_gpio_in);
>>> +        g_free(dev->gpio_in);
>>> +        memcpy(&all_irqs[dev->num_gpio_in], new_irqs, sizeof(*all_irqs) * n),
>>> +        g_free(new_irqs);
>>
>> I think this is rather looking into private details of what qemu_allocate_irqs
>> does, and it would be better to define a new qemu_reallocate_irqs() in irq.c
>> so we can use it here. (when doing so, consider whether g_renew() might help
>> in producing nice looking code)
>
> My understanding is Anthony is doing major refactoring in the GPIO
> area anyways. Long term, will this code in qdev.c/irq.c even going to
> exist? In this patch, I took something of a path of least resistance
> to just make this series work, as I suspect this patch in its current
> form will be short lived due to continued QOM development.

Yes, long term this will all disappear, but I wouldn't hold your breath
(and indeed it's because I don't think the Pin reworking will hit before
next release that I think this patch makes sense at all). But it will
be around for a fair while, which is why the code should be in the
right place. It's only adding a five or ten line function to irq.c.

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub
  2012-08-06  9:13   ` Peter Maydell
  2012-08-06  9:15     ` Peter Maydell
@ 2012-08-10  4:19     ` Peter Crosthwaite
  1 sibling, 0 replies; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-10  4:19 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Anthony Liguori, i.mitsyanko, Juan Quintela, stefanha,
	qemu-devel, paul, edgar.iglesias, john.williams

On Mon, 2012-08-06 at 10:13 +0100, Peter Maydell wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
> > Added VMSD stub for SSI slaves. Fields may be added to this VMSD for generic
> > SSI slave state (e.g. the CS line state).
> 
> This is more me being confused about how this should work than a
> review comment, but it seems a bit odd that we have a hierarchy
> Device->SSI->ADS7846[etc], where the VMState for the ADS7846
> includes a field for the SSI VMState, but the SSI VMState doesn't
> include a field for the Device VMState.
> 
> What you've done here matches how i2c works currently, but I've
> just cc'd Anthony and Juan to check whether there's a better way
> of doing it.
> 

So VMSD is handled by casting to TYPE_DEVICE and populating dc->vmsd.
This is only going to work for one layer of heirachy. But, I notice a
few device models here and there just call vmstate_register() directly.
Question is, can we call vmstate_register() from the object init
function for SSI_SLAVE, seperately? This would mean that a single
TYPE_DEVICE object is going to get two vmsds instead of one. The
original, and one for SSI_SLAVE (which has the cs state). Sound legit?
Any implementation details and pitfalls to be aware of?

Would be nice to get rid of this change pattern applied to all the
existing SSI devices.

Regards,
Peter

> 
> > Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> > ---
> >  hw/ads7846.c   |    1 +
> >  hw/max111x.c   |    1 +
> >  hw/spitz.c     |    2 ++
> >  hw/ssi.c       |   10 ++++++++++
> >  hw/ssi.h       |   10 ++++++++++
> >  hw/stellaris.c |    1 +
> >  hw/z2.c        |    1 +
> >  7 files changed, 26 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/ads7846.c b/hw/ads7846.c
> > index 41c7f10..6a523f6 100644
> > --- a/hw/ads7846.c
> > +++ b/hw/ads7846.c
> > @@ -124,6 +124,7 @@ static const VMStateDescription vmstate_ads7846 = {
> >      .minimum_version_id_old = 0,
> >      .post_load = ads7856_post_load,
> >      .fields      = (VMStateField[]) {
> > +        VMSTATE_SSI_SLAVE(ssidev, ADS7846State),
> >          VMSTATE_INT32_ARRAY(input, ADS7846State, 8),
> >          VMSTATE_INT32(noise, ADS7846State),
> >          VMSTATE_INT32(cycle, ADS7846State),
> > diff --git a/hw/max111x.c b/hw/max111x.c
> > index 706d89f..948fd97 100644
> > --- a/hw/max111x.c
> > +++ b/hw/max111x.c
> > @@ -103,6 +103,7 @@ static const VMStateDescription vmstate_max111x = {
> >      .minimum_version_id = 0,
> >      .minimum_version_id_old = 0,
> >      .fields      = (VMStateField[]) {
> > +        VMSTATE_SSI_SLAVE(ssidev, MAX111xState),
> >          VMSTATE_UINT8(tb1, MAX111xState),
> >          VMSTATE_UINT8(rb2, MAX111xState),
> >          VMSTATE_UINT8(rb3, MAX111xState),
> > diff --git a/hw/spitz.c b/hw/spitz.c
> > index 20e7835..f5d502d 100644
> > --- a/hw/spitz.c
> > +++ b/hw/spitz.c
> > @@ -1087,6 +1087,7 @@ static const VMStateDescription vmstate_corgi_ssp_regs = {
> >      .minimum_version_id = 1,
> >      .minimum_version_id_old = 1,
> >      .fields = (VMStateField []) {
> > +        VMSTATE_SSI_SLAVE(ssidev, CorgiSSPState),
> >          VMSTATE_UINT32_ARRAY(enable, CorgiSSPState, 3),
> >          VMSTATE_END_OF_LIST(),
> >      }
> > @@ -1115,6 +1116,7 @@ static const VMStateDescription vmstate_spitz_lcdtg_regs = {
> >      .minimum_version_id = 1,
> >      .minimum_version_id_old = 1,
> >      .fields = (VMStateField []) {
> > +        VMSTATE_SSI_SLAVE(ssidev, SpitzLCDTG),
> >          VMSTATE_UINT32(bl_intensity, SpitzLCDTG),
> >          VMSTATE_UINT32(bl_power, SpitzLCDTG),
> >          VMSTATE_END_OF_LIST(),
> > diff --git a/hw/ssi.c b/hw/ssi.c
> > index 35d0a04..2db88fc 100644
> > --- a/hw/ssi.c
> > +++ b/hw/ssi.c
> > @@ -80,6 +80,16 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
> >      return r;
> >  }
> >
> > +const VMStateDescription vmstate_ssi_slave = {
> > +    .name = "SSISlave",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .minimum_version_id_old = 1,
> > +    .fields      = (VMStateField[]) {
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  static void ssi_slave_register_types(void)
> >  {
> >      type_register_static(&ssi_bus_info);
> > diff --git a/hw/ssi.h b/hw/ssi.h
> > index 06657d7..975f9fb 100644
> > --- a/hw/ssi.h
> > +++ b/hw/ssi.h
> > @@ -38,6 +38,16 @@ struct SSISlave {
> >  #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
> >  #define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)
> >
> > +extern const VMStateDescription vmstate_ssi_slave;
> > +
> > +#define VMSTATE_SSI_SLAVE(_field, _state) {                          \
> > +    .name       = (stringify(_field)),                               \
> > +    .size       = sizeof(SSISlave),                                  \
> > +    .vmsd       = &vmstate_ssi_slave,                                \
> > +    .flags      = VMS_STRUCT,                                        \
> > +    .offset     = vmstate_offset_value(_state, _field, SSISlave),    \
> > +}
> > +
> >  DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
> >
> >  /* Master interface.  */
> > diff --git a/hw/stellaris.c b/hw/stellaris.c
> > index 562fbbf..4d26857 100644
> > --- a/hw/stellaris.c
> > +++ b/hw/stellaris.c
> > @@ -1188,6 +1188,7 @@ static const VMStateDescription vmstate_stellaris_ssi_bus = {
> >      .minimum_version_id = 1,
> >      .minimum_version_id_old = 1,
> >      .fields      = (VMStateField[]) {
> > +        VMSTATE_SSI_SLAVE(ssidev, stellaris_ssi_bus_state),
> >          VMSTATE_INT32(current_dev, stellaris_ssi_bus_state),
> >          VMSTATE_END_OF_LIST()
> >      }
> > diff --git a/hw/z2.c b/hw/z2.c
> > index 289cee9..721aaf1 100644
> > --- a/hw/z2.c
> > +++ b/hw/z2.c
> > @@ -165,6 +165,7 @@ static VMStateDescription vmstate_zipit_lcd_state = {
> >      .minimum_version_id = 1,
> >      .minimum_version_id_old = 1,
> >      .fields = (VMStateField[]) {
> > +        VMSTATE_SSI_SLAVE(ssidev, ZipitLCD),
> >          VMSTATE_INT32(selected, ZipitLCD),
> >          VMSTATE_INT32(enabled, ZipitLCD),
> >          VMSTATE_BUFFER(buf, ZipitLCD),
> > --
> > 1.7.0.4
> >
> 
> 
> -- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
  2012-08-06  9:41   ` Peter Maydell
@ 2012-08-10 23:31     ` Peter Crosthwaite
  2012-08-11 19:01       ` Peter Maydell
  0 siblings, 1 reply; 39+ messages in thread
From: Peter Crosthwaite @ 2012-08-10 23:31 UTC (permalink / raw)
  To: Peter Maydell
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On Mon, Aug 6, 2012 at 7:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 03:16, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> To be more consistent with the newer ways of error signalling. That and SIGABT
>> is easier to debug with than exit(1).
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>

This is independent of the rest of the series, do we want to en-queue
to arm-devs and ill remove from series? One less diff for me and one
less spam for the mailing list :)

Regards,
Peter

> -- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.
  2012-08-10 23:31     ` Peter Crosthwaite
@ 2012-08-11 19:01       ` Peter Maydell
  0 siblings, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2012-08-11 19:01 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: i.mitsyanko, stefanha, qemu-devel, paul, edgar.iglesias, john.williams

On 11 August 2012 00:31, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> On Mon, Aug 6, 2012 at 7:41 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 6 August 2012 03:16, Peter A. G. Crosthwaite
>> <peter.crosthwaite@petalogix.com> wrote:
>>> To be more consistent with the newer ways of error signalling. That and SIGABT
>>> is easier to debug with than exit(1).
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>
>
> This is independent of the rest of the series, do we want to en-queue
> to arm-devs and ill remove from series? One less diff for me and one
> less spam for the mailing list :)

Might as well. Applied to arm-devs.next.

-- PMM

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2012-08-11 19:01 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-06  2:16 [Qemu-devel] [PATCH v5 00/15] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 01/15] ssi: Support for multiple attached devices Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 02/15] ssi: Added VMSD stub Peter A. G. Crosthwaite
2012-08-06  9:13   ` Peter Maydell
2012-08-06  9:15     ` Peter Maydell
2012-08-10  4:19     ` Peter Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 03/15] ssi: Implemented CS behaviour Peter A. G. Crosthwaite
2012-08-06  9:25   ` Peter Maydell
2012-08-07  5:17     ` Peter Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 04/15] ssi: Added create_slave_no_init() Peter A. G. Crosthwaite
2012-08-06  9:29   ` Peter Maydell
2012-08-07  0:04     ` Peter Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 05/15] qdev: allow multiple qdev_init_gpio_in() calls Peter A. G. Crosthwaite
2012-08-06  9:38   ` Peter Maydell
2012-08-07  0:12     ` Peter Crosthwaite
2012-08-07  8:03       ` Peter Maydell
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 06/15] hw/stellaris: Removed gpio_out init array Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 07/15] stellaris: Removed SSI mux Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error Peter A. G. Crosthwaite
2012-08-06  9:41   ` Peter Maydell
2012-08-10 23:31     ` Peter Crosthwaite
2012-08-11 19:01       ` Peter Maydell
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 09/15] hw: Added generic FIFO API Peter A. G. Crosthwaite
2012-08-06  9:40   ` Igor Mitsyanko
2012-08-06  9:48   ` Peter Maydell
2012-08-06 12:42     ` Igor Mitsyanko
2012-08-07  6:05     ` Peter Crosthwaite
2012-08-06  9:52   ` Igor Mitsyanko
2012-08-07  6:10     ` Peter Crosthwaite
2012-08-07  6:28       ` Igor Mitsyanko
2012-08-07  6:42         ` Peter Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 10/15] m25p80: Initial implementation of SPI flash device Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 11/15] xilinx_spi: Initial impl. of Xilinx SPI controller Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 12/15] petalogix-ml605: added SPI controller with n25q128 Peter A. G. Crosthwaite
2012-08-06  9:50   ` Peter Maydell
2012-08-07  5:24     ` Peter Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 13/15] xilinx_spips: Xilinx Zynq SPI cntrlr device model Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 14/15] xilinx_zynq: Added SPI controllers + flashes Peter A. G. Crosthwaite
2012-08-06  2:16 ` [Qemu-devel] [PATCH v5 15/15] MAINTAINERS: Added maintainerships for SSI Peter A. G. Crosthwaite

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.