Re: [Qemu-devel] [RFC PATCH v1 1/4] SPI: initial support

[Peter Maydell]

On 30 March 2012 07:37, Peter A. G. Crosthwaite
<address@hidden> 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 <address@hidden>
> ---
>  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 <address@hidden>
> + * 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