diff mbox

[v2,5/5] xlnx-ep108: Connect the SPI Flash

Message ID 1381679a85ad75a4b80d553d33608cffef3c9868.1444251567.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis Oct. 7, 2015, 9:34 p.m. UTC
Connect the sst25wf080 SPI flash to the EP108 board.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V2:
 - Use sst25wf080 instead of m25p80

 hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Peter Crosthwaite Oct. 8, 2015, 12:01 a.m. UTC | #1
On Wed, Oct 7, 2015 at 2:34 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Connect the sst25wf080 SPI flash to the EP108 board.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> V2:
>  - Use sst25wf080 instead of m25p80
>
>  hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> index 2899698..9755c30 100644
> --- a/hw/arm/xlnx-ep108.c
> +++ b/hw/arm/xlnx-ep108.c
> @@ -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, j;
>      Error *err = NULL;
>
>      object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
> @@ -60,6 +61,25 @@ 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;
> +        char bus_name[6];
> +
> +        snprintf(bus_name, 6, "spi%d", i);
> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
> +                                               bus_name);

So machine level code shouldn't reach into the SoC like this. Instead
the bus should be passed from the SPI controller to SoC itself, then
use qdev_get_child_bus on the SoC itself.

Regards,
Peter

> +
> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
> +                                                      SSI_GPIO_CS, 0);
> +
> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
> +                               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;
> --
> 2.1.4
>
Alistair Francis Oct. 8, 2015, 11:30 p.m. UTC | #2
On Wed, Oct 7, 2015 at 5:01 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Oct 7, 2015 at 2:34 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> Connect the sst25wf080 SPI flash to the EP108 board.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>> V2:
>>  - Use sst25wf080 instead of m25p80
>>
>>  hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>> index 2899698..9755c30 100644
>> --- a/hw/arm/xlnx-ep108.c
>> +++ b/hw/arm/xlnx-ep108.c
>> @@ -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, j;
>>      Error *err = NULL;
>>
>>      object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
>> @@ -60,6 +61,25 @@ 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;
>> +        char bus_name[6];
>> +
>> +        snprintf(bus_name, 6, "spi%d", i);
>> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
>> +                                               bus_name);
>
> So machine level code shouldn't reach into the SoC like this. Instead
> the bus should be passed from the SPI controller to SoC itself, then
> use qdev_get_child_bus on the SoC itself.

I'm not sure what you mean. The board needs multiple buses and I can't
see how it can do that without reaching into the SoC.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> +
>> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
>> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
>> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
>> +                                                      SSI_GPIO_CS, 0);
>> +
>> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
>> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
>> +                               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;
>> --
>> 2.1.4
>>
>
Alistair Francis Oct. 14, 2015, 6:41 p.m. UTC | #3
On Thu, Oct 8, 2015 at 4:30 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Wed, Oct 7, 2015 at 5:01 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Oct 7, 2015 at 2:34 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> Connect the sst25wf080 SPI flash to the EP108 board.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>> V2:
>>>  - Use sst25wf080 instead of m25p80
>>>
>>>  hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>>> index 2899698..9755c30 100644
>>> --- a/hw/arm/xlnx-ep108.c
>>> +++ b/hw/arm/xlnx-ep108.c
>>> @@ -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, j;
>>>      Error *err = NULL;
>>>
>>>      object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
>>> @@ -60,6 +61,25 @@ 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;
>>> +        char bus_name[6];
>>> +
>>> +        snprintf(bus_name, 6, "spi%d", i);
>>> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
>>> +                                               bus_name);
>>
>> So machine level code shouldn't reach into the SoC like this. Instead
>> the bus should be passed from the SPI controller to SoC itself, then
>> use qdev_get_child_bus on the SoC itself.
>
> I'm not sure what you mean. The board needs multiple buses and I can't
> see how it can do that without reaching into the SoC.

Any updates on this?

Thanks,

Alistair

>
> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> +
>>> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
>>> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
>>> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
>>> +                                                      SSI_GPIO_CS, 0);
>>> +
>>> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
>>> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
>>> +                               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;
>>> --
>>> 2.1.4
>>>
>>
Peter Crosthwaite Oct. 14, 2015, 7:25 p.m. UTC | #4
On Thu, Oct 8, 2015 at 4:30 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Wed, Oct 7, 2015 at 5:01 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Oct 7, 2015 at 2:34 PM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> Connect the sst25wf080 SPI flash to the EP108 board.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>> V2:
>>>  - Use sst25wf080 instead of m25p80
>>>
>>>  hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>
>>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>>> index 2899698..9755c30 100644
>>> --- a/hw/arm/xlnx-ep108.c
>>> +++ b/hw/arm/xlnx-ep108.c
>>> @@ -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, j;
>>>      Error *err = NULL;
>>>
>>>      object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
>>> @@ -60,6 +61,25 @@ 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;
>>> +        char bus_name[6];
>>> +
>>> +        snprintf(bus_name, 6, "spi%d", i);
>>> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
>>> +                                               bus_name);
>>
>> So machine level code shouldn't reach into the SoC like this. Instead
>> the bus should be passed from the SPI controller to SoC itself, then
>> use qdev_get_child_bus on the SoC itself.
>
> I'm not sure what you mean. The board needs multiple buses and I can't
> see how it can do that without reaching into the SoC.
>

