All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller
@ 2012-03-30  6:37 Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
                   ` (3 more replies)
  0 siblings, 4 replies; 23+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-03-30  6:37 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias; +Cc: peter.crosthwaite, john.williams

Add support for Serial Peripheral interface (SPI) as a proper bus standard. Includes an example device (m25p80 SPI flash), an example controller (Xilinx XPS SPI) and adds it to all to a machine model (petalogix_ml605_mmu.c).

Patch 1 adds the Serial Peripheral Interface (SPI) protocol as a bus and defines a QOM type for slave devices. The approach to doing this is based looseley on the existing I2C QOMification.

Patch 2 is a device model for the m25p80 style SPI flash chip.

Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that instantiates a spi bus, and interfaces the two (as per the controllers functionality)

Patch 4 instantiates the XPS SPI controller in the petalogix ML605 reference platform and connects one m25p80 to it.

Peter A. G. Crosthwaite (4):
  SPI: initial support
  m25p80: initial verion
  xilinx_spi: initial version
  petalogix-ml605: added spi controller with m25p80

 Makefile.target          |    3 +
 hw/m25p80.c              |  495 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/petalogix_ml605_mmu.c |   19 ++
 hw/spi.c                 |  175 ++++++++++++++++
 hw/spi.h                 |   86 ++++++++
 hw/xilinx_spi.c          |  477 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 1255 insertions(+), 0 deletions(-)
 create mode 100644 hw/m25p80.c
 create mode 100644 hw/spi.c
 create mode 100644 hw/spi.h
 create mode 100644 hw/xilinx_spi.c

-- 
1.7.3.2

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

* [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-03-30  6:37 [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller Peter A. G. Crosthwaite
@ 2012-03-30  6:37 ` Peter A. G. Crosthwaite
  2012-03-30  7:37   ` Peter Maydell
  2012-04-02 17:39   ` Peter Maydell
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion Peter A. G. Crosthwaite
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 23+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-03-30  6:37 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias; +Cc: peter.crosthwaite, john.williams

defined spi bus and spi slave QOM interfaces. Inspired by and loosley based on
existing I2C framework.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 Makefile.target |    1 +
 hw/spi.c        |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/spi.h        |   86 +++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 hw/spi.c
 create mode 100644 hw/spi.h

diff --git a/Makefile.target b/Makefile.target
index 44b2e83..8fd3718 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -320,6 +320,7 @@ obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o mips_fulong2e.o
 obj-microblaze-y = petalogix_s3adsp1800_mmu.o
 obj-microblaze-y += petalogix_ml605_mmu.o
 obj-microblaze-y += microblaze_boot.o
+obj-microblaze-y += spi.o
 
 obj-microblaze-y += microblaze_pic_cpu.o
 obj-microblaze-y += xilinx_intc.o
diff --git a/hw/spi.c b/hw/spi.c
new file mode 100644
index 0000000..3afef07
--- /dev/null
+++ b/hw/spi.c
@@ -0,0 +1,175 @@
+/*
+ * QEMU SPI bus interface.
+ *
+ * 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 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 "spi.h"
+
+static struct BusInfo spi_bus_info = {
+    .name = "SPI",
+    .size = sizeof(spi_bus)
+};
+
+static const VMStateDescription vmstate_spi_bus = {
+    .name = "spi_bus",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT8(cur_slave, spi_bus),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name)
+{
+    spi_bus *bus;
+
+    bus = FROM_QBUS(spi_bus, qbus_create(&spi_bus_info, parent, name));
+    if (num_slaves >= SPI_BUS_NO_CS) {
+        hw_error("too many slaves on spi bus: %d\n", num_slaves);
+    }
+    bus->num_slaves = num_slaves;
+    bus->slaves = g_malloc0(sizeof(*bus->slaves) * num_slaves);
+    vmstate_register(NULL, -1, &vmstate_spi_bus, bus);
+    return bus;
+}
+
+int spi_attach_slave(spi_bus *bus, SPISlave *slave, int cs)
+{
+    if (bus->slaves[cs]) {
+        return 1;
+    }
+    bus->slaves[cs] = slave;
+    return 0;
+}
+
+int spi_set_cs(spi_bus *bus, int cs)
+{
+    SPISlave *dev;
+    SPISlaveClass *klass;
+
+    if (bus->cur_slave == cs) {
+        return 0;
+    }
+
+    if (bus->cur_slave != SPI_BUS_NO_CS) {
+        dev = bus->slaves[bus->cur_slave];
+        dev->cs = 0;
+        klass = SPI_SLAVE_GET_CLASS(dev);
+        klass->cs(dev, 0);
+    }
+
+    if (cs >= bus->num_slaves && cs != SPI_BUS_NO_CS) {
+        hw_error("attempted to assert non existent spi CS line: %d\n", cs);
+    }
+
+    bus->cur_slave = (uint8_t)cs;
+
+    if (cs != SPI_BUS_NO_CS) {
+        dev = bus->slaves[cs];
+        dev->cs = 1;
+        klass = SPI_SLAVE_GET_CLASS(dev);
+        klass->cs(dev, 1);
+    }
+    return 0;
+};
+
+int spi_get_cs(spi_bus *bus)
+{
+    return bus->cur_slave;
+}
+
+SpiSlaveState spi_get_state(spi_bus *bus)
+{
+    SPISlave *dev;
+    SPISlaveClass *klass;
+
+    if (bus->cur_slave == SPI_BUS_NO_CS) {
+        return SPI_NO_CS;
+    }
+    dev = bus->slaves[bus->cur_slave];
+    klass = SPI_SLAVE_GET_CLASS(dev);
+
+    return klass->get_state(dev);
+}
+
+SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len)
+{
+    SPISlave *dev;
+    SPISlaveClass *klass;
+
+    if (bus->cur_slave == SPI_BUS_NO_CS) {
+        return SPI_NO_CS;
+    }
+    dev = bus->slaves[bus->cur_slave];
+    klass = SPI_SLAVE_GET_CLASS(dev);
+
+    return klass->send(dev, data, len);
+}
+
+SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data)
+{
+    SPISlave *dev;
+    SPISlaveClass *klass;
+
+    if (bus->cur_slave == SPI_BUS_NO_CS) {
+        return SPI_NO_CS;
+    }
+    dev = bus->slaves[bus->cur_slave];
+    klass = SPI_SLAVE_GET_CLASS(dev);
+
+    return klass->recv(dev, data);
+}
+
+const VMStateDescription vmstate_spi_slave = {
+    .name = "SPISlave",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT8(cs, SPISlave),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int spi_slave_qdev_init(DeviceState *dev)
+{
+    SPISlave *s = SPI_SLAVE_FROM_QDEV(dev);
+    SPISlaveClass *sc = SPI_SLAVE_GET_CLASS(s);
+
+    return sc->init(s);
+}
+
+static void spi_slave_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *k = DEVICE_CLASS(klass);
+    k->init = spi_slave_qdev_init;
+    k->bus_info = &spi_bus_info;
+}
+
+static TypeInfo spi_slave_type_info = {
+    .name = TYPE_SPI_SLAVE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(SPISlave),
+    .abstract = true,
+    .class_size = sizeof(SPISlaveClass),
+    .class_init = spi_slave_class_init,
+};
+
+static void spi_slave_register_types(void)
+{
+    type_register_static(&spi_slave_type_info);
+}
+
+type_init(spi_slave_register_types)
diff --git a/hw/spi.h b/hw/spi.h
new file mode 100644
index 0000000..668e9b0
--- /dev/null
+++ b/hw/spi.h
@@ -0,0 +1,86 @@
+#ifndef QEMU_SPI_H
+#define QEMU_SPI_H
+
+#include "qdev.h"
+
+/* pass to spi_set_cs to deslect all devices on bus */
+
+#define SPI_BUS_NO_CS 0xFF
+
+/* state of a SPI device,
+ * SPI_NO_CS -> the CS pin in de-asserted -> device is tristated
+ * SPI_IDLE -> CS is asserted and device ready to recv
+ * SPI_DATA_PENDING -> CS is asserted and the device has pushed data to master
+ */
+
+typedef enum {
+    SPI_NO_CS,
+    SPI_IDLE,
+    SPI_DATA_PENDING
+} SpiSlaveState;
+
+typedef struct SPISlave {
+    DeviceState qdev;
+    uint8_t cs;
+} SPISlave;
+
+#define TYPE_SPI_SLAVE "spi-slave"
+#define SPI_SLAVE(obj) \
+     OBJECT_CHECK(SPISlave, (obj), TYPE_SPI_SLAVE)
+#define SPI_SLAVE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(SPISlaveClass, (klass), TYPE_SPI_SLAVE)
+#define SPI_SLAVE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(SPISlaveClass, (obj), TYPE_SPI_SLAVE)
+
+typedef struct SPISlaveClass {
+    DeviceClass parent_class;
+
+    /* Callbacks provided by the device.  */
+    int (*init)(SPISlave *s);
+
+    /* change the cs pin state */
+    void (*cs)(SPISlave *s, uint8_t select);
+
+    /* Master to slave.  */
+    SpiSlaveState (*send)(SPISlave *s, uint32_t data, int len);
+
+    /* Slave to master.  */
+    SpiSlaveState (*recv)(SPISlave *s, uint32_t *data);
+
+    /* poll the spi device state */
+    SpiSlaveState (*get_state)(SPISlave *s);
+} SPISlaveClass;
+
+#define SPI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SPISlave, qdev, dev)
+#define FROM_SPI_SLAVE(type, dev) DO_UPCAST(type, spi, dev)
+
+extern const VMStateDescription vmstate_spi_slave;
+
+#define VMSTATE_SPI_SLAVE(_field, _state) {                          \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(SPISlave),                                  \
+    .vmsd       = &vmstate_spi_slave,                                \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, SPISlave),    \
+}
+
+typedef struct spi_bus {
+    BusState qbus;
+    SPISlave **slaves;
+    uint8_t num_slaves;
+    uint8_t cur_slave;
+} spi_bus;
+
+/* create a new spi bus */
+spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name);
+int spi_attach_slave(spi_bus *bus, SPISlave *s, int cs);
+
+/* change the chip select. Return 1 on failure. */
+int spi_set_cs(spi_bus *bus, int cs);
+int spi_get_cs(spi_bus *bus);
+SpiSlaveState spi_get_state(spi_bus *bus);
+
+SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len);
+SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data);
+
+#endif
-- 
1.7.3.2

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

