| Message ID | 7e53a8579626017e62bfa696ca343c310360afa8.1455534309.git.jcd@tribudubois.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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"); 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
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 >
On 25 February 2016 at 20:37, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote: > 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? We seem to be using "spi0", "spi1", etc. The zynqmp actually has multiple SPI controllers too so you can use the code it has to loop round creating properties. thanks -- PMM
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"); + 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); + } + sabrelite_binfo.ram_size = machine->ram_size; sabrelite_binfo.kernel_filename = machine->kernel_filename; sabrelite_binfo.kernel_cmdline = machine->kernel_cmdline;
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(+)