diff mbox

[RFC,v1,1/4] SPI: initial support

Message ID CAEgOgz54PRciOofZ0q1JkzTcT-84drinZMdWjMz-2Au+wXufPA@mail.gmail.com
State New
Headers show

Commit Message

Peter A. G. Crosthwaite April 5, 2012, 12:32 a.m. UTC 
>>
>> 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:

> Paul

Peter

Comments

Peter Maydell April 5, 2012, 7:18 a.m. UTC | #1 
On 5 April 2012 01:32, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> 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;

I'm not sure what you intend the tristate masks to be used for?
If the sending device puts the line into high-impedance presumably
the receiving device is just going to read something random which
it's supposed to ignore, so the sender could just send whatever
it likes. We're not modelling multiple devices sending at once,
which is the only case I can think of where you'd need to care
which bits each device was tristating for.

-- PMM
Peter A. G. Crosthwaite April 7, 2012, 1:08 a.m. UTC | #2 
On Thu, Apr 5, 2012 at 5:18 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> I'm not sure what you intend the tristate masks to be used for?
> If the sending device puts the line into high-impedance presumably
> the receiving device is just going to read something random which
> it's supposed to ignore, so the sender could just send whatever
> it likes. We're not modelling multiple devices sending at once,
> which is the only case I can think of where you'd need to care
> which bits each device was tristating for.
>
> -- PMM

Hi Peter,

I managed to get all the tristate behaviours out of my controller and
device models so we can drop this as you have suggested, so we can
drop that change to ssi.

Happy Easter,
Peter
diff mbox

Patch

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

>