From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Havard Skinnemoen <hskinnemoen@google.com>, peter.maydell@linaro.org
Cc: Avi.Fishman@nuvoton.com, qemu-devel@nongnu.org,
kfting@nuvoton.com, qemu-arm@nongnu.org,
"Cédric Le Goater" <clg@kaod.org>,
"Joel Stanley" <joel@jms.id.au>
Subject: Re: [PATCH v5 05/11] hw/arm: Add two NPCM7xx-based machines
Date: Thu, 9 Jul 2020 08:09:45 +0200 [thread overview]
Message-ID: <c506f212-2119-32f3-5bfc-7dbb1e5fdf28@amsat.org> (raw)
In-Reply-To: <46869856-cc4f-e694-d56f-27d42dee5a08@amsat.org>
On 7/9/20 7:57 AM, Philippe Mathieu-Daudé wrote:
> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
>> This adds two new machines, both supported by OpenBMC:
>>
>> - npcm750-evb: Nuvoton NPCM750 Evaluation Board.
>> - quanta-gsj: A board with a NPCM730 chip.
>>
>> They rely on the NPCM7xx SoC device to do the heavy lifting. They are
>> almost completely identical at the moment, apart from the SoC type,
>> which currently only changes the reset contents of one register
>> (GCR.MDLR), but they might grow apart a bit more as more functionality
>> is added.
>>
>> Both machines can boot the Linux kernel into /bin/sh.
>>
>> Reviewed-by: Tyrone Ting <kfting@nuvoton.com>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
>> ---
>> default-configs/arm-softmmu.mak | 1 +
>> include/hw/arm/npcm7xx.h | 19 ++++
>> hw/arm/npcm7xx_boards.c | 156 ++++++++++++++++++++++++++++++++
>> hw/arm/Makefile.objs | 2 +-
>> 4 files changed, 177 insertions(+), 1 deletion(-)
>> create mode 100644 hw/arm/npcm7xx_boards.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 8fc09a4a51..9a94ebd0be 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -27,6 +27,7 @@ CONFIG_GUMSTIX=y
>> CONFIG_SPITZ=y
>> CONFIG_TOSA=y
>> CONFIG_Z2=y
>> +CONFIG_NPCM7XX=y
>> CONFIG_COLLIE=y
>> CONFIG_ASPEED_SOC=y
>> CONFIG_NETDUINO2=y
>> diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
>> index 95d9224f59..b8b76bc07b 100644
>> --- a/include/hw/arm/npcm7xx.h
>> +++ b/include/hw/arm/npcm7xx.h
>> @@ -35,6 +35,25 @@
>> #define NPCM7XX_SMP_BOOTREG_ADDR (0xf080013c) /* GCR.SCRPAD */
>> #define NPCM7XX_GIC_CPU_IF_ADDR (0xf03fe100) /* GIC within A9 */
>>
>> +typedef struct NPCM7xxMachine {
>> + MachineState parent;
>> +} NPCM7xxMachine;
>> +
>> +#define TYPE_NPCM7XX_MACHINE MACHINE_TYPE_NAME("npcm7xx")
>> +#define NPCM7XX_MACHINE(obj) \
>> + OBJECT_CHECK(NPCM7xxMachine, (obj), TYPE_NPCM7XX_MACHINE)
>> +
>> +typedef struct NPCM7xxMachineClass {
>> + MachineClass parent;
>> +
>> + const char *soc_type;
>> +} NPCM7xxMachineClass;
>> +
>> +#define NPCM7XX_MACHINE_CLASS(klass) \
>> + OBJECT_CLASS_CHECK(NPCM7xxMachineClass, (klass), TYPE_NPCM7XX_MACHINE)
>> +#define NPCM7XX_MACHINE_GET_CLASS(obj) \
>> + OBJECT_GET_CLASS(NPCM7xxMachineClass, (obj), TYPE_NPCM7XX_MACHINE)
>> +
>> typedef struct NPCM7xxState {
>> DeviceState parent;
>>
>> diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
>> new file mode 100644
>> index 0000000000..d78d9f991b
>> --- /dev/null
>> +++ b/hw/arm/npcm7xx_boards.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * Machine definitions for boards featuring an NPCM7xx SoC.
>> + *
>> + * Copyright 2020 Google LLC
>> + *
>> + * 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.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "hw/arm/boot.h"
>> +#include "hw/arm/npcm7xx.h"
>> +#include "hw/core/cpu.h"
>> +#include "qapi/error.h"
>> +#include "qemu/units.h"
>> +
>> +#define NPCM750_EVB_POWER_ON_STRAPS 0x00001ff7
>> +#define QUANTA_GSJ_POWER_ON_STRAPS 0x00001fff
>> +
>> +static struct arm_boot_info npcm7xx_binfo = {
>> + .loader_start = NPCM7XX_LOADER_START,
>> + .smp_loader_start = NPCM7XX_SMP_LOADER_START,
>> + .smp_bootreg_addr = NPCM7XX_SMP_BOOTREG_ADDR,
>> + .gic_cpu_if_addr = NPCM7XX_GIC_CPU_IF_ADDR,
>> + .write_secondary_boot = npcm7xx_write_secondary_boot,
>> + .board_id = -1,
>> +};
>> +
>> +static void npcm7xx_load_kernel(MachineState *machine, NPCM7xxState *soc)
>> +{
>> + NPCM7xxClass *sc = NPCM7XX_GET_CLASS(soc);
>> +
>> + npcm7xx_binfo.ram_size = machine->ram_size;
>> + npcm7xx_binfo.nb_cpus = sc->num_cpus;
>> +
>> + arm_load_kernel(&soc->cpu[0], machine, &npcm7xx_binfo);
>> +}
>> +
>> +static void npcm7xx_connect_dram(NPCM7xxState *soc, MemoryRegion *dram)
>> +{
>
> You might need a check such:
>
> if (memory_region_size(dram) > NPCM7XX_DRAM_SZ) {
> error_report("Invalid RAM size, maximum allowed is 2GB");
> exit(1);
> }
>
> Or add a npcm7xx_verify_dram_size(MachineState *machine) and use
> machine->ram_size.
>
> Hardware don't really care you use more than 2GB, the memory
> above 2GB is not addressable by the SoC.
>
> Maybe the check belongs to npcm7xx_gcr_realize() instead?
>
> if (dram_size > 2 * GiB) {
> warn_report(...)
> }
> if (dram_size >= 2 * GiB) {
> s->reset_intcr3 |= 4 << 8;
> ...
>
> If the GCR is generic to a family of Nuvoton chipsets and expected to
> also work when using ARMv8-A cores (able to address >4GB), then the
> check belongs to npcm7xx_realize().
>
> With this addressed:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> + memory_region_add_subregion(get_system_memory(), NPCM7XX_DRAM_BA, dram);
>> +
>> + object_property_set_link(OBJECT(soc), OBJECT(dram), "dram-mr",
>> + &error_abort);
>> +}
>> +
>> +static NPCM7xxState *npcm7xx_create_soc(MachineState *machine,
>> + uint32_t hw_straps)
>> +{
>> + NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_GET_CLASS(machine);
>> + Object *obj;
>> +
Oh I forgot, you also need:
if (strcmp(machine->cpu_type, mc->default_cpu_type) != 0) {
error_report("This board can only be used with CPU %s",
mc->default_cpu_type);
exit(1);
}
With ...
>> + obj = object_new_with_props(nmc->soc_type, OBJECT(machine), "soc",
>> + &error_abort, NULL);
>> + object_property_set_uint(obj, hw_straps, "power-on-straps", &error_abort);
>> +
>> + return NPCM7XX(obj);
>> +}
>> +
>> +static void npcm750_evb_init(MachineState *machine)
>> +{
>> + NPCM7xxState *soc;
>> +
>> + soc = npcm7xx_create_soc(machine, NPCM750_EVB_POWER_ON_STRAPS);
>> + npcm7xx_connect_dram(soc, machine->ram);
>> + qdev_realize(DEVICE(soc), NULL, &error_abort);
>> +
>> + npcm7xx_load_kernel(machine, soc);
>> +}
>> +
>> +static void quanta_gsj_init(MachineState *machine)
>> +{
>> + NPCM7xxState *soc;
>> +
>> + soc = npcm7xx_create_soc(machine, QUANTA_GSJ_POWER_ON_STRAPS);
>> + npcm7xx_connect_dram(soc, machine->ram);
>> + qdev_realize(DEVICE(soc), NULL, &error_abort);
>> +
>> + npcm7xx_load_kernel(machine, soc);
>> +}
>> +
>> +static void npcm7xx_set_soc_type(NPCM7xxMachineClass *nmc, const char *type)
>> +{
>> + NPCM7xxClass *sc = NPCM7XX_CLASS(object_class_by_name(type));
>> + MachineClass *mc = MACHINE_CLASS(nmc);
>> +
>> + nmc->soc_type = type;
>> + mc->default_cpus = mc->min_cpus = mc->max_cpus = sc->num_cpus;
>> +}
>> +
>> +static void npcm7xx_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> + MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> + mc->no_floppy = 1;
>> + mc->no_cdrom = 1;
>> + mc->no_parallel = 1;
>> + mc->default_ram_id = "ram";
... this here:
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
>> +}
>> +
>> +/*
>> + * Schematics:
>> + * https://github.com/Nuvoton-Israel/nuvoton-info/blob/master/npcm7xx-poleg/evaluation-board/board_deliverables/NPCM750x_EB_ver.A1.1_COMPLETE.pdf
>> + */
>> +static void npcm750_evb_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> + NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_CLASS(oc);
>> + MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> + npcm7xx_set_soc_type(nmc, TYPE_NPCM750);
>> +
>> + mc->desc = "Nuvoton NPCM750 Evaluation Board (Cortex A9)";
>> + mc->init = npcm750_evb_init;
>
> Either use 'var = value' or pad and align the '=' :)
>
>> + mc->default_ram_size = 512 * MiB;
>> +};
>> +
>> +static void gsj_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> + NPCM7xxMachineClass *nmc = NPCM7XX_MACHINE_CLASS(oc);
>> + MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> + npcm7xx_set_soc_type(nmc, TYPE_NPCM730);
>> +
>> + mc->desc = "Quanta GSJ (Cortex A9)";
>> + mc->init = quanta_gsj_init;
>> + mc->default_ram_size = 512 * MiB;
>> +};
>> +
>> +static const TypeInfo npcm7xx_machine_types[] = {
>> + {
>> + .name = TYPE_NPCM7XX_MACHINE,
>> + .parent = TYPE_MACHINE,
>> + .instance_size = sizeof(NPCM7xxMachine),
>> + .class_size = sizeof(NPCM7xxMachineClass),
>> + .class_init = npcm7xx_machine_class_init,
>> + .abstract = true,
>> + }, {
>> + .name = MACHINE_TYPE_NAME("npcm750-evb"),
>> + .parent = TYPE_NPCM7XX_MACHINE,
>> + .class_init = npcm750_evb_machine_class_init,
>> + }, {
>> + .name = MACHINE_TYPE_NAME("quanta-gsj"),
>> + .parent = TYPE_NPCM7XX_MACHINE,
>> + .class_init = gsj_machine_class_init,
>> + },
>> +};
>> +
>> +DEFINE_TYPES(npcm7xx_machine_types)
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 13d163a599..c333548ce1 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -41,7 +41,7 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
>> obj-$(CONFIG_STM32F405_SOC) += stm32f405_soc.o
>> obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
>> obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
>> -obj-$(CONFIG_NPCM7XX) += npcm7xx.o
>> +obj-$(CONFIG_NPCM7XX) += npcm7xx.o npcm7xx_boards.o
>> obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
>> obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
>> obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
>>
>
next prev parent reply other threads:[~2020-07-09 6:10 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 0:35 [PATCH v5 00/11] Add Nuvoton NPCM730/NPCM750 SoCs and two BMC machines Havard Skinnemoen
2020-07-09 0:35 ` [PATCH v5 01/11] hw/misc: Add NPCM7xx System Global Control Registers device model Havard Skinnemoen
2020-07-09 6:04 ` Philippe Mathieu-Daudé
2020-07-09 6:43 ` Havard Skinnemoen
2020-07-09 16:23 ` Philippe Mathieu-Daudé
2020-07-09 17:09 ` Havard Skinnemoen
2020-07-09 17:24 ` Philippe Mathieu-Daudé
2020-07-09 17:42 ` Havard Skinnemoen
2020-07-10 9:31 ` Philippe Mathieu-Daudé
2020-07-11 6:46 ` Havard Skinnemoen
2020-07-12 5:49 ` Havard Skinnemoen
2020-07-09 0:35 ` [PATCH v5 02/11] hw/misc: Add NPCM7xx Clock Controller " Havard Skinnemoen
2020-07-15 7:18 ` Philippe Mathieu-Daudé
2020-07-09 0:36 ` [PATCH v5 03/11] hw/timer: Add NPCM7xx Timer " Havard Skinnemoen
2020-07-15 7:25 ` Philippe Mathieu-Daudé
2020-07-15 23:04 ` Havard Skinnemoen
2020-07-16 8:04 ` Philippe Mathieu-Daudé
2020-07-09 0:36 ` [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models Havard Skinnemoen
2020-07-09 6:11 ` Philippe Mathieu-Daudé
2020-07-13 15:02 ` Cédric Le Goater
2020-07-14 0:44 ` Havard Skinnemoen
2020-07-14 11:37 ` Philippe Mathieu-Daudé
2020-07-14 16:01 ` Markus Armbruster
2020-07-14 17:11 ` Philippe Mathieu-Daudé
2020-07-15 1:03 ` Havard Skinnemoen
2020-07-15 9:35 ` Markus Armbruster
2020-07-09 0:36 ` [PATCH v5 05/11] hw/arm: Add two NPCM7xx-based machines Havard Skinnemoen
2020-07-09 5:57 ` Philippe Mathieu-Daudé
2020-07-09 6:09 ` Philippe Mathieu-Daudé [this message]
2020-07-09 0:36 ` [PATCH v5 06/11] hw/arm: Load -bios image as a boot ROM for npcm7xx Havard Skinnemoen
2020-07-13 17:50 ` Philippe Mathieu-Daudé
2020-07-09 0:36 ` [PATCH v5 07/11] hw/nvram: NPCM7xx OTP device model Havard Skinnemoen
2020-07-09 0:36 ` [PATCH v5 08/11] hw/mem: Stubbed out NPCM7xx Memory Controller model Havard Skinnemoen
2020-07-09 16:29 ` Philippe Mathieu-Daudé
2020-07-09 0:36 ` [PATCH v5 09/11] hw/ssi: NPCM7xx Flash Interface Unit device model Havard Skinnemoen
2020-07-09 17:00 ` Philippe Mathieu-Daudé
2020-07-12 5:42 ` Havard Skinnemoen
2020-07-13 17:38 ` Philippe Mathieu-Daudé
2020-07-14 2:39 ` Havard Skinnemoen
2020-07-09 0:36 ` [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj Havard Skinnemoen
2020-07-13 14:57 ` Cédric Le Goater
2020-07-13 17:59 ` Philippe Mathieu-Daudé
2020-07-13 18:02 ` Philippe Mathieu-Daudé
2020-07-14 2:56 ` Havard Skinnemoen
2020-07-14 9:16 ` Markus Armbruster
2020-07-14 11:29 ` Philippe Mathieu-Daudé
2020-07-14 16:21 ` Markus Armbruster
2020-07-14 17:16 ` Philippe Mathieu-Daudé
2020-07-15 9:00 ` Markus Armbruster
2020-07-15 10:57 ` Philippe Mathieu-Daudé
2020-07-15 20:54 ` Havard Skinnemoen
2020-07-16 20:56 ` Havard Skinnemoen
2020-07-17 7:48 ` Philippe Mathieu-Daudé
2020-07-17 8:03 ` Thomas Huth
2020-07-17 8:27 ` Philippe Mathieu-Daudé
2020-07-17 9:00 ` Philippe Mathieu-Daudé
2020-07-17 19:18 ` Havard Skinnemoen
2020-07-17 20:21 ` Cédric Le Goater
2020-07-17 20:52 ` Philippe Mathieu-Daudé
2020-07-17 20:57 ` Havard Skinnemoen
2020-07-20 7:58 ` Markus Armbruster
2020-07-15 7:42 ` Cédric Le Goater
2020-07-15 21:19 ` Havard Skinnemoen
2020-07-09 0:36 ` [PATCH v5 11/11] docs/system: Add Nuvoton machine documentation Havard Skinnemoen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c506f212-2119-32f3-5bfc-7dbb1e5fdf28@amsat.org \
--to=f4bug@amsat.org \
--cc=Avi.Fishman@nuvoton.com \
--cc=clg@kaod.org \
--cc=hskinnemoen@google.com \
--cc=joel@jms.id.au \
--cc=kfting@nuvoton.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).