Patchwork [RFC,v1,2/4] m25p80: initial verion

login
register
mail settings
Submitter Peter A. G. Crosthwaite
Date March 30, 2012, 6:37 a.m.
Message ID <26d832cdffc1b31cfe1aa6fa9ca50f66d6fb9f71.1333088411.git.peter.crosthwaite@petalogix.com>
Download mbox | patch
Permalink /patch/149548/
State New
Headers show

Comments

Peter A. G. Crosthwaite - March 30, 2012, 6:37 a.m.
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
Stefan Hajnoczi - April 3, 2012, 7:03 a.m.
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
Andreas Färber - April 4, 2012, 12:53 p.m.
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
Peter A. G. Crosthwaite - April 19, 2012, 2:33 a.m.
Hi Andreas,

So is there any standard policy on setting maintainer-ships for device
models? When I pushed the cadence IP device models (hw/cadence*) for
xilinx zynq there was insistence that they be in the MAINTAINERS,
however like m25p80, they are not necessarily zynq specific (indeed
cadence could tmrw sell those IPs to another SoC vendor).

What qualifies a device model for being in maintainers under a cpu or
machine model?

Peter

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.
>
> 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
Andreas Färber - April 21, 2012, 5:44 a.m.
Hi Peter,

Am 19.04.2012 04:33, schrieb Peter Crosthwaite:
> So is there any standard policy on setting maintainer-ships for device
> models? When I pushed the cadence IP device models (hw/cadence*) for
> xilinx zynq there was insistence that they be in the MAINTAINERS,

Yes, that was my request.

> however like m25p80, they are not necessarily zynq specific (indeed
> cadence could tmrw sell those IPs to another SoC vendor).
> 
> What qualifies a device model for being in maintainers under a cpu or
> machine model?

MAINTAINERS file is newer than most devices, so many are still missing.

I don't think that grouping devices with the TCG target is a good idea.

My reasoning for adding the Zynq devices to the machine was that it
seemed a natural fit - both were created by you and "belonged" to that
one machine.

When that is not the case, you can just create a new subsystem section
in MAINTAINERS, grouping in a way you see fit (Cadence IP, SPI,
MicroBlaze SoC, ...).

Especially when handling large patch series it is unhandy to manually
track maintainers/authors, so an automated way such as MAINTAINERS +
scripts/get_maintainer.pl is very handy. So if I need to apply some
CPUState change (or someone changes Memory API again) and authors can be
cc'ed for review by our script then I'm happy.

Another discussion this touches on is how the status of a MAINTAINERS
section is interpreted (my recent RFC series). Anthony has been
advocating that S: Maintained means to him you should queue patches for
that subsystem yourself rather than just sending/ack'ing patches on the
list.
How that interacts with TCG target maintenance is still somewhat of a
gray zone, i.e. ARM devices, despite not strictly documented in
MAINTAINERS, are in practice going through Peter as ARM maintainer for
coordination. (cc'ing both: Maybe we should find a mechanism to
distinguish between author and person to queue/pull after all? For
MicroBlaze that might be Peter C. vs. Edgar.)

Sorry for the late reply,

Andreas
Peter Maydell - April 21, 2012, 8:40 a.m.
On 21 April 2012 06:44, Andreas Färber <afaerber@suse.de> wrote:
> MAINTAINERS file is newer than most devices, so many are still missing.
>
> I don't think that grouping devices with the TCG target is a good idea.
>
> My reasoning for adding the Zynq devices to the machine was that it
> seemed a natural fit - both were created by you and "belonged" to that
> one machine.

Yes, those cases are clear. The difficult ones are things like
for example the PL330 DMA controller that has been posted to the
list, which happens to be used first by the zynq but will also
be used by the exynos board and perhaps others later. I think
listing this kind of generic device under the board which happened
to first use it is misleading.

> Another discussion this touches on is how the status of a MAINTAINERS
> section is interpreted (my recent RFC series). Anthony has been
> advocating that S: Maintained means to him you should queue patches for
> that subsystem yourself rather than just sending/ack'ing patches on the
> list.
> How that interacts with TCG target maintenance is still somewhat of a
> gray zone, i.e. ARM devices, despite not strictly documented in
> MAINTAINERS, are in practice going through Peter as ARM maintainer for
> coordination.

Yes. I've been doing this because a lot of the ARM boards are in
the 'occasional fixes' state where they get patches sometimes but
don't have a continously active maintainer, so it's partly historical.
(I do also value the code-review gate that running these board patches
through me gives.) Would people prefer board maintainers to submit
direct pull requests?

-- PMM
Peter A. G. Crosthwaite - June 5, 2012, 12:47 a.m.
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
Andreas Färber - June 5, 2012, 12:38 p.m.
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

Patch

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)