The bus child object "spi0" should be a bus of the SoC, so the
instantiator of the SoC (ep108) does not need to reach into the SoC
implementation. You should end up with

        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), "spi0");

The bus should be exported by the SoC as a bus of its own to avoid
exposing internals. Not sure if we need some new QOM infrastructure to
support this yet. Wondering if you just QOM detach the child bus from
the SPI controller and reparent to the SoC. That however is non-ideal
as we ultimately want a mechanism whereby both a SoC and a board can
attach to a bus (mix of internal and external connections).

So I think the SPI bus as a child of the SPI controller needs to be
aliased to the SoC (might be similar to that prop aliasing stuff for
processors to MPCore). qdev_get_child_bus might need a patch to scan
for such aliases.

Alternatively, you could do the alias and then ditch
qdev_get_child_bus and use QOM APIs instead.

Regards,
Peter

> Thanks,
>
> Alistair
>
>>
>> Regards,
>> Peter
>>
>>> +
>>> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
>>> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
>>> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
>>> +                                                      SSI_GPIO_CS, 0);
>>> +
>>> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
>>> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
>>> +                               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;
>>> --
>>> 2.1.4
>>>
>>
Alistair Francis Oct. 27, 2015, 12:39 a.m. UTC | #5
On Wed, Oct 14, 2015 at 12:25 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Thu, Oct 8, 2015 at 4:30 PM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Wed, Oct 7, 2015 at 5:01 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Wed, Oct 7, 2015 at 2:34 PM, Alistair Francis
>>> <alistair.francis@xilinx.com> wrote:
>>>> Connect the sst25wf080 SPI flash to the EP108 board.
>>>>
>>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>>> ---
>>>> V2:
>>>>  - Use sst25wf080 instead of m25p80
>>>>
>>>>  hw/arm/xlnx-ep108.c | 20 ++++++++++++++++++++
>>>>  1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>>>> index 2899698..9755c30 100644
>>>> --- a/hw/arm/xlnx-ep108.c
>>>> +++ b/hw/arm/xlnx-ep108.c
>>>> @@ -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, j;
>>>>      Error *err = NULL;
>>>>
>>>>      object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
>>>> @@ -60,6 +61,25 @@ 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;
>>>> +        char bus_name[6];
>>>> +
>>>> +        snprintf(bus_name, 6, "spi%d", i);
>>>> +        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
>>>> +                                               bus_name);
>>>
>>> So machine level code shouldn't reach into the SoC like this. Instead
>>> the bus should be passed from the SPI controller to SoC itself, then
>>> use qdev_get_child_bus on the SoC itself.
>>
>> I'm not sure what you mean. The board needs multiple buses and I can't
>> see how it can do that without reaching into the SoC.
>>
>
> The bus child object "spi0" should be a bus of the SoC, so the
> instantiator of the SoC (ep108) does not need to reach into the SoC
> implementation. You should end up with
>
>         spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), "spi0");
>
> The bus should be exported by the SoC as a bus of its own to avoid
> exposing internals. Not sure if we need some new QOM infrastructure to
> support this yet. Wondering if you just QOM detach the child bus from
> the SPI controller and reparent to the SoC. That however is non-ideal
> as we ultimately want a mechanism whereby both a SoC and a board can
> attach to a bus (mix of internal and external connections).
>
> So I think the SPI bus as a child of the SPI controller needs to be
> aliased to the SoC (might be similar to that prop aliasing stuff for
> processors to MPCore). qdev_get_child_bus might need a patch to scan
> for such aliases.

Ok, this aliasing is tricky. the QOM infrastructure for aliases is
built on object propertys, which doesn't really help too much with
qdev buses/devices.

I was looking at adding a qdev_get_child_bus() similar function which
can find the devices children's buses as well, but that seems
difficult as well. Though it might be the best option.

I'm just not sure how yet... I'll look into that tomorrow

Thanks,

Alistair

>
> Alternatively, you could do the alias and then ditch
> qdev_get_child_bus and use QOM APIs instead.
>
> Regards,
> Peter
>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Regards,
>>> Peter
>>>
>>>> +
>>>> +        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
>>>> +            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
>>>> +            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
>>>> +                                                      SSI_GPIO_CS, 0);
>>>> +
>>>> +            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
>>>> +                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
>>>> +                               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;
>>>> --
>>>> 2.1.4
>>>>
>>>
>
diff mbox

Patch

diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 2899698..9755c30 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -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, j;
     Error *err = NULL;
 
     object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
@@ -60,6 +61,25 @@  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;
+        char bus_name[6];
+
+        snprintf(bus_name, 6, "spi%d", i);
+        spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[i]),
+                                               bus_name);
+
+        for (j = 0; j < XLNX_ZYNQMP_NUM_SPI_FLASHES; ++j) {
+            DeviceState *flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
+            qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev,
+                                                      SSI_GPIO_CS, 0);
+
+            sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]),
+                               i * XLNX_ZYNQMP_NUM_SPI_FLASHES + j,
+                               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;