* [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
  2012-03-30  6:37 [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
@ 2012-03-30  6:37 ` Peter A. G. Crosthwaite
  2012-04-03  7:03   ` Stefan Hajnoczi
  2012-04-04 12:53   ` Andreas Färber
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 3/4] xilinx_spi: initial version Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 4/4] petalogix-ml605: added spi controller with m25p80 Peter A. G. Crosthwaite
  3 siblings, 2 replies; 23+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-03-30  6:37 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias; +Cc: peter.crosthwaite, john.williams

Added device model for m25p80 SPI flash

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

diff --git a/Makefile.target b/Makefile.target
index 8fd3718..fcccf1b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -321,6 +321,7 @@ obj-microblaze-y = petalogix_s3adsp1800_mmu.o
 obj-microblaze-y += petalogix_ml605_mmu.o
 obj-microblaze-y += microblaze_boot.o
 obj-microblaze-y += spi.o
+obj-microblaze-y += m25p80.o
 
 obj-microblaze-y += microblaze_pic_cpu.o
 obj-microblaze-y += xilinx_intc.o
diff --git a/hw/m25p80.c b/hw/m25p80.c
new file mode 100644
index 0000000..2b67375
--- /dev/null
+++ b/hw/m25p80.c
@@ -0,0 +1,495 @@
+/*
+ * ST M25P80 emulator.
+ *
+ * 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) version 3 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 "spi.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
+
+enum FlashCMD {
+    NOP = 0,
+    PP = 0x2,
+    READ = 0x3,
+    WRDI = 0x4,
+    RDSR = 0x5,
+    WREN = 0x6,
+    FAST_READ = 0xb,
+    SECTOR_ERASE = 0x20,
+    BLOCK_ERASE32 = 0x52,
+    JEDEC_READ = 0x9f,
+    CHIP_ERASE = 0xc7,
+};
+
+enum CMDState {
+    STATE_IDLE,
+    STATE_PAGE_PROGRAM,
+    STATE_SECTOR_ERASE,
+    STATE_BLOCK_ERASE32,
+    STATE_READ,
+    STATE_FAST_READ,
+    STATE_COLLECTING_DATA,
+    STATE_READING_DATA,
+};
+
+struct flash {
+    SPISlave spi;
+    SpiSlaveState spi_state;
+    uint32_t r;
+
+    BlockDriverState *bdrv;
+    enum CMDState state;
+
+    uint8_t *storage;
+    uint64_t size;
+    int pagesize;
+    int sectorsize;
+    int blocksize;
+
+    uint8_t data[16];
+    int len;
+    int pos;
+    int wrap_read;
+    int needed_bytes;
+    int next_state;
+
+    int64_t dirty_page;
+
+    uint64_t waddr;
+    int write_enable;
+};
+
+static void flash_sync_page(struct flash *s, int page)
+{
+    if (s->bdrv) {
+        int bdrv_sector;
+        int offset;
+
+        bdrv_sector = (page * s->pagesize) / 512;
+        offset = bdrv_sector * 512;
+        bdrv_write(s->bdrv, bdrv_sector,
+                   s->storage + offset, (s->pagesize + 511) / 512);
+    }
+}
+
+static inline void flash_sync_area(struct flash *s, int64_t off, int64_t len)
+{
+    int64_t start, end;
+
+    if (!s->bdrv) {
+        return;
+    }
+
+    start = off / 512;
+    end = (off + len) / 512;
+    bdrv_write(s->bdrv, start, s->storage + (start * 512), end - start);
+}
+
+static void flash_sector_erase(struct flash *s, int sector)
+{
+    if (!s->write_enable) {
+        DB_PRINT("write with write protect!\n");
+    }
+    memset(s->storage + sector, 0xff, s->sectorsize);
+    flash_sync_area(s, sector, s->sectorsize);
+}
+
+static void flash_block_erase32k(struct flash *s, int addr)
+{
+    if (!s->write_enable) {
+        DB_PRINT("write with write protect!\n");
+    }
+    memset(s->storage + addr, 0xff, 32 * 1024);
+    flash_sync_area(s, addr, 32 * 1024);
+}
+
+static void flash_chip_erase(struct flash *s)
+{
+    if (!s->write_enable) {
+        DB_PRINT("write with write protect!\n");
+    }
+    memset(s->storage, 0xff, s->size);
+    flash_sync_area(s, 0, s->size);
+}
+
+static inline void flash_sync_dirty(struct 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(struct flash *s, uint64_t addr, uint8_t data)
+{
+    int64_t page = addr / s->pagesize;
+    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);
+    }
+    s->storage[s->waddr] ^= ~data & s->storage[s->waddr];
+
+    flash_sync_dirty(s, page);
+    s->dirty_page = page;
+}
+
+static int decode_new_cmd(struct flash *s, uint32_t value)
+{
+    int state = STATE_IDLE;
+    int val;
+
+    switch (value) {
+    case PP:
+        s->needed_bytes = 3;
+        s->pos = 0; s->len = 0;
+        state = STATE_COLLECTING_DATA;
+        s->next_state = STATE_PAGE_PROGRAM;
+        break;
+    case READ:
+        s->needed_bytes = 2;
+        s->pos = 0; s->len = 0;
+        state = STATE_COLLECTING_DATA;
+        s->next_state = STATE_READ;
+        break;
+    case FAST_READ:
+        /* Dummy byte must return data!  */
+        s->needed_bytes = 3;
+        s->pos = 0; s->len = 0;
+        state = STATE_COLLECTING_DATA;
+        s->next_state = STATE_FAST_READ;
+        break;
+    case WRDI:
+        s->write_enable = 0;
+        break;
+    case WREN:
+        s->write_enable = 1;
+        break;
+
+    case RDSR:
+        val = (!!s->write_enable) << 1;
+
+        s->data[0] = val;
+        s->pos = 0; s->len = 1; s->wrap_read = 0;
+        state = STATE_READING_DATA;
+        break;
+
+    case JEDEC_READ:
+        s->data[0] = 0xef;
+        s->data[1] = 0x40;
+        s->data[2] = 0x17;
+        s->pos = 0;
+        s->len = 3;
+        s->wrap_read = 0;
+        state = STATE_READING_DATA;
+        break;
+
+    case SECTOR_ERASE:
+        s->needed_bytes = 2;
+        s->pos = 0; s->len = 0;
+        state = STATE_COLLECTING_DATA;
+        s->next_state = STATE_SECTOR_ERASE;
+        break;
+
+    case BLOCK_ERASE32:
+        s->needed_bytes = 2;
+        s->pos = 0; s->len = 0;
+        state = STATE_COLLECTING_DATA;
+        s->next_state = STATE_BLOCK_ERASE32;
+        break;
+
+    case CHIP_ERASE:
+        if (s->write_enable) {
+            DB_PRINT("chip erase\n");
+            flash_chip_erase(s);
+        } else {
+            DB_PRINT("chip erase with write protect!\n");
+        }
+        break;
+    case NOP:
+        break;
+    default:
+        DB_PRINT("Unknown cmd %x\n", value);
+        break;
+    }
+
+    return state;
+}
+
+static void sector_erase(struct flash *s, uint32_t data)
+{
+    int sector;
+
+    if (s->next_state == STATE_SECTOR_ERASE) {
+        /* Transition just happened, pick up the address.  */
+        sector = s->data[0] << 16;
+        sector |= s->data[1] << 8;
+        sector |= data;
+        s->next_state = STATE_IDLE;
+    }
+    DB_PRINT("sector_erase sector=%d\n", sector);
+    flash_sector_erase(s, sector);
+}
+
+static void block_erase32(struct flash *s, uint32_t data)
+{
+    int addr;
+
+    if (s->next_state == STATE_BLOCK_ERASE32) {
+        /* Transition just happened, pick up the address.  */
+        addr = s->data[0] << 16;
+        addr |= s->data[1] << 8;
+        addr |= data;
+        s->next_state = STATE_IDLE;
+    }
+    DB_PRINT("block_erase addr=%08x\n", addr);
+    flash_block_erase32k(s, addr);
+}
+
+static void page_program(struct flash *s, uint32_t data)
+{
+    if (s->next_state == STATE_PAGE_PROGRAM) {
+        /* Transition just happened, pick up the address.  */
+        s->waddr = s->data[0] << 16;
+        s->waddr |= s->data[1] << 8;
+        s->waddr |= s->data[2];
+        s->next_state = STATE_IDLE;
+    }
+    DB_PRINT("page program waddr=%lx data=%x\n", s->waddr, data);
+    flash_write8(s, s->waddr, data);
+    s->waddr++;
+}
+
+/* FIXME: these two functions are near identical, merge */
+
+static void state_read(struct flash *s, uint32_t data)
+{
+    int addr;
+
+    if (s->next_state == STATE_READ) {
+        /* Transition just happened, pick up the address.  */
+        s->waddr = s->data[0] << 16;
+        s->waddr |= s->data[1] << 8;
+        s->waddr |= data;
+        s->next_state = STATE_IDLE;
+        s->spi_state = SPI_IDLE;
+    }
+    addr = s->waddr;
+    s->r = s->storage[addr];
+    DB_PRINT("READ 0x%lx.0x%x=%x\n", s->waddr, addr, s->r);
+    addr += 1;
+    if (addr >= s->size) {
+        addr = 0;
+    }
+    s->waddr = addr;
+    s->spi_state = SPI_DATA_PENDING;
+}
+
+static void state_fast_read(struct flash *s, uint32_t data)
+{
+    int addr;
+
+    if (s->next_state == STATE_FAST_READ) {
+        /* Transition just happened, pick up the address.  */
+        s->waddr = s->data[0] << 16;
+        s->waddr |= s->data[1] << 8;
+        s->waddr |= s->data[2];
+        if (s->data[3] != 0) {
+            hw_error("Fast read dummy byte=%x\n", s->data[3]);
+        }
+        s->next_state = STATE_IDLE;
+        s->spi_state = SPI_IDLE;
+    }
+
+    addr = s->waddr;
+    s->r = s->storage[addr];
+    DB_PRINT("FAST-READ 0x%lx.0x%x=%x\n", s->waddr, addr, s->r);
+    addr += 1;
+    if (addr >= s->size) {
+        addr = 0;
+    }
+    s->waddr = addr;
+    s->spi_state = SPI_DATA_PENDING;
+}
+
+static void m25p80_cs(SPISlave *ss, uint8_t select)
+{
+    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
+
+    if (!select) {
+        s->len = 0;
+        s->pos = 0;
+        s->state = STATE_IDLE;
+        s->next_state = STATE_IDLE;
+        flash_sync_dirty(s, -1);
+        DB_PRINT("deselect\n");
+        s->spi_state = SPI_NO_CS;
+    } else {
+        s->spi_state = SPI_IDLE;
+    }
+}
+
+static SpiSlaveState m25p80_send(SPISlave *ss, uint32_t value, int len)
+{
+    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
+
+    switch (s->state) {
+    case STATE_PAGE_PROGRAM:
+        page_program(s, value);
+        break;
+
+    case STATE_READ:
+        state_read(s, value);
+        break;
+
+    case STATE_FAST_READ:
+        state_fast_read(s, value);
+        break;
+
+    case STATE_SECTOR_ERASE:
+        sector_erase(s, value);
+        break;
+
+    case STATE_BLOCK_ERASE32:
+        block_erase32(s, value);
+        break;
+
+    case STATE_COLLECTING_DATA:
+        if (len > 0) {
+            s->data[s->len] = value;
+            s->len++;
+
+            if (s->len == s->needed_bytes) {
+                s->state = s->next_state;
+            }
+        }
+        break;
+
+    case STATE_READING_DATA:
+        s->r = s->data[s->pos];
+        s->spi_state = SPI_DATA_PENDING;
+        s->pos++;
+        if (s->pos == s->len) {
+            s->pos = 0;
+            if (!s->wrap_read) {
+                s->state = STATE_IDLE;
+            }
+        }
+        break;
+
+    default:
+    case STATE_IDLE:
+        if (len > 0) {
+            s->state = decode_new_cmd(s, value);
+        }
+        break;
+    }
+    return s->spi_state;
+}
+
+static SpiSlaveState m25p80_recv(SPISlave *ss, uint32_t *data)
+{
+    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
+
+    *data = s->r;
+    s->spi_state = SPI_IDLE;
+    return SPI_IDLE;
+}
+
+static SpiSlaveState m25p80_get_state(SPISlave *ss)
+{
+    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
+
+    return s->spi_state;
+}
+
+static int m25p80_init(SPISlave *ss)
+{
+    DriveInfo *dinfo;
+    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
+    /* FIXME: This should be handled centrally!  */
+    static int mtdblock_idx;
+    dinfo = drive_get(IF_MTD, 0, mtdblock_idx++);
+
+    DB_PRINT("inited m25p80 device model - dinfo = %p\n", dinfo);
+    /* TODO: parameterize */
+    s->size = 8 * 1024 * 1024;
+    s->pagesize = 256;
+    s->sectorsize = 4 * 1024;
+    s->dirty_page = -1;
+    s->storage = g_malloc0(s->size);
+
+    if (dinfo && dinfo->bdrv) {
+        int rsize;
+
+        s->bdrv = dinfo->bdrv;
+        rsize = MIN(bdrv_getlength(s->bdrv), s->size);
+        if (bdrv_read(s->bdrv, 0, s->storage, (s->size + 511) / 512)) {
+            fprintf(stderr, "Failed to initialize SPI flash!\n");
+            return 1;
+        }
+    } else {
+        s->write_enable = 1;
+        flash_chip_erase(s);
+        s->write_enable = 0;
+    }
+
+    return 0;
+}
+
+static void m25p80_class_init(ObjectClass *klass, void *data)
+{
+    SPISlaveClass *k = SPI_SLAVE_CLASS(klass);
+
+    k->init = m25p80_init;
+    k->send = m25p80_send;
+    k->recv = m25p80_recv;
+    k->cs = m25p80_cs;
+    k->get_state = m25p80_get_state;
+}
+
+static TypeInfo m25p80_info = {
+    .name           = "m25p80",
+    .parent         = TYPE_SPI_SLAVE,
+    .instance_size  = sizeof(struct flash),
+    .class_init     = m25p80_class_init,
+};
+
+static void m25p80_register_types(void)
+{
+    type_register_static(&m25p80_info);
+}
+
+type_init(m25p80_register_types)
-- 
1.7.3.2

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

* [Qemu-devel] [RFC PATCH v1 3/4] xilinx_spi: initial version
  2012-03-30  6:37 [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion Peter A. G. Crosthwaite
@ 2012-03-30  6:37 ` Peter A. G. Crosthwaite
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 4/4] petalogix-ml605: added spi controller with m25p80 Peter A. G. Crosthwaite
  3 siblings, 0 replies; 23+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-03-30  6:37 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias; +Cc: peter.crosthwaite, john.williams

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

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

diff --git a/Makefile.target b/Makefile.target
index fcccf1b..15edfa4 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -322,6 +322,7 @@ obj-microblaze-y += petalogix_ml605_mmu.o
 obj-microblaze-y += microblaze_boot.o
 obj-microblaze-y += spi.o
 obj-microblaze-y += m25p80.o
+obj-microblaze-y += xilinx_spi.o
 
 obj-microblaze-y += microblaze_pic_cpu.o
 obj-microblaze-y += xilinx_intc.o
diff --git a/hw/xilinx_spi.c b/hw/xilinx_spi.c
new file mode 100644
index 0000000..a5c8aa8
--- /dev/null
+++ b/hw/xilinx_spi.c
@@ -0,0 +1,477 @@
+/*
+ * 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 "ptimer.h"
+#include "qemu-log.h"
+
+#include "spi.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)
+
+struct XilinxSPI {
+    SysBusDevice busdev;
+    MemoryRegion mmio;
+    qemu_irq irq;
+    int irqline;
+
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+
+    spi_bus *spi;
+
+    uint32_t c_fifo_exist;
+
+    uint8_t rx_fifo[256];
+    unsigned int rx_fifo_pos;
+    unsigned int rx_fifo_len;
+
+    uint8_t tx_fifo[256];
+    unsigned int tx_fifo_pos;
+    unsigned int tx_fifo_len;
+
+    /* Slave select.  */
+    uint8_t num_cs;
+    int cmd_ongoing;
+
+    uint32_t regs[R_MAX];
+};
+
+static void txfifo_reset(struct XilinxSPI *s)
+{
+    s->tx_fifo_pos = 0;
+    s->tx_fifo_len = 0;
+
+    s->regs[R_SPISR] &= ~SR_TX_FULL;
+    s->regs[R_SPISR] |= SR_TX_EMPTY;
+    s->regs[R_SPISR] &= ~SR_TX_FULL;
+    s->regs[R_IPISR] |= IRQ_DTR_EMPTY;
+}
+
+static void rxfifo_reset(struct XilinxSPI *s)
+{
+    s->rx_fifo_pos = 0;
+    s->rx_fifo_len = 0;
+
+    s->regs[R_SPISR] |= SR_RX_EMPTY;
+    s->regs[R_SPISR] &= ~SR_RX_FULL;
+    s->regs[R_IPISR] &= ~IRQ_DRR_NOT_EMPTY;
+    s->regs[R_IPISR] &= ~IRQ_DRR_OVERRUN;
+}
+
+static void xlx_spi_reset(struct XilinxSPI *s)
+{
+    memset(s->regs, 0, sizeof s->regs);
+
+    rxfifo_reset(s);
+    txfifo_reset(s);
+
+    s->regs[R_SPISSR] = 1;
+    spi_set_cs(s->spi, 0);
+}
+
+static void xlx_spi_update_irq(struct XilinxSPI *s)
+{
+    uint32_t pending;
+    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 dont call the
+       irq chain unless things really changed.  */
+    if (pending != s->irqline) {
+        s->irqline = pending;
+        qemu_set_irq(s->irq, pending);
+    }
+}
+
+static inline int spi_master_enabled(struct XilinxSPI *s)
+{
+    return !(s->regs[R_SPICR] & R_SPICR_MTI);
+}
+
+static int spi_slave_select(struct XilinxSPI *s, uint32_t v)
+{
+    unsigned int ss;
+
+    ss = ffs(v) - 1;
+    return ss < s->num_cs ? ss : SPI_BUS_NO_CS;
+}
+
+static inline int txfifo_empty(struct XilinxSPI *s)
+{
+    return s->tx_fifo_len == 0;
+}
+
+static inline int txfifo_full(struct XilinxSPI *s)
+{
+    return s->tx_fifo_len >= ARRAY_SIZE(s->tx_fifo);
+}
+
+static inline int rxfifo_empty(struct XilinxSPI *s)
+{
+    return s->rx_fifo_len == 0;
+}
+
+static inline int rxfifo_full(struct XilinxSPI *s)
+{
+    return s->rx_fifo_len >= ARRAY_SIZE(s->rx_fifo);
+}
+
+static inline void txfifo_put(struct XilinxSPI *s, uint8_t v)
+{
+    s->regs[R_SPISR] &= ~SR_TX_EMPTY;
+    s->regs[R_IPISR] &= ~IRQ_DTR_EMPTY;
+
+    s->tx_fifo[s->tx_fifo_pos] = v;
+    s->tx_fifo_pos++;
+    s->tx_fifo_pos &= ARRAY_SIZE(s->tx_fifo) - 1;
+    s->tx_fifo_len++;
+
+    s->regs[R_SPISR] &= ~SR_TX_FULL;
+    if (txfifo_full(s)) {
+        s->regs[R_SPISR] |= SR_TX_FULL;
+    }
+}
+
+static inline uint32_t txfifo_get(struct XilinxSPI *s)
+{
+    uint32_t r = 0;
+    assert(s->tx_fifo_len);
+
+    r = s->tx_fifo[(s->tx_fifo_pos - s->tx_fifo_len) &
+                                (ARRAY_SIZE(s->tx_fifo) - 1)];
+    s->tx_fifo_len--;
+
+    s->regs[R_SPISR] &= ~SR_TX_FULL;
+    if (txfifo_empty(s)) {
+        s->regs[R_SPISR] |= SR_TX_EMPTY;
+        s->regs[R_IPISR] |= IRQ_DTR_EMPTY;
+    }
+
+    return r;
+}
+
+static inline void rxfifo_put(struct XilinxSPI *s, uint8_t v)
+{
+    DB_PRINT("%x\n", v);
+    s->regs[R_SPISR] &= ~SR_RX_EMPTY;
+    s->regs[R_IPISR] |= IRQ_DRR_NOT_EMPTY;
+
+    s->rx_fifo[s->rx_fifo_pos] = v;
+    s->rx_fifo_pos++;
+    s->rx_fifo_pos &= ARRAY_SIZE(s->rx_fifo) - 1;
+    s->rx_fifo_len++;
+
+    s->regs[R_SPISR] &= ~SR_RX_FULL;
+    if (s->rx_fifo_len >= ARRAY_SIZE(s->rx_fifo)) {
+        s->regs[R_SPISR] |= SR_RX_FULL;
+        s->regs[R_IPISR] |= IRQ_DRR_OVERRUN;
+    }
+}
+
+static inline uint32_t rxfifo_get(struct XilinxSPI *s)
+{
+    uint32_t r = 0;
+    assert(s->rx_fifo_len);
+
+    r = s->rx_fifo[(s->rx_fifo_pos - s->rx_fifo_len) &
+                            (ARRAY_SIZE(s->rx_fifo) - 1)];
+    s->rx_fifo_len--;
+
+    s->regs[R_SPISR] &= ~SR_RX_FULL;
+    if (rxfifo_empty(s)) {
+        s->regs[R_SPISR] |= SR_RX_EMPTY;
+        s->regs[R_IPISR] &= ~IRQ_DRR_NOT_EMPTY;
+    }
+
+    return r;
+}
+
+static void spi_timer_run(struct XilinxSPI *s, int delay)
+{
+    ptimer_set_count(s->ptimer, delay);
+    ptimer_run(s->ptimer, 1);
+}
+
+static void
+spi_flush_txfifo(struct XilinxSPI *s)
+{
+    uint32_t value;
+    SpiSlaveState st;
+
+    while (!txfifo_empty(s)) {
+        value = txfifo_get(s);
+        DB_PRINT("data transfer:%x\n", value);
+        st = spi_send(s->spi, value, 8);
+        while (st == SPI_DATA_PENDING) {
+            uint32_t d;
+            st = spi_recv(s->spi, &d);
+            rxfifo_put(s, d);
+        }
+    }
+}
+
+static uint64_t
+spi_read(void *opaque, target_phys_addr_t addr, unsigned int size)
+{
+    struct XilinxSPI *s = opaque;
+    uint32_t r = 0;
+
+    addr >>= 2;
+    switch (addr) {
+    case R_SPIDRR:
+        if (rxfifo_empty(s)) {
+            DB_PRINT("Read from empty FIFO!\n");
+            return 0xdeadbeef;
+        }
+
+        r = rxfifo_get(s);
+        break;
+
+    case R_SPISR:
+        r = s->regs[addr];
+        if (rxfifo_empty(s)) {
+            spi_timer_run(s, 1);
+        }
+        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)
+{
+    struct 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_reset(s);
+        }
+        break;
+
+    case R_SPIDTR:
+        txfifo_put(s, value);
+
+        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:
+        spi_set_cs(s->spi, spi_slave_select(s, ~value));
+        s->regs[addr] = value;
+        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)) {
+            /*
+             * The linux driver, when issuing READS to a flash memory,
+             * first sets up the tx part, starts the transmition and
+             * waits for the tx part to end. After that it sets up
+             * the rx part, assumeing the CPU is faster than the flash.
+             *
+             * If we simply flush the txfifo and consume all rx data
+             * from the flash, we'll hit a race in the linux driver and
+             * some rx data will be arrive to the linux driver too early.
+             *
+             * That's why we implement this hack. If we are at the start
+             * of a new cmd (i.e s->cmd_ongoing == 0), then we delay
+             * the processing a bit, to give linux time to set things
+             * up.
+             *
+             * FIXME: The mentione hack doesn't work. Investigate why.
+             */
+            if (0 && s->cmd_ongoing) {
+                spi_flush_txfifo(s);
+            } else {
+                /* When releasing the master disable, initiate a timer
+                   that eventually will flush the txfifo.  */
+                spi_timer_run(s, 1);
+            }
+        }
+        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 void timer_hit(void *opaque)
+{
+    struct XilinxSPI *s = opaque;
+
+    if (!txfifo_empty(s)) {
+        spi_flush_txfifo(s);
+        s->cmd_ongoing = 1;
+    }
+    xlx_spi_update_irq(s);
+}
+
+static int xilinx_spi_init(SysBusDevice *dev)
+{
+    struct XilinxSPI *s = FROM_SYSBUS(typeof(*s), dev);
+
+    DB_PRINT("\n");
+    sysbus_init_irq(dev, &s->irq);
+
+    memory_region_init_io(&s->mmio, &spi_ops, s, "xilinx-spi", R_MAX * 4);
+    sysbus_init_mmio(dev, &s->mmio);
+
+    s->bh = qemu_bh_new(timer_hit, s);
+    s->ptimer = ptimer_init(s->bh);
+    ptimer_set_freq(s->ptimer, 10 * 1000 * 1000);
+
+    s->spi = spi_init_bus(&dev->qdev, s->num_cs, "spi");
+
+    xlx_spi_reset(s);
+    return 0;
+}
+
+static Property xilinx_spi_properties[] = {
+    DEFINE_PROP_UINT8("num-cs", struct 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->props = xilinx_spi_properties;
+}
+
+static TypeInfo xilinx_spi_info = {
+    .name           = "xilinx,spi",
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(struct 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.3.2

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

* [Qemu-devel] [RFC PATCH v1 4/4] petalogix-ml605: added spi controller with m25p80
  2012-03-30  6:37 [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller Peter A. G. Crosthwaite
                   ` (2 preceding siblings ...)
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 3/4] xilinx_spi: initial version Peter A. G. Crosthwaite
@ 2012-03-30  6:37 ` Peter A. G. Crosthwaite
  3 siblings, 0 replies; 23+ messages in thread
From: Peter A. G. Crosthwaite @ 2012-03-30  6:37 UTC (permalink / raw)
  To: qemu-devel, paul, edgar.iglesias; +Cc: peter.crosthwaite, john.williams

Added spi controller to the reference design, with a single cs line and a
m25p80 style spi-flash connected

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

diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index 31a4348..b2359a8 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 "spi.h"
 
 #include "microblaze_boot.h"
 #include "microblaze_pic_cpu.h"
@@ -75,6 +76,7 @@ petalogix_ml605_init(ram_addr_t ram_size,
 {
     MemoryRegion *address_space_mem = get_system_memory();
     DeviceState *dev;
+    SysBusDevice *busdev;
     CPUMBState *env;
     DriveInfo *dinfo;
     int i;
@@ -131,6 +133,23 @@ petalogix_ml605_init(ram_addr_t ram_size,
                                      irq[1], irq[0], 100 * 1000000);
     }
 
+    {
+        spi_bus *spi;
+
+        dev = qdev_create(NULL, "xilinx,spi");
+        qdev_prop_set_uint8(dev, "num-cs", 1);
+        qdev_init_nofail(dev);
+        busdev = sysbus_from_qdev(dev);
+        sysbus_mmio_map(busdev, 0, 0x40a00000);
+        sysbus_connect_irq(busdev, 0, irq[6]);
+
+        spi = FROM_QBUS(spi_bus, qdev_get_child_bus(dev, "spi"));
+
+        dev = qdev_create(NULL, "m25p80");
+        qdev_init_nofail(dev);
+        spi_attach_slave(spi, SPI_SLAVE(dev), 0);
+    }
+
     microblaze_load_kernel(env, ddr_base, ram_size, BINARY_DEVICE_TREE_FILE,
                                                             machine_cpu_reset);
 
-- 
1.7.3.2

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
@ 2012-03-30  7:37   ` Peter Maydell
  2012-03-30  7:50     ` Peter Crosthwaite
  2012-04-02 17:39   ` Peter Maydell
  1 sibling, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2012-03-30  7:37 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

On 30 March 2012 07:37, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> defined spi bus and spi slave QOM interfaces. Inspired by and loosley based on
> existing I2C framework.

There's also SPI support in qemu-linaro thanks to the folks at Nokia:
http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=blob;f=hw/spi.h

I guess we need to look at both and figure out which is the better
implementation...

-- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-03-30  7:37   ` Peter Maydell
@ 2012-03-30  7:50     ` Peter Crosthwaite
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Crosthwaite @ 2012-03-30  7:50 UTC (permalink / raw)
  To: Peter Maydell; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

Hi Peter,

One major flaw in that interface (which is similar to another that we
originally used) is it doesnt have an API for wiggling the CS lines.
Some spi devices (e.g. the m25p80 in this series) have side effects
from cs strobing whether or not a txrx occurs during cs assertion,
which is not possible in that api. Thats the main difference between
the two implementations.

Regards,
Peter


On Fri, Mar 30, 2012 at 5:37 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 March 2012 07:37, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> defined spi bus and spi slave QOM interfaces. Inspired by and loosley based on
>> existing I2C framework.
>
> There's also SPI support in qemu-linaro thanks to the folks at Nokia:
> http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=blob;f=hw/spi.h
>
> I guess we need to look at both and figure out which is the better
> implementation...
>
> -- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
  2012-03-30  7:37   ` Peter Maydell
@ 2012-04-02 17:39   ` Peter Maydell
  2012-04-02 23:51     ` Peter Crosthwaite
  2012-04-02 23:57     ` Peter Crosthwaite
  1 sibling, 2 replies; 23+ messages in thread
From: Peter Maydell @ 2012-04-02 17:39 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

On 30 March 2012 07:37, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> defined spi bus and spi slave QOM interfaces. Inspired by and loosley based on

"Define"; "loosely".

> existing I2C framework.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  Makefile.target |    1 +
>  hw/spi.c        |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/spi.h        |   86 +++++++++++++++++++++++++++
>  3 files changed, 262 insertions(+), 0 deletions(-)
>  create mode 100644 hw/spi.c
>  create mode 100644 hw/spi.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 44b2e83..8fd3718 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -320,6 +320,7 @@ obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o mips_fulong2e.o
>  obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>  obj-microblaze-y += petalogix_ml605_mmu.o
>  obj-microblaze-y += microblaze_boot.o
> +obj-microblaze-y += spi.o

This should be in common-obj-y with i2c.o, I think -- microblaze isn't
the only target with SPI (eg hw/omap_spi.c).

>  obj-microblaze-y += microblaze_pic_cpu.o
>  obj-microblaze-y += xilinx_intc.o
> diff --git a/hw/spi.c b/hw/spi.c
> new file mode 100644
> index 0000000..3afef07
> --- /dev/null
> +++ b/hw/spi.c
> @@ -0,0 +1,175 @@
> +/*
> + * QEMU SPI bus interface.
> + *
> + * 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 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 "spi.h"
> +
> +static struct BusInfo spi_bus_info = {
> +    .name = "SPI",
> +    .size = sizeof(spi_bus)
> +};
> +
> +static const VMStateDescription vmstate_spi_bus = {
> +    .name = "spi_bus",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT8(cur_slave, spi_bus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name)
> +{
> +    spi_bus *bus;
> +
> +    bus = FROM_QBUS(spi_bus, qbus_create(&spi_bus_info, parent, name));
> +    if (num_slaves >= SPI_BUS_NO_CS) {
> +        hw_error("too many slaves on spi bus: %d\n", num_slaves);
> +    }
> +    bus->num_slaves = num_slaves;
> +    bus->slaves = g_malloc0(sizeof(*bus->slaves) * num_slaves);
> +    vmstate_register(NULL, -1, &vmstate_spi_bus, bus);
> +    return bus;
> +}
> +
> +int spi_attach_slave(spi_bus *bus, SPISlave *slave, int cs)
> +{
> +    if (bus->slaves[cs]) {
> +        return 1;
> +    }
> +    bus->slaves[cs] = slave;
> +    return 0;
> +}
> +
> +int spi_set_cs(spi_bus *bus, int cs)
> +{
> +    SPISlave *dev;
> +    SPISlaveClass *klass;
> +
> +    if (bus->cur_slave == cs) {
> +        return 0;
> +    }
> +
> +    if (bus->cur_slave != SPI_BUS_NO_CS) {
> +        dev = bus->slaves[bus->cur_slave];
> +        dev->cs = 0;
> +        klass = SPI_SLAVE_GET_CLASS(dev);
> +        klass->cs(dev, 0);
> +    }
> +
> +    if (cs >= bus->num_slaves && cs != SPI_BUS_NO_CS) {
> +        hw_error("attempted to assert non existent spi CS line: %d\n", cs);
> +    }
> +
> +    bus->cur_slave = (uint8_t)cs;
> +
> +    if (cs != SPI_BUS_NO_CS) {
> +        dev = bus->slaves[cs];
> +        dev->cs = 1;
> +        klass = SPI_SLAVE_GET_CLASS(dev);
> +        klass->cs(dev, 1);
> +    }
> +    return 0;
> +};
> +
> +int spi_get_cs(spi_bus *bus)
> +{
> +    return bus->cur_slave;
> +}
> +
> +SpiSlaveState spi_get_state(spi_bus *bus)
> +{
> +    SPISlave *dev;
> +    SPISlaveClass *klass;
> +
> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
> +        return SPI_NO_CS;
> +    }
> +    dev = bus->slaves[bus->cur_slave];
> +    klass = SPI_SLAVE_GET_CLASS(dev);
> +
> +    return klass->get_state(dev);
> +}
> +
> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len)
> +{
> +    SPISlave *dev;
> +    SPISlaveClass *klass;
> +
> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
> +        return SPI_NO_CS;
> +    }
> +    dev = bus->slaves[bus->cur_slave];
> +    klass = SPI_SLAVE_GET_CLASS(dev);
> +
> +    return klass->send(dev, data, len);
> +}
> +
> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data)
> +{
> +    SPISlave *dev;
> +    SPISlaveClass *klass;
> +
> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
> +        return SPI_NO_CS;
> +    }
> +    dev = bus->slaves[bus->cur_slave];
> +    klass = SPI_SLAVE_GET_CLASS(dev);
> +
> +    return klass->recv(dev, data);
> +}
> +
> +const VMStateDescription vmstate_spi_slave = {
> +    .name = "SPISlave",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT8(cs, SPISlave),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static int spi_slave_qdev_init(DeviceState *dev)
> +{
> +    SPISlave *s = SPI_SLAVE_FROM_QDEV(dev);
> +    SPISlaveClass *sc = SPI_SLAVE_GET_CLASS(s);
> +
> +    return sc->init(s);
> +}
> +
> +static void spi_slave_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *k = DEVICE_CLASS(klass);
> +    k->init = spi_slave_qdev_init;
> +    k->bus_info = &spi_bus_info;
> +}
> +
> +static TypeInfo spi_slave_type_info = {
> +    .name = TYPE_SPI_SLAVE,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(SPISlave),
> +    .abstract = true,
> +    .class_size = sizeof(SPISlaveClass),
> +    .class_init = spi_slave_class_init,
> +};
> +
> +static void spi_slave_register_types(void)
> +{
> +    type_register_static(&spi_slave_type_info);
> +}
> +
> +type_init(spi_slave_register_types)
> diff --git a/hw/spi.h b/hw/spi.h
> new file mode 100644
> index 0000000..668e9b0
> --- /dev/null
> +++ b/hw/spi.h
> @@ -0,0 +1,86 @@
> +#ifndef QEMU_SPI_H
> +#define QEMU_SPI_H
> +
> +#include "qdev.h"
> +
> +/* pass to spi_set_cs to deslect all devices on bus */

"deselect"

> +
> +#define SPI_BUS_NO_CS 0xFF
> +
> +/* state of a SPI device,
> + * SPI_NO_CS -> the CS pin in de-asserted -> device is tristated

"is"

> + * SPI_IDLE -> CS is asserted and device ready to recv
> + * SPI_DATA_PENDING -> CS is asserted and the device has pushed data to master
> + */
> +
> +typedef enum {
> +    SPI_NO_CS,
> +    SPI_IDLE,
> +    SPI_DATA_PENDING
> +} SpiSlaveState;

Why not SPISlaveState ?

> +
> +typedef struct SPISlave {
> +    DeviceState qdev;
> +    uint8_t cs;
> +} SPISlave;
> +
> +#define TYPE_SPI_SLAVE "spi-slave"
> +#define SPI_SLAVE(obj) \
> +     OBJECT_CHECK(SPISlave, (obj), TYPE_SPI_SLAVE)
> +#define SPI_SLAVE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(SPISlaveClass, (klass), TYPE_SPI_SLAVE)
> +#define SPI_SLAVE_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(SPISlaveClass, (obj), TYPE_SPI_SLAVE)
> +
> +typedef struct SPISlaveClass {
> +    DeviceClass parent_class;
> +
> +    /* Callbacks provided by the device.  */
> +    int (*init)(SPISlave *s);
> +
> +    /* change the cs pin state */
> +    void (*cs)(SPISlave *s, uint8_t select);
> +
> +    /* Master to slave.  */
> +    SpiSlaveState (*send)(SPISlave *s, uint32_t data, int len);
> +
> +    /* Slave to master.  */
> +    SpiSlaveState (*recv)(SPISlave *s, uint32_t *data);
> +
> +    /* poll the spi device state */
> +    SpiSlaveState (*get_state)(SPISlave *s);
> +} SPISlaveClass;
> +
> +#define SPI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SPISlave, qdev, dev)
> +#define FROM_SPI_SLAVE(type, dev) DO_UPCAST(type, spi, dev)
> +
> +extern const VMStateDescription vmstate_spi_slave;
> +
> +#define VMSTATE_SPI_SLAVE(_field, _state) {                          \
> +    .name       = (stringify(_field)),                               \
> +    .size       = sizeof(SPISlave),                                  \
> +    .vmsd       = &vmstate_spi_slave,                                \
> +    .flags      = VMS_STRUCT,                                        \
> +    .offset     = vmstate_offset_value(_state, _field, SPISlave),    \
> +}
> +
> +typedef struct spi_bus {
> +    BusState qbus;
> +    SPISlave **slaves;
> +    uint8_t num_slaves;
> +    uint8_t cur_slave;
> +} spi_bus;

CODING_STYLE demands camelcase for type names, so SPIBus.

> +
> +/* create a new spi bus */
> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name);
> +int spi_attach_slave(spi_bus *bus, SPISlave *s, int cs);
> +
> +/* change the chip select. Return 1 on failure. */
> +int spi_set_cs(spi_bus *bus, int cs);
> +int spi_get_cs(spi_bus *bus);
> +SpiSlaveState spi_get_state(spi_bus *bus);
> +
> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len);
> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data);

I'm no SPI expert, but a bit of googling suggests that it's
a synchronous duplex bus, so you always send a byte of data
to the slave and get one back in return (even if for some slaves
it might be random garbage). The current OMAP SPI devices in QEMU
master have an API that reflects this: eg tsc210x_txrx() which
takes an input value and returns an output value. This API
seems to have separate send and recv methods -- can you explain
how this works?

(Incidentally if we're going to qdevify the SPI interface we
should also convert the existing omap SPI devices...)

thanks
-- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-02 17:39   ` Peter Maydell
@ 2012-04-02 23:51     ` Peter Crosthwaite
  2012-04-03  0:48       ` Peter Crosthwaite
  2012-04-03 17:45       ` Paul Brook
  2012-04-02 23:57     ` Peter Crosthwaite
  1 sibling, 2 replies; 23+ messages in thread
From: Peter Crosthwaite @ 2012-04-02 23:51 UTC (permalink / raw)
  To: Peter Maydell; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

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

>> +
>> +/* create a new spi bus */
>> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name);
>> +int spi_attach_slave(spi_bus *bus, SPISlave *s, int cs);
>> +
>> +/* change the chip select. Return 1 on failure. */
>> +int spi_set_cs(spi_bus *bus, int cs);
>> +int spi_get_cs(spi_bus *bus);
>> +SpiSlaveState spi_get_state(spi_bus *bus);
>> +
>> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len);
>> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data);
>
> I'm no SPI expert, but a bit of googling suggests that it's
> a synchronous duplex bus, so you always send a byte of data
> to the slave and get one back in return (even if for some slaves
> it might be random garbage).

Hi Peter,

Thats not correct, SPI txs can trigger rxs of arbitrary length. Heres
the data sheet for the m25p80 spi flash device:
www.datasheetcatalog.org/datasheet/stmicroelectronics/8495.pdf.
Figure 12, page 15 is a good example of this, the master transmits 4
bytes back to back for a opcode and an addresses, and the slave bursts
the entire playload back. To hack around this, Edgar had to put escape
sequences in that txrx api to flag all the txs and rxs for devices and
controllers respecitvely, ive attached a file (xilinx_spi.h) which is
Eds original hack of the txrx interface that im trying to fix here by
changing the API.

This api allows controllers to manage that without dummy txs or invalid rxs.

The current OMAP SPI devices in QEMU
> master have an API that reflects this: eg tsc210x_txrx() which
> takes an input value and returns an output value. This API
> seems to have separate send and recv methods -- can you explain
> how this works?
>

Each send or receive command will return a enum SPISlaveState. If
SPI_DATA_PENDING the master can receive a byte back from the slave
with recv. If that recv returns SPI_DATA_PENDING then the master
should recieve again i.e. recv should be looped. Heres how its handled
in the xilinx_spi controller:

    uint32_t value;
    SpiSlaveState st;

    while (!txfifo_empty(s)) {
        value = txfifo_get(s);
        DB_PRINT("data transfer:%x\n", value);
        st = spi_send(s->spi, value, 8);
        while (st == SPI_DATA_PENDING) {
            uint32_t d;
            st = spi_recv(s->spi, &d);
            rxfifo_put(s, d);
        }
    }

The inner loop is basically a poll, waiting for the recv to return !=
SPI_DATA_PENDING.

> (Incidentally if we're going to qdevify the SPI interface we
> should also convert the existing omap SPI devices...)

Yes I havent looked into the details, but shouldnt be too much work. I
can provide the diff for the m25p80 to change it over from txrx style
to this, to give an idea of the change pattern. Little bit of this
that and the other in the machine models too.

If we really want to minimise work, the old txrx function can be added
(to spi.c) as something like:

uint32_t spi_txrx(spi_bus *bus, uint32_t data) {
      uint32_t ret;

      assert(spi_send(bus, data) == SPI_DATA_PENDING);
      assert(spi_recv(bus, &ret) == SPI_DATA_IDLE);
      return ret;
}

>
> thanks
> -- PMM

Regards,
Peter

[-- Attachment #2: xilinx_spi.h --]
[-- Type: text/x-chdr, Size: 1022 bytes --]

/* SPI connection.  */

#define SPI_LEN_UNSELECT -1  /* Used to simulate CS.  */
#define SPI_LEN_NODATA    0  /* Used to read data without writing.  */

#define SPI_DATA_EMPTY (1 << 31)
#define SPI_DATA_PENDING (1 << 30)

static inline int xlx_spi_valid_data(uint32_t data)
{
    return !(data & SPI_DATA_EMPTY);
}

static inline int xlx_spi_data_pending(uint32_t data)
{
    return data & SPI_DATA_PENDING;
}

typedef uint32_t (*spi_push_fn)(void *opaque, uint32_t data, int len);

struct xlx_spi_ch
{
    void *slave[32];
    spi_push_fn txrx[32];
};

struct xlx_spi_device
{
    void * (*init) (void);
    spi_push_fn txrx;
    const char * device_name;
};

static inline void xlx_spi_connect_slave(struct xlx_spi_ch *spich, int nr,
                                          void *slave, spi_push_fn f)
{
    spich->slave[nr] = slave;
    spich->txrx[nr] = f;
}

static inline
uint32_t xlx_spi_txrx(struct xlx_spi_ch *spich, int nr, uint32_t data, int len)
{
    return spich->txrx[nr](spich->slave[nr], data, len);
}


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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-02 17:39   ` Peter Maydell
  2012-04-02 23:51     ` Peter Crosthwaite
@ 2012-04-02 23:57     ` Peter Crosthwaite
  2012-04-03  0:27       ` Andreas Färber
  1 sibling, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2012-04-02 23:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

On Tue, Apr 3, 2012 at 3:39 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 30 March 2012 07:37, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> defined spi bus and spi slave QOM interfaces. Inspired by and loosley based on
>
> "Define"; "loosely".
>

ack

>> existing I2C framework.
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  Makefile.target |    1 +
>>  hw/spi.c        |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/spi.h        |   86 +++++++++++++++++++++++++++
>>  3 files changed, 262 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/spi.c
>>  create mode 100644 hw/spi.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 44b2e83..8fd3718 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -320,6 +320,7 @@ obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o mips_fulong2e.o
>>  obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>>  obj-microblaze-y += petalogix_ml605_mmu.o
>>  obj-microblaze-y += microblaze_boot.o
>> +obj-microblaze-y += spi.o
>
> This should be in common-obj-y with i2c.o, I think -- microblaze isn't
> the only target with SPI (eg hw/omap_spi.c).
>

ok

>>  obj-microblaze-y += microblaze_pic_cpu.o
>>  obj-microblaze-y += xilinx_intc.o
>> diff --git a/hw/spi.c b/hw/spi.c
>> new file mode 100644
>> index 0000000..3afef07
>> --- /dev/null
>> +++ b/hw/spi.c
>> @@ -0,0 +1,175 @@
>> +/*
>> + * QEMU SPI bus interface.
>> + *
>> + * 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 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 "spi.h"
>> +
>> +static struct BusInfo spi_bus_info = {
>> +    .name = "SPI",
>> +    .size = sizeof(spi_bus)
>> +};
>> +
>> +static const VMStateDescription vmstate_spi_bus = {
>> +    .name = "spi_bus",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields      = (VMStateField[]) {
>> +        VMSTATE_UINT8(cur_slave, spi_bus),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name)
>> +{
>> +    spi_bus *bus;
>> +
>> +    bus = FROM_QBUS(spi_bus, qbus_create(&spi_bus_info, parent, name));
>> +    if (num_slaves >= SPI_BUS_NO_CS) {
>> +        hw_error("too many slaves on spi bus: %d\n", num_slaves);
>> +    }
>> +    bus->num_slaves = num_slaves;
>> +    bus->slaves = g_malloc0(sizeof(*bus->slaves) * num_slaves);
>> +    vmstate_register(NULL, -1, &vmstate_spi_bus, bus);
>> +    return bus;
>> +}
>> +
>> +int spi_attach_slave(spi_bus *bus, SPISlave *slave, int cs)
>> +{
>> +    if (bus->slaves[cs]) {
>> +        return 1;
>> +    }
>> +    bus->slaves[cs] = slave;
>> +    return 0;
>> +}
>> +
>> +int spi_set_cs(spi_bus *bus, int cs)
>> +{
>> +    SPISlave *dev;
>> +    SPISlaveClass *klass;
>> +
>> +    if (bus->cur_slave == cs) {
>> +        return 0;
>> +    }
>> +
>> +    if (bus->cur_slave != SPI_BUS_NO_CS) {
>> +        dev = bus->slaves[bus->cur_slave];
>> +        dev->cs = 0;
>> +        klass = SPI_SLAVE_GET_CLASS(dev);
>> +        klass->cs(dev, 0);
>> +    }
>> +
>> +    if (cs >= bus->num_slaves && cs != SPI_BUS_NO_CS) {
>> +        hw_error("attempted to assert non existent spi CS line: %d\n", cs);
>> +    }
>> +
>> +    bus->cur_slave = (uint8_t)cs;
>> +
>> +    if (cs != SPI_BUS_NO_CS) {
>> +        dev = bus->slaves[cs];
>> +        dev->cs = 1;
>> +        klass = SPI_SLAVE_GET_CLASS(dev);
>> +        klass->cs(dev, 1);
>> +    }
>> +    return 0;
>> +};
>> +
>> +int spi_get_cs(spi_bus *bus)
>> +{
>> +    return bus->cur_slave;
>> +}
>> +
>> +SpiSlaveState spi_get_state(spi_bus *bus)
>> +{
>> +    SPISlave *dev;
>> +    SPISlaveClass *klass;
>> +
>> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
>> +        return SPI_NO_CS;
>> +    }
>> +    dev = bus->slaves[bus->cur_slave];
>> +    klass = SPI_SLAVE_GET_CLASS(dev);
>> +
>> +    return klass->get_state(dev);
>> +}
>> +
>> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len)
>> +{
>> +    SPISlave *dev;
>> +    SPISlaveClass *klass;
>> +
>> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
>> +        return SPI_NO_CS;
>> +    }
>> +    dev = bus->slaves[bus->cur_slave];
>> +    klass = SPI_SLAVE_GET_CLASS(dev);
>> +
>> +    return klass->send(dev, data, len);
>> +}
>> +
>> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data)
>> +{
>> +    SPISlave *dev;
>> +    SPISlaveClass *klass;
>> +
>> +    if (bus->cur_slave == SPI_BUS_NO_CS) {
>> +        return SPI_NO_CS;
>> +    }
>> +    dev = bus->slaves[bus->cur_slave];
>> +    klass = SPI_SLAVE_GET_CLASS(dev);
>> +
>> +    return klass->recv(dev, data);
>> +}
>> +
>> +const VMStateDescription vmstate_spi_slave = {
>> +    .name = "SPISlave",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields      = (VMStateField[]) {
>> +        VMSTATE_UINT8(cs, SPISlave),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static int spi_slave_qdev_init(DeviceState *dev)
>> +{
>> +    SPISlave *s = SPI_SLAVE_FROM_QDEV(dev);
>> +    SPISlaveClass *sc = SPI_SLAVE_GET_CLASS(s);
>> +
>> +    return sc->init(s);
>> +}
>> +
>> +static void spi_slave_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *k = DEVICE_CLASS(klass);
>> +    k->init = spi_slave_qdev_init;
>> +    k->bus_info = &spi_bus_info;
>> +}
>> +
>> +static TypeInfo spi_slave_type_info = {
>> +    .name = TYPE_SPI_SLAVE,
>> +    .parent = TYPE_DEVICE,
>> +    .instance_size = sizeof(SPISlave),
>> +    .abstract = true,
>> +    .class_size = sizeof(SPISlaveClass),
>> +    .class_init = spi_slave_class_init,
>> +};
>> +
>> +static void spi_slave_register_types(void)
>> +{
>> +    type_register_static(&spi_slave_type_info);
>> +}
>> +
>> +type_init(spi_slave_register_types)
>> diff --git a/hw/spi.h b/hw/spi.h
>> new file mode 100644
>> index 0000000..668e9b0
>> --- /dev/null
>> +++ b/hw/spi.h
>> @@ -0,0 +1,86 @@
>> +#ifndef QEMU_SPI_H
>> +#define QEMU_SPI_H
>> +
>> +#include "qdev.h"
>> +
>> +/* pass to spi_set_cs to deslect all devices on bus */
>
> "deselect"
>
>> +
>> +#define SPI_BUS_NO_CS 0xFF
>> +
>> +/* state of a SPI device,
>> + * SPI_NO_CS -> the CS pin in de-asserted -> device is tristated
>
> "is"
>

ack

>> + * SPI_IDLE -> CS is asserted and device ready to recv
>> + * SPI_DATA_PENDING -> CS is asserted and the device has pushed data to master
>> + */
>> +
>> +typedef enum {
>> +    SPI_NO_CS,
>> +    SPI_IDLE,
>> +    SPI_DATA_PENDING
>> +} SpiSlaveState;
>
> Why not SPISlaveState ?
>

ack

>> +
>> +typedef struct SPISlave {
>> +    DeviceState qdev;
>> +    uint8_t cs;
>> +} SPISlave;
>> +
>> +#define TYPE_SPI_SLAVE "spi-slave"
>> +#define SPI_SLAVE(obj) \
>> +     OBJECT_CHECK(SPISlave, (obj), TYPE_SPI_SLAVE)
>> +#define SPI_SLAVE_CLASS(klass) \
>> +     OBJECT_CLASS_CHECK(SPISlaveClass, (klass), TYPE_SPI_SLAVE)
>> +#define SPI_SLAVE_GET_CLASS(obj) \
>> +     OBJECT_GET_CLASS(SPISlaveClass, (obj), TYPE_SPI_SLAVE)
>> +
>> +typedef struct SPISlaveClass {
>> +    DeviceClass parent_class;
>> +
>> +    /* Callbacks provided by the device.  */
>> +    int (*init)(SPISlave *s);
>> +
>> +    /* change the cs pin state */
>> +    void (*cs)(SPISlave *s, uint8_t select);
>> +
>> +    /* Master to slave.  */
>> +    SpiSlaveState (*send)(SPISlave *s, uint32_t data, int len);
>> +
>> +    /* Slave to master.  */
>> +    SpiSlaveState (*recv)(SPISlave *s, uint32_t *data);
>> +
>> +    /* poll the spi device state */
>> +    SpiSlaveState (*get_state)(SPISlave *s);
>> +} SPISlaveClass;
>> +
>> +#define SPI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SPISlave, qdev, dev)
>> +#define FROM_SPI_SLAVE(type, dev) DO_UPCAST(type, spi, dev)
>> +
>> +extern const VMStateDescription vmstate_spi_slave;
>> +
>> +#define VMSTATE_SPI_SLAVE(_field, _state) {                          \
>> +    .name       = (stringify(_field)),                               \
>> +    .size       = sizeof(SPISlave),                                  \
>> +    .vmsd       = &vmstate_spi_slave,                                \
>> +    .flags      = VMS_STRUCT,                                        \
>> +    .offset     = vmstate_offset_value(_state, _field, SPISlave),    \
>> +}
>> +
>> +typedef struct spi_bus {
>> +    BusState qbus;
>> +    SPISlave **slaves;
>> +    uint8_t num_slaves;
>> +    uint8_t cur_slave;
>> +} spi_bus;
>
> CODING_STYLE demands camelcase for type names, so SPIBus.
>

Ok, I have a related question tho, if camel casing with acronyms,
should a space perhaps be inserted after for readability? As in
SPI_Bus rather than SPIBus.

>> +
>> +/* create a new spi bus */
>> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name);
>> +int spi_attach_slave(spi_bus *bus, SPISlave *s, int cs);
>> +
>> +/* change the chip select. Return 1 on failure. */
>> +int spi_set_cs(spi_bus *bus, int cs);
>> +int spi_get_cs(spi_bus *bus);
>> +SpiSlaveState spi_get_state(spi_bus *bus);
>> +
>> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len);
>> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data);
>
> I'm no SPI expert, but a bit of googling suggests that it's
> a synchronous duplex bus, so you always send a byte of data
> to the slave and get one back in return (even if for some slaves
> it might be random garbage). The current OMAP SPI devices in QEMU
> master have an API that reflects this: eg tsc210x_txrx() which
> takes an input value and returns an output value. This API
> seems to have separate send and recv methods -- can you explain
> how this works?
>
> (Incidentally if we're going to qdevify the SPI interface we
> should also convert the existing omap SPI devices...)
>
> thanks
> -- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-02 23:57     ` Peter Crosthwaite
@ 2012-04-03  0:27       ` Andreas Färber
  0 siblings, 0 replies; 23+ messages in thread
