Re: [Qemu-devel] [PATCH v3 01/14] ARM: Samsung exynos4210-based boards emulation

[Peter Maydell]

On 12 December 2011 06:43, Evgeny Voevodin <address@hidden> wrote:
> From: Maksim Kozlov <address@hidden>
>
> Add initial code for support of NURI and SMDKC210 boards

This patch doesn't compile:

/home/pm215/src/qemu/qemu/hw/exynos4210.c: In function ‘exynos4210_init’:
/home/pm215/src/qemu/qemu/hw/exynos4210.c:98:14: error: variable
‘cpu_irq’ set but not used [-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors


> ---
>  Makefile.target |    1 +
>  hw/exynos4210.c |  224 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/exynos4210.h |   34 ++++++++
>  3 files changed, 259 insertions(+), 0 deletions(-)
>  create mode 100644 hw/exynos4210.c
>  create mode 100644 hw/exynos4210.h
>
> diff --git a/Makefile.target b/Makefile.target
> index a111521..624a142 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -344,6 +344,7 @@ obj-arm-y = integratorcp.o versatilepb.o arm_pic.o 
> arm_timer.o
>  obj-arm-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o 
> pl190.o
>  obj-arm-y += versatile_pci.o
>  obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
> +obj-arm-y += exynos4210.o
>  obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o
>  obj-arm-y += pl061.o
>  obj-arm-y += arm-semi.o
> diff --git a/hw/exynos4210.c b/hw/exynos4210.c
> new file mode 100644
> index 0000000..1550016
> --- /dev/null
> +++ b/hw/exynos4210.c
> @@ -0,0 +1,224 @@
> +/*
> + *  Samsung exynos4210-based boards emulation
> + *
> + *  Copyright (c) 2011 Samsung Electronics Co., Ltd. All rights reserved.
> + *    Maksim Kozlov <address@hidden>
> + *    Evgeny Voevodin <address@hidden>
> + *    Igor Mitsyanko  <address@hidden>
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License as published by the
> + *  Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + *  FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + *  for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc., 
> 51
> + *  Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#include "boards.h"
> +#include "sysemu.h"
> +#include "sysbus.h"
> +#include "arm-misc.h"
> +#include "exec-memory.h"
> +#include "exynos4210.h"
> +
> +#undef DEBUG
> +
> +//#define DEBUG
> +
> +#ifdef DEBUG
> +    #undef PRINT_DEBUG
> +    #define  PRINT_DEBUG(fmt, args...) \
> +        do { \
> +            fprintf(stderr, "  [%s:%d]   "fmt, __func__, __LINE__, ##args); \
> +        } while (0)
> +#else
> +    #define  PRINT_DEBUG(fmt, args...) \
> +        do {} while (0)
> +#endif
> +
> +#define EXYNOS4210_DRAM0_BASE_ADDR          0x40000000
> +#define EXYNOS4210_DRAM1_BASE_ADDR          0xa0000000
> +#define EXYNOS4210_DRAM_MAX_SIZE            0x60000000  /* 1.5 GB */
> +
> +#define EXYNOS4210_IROM_BASE_ADDR           0x00000000
> +#define EXYNOS4210_IROM_SIZE                0x00010000  /* 64 KB */
> +#define EXYNOS4210_IROM_MIRROR_BASE_ADDR    0x02000000
> +#define EXYNOS4210_IROM_MIRROR_SIZE         0x00010000  /* 64 KB */
> +
> +#define EXYNOS4210_IRAM_BASE_ADDR           0x02020000
> +#define EXYNOS4210_IRAM_SIZE                0x00020000  /* 128 KB */
> +
> +#define EXYNOS4210_SFR_BASE_ADDR            0x10000000
> +
> +#define EXYNOS4210_BASE_BOOT_ADDR           EXYNOS4210_DRAM0_BASE_ADDR
> +
> +static struct arm_boot_info exynos4210_binfo = {
> +        .loader_start     = EXYNOS4210_BASE_BOOT_ADDR,
> +};
> +
> +static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
> +                                    0x09, 0x00, 0x00, 0x00 };
> +
> +enum exynos4210_board_type {
> +    BOARD_EXYNOS4210_NURI,
> +    BOARD_EXYNOS4210_SMDKC210,
> +};
> +
> +enum exynos4210_mach_id {
> +    MACH_NURI_ID     = 0xD33,
> +    MACH_SMDKC210_ID = 0xB16,
> +};
> +
> +
> +static void exynos4210_init(ram_addr_t ram_size,
> +        const char *boot_device,
> +        const char *kernel_filename,
> +        const char *kernel_cmdline,
> +        const char *initrd_filename,
> +        const char *cpu_model,
> +        enum exynos4210_board_type board_type)
> +{
> +    CPUState *env;
> +    MemoryRegion *system_mem = get_system_memory();
> +    MemoryRegion *chipid_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *iram_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *irom_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *irom_alias_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *dram0_mem = g_new(MemoryRegion, 1);
> +    MemoryRegion *dram1_mem = NULL;
> +    qemu_irq *irqp;
> +    qemu_irq cpu_irq[4];
> +    ram_addr_t mem_size;
> +    int n;
> +
> +    switch (board_type) {
> +    case BOARD_EXYNOS4210_NURI:
> +        exynos4210_binfo.board_id      = MACH_NURI_ID;
> +    break;
> +    case BOARD_EXYNOS4210_SMDKC210:
> +        exynos4210_binfo.board_id = MACH_SMDKC210_ID;
> +    break;
> +    default:
> +    break;
> +    }
> +    if (!cpu_model) {
> +        cpu_model = "cortex-a9";
> +    }
> +
> +    for (n = 0; n < smp_cpus; n++) {
> +        env = cpu_init(cpu_model);
> +        if (!env) {
> +            fprintf(stderr, "Unable to find CPU %d definition\n", n);
> +            exit(1);
> +        }
> +        /* Create PIC controller for each processor instance */
> +        irqp = arm_pic_init_cpu(env);
> +
> +        /*
> +         * Get GICs gpio_in cpu_irq to connect a combiner to them later.
> +         * Use only IRQ for a while.
> +         */
> +        cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
> +    }
> +
> +    /*** Memory ***/
> +
> +    /* Chip-ID and OMR */
> +    memory_region_init_ram_ptr(chipid_mem, NULL, "exynos4210.chipid",
> +            sizeof(chipid_and_omr), chipid_and_omr);
> +    memory_region_set_readonly(chipid_mem, true);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_SFR_BASE_ADDR,
> +                                chipid_mem);
> +
> +    /* Internal ROM */
> +    memory_region_init_ram(irom_mem, NULL, "exynos4210.irom",
> +                           EXYNOS4210_IROM_SIZE);
> +    memory_region_set_readonly(irom_mem, true);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_IROM_BASE_ADDR,
> +                                irom_mem);
> +    /* mirror of 0x0 – 0x10000 */
> +    memory_region_init_alias(irom_alias_mem, "exynos4210.irom_alias",
> +            irom_mem, 0, EXYNOS4210_IROM_SIZE);
> +    memory_region_set_readonly(irom_alias_mem, true);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_IROM_MIRROR_BASE_ADDR,
> +            irom_alias_mem);
> +
> +    /* Internal RAM */
> +    memory_region_init_ram(iram_mem, NULL, "exynos4210.iram",
> +                           EXYNOS4210_IRAM_SIZE);
> +    memory_region_set_readonly(iram_mem, false);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_IRAM_BASE_ADDR,
> +                                iram_mem);
> +
> +    /* DRAM */
> +    mem_size = ram_size;
> +    if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
> +        dram1_mem = g_new(MemoryRegion, 1);
> +        memory_region_init_ram(dram1_mem, NULL, "exynos4210.dram1",
> +                mem_size - EXYNOS4210_DRAM_MAX_SIZE);
> +        memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
> +                dram1_mem);
> +        mem_size = EXYNOS4210_DRAM_MAX_SIZE;
> +    }
> +    memory_region_init_ram(dram0_mem, NULL, "exynos4210.dram0", mem_size);
> +    memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
> +            dram0_mem);
> +
> +    /*** Load kernel ***/
> +
> +    exynos4210_binfo.ram_size = ram_size;
> +    exynos4210_binfo.nb_cpus = smp_cpus;
> +    exynos4210_binfo.kernel_filename = kernel_filename;
> +    exynos4210_binfo.initrd_filename = initrd_filename;
> +    exynos4210_binfo.kernel_cmdline = kernel_cmdline;
> +
> +    arm_load_kernel(first_cpu, &exynos4210_binfo);
> +}
> +
> +static void exynos4210_nuri_init(ram_addr_t ram_size,
> +        const char *boot_device,
> +        const char *kernel_filename, const char *kernel_cmdline,
> +        const char *initrd_filename, const char *cpu_model)
> +{
> +    exynos4210_init(ram_size, boot_device, kernel_filename, kernel_cmdline,
> +            initrd_filename, cpu_model, BOARD_EXYNOS4210_NURI);
> +}
> +
> +static void exynos4210_smdkc210_init(ram_addr_t ram_size,
> +        const char *boot_device,
> +        const char *kernel_filename, const char *kernel_cmdline,
> +        const char *initrd_filename, const char *cpu_model)
> +{
> +    exynos4210_init(ram_size, boot_device, kernel_filename, kernel_cmdline,
> +            initrd_filename, cpu_model, BOARD_EXYNOS4210_SMDKC210);
> +}
> +
> +static QEMUMachine exynos4210_nuri_machine = {
> +        .name = "exynos4210-nuri",
> +        .desc = "Samsung Exynos4210 NURI board",
> +        .init = exynos4210_nuri_init,
> +        .max_cpus = EXYNOS4210_MAX_CPUS,
> +};
> +
> +static QEMUMachine exynos4210_smdkc210_machine = {
> +        .name = "exynos4210-smdkc210",
> +        .desc = "Samsung Exynos4210 SMDKC210 board",
> +        .init = exynos4210_smdkc210_init,
> +        .max_cpus = EXYNOS4210_MAX_CPUS,
> +};
> +
> +static void exynos4210_machine_init(void)
> +{
> +    qemu_register_machine(&exynos4210_nuri_machine);
> +    qemu_register_machine(&exynos4210_smdkc210_machine);
> +}
> +
> +machine_init(exynos4210_machine_init);

This file is mixing up code for the SoC with code for individual
boards. The SoC specific code should be in its own file, and then
the board initialisation file should say "init the SoC, now add
the things which are external to the SoC and specific to this
board". (This is how the omap code is structured, for instance.)


> diff --git a/hw/exynos4210.h b/hw/exynos4210.h
> new file mode 100644
> index 0000000..7137630
> --- /dev/null
> +++ b/hw/exynos4210.h
> @@ -0,0 +1,34 @@
> +/*
> + *  Samsung exynos4210-based boards emulation
> + *
> + *  Copyright (c) 2011 Samsung Electronics Co., Ltd. All rights reserved.
> + *    Maksim Kozlov <address@hidden>
> + *    Evgeny Voevodin <address@hidden>
> + *    Igor Mitsyanko <address@hidden>
> + *
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License as published by the
> + *  Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + *  FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + *  for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc., 
> 51
> + *  Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +
> +#ifndef EXYNOS4210_H_
> +#define EXYNOS4210_H_
> +
> +#include "qemu-common.h"
> +
> +#define EXYNOS4210_MAX_CPUS                2
> +
> +#endif /* EXYNOS4210_H_ */
> --
> 1.7.4.1

-- PMM