Skip to contentSkip to conversation

Connect the SPI devices to ZynqMP

PATCH In review Peter Crosthwaite 3 participants 14 messages Dec 20, 2015 – Jan 15, 2016 Arm/Zynq architecture

lore
Share & tools
lore
#0 lore raw

v6 cover Connect the SPI devices to ZynqMP

Hi All,

This is a modification of Alistair's ZynqMP SPI work, which proposes a solution to the SoC bus-aliasing problem. Patch 1 is the big change, and patches 5 and 6 demo the capability.

I have only tested this -S as I do not have images that test SPI handy, but qtree looks correct with the new SPI flashes.

V6:
Allow use of QOM paths for referencing qbusses
Use Alias to implement bus pinout.

Regards,
Peter

Original Cover from Alistair:

Connect the SPI devices to Xilinx's ZynqMP.

I also need to make some changes to the actual SPI device to imporove the fuctionality, but for the time being this works.

V5:
- Fix a typo
- Use a qdev API to rename the SPI bus
V4:
- Rebase
- Rename the SPI busses so that they can all be accessed from the SoC
- Only create one SPI flash device
V3:
- Don't reach into the SoC to get the SPI Bus
V2:
- Connect the SPI flash in the board code
- Update git patches to properly indicate rename
- Add sst25wf080 as a SPI flash

Alistair Francis (5):
m25p80.c: Add sst25wf080 SPI flash device
ssi: Move ssi.h into a separate directory
xilinx_spips: Separate the state struct into a header
xlnx-zynqmp: Connect the SPI devices
xlnx-ep108: Connect the SPI Flash

Peter Crosthwaite (1):
qdev: get_child_bus(): Use QOM lookup if available

 hw/arm/pxa2xx.c                     |  2 +-
 hw/arm/spitz.c                      |  2 +-
 hw/arm/stellaris.c                  |  2 +-
 hw/arm/strongarm.c                  |  2 +-
 hw/arm/tosa.c                       |  2 +-
 hw/arm/xilinx_zynq.c                |  2 +-
 hw/arm/xlnx-ep108.c                 | 16 +++++++++
 hw/arm/xlnx-zynqmp.c                | 30 ++++++++++++++++
 hw/arm/z2.c                         |  2 +-
 hw/block/m25p80.c                   |  3 +-
 hw/core/qdev.c                      |  6 ++++
 hw/display/ads7846.c                |  2 +-
 hw/display/ssd0323.c                |  2 +-
 hw/microblaze/petalogix_ml605_mmu.c |  2 +-
 hw/misc/max111x.c                   |  2 +-
 hw/sd/ssi-sd.c                      |  2 +-
 hw/ssi/pl022.c                      |  2 +-
 hw/ssi/ssi.c                        |  2 +-
 hw/ssi/xilinx_spi.c                 |  2 +-
 hw/ssi/xilinx_spips.c               | 48 +++----------------------
 include/hw/arm/xlnx-zynqmp.h        |  3 ++
 include/hw/{ => ssi}/ssi.h          | 10 +++---
 include/hw/ssi/xilinx_spips.h       | 72 +++++++++++++++++++++++++++++++++++++
 23 files changed, 155 insertions(+), 63 deletions(-)
 rename include/hw/{ => ssi}/ssi.h (96%)
 create mode 100644 include/hw/ssi/xilinx_spips.h
Signature · 1 line
1.9.1
#1 lore raw

PATCH v6 1/6 qdev: get_child_bus(): Use QOM lookup if available

qbus_realize() adds busses as a QOM child of the device in addition to adding it to the qdev bus list. Change get_child_bus() to use the QOM child if it is available. This takes priority over the bus-list, but the child object is checked for type correctness.

This prepares support for aliasing of buses. The use case is SoCs, where a SoC container needs to present buses to the board level, but the buses are implemented by controller IP we already model as self contained qbus-containing devices.

