Re: [Qemu-devel] [PATCH v2 4/4] i.MX: Add SPI NOR FLASH memory to sabrelite board.

[Peter Maydell]

On 15 February 2016 at 11:18, Jean-Christophe Dubois
<address@hidden> wrote:
> Signed-off-by: Jean-Christophe Dubois <address@hidden>
> ---
>
> Changes since v1:
>  * Not present on v1.
>
>  hw/arm/sabrelite.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
> index 8db9bbc..237dfa1 100644
> --- a/hw/arm/sabrelite.c
> +++ b/hw/arm/sabrelite.c
> @@ -70,6 +70,15 @@ static void sabrelite_init(MachineState *machine)
>      memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR,
>                                  &s->ram);
>
> +    {
> +        /* Add the sst25vf016b NOR FLASH memory to first SPI */
> +        SSIBus *spi = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc.spi[0]),
> +                                                   "spi");

Rather than having the board code looking into the internals
of the SoC struct like this, you should have the SoC create an
spi bus property for itself, and then here you can just have

      spibus = (SSIBus *)qdev_get_child_bus(DEVICE(&s->soc), "spi");

See hw/arm/xlnx-ep108.c for an example of this.
The code to create the property in the SoC's realize method is

        object_property_add_alias(OBJECT(s), "spi",
                                  OBJECT(&s->spi[i]), "spi",
                                  &error_abort);

(where the first "spi" is the name of the bus property to create on the
SoC object, and the second is the name of the bus property on the
internal SPI device object). Example code in hw/arm/xlnx-zynqmp.c.

> +        DeviceState *flash_dev = ssi_create_slave(spi, "sst25vf016b");
> +        qemu_irq cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[0]), 1, cs_line);

Again, you want the SoC to provide an outward-facing interface to the
chipselect line so you don't have to have the board code looking
into soc.spi[] itself.

> +    }
> +
>      sabrelite_binfo.ram_size = machine->ram_size;
>      sabrelite_binfo.kernel_filename = machine->kernel_filename;
>      sabrelite_binfo.kernel_cmdline = machine->kernel_cmdline;

thanks
-- PMM