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

[Jean-Christophe DUBOIS <jcd@tribudubois.net>]

Le 25/02/2016 15:33, Peter Maydell a écrit :
> On 15 February 2016 at 11:18, Jean-Christophe Dubois
> <jcd@tribudubois.net> wrote:
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>> ---
>>
>> 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
>