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

[Peter Crosthwaite]

>>
>> 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