Re: [Qemu-devel] [PATCH v1 3/3] xlnx-zynqmp: Connect the SPI devices

[Alistair Francis]

On Wed, Sep 30, 2015 at 2:15 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Tue, Sep 29, 2015 at 4:03 PM, Alistair Francis
> <address@hidden> wrote:
>> Connect the Xilinx SPI device to the ZynqMP model.
>>
>> Signed-off-by: Alistair Francis <address@hidden>
>> ---
>>
>>  hw/arm/xlnx-zynqmp.c         | 46 
>> +++++++++++++++++++++++++++++++++++++++++++-
>>  include/hw/arm/xlnx-zynqmp.h |  4 ++++
>>  2 files changed, 49 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index a9097f9..4b8d095 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -48,6 +48,14 @@ static const int uart_intr[XLNX_ZYNQMP_NUM_UARTS] = {
>>      21, 22,
>>  };
>>
>> +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;
>> @@ -97,13 +105,19 @@ static void xlnx_zynqmp_init(Object *obj)
>>
>>      object_initialize(&s->sata, sizeof(s->sata), TYPE_SYSBUS_AHCI);
>>      qdev_set_parent_bus(DEVICE(&s->sata), 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)
>>  {
>>      XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
>>      MemoryRegion *system_memory = get_system_memory();
>> -    uint8_t i;
>> +    uint8_t i, j;
>>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>>      qemu_irq gic_spi[GIC_NUM_SPI_INTR];
>>      Error *err = NULL;
>> @@ -258,6 +272,36 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
>> **errp)
>>
>>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->sata), 0, SATA_ADDR);
>>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->sata), 0, gic_spi[SATA_INTR]);
>> +
>> +    for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
>> +        SSIBus *spi_bus;
>> +        char bus_name[6];
>> +
>> +        object_property_set_int(OBJECT(&s->spi[i]), XLNX_ZYNQMP_NUM_SPIS,
>> +                                "num-busses", &error_abort);
>> +        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", 
>> &err);
>> +        if (err) {
>> +            error_propagate(errp, err);
>> +            return;
>> +        }
>> +
>> +        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]]);
>> +
>> +        snprintf(bus_name, 6, "spi%d", i);
>> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->spi[i]), 
>> bus_name);
>> +
>> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
>> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "m25p80");
>
> Are they actual m25p80's or are you trying to be generic? "M25P80' is
> overloaded, often used to mean the greater family of SPI flashes while
> also being a specific part. In this usage, it means the specific part.
> M25P80 is a very old part.

Good point, I'll update it with the actual part.

>
> This should however be on the machine level. The bus needs to be
> connected to the SoC object as a child bus, and the board level
> creates the flashes (ep108).

Ok, fixed in v2.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
>> +                                                      SSI_GPIO_CS, 0);
>> +
>> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]),
>> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
>> +                               cs_line);
>> +        }
>> +    }
>>  }
>>
>>  static Property xlnx_zynqmp_props[] = {
>> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
>> index 4005a99..6d1d2a9 100644
>> --- a/include/hw/arm/xlnx-zynqmp.h
>> +++ b/include/hw/arm/xlnx-zynqmp.h
>> @@ -24,6 +24,7 @@
>>  #include "hw/char/cadence_uart.h"
>>  #include "hw/ide/pci.h"
>>  #include "hw/ide/ahci.h"
>> +#include "hw/ssi/xilinx_spips.h"
>>
>>  #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
>>  #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
>> @@ -33,6 +34,8 @@
>>  #define XLNX_ZYNQMP_NUM_RPU_CPUS 2
>>  #define XLNX_ZYNQMP_NUM_GEMS 4
>>  #define XLNX_ZYNQMP_NUM_UARTS 2
>> +#define XLNX_ZYNQMP_NUM_SPIS 2
>> +#define XLNX_ZYNQMP_NUM_SPI_FLASHES 4
>>
>>  #define XLNX_ZYNQMP_NUM_OCM_BANKS 4
>>  #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC0000
>> @@ -63,6 +66,7 @@ typedef struct XlnxZynqMPState {
>>      CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
>>      CadenceUARTState uart[XLNX_ZYNQMP_NUM_UARTS];
>>      SysbusAHCIState sata;
>> +    XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
>>
>>      char *boot_cpu;
>>      ARMCPU *boot_cpu_ptr;
>> --
>> 1.9.1
>>
>