Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-06 Thread Paul Brook
> > I'm still not convinced modelling this as a multipoint bus is a good
> > idea.  If nothing else you've failed to model the case where multiple
> > slaves are selected simultanously.
> 
> The bus can easily be changed such that multiple devices are
> selectable at once to get your desired multi device behaviour. AFAICT
> though nothing in QEMU behaves like this ATM.

By my reading your xilinx device *should* behave like this.
 
> >  Given the chip selects are actual wires, not part of
> > the bus itself, I think multiple point-point busses are a better fit.
> > 
> > For the stellaris device we still have the synthetic mux device and
> > intermediate bus.
> 
> Yes, because in your stellaris architecture, the SSI controller
> (pl022) is point to point so that exactly matches the hardware.
> 
> In the microblaze controller in this series, the controller has
> inbuilt muxing with one-hot CS behavior. To implement with point to
> point, I would have to dynamically create a number of sub-busses
> (driven by a qdev property). I would also have to have a device within
> a device to model the internal mux which increases my code volume
> significantly. Also you end up with this little piece of ugliness in
> your machine model and device model:

I don't see why would would need a separate mux device.

One of my issues is that you've made this a device property.  A SPI device has 
no concept of address.  This really is a property of the controller.

> The multi-slave bus is a direct superset on point-to-point. There is
> nothing stopping anyone from using it as p2p. Its just things are very
> ugly for SPI controllers with integrated muxes to treat everything as
> point to point.

IMHO the resulting tree device is better with multiple point-point links.  I'm 
hoping the hardcoded board descriptions (i.e. everything using 
ssi_create_slave) will go away sooner rather than later.  Having two m25p80 
devices that are indistinguishable apart from one minor property seems 
undesirable.

Paul



Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-06 Thread Paul Brook
> On 5th April, when we first RFC'd our SPI layer support, you said to Peter:
> 
> ==
> 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.
> ==
> 
> He's gone and done exactly that, indeed generalised it with the
> proposed changes to SSI.

No.  There are two changes.  Modelling the CS line in the SPI bus, and having 
SSI be a multipoint bus rather than point-point.

Paul



Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-04 Thread John Williams
Hi Paul,

On Tue, Jun 5, 2012 at 11:34 AM, Paul Brook  wrote:
>> Patch 1 Enhances SSI bus support to properly support multiple attached
>> devices. An api is provided for SSI/SPI masters to select a particular
>> device attached to the bus.
>>
>> Patch 2 is a device model for the m25p80 style SPI flash chip.
>>
>> Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that
>> instantiates a ssi bus, and interfaces the two (as per the controllers
>> functionality)
>>
>> Patch 4 instantiates the XPS SPI controller in the petalogix ML605
>> reference platform and connects two m25p80s to it.
>>
>> Patch 5 updates the stellaris machine model to use the multi slave SSI
>> support
>
> I'm still not convinced modelling this as a multipoint bus is a good idea.  If
> nothing else you've failed to model the case where multiple slaves are
> selected simultanously.  Given the chip selects are actual wires, not part of
> the bus itself, I think multiple point-point busses are a better fit.
>

On 5th April, when we first RFC'd our SPI layer support, you said to Peter:

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

He's gone and done exactly that, indeed generalised it with the
proposed changes to SSI.

If you don't like how he's done the Stellaris change then feel free to
NACK that specific patch, but otherwise it seems this generic SSI set
should be good to go now?

Thanks,

John
--
John Williams, PhD, B. Eng, B. IT
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com  p: +61-7-30090663  f: +61-7-30090663



Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-04 Thread Peter Crosthwaite
Hi Paul,

Responses inline below.

On Tue, Jun 5, 2012 at 11:34 AM, Paul Brook  wrote:
>> Patch 1 Enhances SSI bus support to properly support multiple attached
>> devices. An api is provided for SSI/SPI masters to select a particular
>> device attached to the bus.
>>
>> Patch 2 is a device model for the m25p80 style SPI flash chip.
>>
>> Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that
>> instantiates a ssi bus, and interfaces the two (as per the controllers
>> functionality)
>>
>> Patch 4 instantiates the XPS SPI controller in the petalogix ML605
>> reference platform and connects two m25p80s to it.
>>
>> Patch 5 updates the stellaris machine model to use the multi slave SSI
>> support
>
> I'm still not convinced modelling this as a multipoint bus is a good idea.  If
> nothing else you've failed to model the case where multiple slaves are
> selected simultanously.

The bus can easily be changed such that multiple devices are
selectable at once to get your desired multi device behaviour. AFAICT
though nothing in QEMU behaves like this ATM.

  Given the chip selects are actual wires, not part of
> the bus itself, I think multiple point-point busses are a better fit.
>
> For the stellaris device we still have the synthetic mux device and
> intermediate bus.
>

Yes, because in your stellaris architecture, the SSI controller
(pl022) is point to point so that exactly matches the hardware.

In the microblaze controller in this series, the controller has
inbuilt muxing with one-hot CS behavior. To implement with point to
point, I would have to dynamically create a number of sub-busses
(driven by a qdev property). I would also have to have a device within
a device to model the internal mux which increases my code volume
significantly. Also you end up with this little piece of ugliness in
your machine model and device model:

diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index 01af0da..394a80e 100644
--- a/hw/petalogix_ml605_mmu.c
+++ b/hw/petalogix_ml605_mmu.c
@@ -145,10 +145,11 @@ petalogix_ml605_init(ram_addr_t ram_size,
 sysbus_mmio_map(busdev, 0, 0x40a0);
 sysbus_connect_irq(busdev, 0, irq[4]);

-spi = qdev_get_child_bus(dev, "spi");

 for (i = 0; i < NUM_SPI_FLASHES; i++) {
-dev = ssi_create_slave_no_init(spi, "m25p80", i);
+sprintf(sub_bus_name, "spi%d", i);
+spi = qdev_get_child_bus(dev, sub_bus_name);
+dev = ssi_create_slave_no_init(spi, "m25p80");
 qdev_prop_set_string(dev, "partname", (char *)"s25fl064k");
 qdev_init_nofail(dev);
 }
diff --git a/hw/xilinx_spi.c b/hw/xilinx_spi.c
index cae88ad..6c81f70 100644
--- a/hw/xilinx_spi.c
+++ b/hw/xilinx_spi.c
@@ -420,8 +420,11 @@ static int xilinx_spi_init(SysBusDevice *dev)
 s->irqline = -1;
 ptimer_set_freq(s->ptimer, 10 * 1000 * 1000);

-s->spi = ssi_create_bus(&dev->qdev, "spi");
-
+char sub_bus_name;
+for (i = 0; i < num_cs; i++) {
+sprintf(sub_bus_name, "spi%d", i);
+s->spi[i] = ssi_create_bus(&dev->qdev, sub_bus_name);
+}
 return 0;
 }

The multi-slave bus is a direct superset on point-to-point. There is
nothing stopping anyone from using it as p2p. Its just things are very
ugly for SPI controllers with integrated muxes to treat everything as
point to point.

Regards,
Peter
>
> Paul



Re: [Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-04 Thread Paul Brook
> Patch 1 Enhances SSI bus support to properly support multiple attached
> devices. An api is provided for SSI/SPI masters to select a particular
> device attached to the bus.
> 
> Patch 2 is a device model for the m25p80 style SPI flash chip.
> 
> Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that
> instantiates a ssi bus, and interfaces the two (as per the controllers
> functionality)
> 
> Patch 4 instantiates the XPS SPI controller in the petalogix ML605
> reference platform and connects two m25p80s to it.
> 
> Patch 5 updates the stellaris machine model to use the multi slave SSI
> support

I'm still not convinced modelling this as a multipoint bus is a good idea.  If 
nothing else you've failed to model the case where multiple slaves are 
selected simultanously.  Given the chip selects are actual wires, not part of 
the bus itself, I think multiple point-point busses are a better fit.

For the stellaris device we still have the synthetic mux device and 
intermediate bus.


Paul



[Qemu-devel] [PATCH V4 0/5] Ehnahced SSI bus support + M25P80 SPI flash + Xilinx SPI controller

2012-06-04 Thread Peter A. G. Crosthwaite
Patch 1 Enhances SSI bus support to properly support multiple attached devices. 
An api is provided for SSI/SPI masters to select a particular device attached 
to the bus.

Patch 2 is a device model for the m25p80 style SPI flash chip.

Patch 3 is  the Xilinx XPS SPI contoller. Its a sysbus device that instantiates 
a ssi bus, and interfaces the two (as per the controllers functionality)

Patch 4 instantiates the XPS SPI controller in the petalogix ML605 reference 
platform and connects two m25p80s to it.

Patch 5 updates the stellaris machine model to use the multi slave SSI support

CHANGELOG:
changed from v3:
addressed reviewer comments from P Maydell and S Hajnoczi
added patch 5 (re Paul Brooks request)
changed from v2:
folded former SPI bus functionality into existing SSI infrastructure (suggested 
- Paul Brook) (all patches)
made m25p80 use async io (suggested - Stefan Hajnoczi) (2/4)
instantiated two spi flashes instead of one in ml605 ref design (4/4)
changed from v1:
minor sylistic changes (1/4)
converted spi api to modified txrx style (1-3/4)
heavily refactored m25p80 model (2/4)

Peter A. G. Crosthwaite (5):
  SSI: Built in multiple device support
  m25p80: initial verion
  xilinx_spi: initial version
  petalogix-ml605: added spi controller with m25p80
  stellaris: Updated spi bus implementation

 Makefile.target  |2 +
 default-configs/microblaze-softmmu.mak   |1 +
 default-configs/microblazeel-softmmu.mak |1 +
 hw/m25p80.c  |  557 ++
 hw/petalogix_ml605_mmu.c |   23 ++
 hw/spitz.c   |8 +-
 hw/ssi.c |  107 +-
 hw/ssi.h |   28 ++-
 hw/stellaris.c   |   21 +-
 hw/tosa.c|2 +-
 hw/xilinx_spi.c  |  481 ++
 hw/z2.c  |2 +-
 12 files changed, 1196 insertions(+), 37 deletions(-)
 create mode 100644 hw/m25p80.c
 create mode 100644 hw/xilinx_spi.c

-- 
1.7.3.2