From: Andreas Färber @ 2012-04-03  0:27 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Peter Maydell, paul, qemu-devel, john.williams, edgar.iglesias

Am 03.04.2012 01:57, schrieb Peter Crosthwaite:
> On Tue, Apr 3, 2012 at 3:39 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 30 March 2012 07:37, Peter A. G. Crosthwaite
>> <peter.crosthwaite@petalogix.com> wrote:
>>> +typedef struct spi_bus {
>>> +    BusState qbus;
>>> +    SPISlave **slaves;
>>> +    uint8_t num_slaves;
>>> +    uint8_t cur_slave;
>>> +} spi_bus;
>>
>> CODING_STYLE demands camelcase for type names, so SPIBus.
>>
> 
> Ok, I have a related question tho, if camel casing with acronyms,
> should a space perhaps be inserted after for readability? As in
> SPI_Bus rather than SPIBus.

Negative, we have PCIDevice, I2CSlave, etc.

The underscore is used in macro names, such as X86_CPU() vs. X86CPU.
If we allow underscores in type names as word separator then sooner or
later we'll run into the same ugly uppercase name conflicts that caused
the huge'ish CPUState refactoring.

.NET avoids four consecutive uppercase letter by writing, e.g.,
XmlDocument. So far we have rather adopted the older (e.g., Java) model
of not lower-casing acronyms. My preference would be for consistency.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-02 23:51     ` Peter Crosthwaite
@ 2012-04-03  0:48       ` Peter Crosthwaite
  2012-04-03 17:45       ` Paul Brook
  1 sibling, 0 replies; 23+ messages in thread
From: Peter Crosthwaite @ 2012-04-03  0:48 UTC (permalink / raw)
  To: Peter Maydell; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

Hi Peter,

So ive looked into this further, and I think I better see the
confusion between us. My api attempt is attempting to abstract away
the clock wheras the txrx approach is true'er to the physical
interface. This sense of one-sided communication that im trying to
model is comming from the fact that device can drive their SDO line to
tristate, which effiectly means "no-data". To that end, a compromise
between the two API would be to modify the txrx API to allow capturing
of this information:

int spi_txrx(uint8_t *tx, uint8_t *txz, uint8_t *rx, uint8_t *rxz,  int len);

tx and rx and arbitary length bit-buffers for tx and rx data. txz and
rxz and parrell to their two respective buffers, and indicate whether
the line is in tristate, i.e. between the two arrays you can infer
whether the line is 0, 1 or Z. Len then exactly corresponds to the
number of clock edges in the transaction. This would allow
encapsulation of all the desired behaviour with minimal modification
to the txrx interface and no hacky escape return behaviours.

What your think?

Regards
Peter


On Tue, Apr 3, 2012 at 9:51 AM, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
>>> +
>>> +/* create a new spi bus */
>>> +spi_bus *spi_init_bus(DeviceState *parent, int num_slaves, const char *name);
>>> +int spi_attach_slave(spi_bus *bus, SPISlave *s, int cs);
>>> +
>>> +/* change the chip select. Return 1 on failure. */
>>> +int spi_set_cs(spi_bus *bus, int cs);
>>> +int spi_get_cs(spi_bus *bus);
>>> +SpiSlaveState spi_get_state(spi_bus *bus);
>>> +
>>> +SpiSlaveState spi_send(spi_bus *bus, uint32_t data, int len);
>>> +SpiSlaveState spi_recv(spi_bus *bus, uint32_t *data);
>>
>> I'm no SPI expert, but a bit of googling suggests that it's
>> a synchronous duplex bus, so you always send a byte of data
>> to the slave and get one back in return (even if for some slaves
>> it might be random garbage).
>
> Hi Peter,
>
> Thats not correct, SPI txs can trigger rxs of arbitrary length. Heres
> the data sheet for the m25p80 spi flash device:
> www.datasheetcatalog.org/datasheet/stmicroelectronics/8495.pdf.
> Figure 12, page 15 is a good example of this, the master transmits 4
> bytes back to back for a opcode and an addresses, and the slave bursts
> the entire playload back. To hack around this, Edgar had to put escape
> sequences in that txrx api to flag all the txs and rxs for devices and
> controllers respecitvely, ive attached a file (xilinx_spi.h) which is
> Eds original hack of the txrx interface that im trying to fix here by
> changing the API.
>
> This api allows controllers to manage that without dummy txs or invalid rxs.
>
> The current OMAP SPI devices in QEMU
>> master have an API that reflects this: eg tsc210x_txrx() which
>> takes an input value and returns an output value. This API
>> seems to have separate send and recv methods -- can you explain
>> how this works?
>>
>
> Each send or receive command will return a enum SPISlaveState. If
> SPI_DATA_PENDING the master can receive a byte back from the slave
> with recv. If that recv returns SPI_DATA_PENDING then the master
> should recieve again i.e. recv should be looped. Heres how its handled
> in the xilinx_spi controller:
>
>    uint32_t value;
>    SpiSlaveState st;
>
>    while (!txfifo_empty(s)) {
>        value = txfifo_get(s);
>        DB_PRINT("data transfer:%x\n", value);
>        st = spi_send(s->spi, value, 8);
>        while (st == SPI_DATA_PENDING) {
>            uint32_t d;
>            st = spi_recv(s->spi, &d);
>            rxfifo_put(s, d);
>        }
>    }
>
> The inner loop is basically a poll, waiting for the recv to return !=
> SPI_DATA_PENDING.
>
>> (Incidentally if we're going to qdevify the SPI interface we
>> should also convert the existing omap SPI devices...)
>
> Yes I havent looked into the details, but shouldnt be too much work. I
> can provide the diff for the m25p80 to change it over from txrx style
> to this, to give an idea of the change pattern. Little bit of this
> that and the other in the machine models too.
>
> If we really want to minimise work, the old txrx function can be added
> (to spi.c) as something like:
>
> uint32_t spi_txrx(spi_bus *bus, uint32_t data) {
>      uint32_t ret;
>
>      assert(spi_send(bus, data) == SPI_DATA_PENDING);
>      assert(spi_recv(bus, &ret) == SPI_DATA_IDLE);
>      return ret;
> }
>
>>
>> thanks
>> -- PMM
>
> Regards,
> Peter

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

* Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion Peter A. G. Crosthwaite
@ 2012-04-03  7:03   ` Stefan Hajnoczi
  2012-04-04 12:53   ` Andreas Färber
  1 sibling, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2012-04-03  7:03 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

On Fri, Mar 30, 2012 at 04:37:11PM +1000, Peter A. G. Crosthwaite wrote:
> +static void flash_sync_page(struct flash *s, int page)
> +{
> +    if (s->bdrv) {
> +        int bdrv_sector;
> +        int offset;
> +
> +        bdrv_sector = (page * s->pagesize) / 512;
> +        offset = bdrv_sector * 512;
> +        bdrv_write(s->bdrv, bdrv_sector,
> +                   s->storage + offset, (s->pagesize + 511) / 512);

Devices should not use synchronous block I/O interfaces.  sd, flash, and
a couple others still do for historical reasons but new devices should
not.

The vcpu, QEMU monitor, and VNC are all blocked while I/O takes place.
This can be avoided by using bdrv_aio_writev() instead.

Can you change this code to use bdrv_aio_writev() or is this flash
device specified to complete operations within certain time constraints?
(The problem is that the image file could be on a slow harddisk or other
media that don't meet those timing requirements.)

> +static int m25p80_init(SPISlave *ss)
> +{
> +    DriveInfo *dinfo;
> +    struct flash *s = FROM_SPI_SLAVE(struct flash, ss);
> +    /* FIXME: This should be handled centrally!  */
> +    static int mtdblock_idx;
> +    dinfo = drive_get(IF_MTD, 0, mtdblock_idx++);
> +
> +    DB_PRINT("inited m25p80 device model - dinfo = %p\n", dinfo);
> +    /* TODO: parameterize */
> +    s->size = 8 * 1024 * 1024;
> +    s->pagesize = 256;
> +    s->sectorsize = 4 * 1024;
> +    s->dirty_page = -1;
> +    s->storage = g_malloc0(s->size);

Please use qemu_blockalign(s->bdrv, s->size) to allocate I/O buffers.
It honors memory alignment requirements (necessary for O_DIRECT files).

Stefan

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-02 23:51     ` Peter Crosthwaite
  2012-04-03  0:48       ` Peter Crosthwaite
@ 2012-04-03 17:45       ` Paul Brook
  2012-04-03 18:08         ` Peter Maydell
  1 sibling, 1 reply; 23+ messages in thread
From: Paul Brook @ 2012-04-03 17:45 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Peter Maydell, qemu-devel, john.williams, edgar.iglesias

> > I'm no SPI expert, but a bit of googling suggests that it's
> > a synchronous duplex bus, so you always send a byte of data
> > to the slave and get one back in return (even if for some slaves
> > it might be random garbage).

Waitaminute. So this is just basic synchronous serial?

We already have an API for this. hw/spi.c.

If you're building higer level protocols they should be layered on top of the 
SPI, same as smbus is a protocol layer on top of I2C.

Paul

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-03 17:45       ` Paul Brook
@ 2012-04-03 18:08         ` Peter Maydell
  2012-04-03 21:22           ` Paul Brook
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2012-04-03 18:08 UTC (permalink / raw)
  To: Paul Brook; +Cc: Peter Crosthwaite, edgar.iglesias, qemu-devel, john.williams

