[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 14/21] i.MX: Add SOC support for i.MX31
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v10 14/21] i.MX: Add SOC support for i.MX31 |
Date: |
Mon, 6 Jul 2015 00:18:16 -0700 |
Needs a short paragraph. List out the initally support peripherals.
On Sun, Jul 5, 2015 at 5:05 PM, Jean-Christophe Dubois
<address@hidden> wrote:
> 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:
> * not present on v7
>
> Changes since v8:
> * use defines instead of hardcoded values for IRQ and ADDR
> * Add i.MX31 SOC support
>
> Changes since v9:
> * no change.
>
> default-configs/arm-softmmu.mak | 2 +
> hw/arm/Makefile.objs | 1 +
> hw/arm/fsl-imx31.c | 219
> ++++++++++++++++++++++++++++++++++++++++
> include/hw/arm/fsl-imx31.h | 99 ++++++++++++++++++
> 4 files changed, 321 insertions(+)
> create mode 100644 hw/arm/fsl-imx31.c
> create mode 100644 include/hw/arm/fsl-imx31.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 74f1db3..3f86e7e 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -98,6 +98,8 @@ CONFIG_ALLWINNER_A10_PIT=y
> CONFIG_ALLWINNER_A10_PIC=y
> CONFIG_ALLWINNER_A10=y
>
> +CONFIG_FSL_IMX31=y
> +
> CONFIG_XIO3130=y
> CONFIG_IOH3420=y
> CONFIG_I82801B11=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index cf346c1..f35f731 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -13,3 +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
> diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
> new file mode 100644
> index 0000000..809070b
> --- /dev/null
> +++ b/hw/arm/fsl-imx31.c
> @@ -0,0 +1,219 @@
> +/*
> + * Copyright (c) 2013 Jean-Christophe Dubois <address@hidden>
> + *
> + * i.MX31 SOC emulation.
> + *
> + * Based on hw/arm/fsl-imx31.c
> + *
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/arm/fsl-imx31.h"
> +#include "sysemu/sysemu.h"
> +#include "exec/address-spaces.h"
> +
> +static void fsl_imx31_init(Object *obj)
> +{
> + FslImx31State *s = FSL_IMX31(obj);
> + int i;
> +
> + object_initialize(&s->cpu, sizeof(s->cpu), "arm1136-" TYPE_ARM_CPU);
> +
> + object_initialize(&s->avic, sizeof(s->avic), TYPE_IMX_AVIC);
> + qdev_set_parent_bus(DEVICE(&s->avic), sysbus_get_default());
> +
> + object_initialize(&s->ccm, sizeof(s->ccm), TYPE_IMX_CCM);
> + qdev_set_parent_bus(DEVICE(&s->ccm), sysbus_get_default());
> +
> + for (i = 0; i < FSL_IMX31_NUM_UARTS; i++) {
> + if (i >= MAX_SERIAL_PORTS) {
> + break;
> + }
You don't need this bail-out. For SoCs, the device should always be
created. If QEMU can't attach the serial port to anything that is ok.
If the UART model can't handle being un-attached (chr == NULL) it
should be patched (but thats out of scope for the moment I think).
> + object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_IMX_SERIAL);
> + qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
> + }
> +
> + for (i = 0; i < FSL_IMX31_NUM_GPTS; i++) {
> + object_initialize(&s->gpt[i], sizeof(s->gpt[i]), TYPE_IMX_GPT);
> + qdev_set_parent_bus(DEVICE(&s->gpt[i]), sysbus_get_default());
> + }
> +
> + for (i = 0; i < FSL_IMX31_NUM_EPITS; i++) {
> + object_initialize(&s->epit[i], sizeof(s->epit[i]), TYPE_IMX_EPIT);
> + qdev_set_parent_bus(DEVICE(&s->epit[i]), sysbus_get_default());
> + }
> +}
> +
> +static void fsl_imx31_realize(DeviceState *dev, Error **errp)
> +{
> + FslImx31State *s = FSL_IMX31(dev);
> + uint16_t i;
> + Error *err = NULL;
> +
> + /* Initialize the CPU */
Self documented, drop comment.
> + object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
> + if (err) {
> + error_propagate((errp), (err));
parentheses not needed (x2). Happens a few times below as well.
> + return;
> + }
> +
> + /* Initialize the PIC */
What is PIC? I would just say 'interrupt controller" (if you are going
for programmable-interrupt-controller) or drop the comment.
> + object_property_set_bool(OBJECT(&s->avic), true, "realized", &err);
> + if (err) {
> + error_propagate((errp), (err));
> + return;
> + }
> + /* Connect the PIC interrupt to the CPU */
"interrupts" (you have both IRQ and FIQ).
This comment...
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->avic), 0, FSL_IMX31_AVIC_ADDR);
... goes here. Or just drop it. It is self documented.
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->avic), 0,
> + qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->avic), 1,
> + qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ));
> +
> + /* Initialize the CCM */
> + object_property_set_bool(OBJECT(&s->ccm), true, "realized", &err);
> + if (err) {
> + error_propagate((errp), (err));
> + return;
> + }
> + /* Map CCM memory */
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ccm), 0, 0x53f80000);
Missed address macroification.
> +
> + /* Initialize all UARTS */
> + for (i = 0; i < FSL_IMX31_NUM_UARTS; i++) {
> + static const struct {
> + hwaddr addr;
> + unsigned int irq;
> + } serial_table[FSL_IMX31_NUM_UARTS] = {
> + { FSL_IMX31_UART1_ADDR, FSL_IMX31_UART1_IRQ },
> + { FSL_IMX31_UART2_ADDR, FSL_IMX31_UART2_IRQ },
> + };
> +
> + /* Bail out if we exeeded Qemu UART count */
"exceeded" "QEMU"
> + if (i >= MAX_SERIAL_PORTS) {
> + break;
> + }
Is this where the chr property should be set?
> + /* Initialize the UART */
> + object_property_set_bool(OBJECT(&s->uart[i]), true, "realized",
> &err);
> + if (err) {
> + error_propagate((errp), (err));
> + return;
> + }
> + /* Map UART memory */
Self-documented - drop comment.
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0,
> serial_table[i].addr);
> + /* Connet UART IRQ to PIC */
Self-documented - drop comment.
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
> + qdev_get_gpio_in(DEVICE(&s->avic),
> + serial_table[i].irq));
I'd pull the GPIO in to a local variable to save on the double 80 char
wrap of mixed indents.
> + }
> +
> + /* Initialize all GPT timers */
> + for (i = 0; i < FSL_IMX31_NUM_GPTS; i++) {
> + static const struct {
> + hwaddr addr;
> + unsigned int irq;
> + } gpt_table[FSL_IMX31_NUM_GPTS] = {
> + { FSL_IMX31_GPT_ADDR, FSL_IMX31_GPT_IRQ }
> + };
> +
> + s->gpt[i].ccm = DEVICE(&s->ccm);
> +
We should consider making this a QOM link, but is out of scope of
series so no change needed.
> + /* Initialize the GPT */
> + object_property_set_bool(OBJECT(&s->gpt[i]), true, "realized", &err);
> + if (err) {
> + error_propagate((errp), (err));
> + return;
> + }
> + /* Map GPT memory */
Self-documented - drop comment.
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpt[i]), 0, gpt_table[i].addr);
> + /* Connet GPT IRQ to PIC */
Self-documented - drop comment.
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpt[i]), 0,
> + qdev_get_gpio_in(DEVICE(&s->avic),
> + gpt_table[i].irq));
> + }
> +
> + /* Initialize all EPIT timers */
Self-documented - drop comment.
> + for (i = 0; i < FSL_IMX31_NUM_EPITS; i++) {
> + static const struct {
> + hwaddr addr;
> + unsigned int irq;
> + } epit_table[FSL_IMX31_NUM_EPITS] = {
> + { FSL_IMX31_EPIT1_ADDR, FSL_IMX31_EPIT1_IRQ },
> + { FSL_IMX31_EPIT2_ADDR, FSL_IMX31_EPIT2_IRQ },
> + };
> +
> + s->epit[i].ccm = DEVICE(&s->ccm);
> +
> + /* Initialize the EPIT */
Self-documented - drop comment.
> + object_property_set_bool(OBJECT(&s->epit[i]), true, "realized",
> &err);
> + if (err) {
> + error_propagate((errp), (err));
> + return;
> + }
> + /* Map EPIT memory */
Self-documented - drop comment.
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->epit[i]), 0, epit_table[i].addr);
> + /* Connet EPIT IRQ to PIC */
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->epit[i]), 0,
> + qdev_get_gpio_in(DEVICE(&s->avic),
> + epit_table[i].irq));
> + }
> +
> + /* On a real system, the first 16k is a `secure boot rom' */
> + memory_region_init_rom_device(&s->secure_rom, NULL, NULL, NULL,
> + "imx31.secure_rom",
> + FSL_IMX31_SECURE_ROM_SIZE, &err);
> + memory_region_add_subregion(get_system_memory(),
> FSL_IMX31_SECURE_ROM_ADDR,
> + &s->secure_rom);
> +
> + /* There is also a 16k ROM */
> + memory_region_init_rom_device(&s->rom, NULL, NULL, NULL, "imx31.rom",
> + FSL_IMX31_ROM_SIZE, &err);
> + memory_region_add_subregion(get_system_memory(), FSL_IMX31_ROM_ADDR,
> + &s->rom);
> +
> + /* initialize internal RAM (16 KB) */
> + memory_region_init_ram(&s->iram, NULL, "imx31.iram", FSL_IMX31_IRAM_SIZE,
> + &error_abort);
> + vmstate_register_ram_global(&s->iram);
> + memory_region_add_subregion(get_system_memory(), FSL_IMX31_IRAM_ADDR,
> + &s->iram);
I think there is a new feature for this,
memory_region_allocate_system_memory. Check xlnx-ep108.
> +
> + /* internal RAM (16 KB) is aliased over 256 MB - 16 KB */
> + memory_region_init_alias(&s->iram_alias, NULL, "imx31.iram_alias",
> + &s->iram, 0, FSL_IMX31_IRAM_ALIAS_SIZE);
> + memory_region_add_subregion(get_system_memory(),
> FSL_IMX31_IRAM_ALIAS_ADDR,
> + &s->iram_alias);
> +}
> +
> +static void fsl_imx31_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->realize = fsl_imx31_realize;
> +}
> +
> +static const TypeInfo fsl_imx31_type_info = {
> + .name = TYPE_FSL_IMX31,
> + .parent = TYPE_DEVICE,
> + .instance_size = sizeof(FslImx31State),
> + .instance_init = fsl_imx31_init,
> + .class_init = fsl_imx31_class_init,
> +};
> +
> +static void fsl_imx31_register_types(void)
> +{
> + type_register_static(&fsl_imx31_type_info);
> +}
> +
> +type_init(fsl_imx31_register_types)
> diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
> new file mode 100644
> index 0000000..f1bb299
> --- /dev/null
> +++ b/include/hw/arm/fsl-imx31.h
> @@ -0,0 +1,99 @@
> +/*
> + * Freescale i.MX31 SoC emulation
> + *
> + * Copyright (C) 2015 Jean-Christophe Dubois <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.
> + */
> +
> +#ifndef FSL_IMX31_H
> +#define FSL_IMX31_H
> +
> +#include "hw/arm/arm.h"
> +#include "hw/intc/imx_avic.h"
> +#include "hw/misc/imx_ccm.h"
> +#include "hw/char/imx_serial.h"
> +#include "hw/timer/imx_gpt.h"
> +#include "hw/timer/imx_epit.h"
> +#include "exec/memory.h"
> +
> +#define TYPE_FSL_IMX31 "fsl,imx31"
> +#define FSL_IMX31(obj) OBJECT_CHECK(FslImx31State, (obj), TYPE_FSL_IMX31)
> +
> +#define FSL_IMX31_NUM_UARTS 2
> +#define FSL_IMX31_NUM_GPTS 1
Can/should we de-arrayify a singleton device? Are you planning code
share with more SoCs that may have multiple of these?
> +#define FSL_IMX31_NUM_EPITS 2
> +
> +typedef struct {
> + /*< private >*/
> + DeviceState parent_obj;
> +
> + /*< public >*/
> + ARMCPU cpu;
> + IMXAVICState avic;
> + IMXCCMState ccm;
> + IMXSerialState uart[FSL_IMX31_NUM_UARTS];
> + IMXGPTState gpt[FSL_IMX31_NUM_GPTS];
> + IMXEPITState epit[FSL_IMX31_NUM_EPITS];
> + MemoryRegion secure_rom;
> + MemoryRegion rom;
> + MemoryRegion iram;
> + MemoryRegion iram_alias;
> +} FslImx31State;
FslIMX21State
> +
> +#define FSL_IMX31_SECURE_ROM_ADDR 0x00000000
> +#define FSL_IMX31_SECURE_ROM_SIZE 0x4000
> +#define FSL_IMX31_ROM_ADDR 0x00404000
> +#define FSL_IMX31_ROM_SIZE 0x4000
> +#define FSL_IMX31_IRAM_ALIAS_ADDR 0x10000000
> +#define FSL_IMX31_IRAM_ALIAS_SIZE 0xFFC0000
> +#define FSL_IMX31_IRAM_ADDR 0x1FFFC000
> +#define FSL_IMX31_IRAM_SIZE 0x4000
> +#define FSL_IMX31_UART1_ADDR 0x43F90000
> +#define FSL_IMX31_UART1_SIZE 0x4000
> +#define FSL_IMX31_UART2_ADDR 0x43F94000
> +#define FSL_IMX31_UART2_SIZE 0x4000
> +#define FSL_IMX31_CCM_ADDR 0x53F80000
> +#define FSL_IMX31_CCM_SIZE 0x4000
> +#define FSL_IMX31_GPT_ADDR 0x53F90000
> +#define FSL_IMX31_GPT_SIZE 0x4000
> +#define FSL_IMX31_EPIT1_ADDR 0x53F94000
> +#define FSL_IMX31_EPIT1_SIZE 0x4000
> +#define FSL_IMX31_EPIT2_ADDR 0x53F98000
> +#define FSL_IMX31_EPIT2_SIZE 0x4000
> +#define FSL_IMX31_AVIC_ADDR 0x68000000
> +#define FSL_IMX31_AVIC_SIZE 0x100
> +#define FSL_IMX31_SDRAM0_ADDR 0x80000000
> +#define FSL_IMX31_SDRAM0_SIZE 0x10000000
> +#define FSL_IMX31_SDRAM1_ADDR 0x90000000
> +#define FSL_IMX31_SDRAM1_SIZE 0x10000000
> +#define FSL_IMX31_FLASH0_ADDR 0xA0000000
> +#define FSL_IMX31_FLASH0_SIZE 0x8000000
> +#define FSL_IMX31_FLASH1_ADDR 0xA8000000
> +#define FSL_IMX31_FLASH1_SIZE 0x8000000
> +#define FSL_IMX31_CS2_ADDR 0xB0000000
> +#define FSL_IMX31_CS2_SIZE 0x2000000
> +#define FSL_IMX31_CS3_ADDR 0xB2000000
> +#define FSL_IMX31_CS3_SIZE 0x2000000
> +#define FSL_IMX31_CS4_ADDR 0xB4000000
> +#define FSL_IMX31_CS4_SIZE 0x2000000
> +#define FSL_IMX31_CS5_ADDR 0xB6000000
> +#define FSL_IMX31_CS5_SIZE 0x2000000
> +#define FSL_IMX31_NAND_ADDR 0xB8000000
> +#define FSL_IMX31_NAND_SIZE 0x1000
> +
> +#define FSL_IMX31_EPIT2_IRQ 27
> +#define FSL_IMX31_EPIT1_IRQ 28
> +#define FSL_IMX31_GPT_IRQ 29
> +#define FSL_IMX31_UART2_IRQ 32
> +#define FSL_IMX31_UART1_IRQ 45
> +
FWIW, I prefered these memory maps in the C file to minimise the
header to just the embeddable state, but it could go either way. You
could in-theory use these macros from the header to drive SoC-level
tests I guess. No change needed.
> +#endif // FSL_IMX31_H
C++ style comment
Regards,
Peter
> --
> 2.1.4
>
>
- [Qemu-devel] [PATCH v10 08/21] i.MX: Split EPIT emulator in a header file and a source file, (continued)
- [Qemu-devel] [PATCH v10 08/21] i.MX: Split EPIT emulator in a header file and a source file, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 10/21] i.MX: Fix Coding style for EPIT emulator, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 09/21] i.MX: Move Qdev EPIT construction helper as inline function., Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 11/21] i.MX: Split GPT emulator in a header file and a source file, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 12/21] i.MX: Move Qdev GPT construction helper as inline function., Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 13/21] i.MX: Fix Coding style for GPT emulator, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 14/21] i.MX: Add SOC support for i.MX31, Jean-Christophe Dubois, 2015/07/05
- Re: [Qemu-devel] [PATCH v10 14/21] i.MX: Add SOC support for i.MX31,
Peter Crosthwaite <=
- [Qemu-devel] [PATCH v10 15/21] i.MX: KZM now uses the standalone i.MX31 SOC support, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 16/21] i.MX: Add I2C controller emulator, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 17/21] i.MX: Add FEC Ethernet Emulator, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 18/21] i.MX: Add SOC support for i.MX25, Jean-Christophe Dubois, 2015/07/05
- [Qemu-devel] [PATCH v10 19/21] i.MX: Add the i.MX25 3DS PDK plateform, Jean-Christophe Dubois, 2015/07/05