Re: [Qemu-devel] [PATCH v10 15/21] i.MX: KZM now uses the standalone i.MX31 SOC support

[Peter Crosthwaite]

On Sun, Jul 5, 2015 at 5:05 PM, Jean-Christophe Dubois
<address@hidden> wrote:
> Tested by booting a minimal linux system on the emulated plateform

"Linux" "platform"

>
> Note: Qdev construction helper functions are removed with this patch.
>

So is the goal of the inline header movements to minimise the file
scope of this patch? I would say it is ok to have this one touch them
all or if you are worried about it, what I suggest is you do the soc
conversion as a patch and leave the _create helpers as dead functions.
GCC won't complain about non-static unused functions so it is ok to
leave them dead.

> Signed-off-by: Jean-Christophe Dubois <address@hidden>
> ---
>
> Changes since v1:
>     * not present on v1
>
> Changes since v2:
>     * not present on v2
>
> Changes since v3:
>     * not present on v3
>
> Changes since v4:
>     * not present on v4
>
> Changes since v5:
>     * not present on v5
>
> Changes since v6:
>     * not present on v6
>
> Changes since v7:
>     * update KZM target to use new emulators
>
> Changes since v8:
>     * update KZM to user i.MX31 SOC
>     * rework SDRAM memory initialisation
>
> Changes since v9:
>     * remove all Qdev construction helper fucntions.
>
>  hw/arm/Makefile.objs |   4 +-
>  hw/arm/kzm.c         | 213 
> ++++++++++++++++++++++++++-------------------------
>  include/hw/arm/imx.h |  72 -----------------
>  3 files changed, 112 insertions(+), 177 deletions(-)
>  delete mode 100644 include/hw/arm/imx.h
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index f35f731..2fbe344 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,6 +1,6 @@
>  obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
>  obj-$(CONFIG_DIGIC) += digic_boards.o
> -obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
> +obj-y += integratorcp.o mainstone.o musicpal.o nseries.o
>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>  obj-$(CONFIG_ACPI) += virt-acpi-build.o
> @@ -13,4 +13,4 @@ obj-y += omap1.o omap2.o strongarm.o
>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
>  obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
>  obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
> -obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o
> +obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> index d7af230..6925833 100644
> --- a/hw/arm/kzm.c
> +++ b/hw/arm/kzm.c
> @@ -13,131 +13,138 @@
>   * i.MX31 SoC
>   */
>
> -#include "hw/sysbus.h"
> +#include "hw/arm/fsl-imx31.h"
> +#include "hw/boards.h"
> +#include "qemu/error-report.h"
>  #include "exec/address-spaces.h"
> -#include "hw/hw.h"
> -#include "hw/arm/arm.h"
> -#include "hw/devices.h"
>  #include "net/net.h"
> -#include "sysemu/sysemu.h"
> -#include "hw/boards.h"
> +#include "hw/devices.h"
>  #include "hw/char/serial.h"
> -#include "hw/intc/imx_avic.h"
> -#include "hw/arm/imx.h"
> -
> -    /* Memory map for Kzm Emulation Baseboard:
> -     * 0x00000000-0x00003fff 16k secure ROM       IGNORED
> -     * 0x00004000-0x00407fff Reserved             IGNORED
> -     * 0x00404000-0x00407fff ROM                  IGNORED
> -     * 0x00408000-0x0fffffff Reserved             IGNORED
> -     * 0x10000000-0x1fffbfff RAM aliasing         IGNORED
> -     * 0x1fffc000-0x1fffffff RAM                  EMULATED
> -     * 0x20000000-0x2fffffff Reserved             IGNORED
> -     * 0x30000000-0x7fffffff I.MX31 Internal Register Space
> -     *   0x43f00000 IO_AREA0
> -     *   0x43f90000 UART1                         EMULATED
> -     *   0x43f94000 UART2                         EMULATED
> -     *   0x68000000 AVIC                          EMULATED
> -     *   0x53f80000 CCM                           EMULATED
> -     *   0x53f94000 PIT 1                         EMULATED
> -     *   0x53f98000 PIT 2                         EMULATED
> -     *   0x53f90000 GPT                           EMULATED
> -     * 0x80000000-0x87ffffff RAM                  EMULATED
> -     * 0x88000000-0x8fffffff RAM Aliasing         EMULATED
> -     * 0xa0000000-0xafffffff NAND Flash           IGNORED
> -     * 0xb0000000-0xb3ffffff Unavailable          IGNORED
> -     * 0xb4000000-0xb4000fff 8-bit free space     IGNORED
> -     * 0xb4001000-0xb400100f Board control        IGNORED
> -     *  0xb4001003           DIP switch
> -     * 0xb4001010-0xb400101f 7-segment LED        IGNORED
> -     * 0xb4001020-0xb400102f LED                  IGNORED
> -     * 0xb4001030-0xb400103f LED                  IGNORED
> -     * 0xb4001040-0xb400104f FPGA, UART           EMULATED
> -     * 0xb4001050-0xb400105f FPGA, UART           EMULATED
> -     * 0xb4001060-0xb40fffff FPGA                 IGNORED
> -     * 0xb6000000-0xb61fffff LAN controller       EMULATED
> -     * 0xb6200000-0xb62fffff FPGA NAND Controller IGNORED
> -     * 0xb6300000-0xb7ffffff Free                 IGNORED
> -     * 0xb8000000-0xb8004fff Memory control registers IGNORED
> -     * 0xc0000000-0xc3ffffff PCMCIA/CF            IGNORED
> -     * 0xc4000000-0xffffffff Reserved             IGNORED
> -     */
> -
> -#define KZM_RAMADDRESS (0x80000000)
> -#define KZM_FPGA       (0xb4001040)
> +#include "sysemu/qtest.h"
> +
> +/* Memory map for Kzm Emulation Baseboard:
> + * 0x00000000-0x7fffffff See i.MX31 SOC for support
> + * 0x80000000-0x8fffffff RAM                  EMULATED
> + * 0x90000000-0x9fffffff RAM                  EMULATED
> + * 0xa0000000-0xafffffff Flash                IGNORED
> + * 0xb0000000-0xb3ffffff Unavailable          IGNORED
> + * 0xb4000000-0xb4000fff 8-bit free space     IGNORED
> + * 0xb4001000-0xb400100f Board control        IGNORED
> + *  0xb4001003           DIP switch
> + * 0xb4001010-0xb400101f 7-segment LED        IGNORED
> + * 0xb4001020-0xb400102f LED                  IGNORED
> + * 0xb4001030-0xb400103f LED                  IGNORED
> + * 0xb4001040-0xb400104f FPGA, UART           EMULATED
> + * 0xb4001050-0xb400105f FPGA, UART           EMULATED
> + * 0xb4001060-0xb40fffff FPGA                 IGNORED
> + * 0xb6000000-0xb61fffff LAN controller       EMULATED
> + * 0xb6200000-0xb62fffff FPGA NAND Controller IGNORED
> + * 0xb6300000-0xb7ffffff Free                 IGNORED
> + * 0xb8000000-0xb8004fff Memory control registers IGNORED
> + * 0xc0000000-0xc3ffffff PCMCIA/CF            IGNORED
> + * 0xc4000000-0xffffffff Reserved             IGNORED
> + */
> +
> +typedef struct imx31_kzm {

CamelCase for Typename. IMX all caps.

> +    FslImx31State soc;
> +    MemoryRegion ram[2];
> +    MemoryRegion ram_alias;
> +} imx31_kzm;
> +
> +#define KZM_RAM_ADDR           (FSL_IMX31_SDRAM0_ADDR)
> +#define KZM_FPGA_ADDR          (FSL_IMX31_CS4_ADDR + 0x1040)
> +#define KZM_LAN9118_ADDR       (FSL_IMX31_CS5_ADDR)
>
>  static struct arm_boot_info kzm_binfo = {
> -    .loader_start = KZM_RAMADDRESS,
> +    .loader_start = KZM_RAM_ADDR,
>      .board_id = 1722,
>  };
>
>  static void kzm_init(MachineState *machine)
>  {
> -    ram_addr_t ram_size = machine->ram_size;
> -    const char *cpu_model = machine->cpu_model;
> -    const char *kernel_filename = machine->kernel_filename;
> -    const char *kernel_cmdline = machine->kernel_cmdline;
> -    const char *initrd_filename = machine->initrd_filename;
> -    ARMCPU *cpu;
> -    MemoryRegion *address_space_mem = get_system_memory();
> -    MemoryRegion *ram = g_new(MemoryRegion, 1);
> -    MemoryRegion *sram = g_new(MemoryRegion, 1);
> -    MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
> -    DeviceState *dev;
> -    DeviceState *ccm;
> -
> -    if (!cpu_model) {
> -        cpu_model = "arm1136";
> -    }
> -
> -    cpu = cpu_arm_init(cpu_model);
> -    if (!cpu) {
> -        fprintf(stderr, "Unable to find CPU definition\n");
> +    imx31_kzm *s = g_new0(imx31_kzm, 1);
> +    Error *err = NULL;
> +    unsigned int ram_size;
> +    unsigned int i;
> +
> +    object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX31);
> +    object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
> +                              &error_abort);
> +
> +    object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
> +    if (err != NULL) {
> +        error_report("%s", error_get_pretty(err));
>          exit(1);
>      }
>
> -    /* On a real system, the first 16k is a `secure boot rom' */
> -
> -    memory_region_allocate_system_memory(ram, NULL, "kzm.ram", ram_size);
> -    memory_region_add_subregion(address_space_mem, KZM_RAMADDRESS, ram);
> -
> -    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
> -    memory_region_add_subregion(address_space_mem, 0x88000000, ram_alias);
> -
> -    memory_region_init_ram(sram, NULL, "kzm.sram", 0x4000, &error_abort);
> -    memory_region_add_subregion(address_space_mem, 0x1FFFC000, sram);
> -
> -    dev = sysbus_create_varargs(TYPE_IMX_AVIC, 0x68000000,
> -                                qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ),
> -                                qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ),
> -                                NULL);
> -
> -    imx_serial_create(0, 0x43f90000, qdev_get_gpio_in(dev, 45));
> -    imx_serial_create(1, 0x43f94000, qdev_get_gpio_in(dev, 32));
> -
> -    ccm = sysbus_create_simple(TYPE_IMX_CCM, 0x53f80000, NULL);
> +    /* Check the amount of memory is compatible with the SOC */
> +    if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) 
> {
> +        error_report("WARNING: RAM size " RAM_ADDR_FMT " above max 
> supported, "
> +                     "reduced to %x", machine->ram_size,
> +                     FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE);
> +        machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
> +    }
>
> -    imx_timerp_create(0x53f94000, qdev_get_gpio_in(dev, 28), ccm);
> -    imx_timerp_create(0x53f98000, qdev_get_gpio_in(dev, 27), ccm);
> -    imx_timerg_create(0x53f90000, qdev_get_gpio_in(dev, 29), ccm);
> +    ram_size = machine->ram_size;
> +    i = 0;
> +
> +    /* initialize our memory */
> +    while (ram_size && (i < 2)) {

for loop.

> +        unsigned int size;
> +        char ram_name[20];
> +        static const struct {
> +            hwaddr addr;
> +            unsigned int size;
> +        } ram[2] = {
> +            { FSL_IMX31_SDRAM0_ADDR, FSL_IMX31_SDRAM0_SIZE },
> +            { FSL_IMX31_SDRAM1_ADDR, FSL_IMX31_SDRAM1_SIZE },
> +        };
> +
> +       if (ram_size > ram[i].size) {
> +            size = ram[i].size;
> +       } else {
> +            size = ram_size;
> +       }
> +
> +        sprintf(ram_name, "kzm.ram%d", i);
> +
> +        ram_size -= size;
> +
> +        memory_region_init_ram(&s->ram[i], NULL, ram_name, size,
> +                               &error_abort);
> +        vmstate_register_ram_global(&s->ram[i]);

memory_region_allocate_system_memory().

> +        memory_region_add_subregion(get_system_memory(), ram[i].addr,
> +                                    &s->ram[i]);
> +        if (size < ram[i].size) {
> +            memory_region_init_alias(&s->ram_alias, NULL, "ram.alias",
> +                                     &s->ram[i], 0, ram[i].size - size);
> +            memory_region_add_subregion(get_system_memory(),
> +                                        ram[i].addr + size, &s->ram_alias);
> +        }
> +
> +        i++;
> +    }
>
>      if (nd_table[0].used) {
> -        lan9118_init(&nd_table[0], 0xb6000000, qdev_get_gpio_in(dev, 52));
> +        lan9118_init(&nd_table[0], KZM_LAN9118_ADDR,
> +                     qdev_get_gpio_in(DEVICE(&s->soc.avic), 52));
>      }
>
>      if (serial_hds[2]) { /* touchscreen */

Curious to how this does a touchscreen? Looks like a 3rd UART in an
attached FPGA?

Regards,
Peter

> -        serial_mm_init(address_space_mem, KZM_FPGA+0x10, 0,
> -                       qdev_get_gpio_in(dev, 52),
> -                       14745600, serial_hds[2],
> -                       DEVICE_NATIVE_ENDIAN);
> +        serial_mm_init(get_system_memory(), KZM_FPGA_ADDR+0x10, 0,
> +                       qdev_get_gpio_in(DEVICE(&s->soc.avic), 52),
> +                       14745600, serial_hds[2], DEVICE_NATIVE_ENDIAN);
>      }
>
> -    kzm_binfo.ram_size = ram_size;
> -    kzm_binfo.kernel_filename = kernel_filename;
> -    kzm_binfo.kernel_cmdline = kernel_cmdline;
> -    kzm_binfo.initrd_filename = initrd_filename;
> +    kzm_binfo.ram_size = machine->ram_size;
> +    kzm_binfo.kernel_filename = machine->kernel_filename;
> +    kzm_binfo.kernel_cmdline = machine->kernel_cmdline;
> +    kzm_binfo.initrd_filename = machine->initrd_filename;
>      kzm_binfo.nb_cpus = 1;
> -    arm_load_kernel(cpu, &kzm_binfo);
> +
> +    if (!qtest_enabled()) {
> +        arm_load_kernel(&s->soc.cpu, &kzm_binfo);
> +    }
>  }
>
>  static QEMUMachine kzm_machine = {
> diff --git a/include/hw/arm/imx.h b/include/hw/arm/imx.h
> deleted file mode 100644
> index b7aa4b6..0000000
> --- a/include/hw/arm/imx.h
> +++ /dev/null
> @@ -1,72 +0,0 @@
> -/*
> - * i.MX31 emulation
> - *
> - * Copyright (C) 2012 Peter Chubb
> - * NICTA
> - *
> - * This code is released under the GPL, version 2.0 or later
> - * See the file `../COPYING' for details.
> - */
> -
> -#ifndef IMX_H
> -#define IMX_H
> -
> -#include "hw/qdev-core.h"
> -#include "hw/sysbus.h"
> -#include "hw/char/imx_serial.h"
> -#include "hw/misc/imx_ccm.h"
> -#include "hw/timer/imx_epit.h"
> -#include "hw/timer/imx_gpt.h"
> -
> -/***
> - * This Qdev construction helper is going to be removed soon
> - */
> -static inline void imx_serial_create(int uart, const hwaddr addr, qemu_irq 
> irq)
> -{
> -   /* uart parameter is not used anymore */
> -   DeviceState *dev;
> -
> -   dev = qdev_create(NULL, TYPE_IMX_SERIAL);
> -
> -   if (dev) {
> -       SysBusDevice *bus;
> -
> -       bus = SYS_BUS_DEVICE(dev);
> -
> -       qdev_init_nofail(dev);
> -
> -       if (addr != (hwaddr)-1) {
> -           sysbus_mmio_map(bus, 0, addr);
> -       }
> -
> -       sysbus_connect_irq(bus, 0, irq);
> -   }
> -}
> -
> -static inline
> -void imx_timerp_create(const hwaddr addr,
> -                      qemu_irq irq,
> -                      DeviceState *ccm)
> -{
> -    IMXEPITState *pp;
> -    DeviceState *dev;
> -
> -    dev = sysbus_create_simple(TYPE_IMX_EPIT, addr, irq);
> -    pp = IMX_EPIT(dev);
> -    pp->ccm = ccm;
> -}
> -
> -static inline
> -void imx_timerg_create(const hwaddr addr,
> -                       qemu_irq irq,
> -                       DeviceState *ccm)
> -{
> -    IMXGPTState *pp;
> -    DeviceState *dev;
> -
> -    dev = sysbus_create_simple(TYPE_IMX_GPT, addr, irq);
> -    pp = IMX_GPT(dev);
> -    pp->ccm = ccm;
> -}
> -
> -#endif /* IMX_H */
> --
> 2.1.4
>
>