| Message ID | b991677a2c37323f3408c7ba7e5aaefab2c96b3e.1334886618.git.peter.crosthwaite@petalogix.com |
|---|---|
| State | New |
| Headers | show |
> /* QEMU Synchronous Serial Interface support. */ > > -/* In principle SSI is a point-point interface. As such the qemu > - implementation has a single slave device on a "bus". > +/* In principle SSI is a point-point interface. > However it is fairly common for boards to have multiple slaves > connected to a single master, and select devices with an external > - chip select. This is implemented in qemu by having an explicit mux dev. > + chip select. SSI busses can therefore have any number of slaves, > + of which a master can select using ssi_select_slave(). > It is assumed that master and slave are both using the same transfer This would be much more convincing if you'd also implemented it. In particular the lm3s6965evb has two devices (LCD controller and SD card) connected to a single SSI controller, and selects between the two by twiddling chip selects. Paul
Hi Paul, Im happy to spend the 10 mins updating stellaris.c accordingly, but is someone sitting on a binary package and brief instructions or some such to regression test it? Do you of this machine have some sort of kernel image handy? Peter 2012/4/21 Paul Brook <paul@codesourcery.com>: >> /* QEMU Synchronous Serial Interface support. */ >> >> -/* In principle SSI is a point-point interface. As such the qemu >> - implementation has a single slave device on a "bus". >> +/* In principle SSI is a point-point interface. >> However it is fairly common for boards to have multiple slaves >> connected to a single master, and select devices with an external >> - chip select. This is implemented in qemu by having an explicit mux dev. >> + chip select. SSI busses can therefore have any number of slaves, >> + of which a master can select using ssi_select_slave(). >> It is assumed that master and slave are both using the same transfer > > This would be much more convincing if you'd also implemented it. In particular > the lm3s6965evb has two devices (LCD controller and SD card) connected to a > single SSI controller, and selects between the two by twiddling chip selects. > > Paul
On 20 April 2012 03:12, Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> wrote: > Added support for multiple devices attached to a single SSI bus (Previously > SSI masters with multiple slaves were emulated as multiple point to point SSI > busses) > static struct BusInfo ssi_bus_info = { > .name = "SSI", > .size = sizeof(SSIBus), > + .props = (Property[]) { > + DEFINE_PROP_INT32("ss", struct SSISlave, ss, 0), "ss" is a terrible name for a property. Can we have something a little less abbreviated, please? > SSIBus *ssi_create_bus(DeviceState *parent, const char *name) > { > - BusState *bus; > - bus = qbus_create(&ssi_bus_info, parent, name); > - return FROM_QBUS(SSIBus, bus); > + SSIBus *bus; > + > + bus = FROM_QBUS(SSIBus, qbus_create(&ssi_bus_info, parent, name)); > + vmstate_register(NULL, -1, &vmstate_ssi_bus, bus); > + return bus; Stray double space. > +void ssi_select_slave(SSIBus *bus, int32_t ss) > +{ > + SSISlave *slave; > + SSISlaveClass *ssc; > + > + if (bus->ss == ss) { > + return; > + } > + > + slave = get_current_slave(bus); > + if (slave) { > + ssc = SSI_SLAVE_GET_CLASS(slave); > + if (ssc->set_cs) { > + ssc->set_cs(slave, 0); > + } > + } > + bus->ss = ss; Something wrong here. If bus->ss is a property you can't modify it randomly at runtime. (It won't get put back to the right value at reset, for instance.) -- PMM
> Im happy to spend the 10 mins updating stellaris.c accordingly, but is > someone sitting on a binary package and brief instructions or some > such to regression test it? Do you of this machine have some sort of > kernel image handy? I've attached a tarball with some test binaries. They're built from the example libraries shipped with this board. The first exercises the display, amongst other things. The second exercises the SD card. Simple SD card image also included (remember to ungzip it first). "ls" and "cat readme.txt" over the serial port to make it do something verifiable. Run them with: ./qemu-system-arm -M lm3s6965evb -kernel qs_ek-lm3s6965.bin ./qemu-system-arm -M lm3s6965evb -serial stdio -kernel sd_card.bin -sd sdcard.img I don't have any software handy that exercises both simultaneously. It's probably worth mentioning that we don't currently implement the all CS lines accurately for this board. Most pins on this device are dual-function. They can be configured either as regular GPIO, or driven from a periperal (aka "alternate function", e.g. the SSI controller). Config is cone via the GPIO controllers. There are 7 GPIO contollers (A-G) with 8 pins each. On reset all pins are configured as floating GPIO, and we let D0 float high. The frame start/chip select line from the SPI controller goes via GPIO A3. This is connected to the display controller (ssd0323) CS pin. The SD card CS pin is connected to GPIO D0. When comminucating with the display controller the SSI pins will be configured normally. When communicating with the SD card we configure A3 as a GPIO pin, set high (inactive), and pull D0 low to select the SD card. The current implementation ignores the SSI select pin (A3), and assumes the display controller is selected whenever the SD card (D0) is not. We do not implement the alternate function select in the GPIO controller. It's a bit of a strange setup, but I guess probably not that unusual. Paul
On Wed, Apr 25, 2012 at 2:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 April 2012 03:12, Peter A. G. Crosthwaite > <peter.crosthwaite@petalogix.com> wrote: >> Added support for multiple devices attached to a single SSI bus (Previously >> SSI masters with multiple slaves were emulated as multiple point to point SSI >> busses) > >> static struct BusInfo ssi_bus_info = { >> .name = "SSI", >> .size = sizeof(SSIBus), >> + .props = (Property[]) { >> + DEFINE_PROP_INT32("ss", struct SSISlave, ss, 0), > > "ss" is a terrible name for a property. Can we have something > a little less abbreviated, please? > >> SSIBus *ssi_create_bus(DeviceState *parent, const char *name) >> { >> - BusState *bus; >> - bus = qbus_create(&ssi_bus_info, parent, name); >> - return FROM_QBUS(SSIBus, bus); >> + SSIBus *bus; >> + >> + bus = FROM_QBUS(SSIBus, qbus_create(&ssi_bus_info, parent, name)); >> + vmstate_register(NULL, -1, &vmstate_ssi_bus, bus); >> + return bus; > > Stray double space. > ack >> +void ssi_select_slave(SSIBus *bus, int32_t ss) >> +{ >> + SSISlave *slave; >> + SSISlaveClass *ssc; >> + >> + if (bus->ss == ss) { >> + return; >> + } >> + >> + slave = get_current_slave(bus); >> + if (slave) { >> + ssc = SSI_SLAVE_GET_CLASS(slave); >> + if (ssc->set_cs) { >> + ssc->set_cs(slave, 0); >> + } >> + } >> + bus->ss = ss; > > Something wrong here. If bus->ss is a property you can't modify > it randomly at runtime. (It won't get put back to the right > value at reset, for instance.) > Hi Peter, Thanks for your review. I think there is confusion here in that I have named both the bus and slave properties ss. Each slave has a index which ive called ss and the bus has a property ss. The property defintion above applies the slave device ss property which is constant. I think this will go away when I clear up the name confusion as you have suggested. Regards, Peter > -- PMM
diff --git a/hw/spitz.c b/hw/spitz.c index 1d6d2b0..f63a9bf 100644 --- a/hw/spitz.c +++ b/hw/spitz.c @@ -669,18 +669,18 @@ static void spitz_ssp_attach(PXA2xxState *cpu) DeviceState *dev; void *bus; - mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp"); + mux = ssi_create_slave(cpu->ssp[CORGI_SSP_PORT - 1], "corgi-ssp", 0); bus = qdev_get_child_bus(mux, "ssi0"); - ssi_create_slave(bus, "spitz-lcdtg"); + ssi_create_slave(bus, "spitz-lcdtg", 0); bus = qdev_get_child_bus(mux, "ssi1"); - dev = ssi_create_slave(bus, "ads7846"); + dev = ssi_create_slave(bus, "ads7846", 0); qdev_connect_gpio_out(dev, 0, qdev_get_gpio_in(cpu->gpio, SPITZ_GPIO_TP_INT)); bus = qdev_get_child_bus(mux, "ssi2"); - max1111 = ssi_create_slave(bus, "max1111"); + max1111 = ssi_create_slave(bus, "max1111", 0); max111x_set_input(max1111, MAX1111_BATT_VOLT, SPITZ_BATTERY_VOLT); max111x_set_input(max1111, MAX1111_BATT_TEMP, 0); max111x_set_input(max1111, MAX1111_ACIN_VOLT, SPITZ_CHARGEON_ACIN); diff --git a/hw/ssi.c b/hw/ssi.c index 8f2d9bc..d562203 100644 --- a/hw/ssi.c +++ b/hw/ssi.c @@ -2,6 +2,8 @@ * QEMU Synchronous Serial Interface support * * Copyright (c) 2009 CodeSourcery. + * Copyright (c) 2012 Peter A.G. Crosthwaite (peter.crosthwaite@petalogix.com) + * Copyright (c) 2012 PetaLogix Pty Ltd. * Written by Paul Brook * * This code is licensed under the GNU GPL v2. @@ -14,24 +16,33 @@ struct SSIBus { BusState qbus; + int32_t ss; }; static struct BusInfo ssi_bus_info = { .name = "SSI", .size = sizeof(SSIBus), + .props = (Property[]) { + DEFINE_PROP_INT32("ss", struct SSISlave, ss, 0), + DEFINE_PROP_END_OF_LIST(), + } +}; + +static const VMStateDescription vmstate_ssi_bus = { + .name = "ssi_bus", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_INT32(ss, SSIBus), + VMSTATE_END_OF_LIST() + } }; static int ssi_slave_init(DeviceState *dev) { SSISlave *s = SSI_SLAVE(dev); SSISlaveClass *ssc = SSI_SLAVE_GET_CLASS(s); - SSIBus *bus; - - bus = FROM_QBUS(SSIBus, qdev_get_parent_bus(dev)); - if (QTAILQ_FIRST(&bus->qbus.children) != dev - || QTAILQ_NEXT(dev, sibling) != NULL) { - hw_error("Too many devices on SSI bus"); - } return ssc->init(s); } @@ -46,40 +57,96 @@ static void ssi_slave_class_init(ObjectClass *klass, void *data) static TypeInfo ssi_slave_info = { .name = TYPE_SSI_SLAVE, .parent = TYPE_DEVICE, + .instance_size = sizeof(struct SSISlave), .class_init = ssi_slave_class_init, .class_size = sizeof(SSISlaveClass), .abstract = true, }; -DeviceState *ssi_create_slave(SSIBus *bus, const char *name) +DeviceState *ssi_create_slave(SSIBus *bus, const char *name, int32_t ss) { DeviceState *dev; dev = qdev_create(&bus->qbus, name); + qdev_prop_set_int32(dev, "ss", ss); qdev_init_nofail(dev); return dev; } SSIBus *ssi_create_bus(DeviceState *parent, const char *name) { - BusState *bus; - bus = qbus_create(&ssi_bus_info, parent, name); - return FROM_QBUS(SSIBus, bus); + SSIBus *bus; + + bus = FROM_QBUS(SSIBus, qbus_create(&ssi_bus_info, parent, name)); + vmstate_register(NULL, -1, &vmstate_ssi_bus, bus); + return bus; +} + +static SSISlave *get_current_slave(SSIBus *bus) +{ + DeviceState *qdev; + + QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) { + SSISlave *candidate = SSI_SLAVE_FROM_QDEV(qdev); + if (candidate->ss == bus->ss) { + return candidate; + } + } + + return NULL; +} + +void ssi_select_slave(SSIBus *bus, int32_t ss) +{ + SSISlave *slave; + SSISlaveClass *ssc; + + if (bus->ss == ss) { + return; + } + + slave = get_current_slave(bus); + if (slave) { + ssc = SSI_SLAVE_GET_CLASS(slave); + if (ssc->set_cs) { + ssc->set_cs(slave, 0); + } + } + bus->ss = ss; + + slave = get_current_slave(bus); + if (slave) { + ssc = SSI_SLAVE_GET_CLASS(slave); + if (ssc->set_cs) { + ssc->set_cs(slave, 1); + } + } + } uint32_t ssi_transfer(SSIBus *bus, uint32_t val) { - DeviceState *dev; SSISlave *slave; SSISlaveClass *ssc; - dev = QTAILQ_FIRST(&bus->qbus.children); - if (!dev) { + + slave = get_current_slave(bus); + if (!slave) { return 0; } - slave = SSI_SLAVE(dev); ssc = SSI_SLAVE_GET_CLASS(slave); return ssc->transfer(slave, val); } +const VMStateDescription vmstate_ssi_slave = { + .name = "SSISlave", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_INT32(ss, SSISlave), + VMSTATE_END_OF_LIST() + } +}; + static void ssi_slave_register_types(void) { type_register_static(&ssi_slave_info); diff --git a/hw/ssi.h b/hw/ssi.h index 06657d7..000d19f 100644 --- a/hw/ssi.h +++ b/hw/ssi.h @@ -1,10 +1,10 @@ /* QEMU Synchronous Serial Interface support. */ -/* In principle SSI is a point-point interface. As such the qemu - implementation has a single slave device on a "bus". +/* In principle SSI is a point-point interface. However it is fairly common for boards to have multiple slaves connected to a single master, and select devices with an external - chip select. This is implemented in qemu by having an explicit mux device. + chip select. SSI busses can therefore have any number of slaves, + of which a master can select using ssi_select_slave(). It is assumed that master and slave are both using the same transfer width. */ @@ -29,22 +29,39 @@ 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 { DeviceState qdev; + + int32_t ss; }; #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, int32_t ss); /* Master interface. */ SSIBus *ssi_create_bus(DeviceState *parent, const char *name); +#define SSI_SLAVE_SELECT_NONE (-1) + +void ssi_select_slave(SSIBus *bus, int32_t ss); + uint32_t ssi_transfer(SSIBus *bus, uint32_t val); +extern const VMStateDescription vmstate_ssi_slave; + +#define VMSTATE_SSI_SLAVE(_field, _state) { \ + .name = (stringify(_field)), \ + .size = sizeof(SSISlave), \ + .vmsd = &vmstate_ssi_slave, \ + .flags = VMS_STRUCT, \ + .offset = vmstate_offset_value(_state, _field, SSISlave), \ +} + /* max111x.c */ void max111x_set_input(DeviceState *dev, int line, uint8_t value); diff --git a/hw/stellaris.c b/hw/stellaris.c index 562fbbf..e0600a1 100644 --- a/hw/stellaris.c +++ b/hw/stellaris.c @@ -1309,14 +1309,14 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model, void *bus; bus = qdev_get_child_bus(dev, "ssi"); - mux = ssi_create_slave(bus, "evb6965-ssi"); + mux = ssi_create_slave(bus, "evb6965-ssi", 0); gpio_out[GPIO_D][0] = qdev_get_gpio_in(mux, 0); bus = qdev_get_child_bus(mux, "ssi0"); - ssi_create_slave(bus, "ssi-sd"); + ssi_create_slave(bus, "ssi-sd", 0); bus = qdev_get_child_bus(mux, "ssi1"); - dev = ssi_create_slave(bus, "ssd0323"); + dev = ssi_create_slave(bus, "ssd0323", 0); gpio_out[GPIO_C][7] = qdev_get_gpio_in(dev, 0); /* Make sure the select pin is high. */ diff --git a/hw/tosa.c b/hw/tosa.c index 6baa17d..3986810 100644 --- a/hw/tosa.c +++ b/hw/tosa.c @@ -196,7 +196,7 @@ static void tosa_tg_init(PXA2xxState *cpu) { i2c_bus *bus = pxa2xx_i2c_bus(cpu->i2c[0]); i2c_create_slave(bus, "tosa_dac", DAC_BASE); - ssi_create_slave(cpu->ssp[1], "tosa-ssp"); + ssi_create_slave(cpu->ssp[1], "tosa-ssp", 0); } diff --git a/hw/z2.c b/hw/z2.c index 654ac55..2a0ecf5 100644 --- a/hw/z2.c +++ b/hw/z2.c @@ -346,7 +346,7 @@ static void z2_init(ram_addr_t ram_size, type_register_static(&zipit_lcd_info); type_register_static(&aer915_info); - z2_lcd = ssi_create_slave(cpu->ssp[1], "zipit-lcd"); + z2_lcd = ssi_create_slave(cpu->ssp[1], "zipit-lcd", 0); bus = pxa2xx_i2c_bus(cpu->i2c[0]); i2c_create_slave(bus, "aer915", 0x55); wm = i2c_create_slave(bus, "wm8750", 0x1b);
Added support for multiple devices attached to a single SSI bus (Previously SSI masters with multiple slaves were emulated as multiple point to point SSI busses) Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com> --- changed from v2: This patch is new (totally rewitten replacement of (1/4) from v2) hw/spitz.c | 8 ++-- hw/ssi.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++--------- hw/ssi.h | 25 ++++++++++++-- hw/stellaris.c | 6 ++-- hw/tosa.c | 2 +- hw/z2.c | 2 +- 6 files changed, 112 insertions(+), 28 deletions(-)