From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48565) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZriYK-0005lS-Js for qemu-devel@nongnu.org; Thu, 29 Oct 2015 04:27:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZriYH-0007nO-D5 for qemu-devel@nongnu.org; Thu, 29 Oct 2015 04:27:20 -0400 Received: from greensocs.com ([193.104.36.180]:46074) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZriYH-0007mY-1W for qemu-devel@nongnu.org; Thu, 29 Oct 2015 04:27:17 -0400 Message-ID: <5631D858.7060001@greensocs.com> Date: Thu, 29 Oct 2015 09:27:04 +0100 From: Frederic Konrad MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 4/5] xlnx-zynqmp: Connect the SPI devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Edgar Iglesias , Peter Maydell , "Edgar E. Iglesias" , "qemu-devel@nongnu.org Developers" , Alistair Francis On 29/10/2015 03:00, Peter Crosthwaite wrote: > On Wed, Oct 28, 2015 at 10:32 AM, Alistair Francis < > alistair.francis@xilinx.com> wrote: > >> Connect the Xilinx SPI device to the ZynqMP model. >> >> > "devices" > > >> Signed-off-by: Alistair Francis >> --- >> 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 | 37 +++++++++++++++++++++++++++++++++++++ >> include/hw/arm/xlnx-zynqmp.h | 4 ++++ >> 2 files changed, 41 insertions(+) >> >> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c >> index b36ca3d..5671d7a 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,6 +105,12 @@ 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) >> @@ -258,6 +272,29 @@ 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++) { >> + BusState *spi_bus; >> + char bus_name[6]; >> + >> + object_property_set_int(OBJECT(&s->spi[i]), XLNX_ZYNQMP_NUM_SPIS, >> + "num-busses", &error_abort); >> > The number of busses-per-controller is unrelated to the number of > controllers. Setting num_busses != 1 is primarily a QSPI thing, so should > this just default to 1? I think you can drop this setter completely. > > >> + 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 = qdev_get_child_bus(DEVICE(&s->spi), bus_name); >> + >> + /* Add the SPI buses to the SoC child bus */ >> + QLIST_INSERT_HEAD(&dev->child_bus, spi_bus, sibling); >> > Nice! That is pretty simple in the end. One, question though, what happen > with info qtree? Do you get doubles because the bus is double parented? > > I think this concept also might apply to the DP/DPDMA work, where the > display port (or AUX bus?) should be put on the SoC container. Then the > machine model (ep108) is responsible for detecting if the user wants a > display and connecting it. I.e. the DP controller shouldn't be doing the UI > init. You mean get the AUX and I2C bus here and connect the edid and the dpcd? I can take a look. Fred > >> + } >> } >> >> 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 >> > NUM_SPI_FLASHES is local to ep108 so it should just be in ep108.c > > Regards, > Peter > > >> #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; >> -- >> 2.5.0 >> >>