[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 02/10] hw: arm: add Xunlong Orange Pi PC machine
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 02/10] hw: arm: add Xunlong Orange Pi PC machine |
Date: |
Tue, 17 Dec 2019 08:31:27 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
Hi Niek,
On 12/17/19 12:35 AM, Niek Linnenbank wrote:
The Xunlong Orange Pi PC is an Allwinner H3 System on Chip
based embedded computer with mainline support in both U-Boot
and Linux. The board comes with a Quad Core Cortex A7 @ 1.3GHz,
512MB RAM, 100Mbit ethernet, USB, SD/MMC, USB, HDMI and
various other I/O. This commit add support for the Xunlong
Orange Pi PC machine.
Signed-off-by: Niek Linnenbank <address@hidden>
Tested-by: KONRAD Frederic <address@hidden>
---
hw/arm/orangepi.c | 101 +++++++++++++++++++++++++++++++++++++++++++
MAINTAINERS | 1 +
hw/arm/Makefile.objs | 2 +-
3 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 hw/arm/orangepi.c
diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
new file mode 100644
index 0000000000..62cefc8c06
--- /dev/null
+++ b/hw/arm/orangepi.c
@@ -0,0 +1,101 @@
+/*
+ * Orange Pi emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank <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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "hw/sysbus.h"
+#include "hw/boards.h"
+#include "hw/qdev-properties.h"
+#include "hw/arm/allwinner-h3.h"
+
+static struct arm_boot_info orangepi_binfo = {
+ .board_id = -1,
+};
+
+typedef struct OrangePiState {
+ AwH3State *h3;
+ MemoryRegion sdram;
+} OrangePiState;
+
+static void orangepi_init(MachineState *machine)
+{
+ OrangePiState *s = g_new(OrangePiState, 1);
+
+ /* Only allow Cortex-A7 for this board */
+ if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a7")) != 0) {
+ error_report("This board can only be used with cortex-a7 CPU");
+ exit(1);
+ }
+
+ s->h3 = AW_H3(object_new(TYPE_AW_H3));
+
+ /* Setup timer properties */
+ object_property_set_int(OBJECT(&s->h3->timer), 32768, "clk0-freq",
+ &error_abort);
You access the timer object which is contained inside the soc object,
but the soc isn't realized yet... I wonder if this is OK. Usually what
we do is, either:
- add a 'xtal-freq-hz' property to the SoC, set it here in the board,
then in soc::realize() set the property to the timer
- add an alias in the SoC to the timer 'freq-hz' property:
object_property_add_alias(soc, "xtal-freq-hz", OBJECT(&s->timer),
"freq-hz", &error_abort);
Also, if you use &error_abort, a failure in object_property_set_int()
triggers abort(). See "qapi/error.h":
* If @errp is &error_abort, print a suitable message and abort().
* If @errp is &error_fatal, print a suitable message and exit(1).
+ if (error_abort != NULL) {
+ error_reportf_err(error_abort, "Couldn't set clk0 frequency: ");
+ exit(1);
+ }
So this if() block is useless.
+
+ object_property_set_int(OBJECT(&s->h3->timer), 24000000, "clk1-freq",
+ &error_abort);
+ if (error_abort != NULL) {
+ error_reportf_err(error_abort, "Couldn't set clk1 frequency: ");
+ exit(1);
+ }
Similarly, remove if() block.
+
+ /* Mark H3 object realized */
+ object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort);
+ if (error_abort != NULL) {
+ error_reportf_err(error_abort, "Couldn't realize Allwinner H3: ");
+ exit(1);
+ }
Similarly, remove if() block.
+
+ /* RAM */
+ if (machine->ram_size > 1 * GiB) {
+ error_report("Requested ram size is too large for this machine: "
+ "maximum is 1GB");
Per http://www.orangepi.org/orangepipc/ this board comes with a specific
amount of RAM. I'd enforce the default (1GiB) and refuse other cases.
I noticed this by testing your series, without specifying the memory
size you suggested in the cover (512) it defaults to 128 MiB, and the
Raspian userland fails:
[ *** ] (2 of 4) A start job is running for…Persistent Storage (37s /
2min 1s)
[ *** ] (2 of 4) A start job is running for…Persistent Storage (38s /
2min 1s)
[ OK ] Started Flush Journal to Persistent Storage.
Starting Create Volatile Files and Directories...
Starting Armbian ZRAM config...
[ **] (3 of 6) A start job is running for…s and Directories (55s / no
limit)
[ *] (3 of 6) A start job is running for…s and Directories (55s / no
limit)
[ **] (3 of 6) A start job is running for…s and Directories (56s / no
limit)
[ OK ] Started Create Volatile Files and Directories.
[*** ] (5 of 6) A start job is running for… ZRAM config (1min 10s /
1min 19s)
[** ] (5 of 6) A start job is running for… ZRAM config (1min 12s /
1min 19s)
[* ] (5 of 6) A start job is running for… ZRAM config (1min 13s /
1min 19s)
[FAILED] Failed to start Armbian ZRAM config.
See 'systemctl status armbian-zram-config.service' for details.
+ exit(1);
+ }
+ memory_region_allocate_system_memory(&s->sdram, NULL, "orangepi.ram",
There is only one type of ram on this machine, I'd simply name this "sdram".
+ machine->ram_size);
+ memory_region_add_subregion(get_system_memory(),
s->h3->memmap[AW_H3_SDRAM],
+ &s->sdram);
+
+ /* Load target kernel */
+ orangepi_binfo.loader_start = s->h3->memmap[AW_H3_SDRAM];
+ orangepi_binfo.ram_size = machine->ram_size;
+ orangepi_binfo.nb_cpus = AW_H3_NUM_CPUS;
+ arm_load_kernel(ARM_CPU(first_cpu), machine, &orangepi_binfo);
I wonder if we should tell the user '-bios' is not supported on this
machine.
+}
+
+static void orangepi_machine_init(MachineClass *mc)
+{
+ mc->desc = "Orange Pi PC";
+ mc->init = orangepi_init;
+ mc->units_per_default_bus = 1;
Maybe "units_per_default_bus = 1" belongs to patch 9 "add SD/MMC host
controller".
+ mc->min_cpus = AW_H3_NUM_CPUS;
+ mc->max_cpus = AW_H3_NUM_CPUS;
+ mc->default_cpus = AW_H3_NUM_CPUS;
+ mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7");
Please add:
mc->default_ram_size = 1 * GiB;
+}
+
+DEFINE_MACHINE("orangepi-pc", orangepi_machine_init)
diff --git a/MAINTAINERS b/MAINTAINERS
index aae1a049b4..db682e49ca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -486,6 +486,7 @@ L: address@hidden
S: Maintained
F: hw/*/allwinner-h3*
F: include/hw/*/allwinner-h3*
+F: hw/arm/orangepi.c
ARM PrimeCell and CMSDK devices
M: Peter Maydell <address@hidden>
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 956e496052..8d5ea453d5 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -34,7 +34,7 @@ obj-$(CONFIG_DIGIC) += digic.o
obj-$(CONFIG_OMAP) += omap1.o omap2.o
obj-$(CONFIG_STRONGARM) += strongarm.o
obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o
+obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o orangepi.o
obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
- [PATCH v2 00/10] Add Allwinner H3 SoC and Orange Pi PC Machine, Niek Linnenbank, 2019/12/16
- [PATCH v2 04/10] arm: allwinner-h3: add USB host controller, Niek Linnenbank, 2019/12/16
- [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Niek Linnenbank, 2019/12/16
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Niek Linnenbank, 2019/12/16
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Peter Maydell, 2019/12/17
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Richard Henderson, 2019/12/17
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Peter Maydell, 2019/12/17
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Niek Linnenbank, 2019/12/18
- Re: [PATCH v2 06/10] arm/arm-powerctl: rebuild hflags after setting CP15 bits in arm_set_cpu_on(), Richard Henderson, 2019/12/18