2012/4/3 Paul Brook <paul@codesourcery.com>:
>> > I'm no SPI expert, but a bit of googling suggests that it's
>> > a synchronous duplex bus, so you always send a byte of data
>> > to the slave and get one back in return (even if for some slaves
>> > it might be random garbage).
>
> Waitaminute. So this is just basic synchronous serial?
>
> We already have an API for this. hw/spi.c.

Er, no we don't, not in upstream QEMU. You'll notice that
the patch we're discussing creates hw/spi.c as a new file...

-- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-03 18:08         ` Peter Maydell
@ 2012-04-03 21:22           ` Paul Brook
  2012-04-04  0:48             ` Peter Crosthwaite
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Brook @ 2012-04-03 21:22 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Peter Crosthwaite, edgar.iglesias, qemu-devel, john.williams

> 2012/4/3 Paul Brook <paul@codesourcery.com>:
> >> > I'm no SPI expert, but a bit of googling suggests that it's
> >> > a synchronous duplex bus, so you always send a byte of data
> >> > to the slave and get one back in return (even if for some slaves
> >> > it might be random garbage).
> > 
> > Waitaminute. So this is just basic synchronous serial?
> > 
> > We already have an API for this. hw/spi.c.
> 
> Er, no we don't, not in upstream QEMU. You'll notice that
> the patch we're discussing creates hw/spi.c as a new file...

Sorry, I mean hw/ssi.c

Synchronous serial is something that pretty much every vendor has their own 
name for, but in practice they're actually all the same.

Paul

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-03 21:22           ` Paul Brook
@ 2012-04-04  0:48             ` Peter Crosthwaite
  2012-04-04 16:52               ` Paul Brook
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2012-04-04  0:48 UTC (permalink / raw)
  To: Paul Brook; +Cc: Peter Maydell, qemu-devel, john.williams, edgar.iglesias

