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

[Jean-Christophe DUBOIS]

Le 25/02/2016 15:33, Peter Maydell a écrit :
> 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");

I have 5 SPI Controllers so I should do it for each of them. And maybe 
also for I2C, SDHC  and other devices ...

There might be a lot of properties a the end ...

What would be the rule?

Is there a recommended naming scheme?

> 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.

OK, I'll have a look.
> > +        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.

OK

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