On Fri, Oct 27, 2017 at 2:48 PM, francisco iglesias <frasse.igles...@gmail.com> wrote: > Good day Alistair, > > Yes exactly! The qspi aliases on the soc (named "qspi%d") targets the spi > busses on the qspi dev (named "spi%d"). I did it this way in hope that the > code resembles how we connect the SST flashes to the standard spi devices. > Would you like me to do this in an other way or do you possibly think the > approach is ok?
This looks fine to me. Once you make those new line changes I mentioned: Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> Alistair > > Best regards, > Francisco Iglesias > > > On 27 Oct 2017 14:03, "Alistair Francis" <alistai...@gmail.com> wrote: > > On Fri, Oct 27, 2017 at 11:17 AM, francisco iglesias > <frasse.igles...@gmail.com> wrote: >> Dear alistair, >> >> Thank you for the review comments! I will update according them in the >> next >> version of the patch series! About your question, I might have >> misunderstod, >> but isn't the alias "qspi"? (s/"qspi{0,1}" -> s->qspi/"spi{0,1}") > > Ah! Now I think I understand. The buses on the QSPI device are still > labeled "spi", is that correct? > > Alistair > >> >> Best regards, >> Francisco Iglesias >> >> On 27 Oct 2017 10:49, "Alistair Francis" <alistai...@gmail.com> wrote: >>> >>> On Fri, Oct 27, 2017 at 7:56 AM, Francisco Iglesias >>> <frasse.igles...@gmail.com> wrote: >>> > Add support for the ZynqMP QSPI (consisting of the Generic QSPI and >>> > Legacy >>> > QSPI) and connect Numonyx n25q512a11 flashes to it. >>> > >>> > Signed-off-by: Francisco Iglesias <frasse.igles...@gmail.com> >>> > --- >>> > hw/arm/xlnx-zcu102.c | 23 +++++++++++++++++++++++ >>> > hw/arm/xlnx-zynqmp.c | 24 ++++++++++++++++++++++++ >>> > include/hw/arm/xlnx-zynqmp.h | 5 +++++ >>> > 3 files changed, 52 insertions(+) >>> > >>> > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c >>> > index 519a16e..7d61972 100644 >>> > --- a/hw/arm/xlnx-zcu102.c >>> > +++ b/hw/arm/xlnx-zcu102.c >>> > @@ -150,6 +150,29 @@ static void xlnx_zynqmp_init(XlnxZCU102 *s, >>> > MachineState *machine) >>> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, >>> > cs_line); >>> > } >>> > >>> > + for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_FLASH; i++) { >>> > + SSIBus *spi_bus; >>> > + DeviceState *flash_dev; >>> > + qemu_irq cs_line; >>> > + DriveInfo *dinfo = drive_get_next(IF_MTD); >>> > + int bus = i / XLNX_ZYNQMP_NUM_QSPI_BUS_CS; >>> > + gchar *bus_name = g_strdup_printf("qspi%d", bus); >>> > + >>> > + spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), >>> > bus_name); >>> > + g_free(bus_name); >>> > + >>> > + flash_dev = ssi_create_slave_no_init(spi_bus, "n25q512a11"); >>> > + if (dinfo) { >>> > + qdev_prop_set_drive(flash_dev, "drive", >>> > blk_by_legacy_dinfo(dinfo), >>> > + &error_fatal); >>> > + } >>> > + qdev_init_nofail(flash_dev); >>> > + >>> > + cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0); >>> > + >>> > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.qspi), i + 1, >>> > cs_line); >>> > + } >>> > + >>> > /* TODO create and connect IDE devices for ide_drive_get() */ >>> > >>> > xlnx_zcu102_binfo.ram_size = ram_size; >>> > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c >>> > index d4b6560..f7c8b4b 100644 >>> > --- a/hw/arm/xlnx-zynqmp.c >>> > +++ b/hw/arm/xlnx-zynqmp.c >>> > @@ -40,6 +40,10 @@ >>> > #define SATA_ADDR 0xFD0C0000 >>> > #define SATA_NUM_PORTS 2 >>> > >>> > +#define QSPI_ADDR 0xff0f0000 >>> > +#define LQSPI_ADDR 0xc0000000 >>> > +#define QSPI_IRQ 15 >>> > + >>> > #define DP_ADDR 0xfd4a0000 >>> > #define DP_IRQ 113 >>> > >>> > @@ -169,6 +173,9 @@ static void xlnx_zynqmp_init(Object *obj) >>> > qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default()); >>> > } >>> > >>> > + object_initialize(&s->qspi, sizeof(s->qspi), >>> > TYPE_XLNX_ZYNQMP_QSPIPS); >>> > + qdev_set_parent_bus(DEVICE(&s->qspi), sysbus_get_default()); >>> > + >>> > object_initialize(&s->dp, sizeof(s->dp), TYPE_XLNX_DP); >>> > qdev_set_parent_bus(DEVICE(&s->dp), sysbus_get_default()); >>> > >>> > @@ -405,6 +412,23 @@ static void xlnx_zynqmp_realize(DeviceState *dev, >>> > Error **errp) >>> > g_free(bus_name); >>> > } >>> > >>> > + object_property_set_bool(OBJECT(&s->qspi), true, "realized", >>> > &err); >>> > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR); >>> > + sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR); >>> > + sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, >>> > gic_spi[QSPI_IRQ]); >>> >>> New line here >>> >>> > + for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) { >>> > + gchar *bus_name; >>> > + gchar *target_bus; >>> >>> New line here >>> >>> > + /* Alias controller SPI bus to the SoC itself */ >>> > + bus_name = g_strdup_printf("qspi%d", i); >>> > + target_bus = g_strdup_printf("spi%d", i); >>> > + object_property_add_alias(OBJECT(s), bus_name, >>> > + OBJECT(&s->qspi), target_bus, >>> >>> Why do we alias qspi to spi? >>> >>> Alistair >>> >>> > + &error_abort); >>> > + g_free(bus_name); >>> > + g_free(target_bus); >>> > + } >>> > + >>> > object_property_set_bool(OBJECT(&s->dp), true, "realized", &err); >>> > if (err) { >>> > error_propagate(errp, err); >>> > diff --git a/include/hw/arm/xlnx-zynqmp.h >>> > b/include/hw/arm/xlnx-zynqmp.h >>> > index 6eff81a..3e6fb9b 100644 >>> > --- a/include/hw/arm/xlnx-zynqmp.h >>> > +++ b/include/hw/arm/xlnx-zynqmp.h >>> > @@ -40,6 +40,10 @@ >>> > #define XLNX_ZYNQMP_NUM_SDHCI 2 >>> > #define XLNX_ZYNQMP_NUM_SPIS 2 >>> > >>> > +#define XLNX_ZYNQMP_NUM_QSPI_BUS 2 >>> > +#define XLNX_ZYNQMP_NUM_QSPI_BUS_CS 2 >>> > +#define XLNX_ZYNQMP_NUM_QSPI_FLASH 4 >>> > + >>> > #define XLNX_ZYNQMP_NUM_OCM_BANKS 4 >>> > #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC0000 >>> > #define XLNX_ZYNQMP_OCM_RAM_SIZE 0x10000 >>> > @@ -83,6 +87,7 @@ typedef struct XlnxZynqMPState { >>> > SysbusAHCIState sata; >>> > SDHCIState sdhci[XLNX_ZYNQMP_NUM_SDHCI]; >>> > XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS]; >>> > + XlnxZynqMPQSPIPS qspi; >>> > XlnxDPState dp; >>> > XlnxDPDMAState dpdma; >>> > >>> > -- >>> > 2.9.3 >>> > >>> > > >