Hi Paul,

Regarding using ssi, theres a few things that come to mind:

Theres no sense of it being a multi-slave bus, its just a point to
point link. SPI devices universally have the notion of the CS pin that
tristates the device of the bus. Masters connect to a number of slaves
and one-hot-decode the active slave. It would be tedious if all SPI
controllers (of which i have two (only pushed one in this series),
omap have theirs, Nokia may have something out of tree too), had to
implement the\is common CS decoding behaviour. To that end, my SPI bus
implementation has some infra-structure for managing this -
spi_set_cs(). The spi_bus in this series is single master,
multi-slave, where as ssi is point-to-point.

Also as mentioned in earlier discussions with Peter, that api has no
way of emulating the CS (sometimes called SS) pin. The m25p80 in this
series (and potenitally other devices) has side effect associated with
wiggling the cs pin, which can not be encapsulated by that ssi slave
interface.

It may be a case though that ssi is a superclass of spi - all SPI
devices are SSI devices but not all SSI devices are SPI? To that end
can we make SPI a child object of SSI with the desired extra behaviors
mentioned in this thread? This kind of stuff is the whole reason for
QOM.

Regards,
Peter

On Wed, Apr 4, 2012 at 7:22 AM, Paul Brook <paul@codesourcery.com> wrote:
>> 2012/4/3 Paul Brook <paul@codesourcery.com>:
>> >> > I'm no SPI expert, but a bit of googling suggests that it's
>> >> > a synchronous duplex bus, so you always send a byte of data
>> >> > to the slave and get one back in return (even if for some slaves
>> >> > it might be random garbage).
>> >
>> > Waitaminute. So this is just basic synchronous serial?
>> >
>> > We already have an API for this. hw/spi.c.
>>
>> Er, no we don't, not in upstream QEMU. You'll notice that
>> the patch we're discussing creates hw/spi.c as a new file...
>
> Sorry, I mean hw/ssi.c
>
> Synchronous serial is something that pretty much every vendor has their own
> name for, but in practice they're actually all the same.
>
> Paul

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

* Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
  2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion Peter A. G. Crosthwaite
  2012-04-03  7:03   ` Stefan Hajnoczi
@ 2012-04-04 12:53   ` Andreas Färber
  2012-06-05  0:47     ` Peter Crosthwaite
  1 sibling, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2012-04-04 12:53 UTC (permalink / raw)
  To: Peter A. G. Crosthwaite; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

Am 30.03.2012 08:37, schrieb Peter A. G. Crosthwaite:
> Added device model for m25p80 SPI flash
> 
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
>  Makefile.target |    1 +
>  hw/m25p80.c     |  495 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 496 insertions(+), 0 deletions(-)
>  create mode 100644 hw/m25p80.c
> 
> diff --git a/Makefile.target b/Makefile.target
> index 8fd3718..fcccf1b 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -321,6 +321,7 @@ obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>  obj-microblaze-y += petalogix_ml605_mmu.o
>  obj-microblaze-y += microblaze_boot.o
>  obj-microblaze-y += spi.o
> +obj-microblaze-y += m25p80.o
>  
>  obj-microblaze-y += microblaze_pic_cpu.o
>  obj-microblaze-y += xilinx_intc.o
> diff --git a/hw/m25p80.c b/hw/m25p80.c
> new file mode 100644
> index 0000000..2b67375
> --- /dev/null
> +++ b/hw/m25p80.c
> @@ -0,0 +1,495 @@
> +/*
> + * ST M25P80 emulator.
[snip]

A device by ST does not sound microblaze-specific and in that case,
similar to the recent Atmel maxtouch device, should go into
hw-obj-$(CONFIG_M25P80) instead so that it's compiled only once for
microblaze-softmmu and microblazeel-softmmu.

Same for spi.o since it implements a generic bus AFAIU.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-04  0:48             ` Peter Crosthwaite
@ 2012-04-04 16:52               ` Paul Brook
  2012-04-05  0:32                 ` Peter Crosthwaite
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Brook @ 2012-04-04 16:52 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: Peter Maydell, qemu-devel, john.williams, edgar.iglesias