---
Currently qbus_realize() ignores errors from object_property_add_child, so it is hard to guarantee that the QOM linkage is reliable. If it were the case that that object_property_add_child was supposed to be error asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
hw/core/qdev.c +6 -0
index b3ad467..c96c464 100644
@@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
+    Object *child = object_resolve_path_component(OBJECT(dev), name);
+
+    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
+    if (bus) {
+        return bus;
+    }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
         if (strcmp(name, bus->name) == 0) {
Signature · 1 line
1.9.1
#2 lore raw

Re: [Qemu-devel] [PATCH v6 1/6] qdev: get_child_bus(): Use QOM lookup if available

Quoted 4 lines · Peter Crosthwaite

qbus_realize() adds busses as a QOM child of the device in addition to adding it to the qdev bus list. Change get_child_bus() to use the QOM child if it is available. This takes priority over the bus-list, but the child object is checked for type correctness.

This doesn't cause problems for anything else? Is there anything else with the same name or something where this causes conflicts?

Thanks,

Alistair

Quoted 37 lines

This prepares support for aliasing of buses. The use case is SoCs, where a SoC container needs to present buses to the board level, but the buses are implemented by controller IP we already model as self contained qbus-containing devices.

---
Currently qbus_realize() ignores errors from object_property_add_child, so it is hard to guarantee that the QOM linkage is reliable. If it were the case that that object_property_add_child was supposed to be error asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
hw/core/qdev.c +6 -1
index b3ad467..c96c464 100644
@@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
+    Object *child = object_resolve_path_component(OBJECT(dev), name);
+
+    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
+    if (bus) {
+        return bus;
+    }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
         if (strcmp(name, bus->name) == 0) {
--

1.9.1

#3 lore raw

Re: [Qemu-arm] [Qemu-devel] [PATCH v6 1/6] qdev: get_child_bus(): Use QOM lookup if available

Quoted 10 lines · Alistair Francis
Quoted 4 lines · Peter Crosthwaite

qbus_realize() adds busses as a QOM child of the device in addition to adding it to the qdev bus list. Change get_child_bus() to use the QOM child if it is available. This takes priority over the bus-list, but the child object is checked for type correctness.

This doesn't cause problems for anything else? Is there anything else with the same name or something where this causes conflicts?

Not that I know of, I'm just being defensive as it is core code. I suspect that that ignored failure path on the child setter is incorrect but I can't prove it beyond all doubt.

Regards,
Peter

Quoted 41 lines

Thanks,

Alistair

Quoted 37 lines

This prepares support for aliasing of buses. The use case is SoCs, where a SoC container needs to present buses to the board level, but the buses are implemented by controller IP we already model as self contained qbus-containing devices.

---
Currently qbus_realize() ignores errors from object_property_add_child, so it is hard to guarantee that the QOM linkage is reliable. If it were the case that that object_property_add_child was supposed to be error asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
hw/core/qdev.c +6 -1
index b3ad467..c96c464 100644
@@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
+    Object *child = object_resolve_path_component(OBJECT(dev), name);
+
+    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
+    if (bus) {
+        return bus;
+    }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
         if (strcmp(name, bus->name) == 0) {
--

1.9.1

#4 lore raw

Re: [Qemu-devel] [PATCH v6 1/6] qdev: get_child_bus(): Use QOM lookup if available

Quoted 9 lines · Peter Crosthwaite
Quoted 7 lines · Alistair Francis
Quoted 4 lines · Peter Crosthwaite

qbus_realize() adds busses as a QOM child of the device in addition to adding it to the qdev bus list. Change get_child_bus() to use the QOM child if it is available. This takes priority over the bus-list, but the child object is checked for type correctness.

Quoted 7 lines
Quoted 3 lines

This doesn't cause problems for anything else? Is there anything else with the same name or something where this causes conflicts?

Not that I know of, I'm just being defensive as it is core code. I suspect that that ignored failure path on the child setter is incorrect but I can't prove it beyond all doubt.

I just re-read what I said and it doesn't really make sense. My only concern was busses with the same name somehow having conflicts that weren't previously there. I can't think of any case where this would happen though.

Thanks,

Alistair

Quoted 46 lines

Regards,
Peter

Quoted 41 lines

Thanks,

Alistair

Quoted 37 lines

This prepares support for aliasing of buses. The use case is SoCs, where a SoC container needs to present buses to the board level, but the buses are implemented by controller IP we already model as self contained qbus-containing devices.

---
Currently qbus_realize() ignores errors from object_property_add_child, so it is hard to guarantee that the QOM linkage is reliable. If it were the case that that object_property_add_child was supposed to be error asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
hw/core/qdev.c +6 -1
index b3ad467..c96c464 100644
@@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
+    Object *child = object_resolve_path_component(OBJECT(dev), name);
+
+    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
+    if (bus) {
+        return bus;
+    }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
         if (strcmp(name, bus->name) == 0) {
--

1.9.1

#5 lore raw

Re: [Qemu-arm] [PATCH v6 1/6] qdev: get_child_bus(): Use QOM lookup if available

In reply to Peter Crosthwaite
Quoted 27 lines · Peter Crosthwaite

qbus_realize() adds busses as a QOM child of the device in addition to adding it to the qdev bus list. Change get_child_bus() to use the QOM child if it is available. This takes priority over the bus-list, but the child object is checked for type correctness.

This prepares support for aliasing of buses. The use case is SoCs, where a SoC container needs to present buses to the board level, but the buses are implemented by controller IP we already model as self contained qbus-containing devices.

---
Currently qbus_realize() ignores errors from object_property_add_child, so it is hard to guarantee that the QOM linkage is reliable. If it were the case that that object_property_add_child was supposed to be error asserting, we could remove the old bus-list strcmp iterator altogether.

 hw/core/qdev.c | 6 ++++++
 1 file changed, 6 insertions(+)
hw/core/qdev.c +0 -0
index b3ad467..c96c464 100644
@@ -581,6 +581,12 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {

hw/core/qdev.c void qdev_pass_gpios(DeviceState *dev, DeviceState *container, · line 592

Quoted 10 lines
hw/core/qdev.c +6 -0
     BusState *bus;
+    Object *child = object_resolve_path_component(OBJECT(dev), name);
+
+    bus = (BusState *)object_dynamic_cast(child, TYPE_BUS);
+    if (bus) {
+        return bus;
+    }
 
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
         if (strcmp(name, bus->name) == 0) {

This looks OK to me (and I like the effective API you get for devices using things), but I don't know enough QOM/qdev internals to be completely confident giving it a reviewed-by.

thanks
-- PMM

#6 lore raw

PATCH v6 2/6 m25p80.c: Add sst25wf080 SPI flash device

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Add the sst25wf080 SPI flash device.

---

 hw/block/m25p80.c | 1 +
 1 file changed, 1 insertion(+)
hw/block/m25p80.c +1 -0
index efc43dd..7b9f97c 100644
@@ -163,6 +163,7 @@ static const FlashPartInfo known_devices[] = {
     { INFO("sst25wf010",  0xbf2502,      0,  64 << 10,   2, ER_4K) },
     { INFO("sst25wf020",  0xbf2503,      0,  64 << 10,   4, ER_4K) },
     { INFO("sst25wf040",  0xbf2504,      0,  64 << 10,   8, ER_4K) },
+    { INFO("sst25wf080",  0xbf2505,      0,  64 << 10,  16, ER_4K) },
 
     /* ST Microelectronics -- newer production may have feature updates */
     { INFO("m25p05",      0x202010,      0,  32 << 10,   2, 0) },
Signature · 1 line
1.9.1
#7 lore raw

PATCH v6 3/6 ssi: Move ssi.h into a separate directory

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Move the ssi.h include file into the ssi directory.

While touching the code also fix the typdef lines as checkpatch complains.

---

 hw/arm/pxa2xx.c                     |  2 +-
 hw/arm/spitz.c                      |  2 +-
 hw/arm/stellaris.c                  |  2 +-
 hw/arm/strongarm.c                  |  2 +-
 hw/arm/tosa.c                       |  2 +-
 hw/arm/xilinx_zynq.c                |  2 +-
 hw/arm/z2.c                         |  2 +-
 hw/block/m25p80.c                   |  2 +-
 hw/display/ads7846.c                |  2 +-
 hw/display/ssd0323.c                |  2 +-
 hw/microblaze/petalogix_ml605_mmu.c |  2 +-
 hw/misc/max111x.c                   |  2 +-
 hw/sd/ssi-sd.c                      |  2 +-
 hw/ssi/pl022.c                      |  2 +-
 hw/ssi/ssi.c                        |  2 +-
 hw/ssi/xilinx_spi.c                 |  2 +-
 hw/ssi/xilinx_spips.c               |  2 +-
 include/hw/{ => ssi}/ssi.h          | 10 ++++++----
 18 files changed, 23 insertions(+), 21 deletions(-)
 rename include/hw/{ => ssi}/ssi.h (96%)
hw/arm/pxa2xx.c +1 -1
index 79d22d9..54bf152 100644
@@ -12,7 +12,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/char/serial.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "sysemu/char.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
hw/arm/spitz.c +1 -1
index 8d3cc0b..ee8f889 100644
@@ -16,7 +16,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/pcmcia.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/block/flash.h"
 #include "qemu/timer.h"
 #include "hw/devices.h"
hw/arm/stellaris.c +1 -1
index 0114e0a..4e5cfd1 100644
@@ -8,7 +8,7 @@
  */
 
 #include "hw/sysbus.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/arm/arm.h"
 #include "hw/devices.h"
 #include "qemu/timer.h"
hw/arm/strongarm.c +1 -1
index 9624ecb..4d2ba02 100644
@@ -34,7 +34,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 //#define DEBUG
 
hw/arm/tosa.c +1 -1
index 02814d7..68ad01e 100644
@@ -19,7 +19,7 @@
 #include "hw/pcmcia.h"
 #include "hw/boards.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "sysemu/block-backend.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
hw/arm/xilinx_zynq.c +1 -1
index 1c1a445..11a349b 100644
@@ -25,7 +25,7 @@
 #include "sysemu/block-backend.h"
 #include "hw/loader.h"
 #include "hw/misc/zynq-xadc.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "qemu/error-report.h"
 
 #define NUM_SPI_FLASHES 4
hw/arm/z2.c +1 -1
index b44eb76..c82fe2c 100644
@@ -16,7 +16,7 @@
 #include "hw/arm/arm.h"
 #include "hw/devices.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/boards.h"
 #include "sysemu/sysemu.h"
 #include "hw/block/flash.h"
hw/block/m25p80.c +1 -1
index 7b9f97c..addd907 100644
@@ -24,7 +24,7 @@
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 #ifndef M25P80_ERR_DEBUG
 #define M25P80_ERR_DEBUG 0
hw/display/ads7846.c +1 -1
index 3f35369..cb82317 100644
@@ -10,7 +10,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "ui/console.h"
 
 typedef struct {
hw/display/ssd0323.c +1 -1
index 9727007..7545da8 100644
@@ -10,7 +10,7 @@
 /* The controller can support a variety of different displays, but we only
    implement one.  Most of the commends relating to brightness and geometry
    setup are ignored. */
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "ui/console.h"
 
 //#define DEBUG_SSD0323 1
hw/microblaze/petalogix_ml605_mmu.c +1 -1
index 462060f..5366cec 100644
@@ -35,7 +35,7 @@
 #include "sysemu/block-backend.h"
 #include "hw/char/serial.h"
 #include "exec/address-spaces.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 #include "boot.h"
 
hw/misc/max111x.c +1 -1
index bef3651..d619d61 100644
@@ -10,7 +10,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 typedef struct {
     SSISlave parent_obj;
hw/sd/ssi-sd.c +1 -1
index c49ff62..eeb96b9 100644
@@ -12,7 +12,7 @@
 
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/sd/sd.h"
 
 //#define DEBUG_SSI_SD 1
hw/ssi/pl022.c +1 -1
index 61d568f..0bbf633 100644
@@ -8,7 +8,7 @@
  */
 
 #include "hw/sysbus.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 //#define DEBUG_PL022 1
 
hw/ssi/ssi.c +1 -1
index 2aab79b..a0f57c0 100644
@@ -12,7 +12,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 struct SSIBus {
     BusState parent_obj;
hw/ssi/xilinx_spi.c +1 -1
index 620573c..94bb2a7 100644
@@ -29,7 +29,7 @@
 #include "qemu/log.h"
 #include "qemu/fifo8.h"
 
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 #ifdef XILINX_SPI_ERR_DEBUG
 #define DB_PRINT(...) do { \
hw/ssi/xilinx_spips.c +1 -1
index 0910f54..e9471ff 100644
@@ -27,7 +27,7 @@
 #include "hw/ptimer.h"
 #include "qemu/log.h"
 #include "qemu/fifo8.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "qemu/bitops.h"
 
 #ifndef XILINX_SPIPS_ERR_DEBUG
include/hw/ssi/ssi.h renamed from include/hw/ssi.h +6 -4
similarity index 96%
rename from include/hw/ssi.h
rename to include/hw/ssi/ssi.h
index df0f838..4a0a539 100644
@@ -14,6 +14,8 @@
 #include "hw/qdev.h"
 
 typedef struct SSISlave SSISlave;
+typedef struct SSISlaveClass SSISlaveClass;
+typedef enum SSICSMode SSICSMode;
 
 #define TYPE_SSI_SLAVE "ssi-slave"
 #define SSI_SLAVE(obj) \
@@ -25,14 +27,14 @@ typedef struct SSISlave SSISlave;
 
 #define SSI_GPIO_CS "ssi-gpio-cs"
 
-typedef enum {
+enum SSICSMode {
     SSI_CS_NONE = 0,
     SSI_CS_LOW,
     SSI_CS_HIGH,
-} SSICSMode;
+};
 
 /* Slave devices.  */
-typedef struct SSISlaveClass {
+struct SSISlaveClass {
     DeviceClass parent_class;
 
     int (*init)(SSISlave *dev);
@@ -55,7 +57,7 @@ typedef struct SSISlaveClass {
      * always be called for the device for every txrx access to the parent bus
      */
     uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
-} SSISlaveClass;
+};
 
 struct SSISlave {
     DeviceState parent_obj;
Signature · 1 line
1.9.1
#8 lore raw

PATCH v6 4/6 xilinx_spips: Separate the state struct into a header

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Separate out the XilinxSPIPS struct into a separate header file.

---

 hw/ssi/xilinx_spips.c         | 46 +++------------------------
 include/hw/ssi/xilinx_spips.h | 72 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/ssi/xilinx_spips.h
hw/ssi/xilinx_spips.c +4 -42
index e9471ff..2111719 100644
@@ -29,6 +29,7 @@
 #include "qemu/fifo8.h"
 #include "hw/ssi/ssi.h"
 #include "qemu/bitops.h"
+#include "hw/ssi/xilinx_spips.h"
 
 #ifndef XILINX_SPIPS_ERR_DEBUG
 #define XILINX_SPIPS_ERR_DEBUG 0
@@ -103,8 +104,6 @@
 
 #define R_MOD_ID            (0xFC / 4)
 
-#define R_MAX (R_MOD_ID+1)
-
 /* size of TXRX FIFOs */
 #define RXFF_A          32
 #define TXFF_A          32
@@ -135,30 +134,6 @@ typedef enum {
 } FlashCMD;
 
 typedef struct {
-    SysBusDevice parent_obj;
-
-    MemoryRegion iomem;
-    MemoryRegion mmlqspi;
-
-    qemu_irq irq;
-    int irqline;
-
-    uint8_t num_cs;
-    uint8_t num_busses;
-
-    uint8_t snoop_state;
-    qemu_irq *cs_lines;
-    SSIBus **spi;
-
-    Fifo8 rx_fifo;
-    Fifo8 tx_fifo;
-
-    uint8_t num_txrx_bytes;
-
-    uint32_t regs[R_MAX];
-} XilinxSPIPS;
-
-typedef struct {
     XilinxSPIPS parent_obj;
 
     uint8_t lqspi_buf[LQSPI_CACHE_SIZE];
@@ -174,19 +149,6 @@ typedef struct XilinxSPIPSClass {
     uint32_t tx_fifo_size;
 } XilinxSPIPSClass;
 
-#define TYPE_XILINX_SPIPS "xlnx.ps7-spi"
-#define TYPE_XILINX_QSPIPS "xlnx.ps7-qspi"
-
-#define XILINX_SPIPS(obj) \
-     OBJECT_CHECK(XilinxSPIPS, (obj), TYPE_XILINX_SPIPS)
-#define XILINX_SPIPS_CLASS(klass) \
-     OBJECT_CLASS_CHECK(XilinxSPIPSClass, (klass), TYPE_XILINX_SPIPS)
-#define XILINX_SPIPS_GET_CLASS(obj) \
-     OBJECT_GET_CLASS(XilinxSPIPSClass, (obj), TYPE_XILINX_SPIPS)
-
-#define XILINX_QSPIPS(obj) \
-     OBJECT_CHECK(XilinxQSPIPS, (obj), TYPE_XILINX_QSPIPS)
-
 static inline int num_effective_busses(XilinxSPIPS *s)
 {
     return (s->regs[R_LQSPI_CFG] & LQSPI_CFG_SEP_BUS &&
@@ -257,7 +219,7 @@ static void xilinx_spips_reset(DeviceState *d)
     XilinxSPIPS *s = XILINX_SPIPS(d);
 
     int i;
-    for (i = 0; i < R_MAX; i++) {
+    for (i = 0; i < XLNX_SPIPS_R_MAX; i++) {
         s->regs[i] = 0;
     }
 
@@ -664,7 +626,7 @@ static void xilinx_spips_realize(DeviceState *dev, Error **errp)
     }
 
     memory_region_init_io(&s->iomem, OBJECT(s), xsc->reg_ops, s,
-                          "spi", R_MAX*4);
+                          "spi", XLNX_SPIPS_R_MAX*4);
     sysbus_init_mmio(sbd, &s->iomem);
 
     s->irqline = -1;
@@ -708,7 +670,7 @@ static const VMStateDescription vmstate_xilinx_spips = {
     .fields = (VMStateField[]) {
         VMSTATE_FIFO8(tx_fifo, XilinxSPIPS),
         VMSTATE_FIFO8(rx_fifo, XilinxSPIPS),
-        VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, R_MAX),
+        VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, XLNX_SPIPS_R_MAX),
         VMSTATE_UINT8(snoop_state, XilinxSPIPS),
         VMSTATE_END_OF_LIST()
     }
include/hw/ssi/xilinx_spips.h new file +72 -0

72 lines not on this page · raw message

Signature · 1 line
1.9.1
#9 lore raw

PATCH v6 5/6 xlnx-zynqmp: Connect the SPI devices

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Connect the Xilinx SPI devices to the ZynqMP model.

[ PC changes
* Use QOM alias for bus connectivity on SoC level
]

---
V6:
- Use QOM alias for bus connectivity on SoC level
V5:
- Use the bus renaming function
V4:
- Rename the SPI busses so that they can all be accessed from the SoC
- Don't set the num-busses property
V3:
- Expose the SPI Bus as part of the SoC device
V2:
- Don't connect the SPI flash to the SoC

 hw/arm/xlnx-zynqmp.c         | 30 ++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h |  3 +++
 2 files changed, 33 insertions(+)
hw/arm/xlnx-zynqmp.c +30 -0
index 87553bb..f2e13a4 100644
@@ -56,6 +56,14 @@ static const int sdhci_intr[XLNX_ZYNQMP_NUM_SDHCI] = {
     48, 49,
 };
 
+static const uint64_t spi_addr[XLNX_ZYNQMP_NUM_SPIS] = {
+    0xFF040000, 0xFF050000,
+};
+
+static const int spi_intr[XLNX_ZYNQMP_NUM_SPIS] = {
+    19, 20,
+};
+
 typedef struct XlnxZynqMPGICRegion {
     int region_index;
     uint32_t address;
@@ -112,6 +120,12 @@ static void xlnx_zynqmp_init(Object *obj)
         qdev_set_parent_bus(DEVICE(&s->sdhci[i]),
                             sysbus_get_default());
     }
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+        object_initialize(&s->spi[i], sizeof(s->spi[i]),
+                          TYPE_XILINX_SPIPS);
+        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+    }
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -286,6 +300,22 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci[i]), 0,
                            gic_spi[sdhci_intr[i]]);
     }
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+        char bus_name[6];
+
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
+                           gic_spi[spi_intr[i]]);
+
+        /* Alias controller SPI bus to the SoC itself */
+        snprintf(bus_name, 6, "spi%d", i);
+        object_property_add_alias(OBJECT(s), bus_name,
+                                  OBJECT(&s->spi[i]), "spi0",
+                                  &error_abort);
+    }
 }
 
 static Property xlnx_zynqmp_props[] = {
include/hw/arm/xlnx-zynqmp.h +3 -0
index d116092..f598a43 100644
@@ -25,6 +25,7 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/sd/sdhci.h"
+#include "hw/ssi/xilinx_spips.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -35,6 +36,7 @@
 #define XLNX_ZYNQMP_NUM_GEMS 4
 #define XLNX_ZYNQMP_NUM_UARTS 2
 #define XLNX_ZYNQMP_NUM_SDHCI 2
+#define XLNX_ZYNQMP_NUM_SPIS 2
 
 #define XLNX_ZYNQMP_NUM_OCM_BANKS 4
 #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC0000
@@ -66,6 +68,7 @@ typedef struct XlnxZynqMPState {
     CadenceUARTState uart[XLNX_ZYNQMP_NUM_UARTS];
     SysbusAHCIState sata;
     SDHCIState sdhci[XLNX_ZYNQMP_NUM_SDHCI];
+    XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
Signature · 1 line
1.9.1
#10 lore raw

Re: [Qemu-arm] [PATCH v6 5/6] xlnx-zynqmp: Connect the SPI devices

Quoted 16 lines · Peter Crosthwaite

From: Alistair Francis <[email protected]>

Connect the Xilinx SPI devices to the ZynqMP model.

[ PC changes
* Use QOM alias for bus connectivity on SoC level
]

hw/arm/xlnx-zynqmp.c +2 -0
@@ -286,6 +300,22 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci[i]), 0,
                            gic_spi[sdhci_intr[i]]);
     }
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {

hw/arm/xlnx-zynqmp.c static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) · line 314

Quoted 10 lines
hw/arm/xlnx-zynqmp.c +10 -0
+        char bus_name[6];
+
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
+                           gic_spi[spi_intr[i]]);
+
+        /* Alias controller SPI bus to the SoC itself */
+        snprintf(bus_name, 6, "spi%d", i);

I don't much like these hard coded 6s. This is in init which isn't a hot path, so the simplest thing is to use g_strdup_printf() and then g_free() it when we're done.

Similarly in patch 6.

Otherwise I like this; I did a quick tweak to my sd card patchset to use this approach and it seems to at least compile and do the right thing in info qtree (I don't have a xilinx test image to do more).

Haven't looked at the detail of how we're doing this in patch 1 yet.

thanks
-- PMM

#11 lore raw

Re: [Qemu-arm] [PATCH v6 5/6] xlnx-zynqmp: Connect the SPI devices

Quoted 18 lines · Peter Maydell

hw/arm/xlnx-zynqmp.c static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) · line 314

Quoted 11 lines · Peter Crosthwaite
hw/arm/xlnx-zynqmp.c +11 -0
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+        char bus_name[6];
+
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
+                           gic_spi[spi_intr[i]]);
+
+        /* Alias controller SPI bus to the SoC itself */
+        snprintf(bus_name, 6, "spi%d", i);

I don't much like these hard coded 6s. This is in init which isn't a hot path, so the simplest thing is to use g_strdup_printf() and then g_free() it when we're done.

Quoted 8 lines

Similarly in patch 6.

Otherwise I like this; I did a quick tweak to my sd card patchset to use this approach and it seems to at least compile and do the right thing in info qtree (I don't have a xilinx test image to do more).

Haven't looked at the detail of how we're doing this in patch 1 yet.

Hi Peter (and/or Alistair) -- do you have time to do a quick respin of this patchset to use g_strdup_printf() in these cases? It would be nice to get this series into the tree, because my sd card rework patchset depends on it...

thanks
-- PMM

#12 lore raw

Re: [Qemu-arm] [Qemu-devel] [PATCH v6 5/6] xlnx-zynqmp: Connect the SPI devices

Quoted 26 lines · Peter Maydell
Quoted 25 lines · Peter Maydell

hw/arm/xlnx-zynqmp.c static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) · line 314

Quoted 11 lines · Peter Crosthwaite
hw/arm/xlnx-zynqmp.c +11 -0
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+        char bus_name[6];
+
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
+                           gic_spi[spi_intr[i]]);
+
+        /* Alias controller SPI bus to the SoC itself */
+        snprintf(bus_name, 6, "spi%d", i);

I don't much like these hard coded 6s. This is in init which isn't a hot path, so the simplest thing is to use g_strdup_printf() and then g_free() it when we're done.

Similarly in patch 6.

Otherwise I like this; I did a quick tweak to my sd card patchset to use this approach and it seems to at least compile and do the right thing in info qtree (I don't have a xilinx test image to do more).

Quoted 6 lines
Quoted 1 line

Haven't looked at the detail of how we're doing this in patch 1 yet.

Hi Peter (and/or Alistair) -- do you have time to do a quick respin of this patchset to use g_strdup_printf() in these cases? It would be nice to get this series into the tree, because my sd card rework patchset depends on it...

Yep! I'll do it now.

Thanks,

Alistair

Quoted 4 lines

thanks
-- PMM

#13 lore raw

PATCH v6 6/6 xlnx-ep108: Connect the SPI Flash

In reply to Peter Crosthwaite

From: Alistair Francis <[email protected]>

Connect the sst25wf080 SPI flash to the EP108 board.

---

 hw/arm/xlnx-ep108.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
hw/arm/xlnx-ep108.c +16 -0
index 85b978f..d1912aa 100644
@@ -33,6 +33,7 @@ static struct arm_boot_info xlnx_ep108_binfo;
 static void xlnx_ep108_init(MachineState *machine)
 {
     XlnxEP108 *s = g_new0(XlnxEP108, 1);
+    int i;
     Error *err = NULL;
 
     object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
@@ -60,6 +61,21 @@ static void xlnx_ep108_init(MachineState *machine)
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
 
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+        SSIBus *spi_bus;
+        DeviceState *flash_dev;
+        qemu_irq cs_line;
+        char bus_name[6];
+
+        snprintf(bus_name, 6, "spi%d", i);
+        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), bus_name);
+
+        flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
+        cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
+
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
+    }
+
     xlnx_ep108_binfo.ram_size = machine->ram_size;
     xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
     xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
Signature · 1 line
1.9.1