Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

2012-04-25 Thread Paul Brook 
 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

Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

2012-04-25 Thread Peter Crosthwaite 
On Wed, Apr 25, 2012 at 2:41 AM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 20 April 2012 03:12, Peter A. G. Crosthwaite
 peter.crosthwa...@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

Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

2012-04-24 Thread Peter Maydell 
On 20 April 2012 03:12, Peter A. G. Crosthwaite
peter.crosthwa...@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

Re: [Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

2012-04-20 Thread Paul Brook 
  /* 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

[Qemu-devel] [PATCH v3 1/4] SSI: Built in multiple device support

2012-04-19 Thread Peter A. G. Crosthwaite 
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.crosthwa...@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(-)

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);
-max = ssi_create_slave(bus, max);
+max = ssi_create_slave(bus, max, 0);
 max111x_set_input(max, MAX_BATT_VOLT, SPITZ_BATTERY_VOLT);
 max111x_set_input(max, MAX_BATT_TEMP, 0);
 max111x_set_input(max, MAX_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.crosthwa...@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 =