> Hi Paul,
> 
> Regarding using ssi, theres a few things that come to mind:
> 
> Theres no sense of it being a multi-slave bus, its just a point to
> point link. SPI devices universally have the notion of the CS pin that
> tristates the device of the bus. Masters connect to a number of slaves
> and one-hot-decode the active slave. It would be tedious if all SPI
> controllers (of which i have two (only pushed one in this series),
> omap have theirs, Nokia may have something out of tree too), had to
> implement the\is common CS decoding behaviour. To that end, my SPI bus
> implementation has some infra-structure for managing this -
> spi_set_cs(). The spi_bus in this series is single master,
> multi-slave, where as ssi is point-to-point.
> 
> Also as mentioned in earlier discussions with Peter, that api has no
> way of emulating the CS (sometimes called SS) pin. The m25p80 in this
> series (and potenitally other devices) has side effect associated with
> wiggling the cs pin, which can not be encapsulated by that ssi slave
> interface.
> 
> It may be a case though that ssi is a superclass of spi - all SPI
> devices are SSI devices but not all SSI devices are SPI? To that end
> can we make SPI a child object of SSI with the desired extra behaviors
> mentioned in this thread? This kind of stuff is the whole reason for
> QOM.

I don't believe there is any difference between SSI and SPI.  It's the exact 
same thing - the same way that many devices support a "two-wire interface" 
that is actually just I2C with a different name.

The behavior of the CS pin varies between devices.  It sounds like you need a 
bit of extra logic not present in the current ssi code.  You should fix that, 
not invent a whole new bus.

Paul

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-04 16:52               ` Paul Brook
@ 2012-04-05  0:32                 ` Peter Crosthwaite
  2012-04-05  7:18                   ` Peter Maydell
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2012-04-05  0:32 UTC (permalink / raw)
  To: Paul Brook; +Cc: Peter Maydell, qemu-devel, john.williams, edgar.iglesias

>>
>> It may be a case though that ssi is a superclass of spi - all SPI
>> devices are SSI devices but not all SSI devices are SPI? To that end
>> can we make SPI a child object of SSI with the desired extra behaviors
>> mentioned in this thread? This kind of stuff is the whole reason for
>> QOM.
>
> I don't believe there is any difference between SSI and SPI.  It's the exact
> same thing - the same way that many devices support a "two-wire interface"
> that is actually just I2C with a different name.
>
> The behavior of the CS pin varies between devices.  It sounds like you need a
> bit of extra logic not present in the current ssi code.  You should fix that,
> not invent a whole new bus.

Ok then in that case we are extending SSI to have these as optional
API features. Before withing any implementation, can I get some
consensus on the following approach:

1: extend SSISlave to have the set_cs fuction:

--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -29,6 +29,7 @@ typedef struct SSISlaveClass {

     int (*init)(SSISlave *dev);
     uint32_t (*transfer)(SSISlave *dev, uint32_t val);
+    int (*set_cs)(SSISlave *dev, int state);
 } SSISlaveClass;

 struct SSISlave {

2: set up SSI bus as a multi-slave bus:

--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -38,12 +38,13 @@ struct SSISlave {
 #define SSI_SLAVE_FROM_QDEV(dev) DO_UPCAST(SSISlave, qdev, dev)
 #define FROM_SSI_SLAVE(type, dev) DO_UPCAST(type, ssidev, dev)

-DeviceState *ssi_create_slave(SSIBus *bus, const char *name);
+DeviceState *ssi_create_slave(SSIBus *bus, const char *name, int slave);

 /* Master interface.  */
-SSIBus *ssi_create_bus(DeviceState *parent, const char *name);
+SSIBus *ssi_create_bus(DeviceState *parent, const char *name, int num_slaves);

 uint32_t ssi_transfer(SSIBus *bus, uint32_t val);
+uint32_t ssi_select_slave(SSIBus *bus, int slave);

 /* max111x.c */

3: add support for flagging sdi/sdo bits as tristated:

--- a/hw/ssi.h
+++ b/hw/ssi.h
@@ -28,7 +28,11 @@ typedef struct SSISlaveClass {
     DeviceClass parent_class;

     int (*init)(SSISlave *dev);
-    uint32_t (*transfer)(SSISlave *dev, uint32_t val);
+    /* transfer data. If z if provided, *z is the tristate mask
+     * It is initially populated with tristate value for tx, and
+     * on return should be populated with tristate for rx
+     */
+    uint32_t (*transfer)(SSISlave *dev, uint32_t val, uint32_t *z);
 } SSISlaveClass;

>
> Paul

Peter

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

* Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support
  2012-04-05  0:32                 ` Peter Crosthwaite
@ 2012-04-05  7:18                   ` Peter Maydell
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Maydell @ 2012-04-05  7:18 UTC (permalink / raw)
  To: Peter Crosthwaite; +Cc: edgar.iglesias, Paul Brook, john.williams, qemu-devel

On 5 April 2012 01:32, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> 3: add support for flagging sdi/sdo bits as tristated:
>
> --- a/hw/ssi.h
> +++ b/hw/ssi.h
> @@ -28,7 +28,11 @@ typedef struct SSISlaveClass {
>     DeviceClass parent_class;
>
>     int (*init)(SSISlave *dev);
> -    uint32_t (*transfer)(SSISlave *dev, uint32_t val);
> +    /* transfer data. If z if provided, *z is the tristate mask
> +     * It is initially populated with tristate value for tx, and
> +     * on return should be populated with tristate for rx
> +     */
> +    uint32_t (*transfer)(SSISlave *dev, uint32_t val, uint32_t *z);
>  } SSISlaveClass;

I'm not sure what you intend the tristate masks to be used for?
If the sending device puts the line into high-impedance presumably
the receiving device is just going to read something random which
it's supposed to ignore, so the sender could just send whatever
it likes. We're not modelling multiple devices sending at once,
which is the only case I can think of where you'd need to care
which bits each device was tristating for.

-- PMM

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

* Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
  2012-04-04 12:53   ` Andreas Färber
@ 2012-06-05  0:47     ` Peter Crosthwaite
  2012-06-05 12:38       ` Andreas Färber
  0 siblings, 1 reply; 23+ messages in thread
From: Peter Crosthwaite @ 2012-06-05  0:47 UTC (permalink / raw)
  To: Andreas Färber; +Cc: edgar.iglesias, qemu-devel, john.williams, paul

On Wed, Apr 4, 2012 at 10:53 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 30.03.2012 08:37, schrieb Peter A. G. Crosthwaite:
>> Added device model for m25p80 SPI flash
>>
>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>> ---
>>  Makefile.target |    1 +
>>  hw/m25p80.c     |  495 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 496 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/m25p80.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 8fd3718..fcccf1b 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -321,6 +321,7 @@ obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>>  obj-microblaze-y += petalogix_ml605_mmu.o
>>  obj-microblaze-y += microblaze_boot.o
>>  obj-microblaze-y += spi.o
>> +obj-microblaze-y += m25p80.o
>>
>>  obj-microblaze-y += microblaze_pic_cpu.o
>>  obj-microblaze-y += xilinx_intc.o
>> diff --git a/hw/m25p80.c b/hw/m25p80.c
>> new file mode 100644
>> index 0000000..2b67375
>> --- /dev/null
>> +++ b/hw/m25p80.c
>> @@ -0,0 +1,495 @@
>> +/*
>> + * ST M25P80 emulator.
> [snip]
>
> A device by ST does not sound microblaze-specific and in that case,
> similar to the recent Atmel maxtouch device, should go into
> hw-obj-$(CONFIG_M25P80) instead so that it's compiled only once for
> microblaze-softmmu and microblazeel-softmmu.
>

Hi Andreas,

I have regenerated this series, but have not actioned this just yet. I
think adding a config switch is probably not the right solution, as
its just a device model. If every device model has a config switch
then isnt creating everyone config process going to be excessively
tedious? perhaps it can just live next to obj-y=ssi.o? I.E. if you
have SSI, then you have m25p80. I cant think of a case where you would
want SSI but need to exclude m25p80 from the build.

Regards,
Peter

> Same for spi.o since it implements a generic bus AFAIU.
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion
  2012-06-05  0:47     ` Peter Crosthwaite
@ 2012-06-05 12:38       ` Andreas Färber
  0 siblings, 0 replies; 23+ messages in thread
From: Andreas Färber @ 2012-06-05 12:38 UTC (permalink / raw)
  To: Peter Crosthwaite
  Cc: edgar.iglesias, Paolo Bonzini, qemu-devel, john.williams, paul

Am 05.06.2012 02:47, schrieb Peter Crosthwaite:
> On Wed, Apr 4, 2012 at 10:53 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 30.03.2012 08:37, schrieb Peter A. G. Crosthwaite:
>>> Added device model for m25p80 SPI flash
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
>>> ---
>>>  Makefile.target |    1 +
>>>  hw/m25p80.c     |  495 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 496 insertions(+), 0 deletions(-)
>>>  create mode 100644 hw/m25p80.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 8fd3718..fcccf1b 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -321,6 +321,7 @@ obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>>>  obj-microblaze-y += petalogix_ml605_mmu.o
>>>  obj-microblaze-y += microblaze_boot.o
>>>  obj-microblaze-y += spi.o
>>> +obj-microblaze-y += m25p80.o
>>>
>>>  obj-microblaze-y += microblaze_pic_cpu.o
>>>  obj-microblaze-y += xilinx_intc.o
>>> diff --git a/hw/m25p80.c b/hw/m25p80.c
>>> new file mode 100644
>>> index 0000000..2b67375
>>> --- /dev/null
>>> +++ b/hw/m25p80.c
>>> @@ -0,0 +1,495 @@
>>> +/*
>>> + * ST M25P80 emulator.
>> [snip]
>>
>> A device by ST does not sound microblaze-specific and in that case,
>> similar to the recent Atmel maxtouch device, should go into
>> hw-obj-$(CONFIG_M25P80) instead so that it's compiled only once for
>> microblaze-softmmu and microblazeel-softmmu.
>>
> 
> Hi Andreas,
> 
> I have regenerated this series, but have not actioned this just yet. I
> think adding a config switch is probably not the right solution, as
> its just a device model. If every device model has a config switch
> then isnt creating everyone config process going to be excessively
> tedious? perhaps it can just live next to obj-y=ssi.o? I.E. if you
> have SSI, then you have m25p80. I cant think of a case where you would
> want SSI but need to exclude m25p80 from the build.

I think you're missing the point here. Whenever I touch, e.g.,
include/qemu/object.h, it takes an awfully long time to rebuild all
targets because, among others, you are unneccessarily duplicating device
model objects between microblaze and microblazeel. Devices by definition
do not depend on target endianness (they specify their endianness in
code) and should be built in libhw32/64. The mechanism to do so is
defining flags in default-configs/microblaze[el]-softmmu.mak. For
example, I had locally started a CONFIG_XILINX iirc, to share devices
between microblaze and ppc4xx, then there might be CONFIG_XILINX_PPC and
CONFIG_XILINX_MICROBLAZE for target-specific ones. My suggestion here
was to use CONFIG_M25P80 but if you have a better grouping that would be
fine, too. CONFIG_SSI maybe? Or if the amount of such devices is large
and to be shared across targets you might consider your own Makefile
like default-configs/pci.mak that can be included from multiple
*-softmmu.mak files.

Note however that Paolo has posted a series on the list refactoring the
whole Makefile system with which any such changes / additions clash.
Cc'ing Paolo so he can comment on the intended timing.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

end of thread, other threads:[~2012-06-05 12:38 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-30  6:37 [Qemu-devel] [RFC PATCH v1 0/4] SPI bus support + Xilinx SPI controller Peter A. G. Crosthwaite
2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support Peter A. G. Crosthwaite
2012-03-30  7:37   ` Peter Maydell
2012-03-30  7:50     ` Peter Crosthwaite
2012-04-02 17:39   ` Peter Maydell
2012-04-02 23:51     ` Peter Crosthwaite
2012-04-03  0:48       ` Peter Crosthwaite
2012-04-03 17:45       ` Paul Brook
2012-04-03 18:08         ` Peter Maydell
2012-04-03 21:22           ` Paul Brook
2012-04-04  0:48             ` Peter Crosthwaite
2012-04-04 16:52               ` Paul Brook
2012-04-05  0:32                 ` Peter Crosthwaite
2012-04-05  7:18                   ` Peter Maydell
2012-04-02 23:57     ` Peter Crosthwaite
2012-04-03  0:27       ` Andreas Färber
2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 2/4] m25p80: initial verion Peter A. G. Crosthwaite
2012-04-03  7:03   ` Stefan Hajnoczi
2012-04-04 12:53   ` Andreas Färber
2012-06-05  0:47     ` Peter Crosthwaite
2012-06-05 12:38       ` Andreas Färber
2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 3/4] xilinx_spi: initial version Peter A. G. Crosthwaite
2012-03-30  6:37 ` [Qemu-devel] [RFC PATCH v1 4/4] petalogix-ml605: added spi controller with m25p80 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.