Re: [PATCH v3 02/17] hw/arm: add Xunlong Orange Pi PC machine

[Niek Linnenbank]

On Wed, Jan 8, 2020 at 11:44 PM Philippe Mathieu-Daudé 
wrote:

> On 1/8/20 9:00 PM, 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 
> > Tested-by: KONRAD Frederic 
> > ---
> >   hw/arm/orangepi.c| 93 
> >   MAINTAINERS  |  1 +
> >   hw/arm/Makefile.objs |  2 +-
> >   3 files changed, 95 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 00..051184f14f
> > --- /dev/null
> > +++ b/hw/arm/orangepi.c
> > @@ -0,0 +1,93 @@
> > +/*
> > + * Orange Pi emulation
> > + *
> > + * Copyright (C) 2019 Niek Linnenbank 
> > + *
> > + * 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  >.
> > + */
> > +
> > +#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"
> > +#include "sysemu/sysemu.h"
> > +
> > +static struct arm_boot_info orangepi_binfo = {
> > +.board_id = -1,
>
> Since you use a dtb, I suppose Linux ignores this board_id.
>
> Nitpick, you can set this here:
>
> .nb_cpus = AW_H3_NUM_CPUS,
>
> > +};
> > +
> > +typedef struct OrangePiState {
> > +AwH3State *h3;
> > +MemoryRegion sdram;
> > +} OrangePiState;
> > +
> > +static void orangepi_init(MachineState *machine)
> > +{
> > +OrangePiState *s = g_new(OrangePiState, 1);
> > +
>
> I'd do the bios check first, as it is cheap and avoid allocating RAM + H3:
>
> /* Load target kernel or start using BootROM */
> if (bios_name) {
> error_report("BIOS not supported for this machine");
> exit(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), 32768, "clk0-freq",
> > +&error_abort);
> > +object_property_set_int(OBJECT(s->h3), 2400, "clk1-freq",
>
> I find 24 * 1000 * 1000 easier to review.
>
> > +&error_abort);
> > +
> > +/* Mark H3 object realized */
> > +object_property_set_bool(OBJECT(s->h3), true, "realized",
> &error_abort);
> > +
> > +/* SDRAM */
> > +if (machine->ram_size != 1 * GiB) {
> > +error_report("Requested ram size is not supported for this
> machine: "
> > + "restricted to 1GiB");
>
> Maybe easier as "This machine can only be used with 1GiB of RAM"
>
> > +exit(1);
> > +}
> > +memory_region_allocate_system_memory(&s->sdram, NULL, "sdram",
> > + machine->ram_size);
> > +memory_region_add_subregion(get_system_memory(),
> s->h3->memmap[AW_H3_SDRAM],
> > +&s->sdram);
> > +
> > +/* Load target kernel or start using BootROM */
> > +if (bios_name) {
> > +error_report("BIOS not supported for this machine");
> > +exit(1);
> > +}
> > +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);
> > +}
> > +
> > +static void orangepi_machine_init(MachineClass *mc)
> > +{
> > +mc->desc = "Orange Pi PC";
> > +mc->init = orangepi_init;
> > +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");
> > +mc->default_ram_size = 1 * GiB;
> > +}
> > +
> > +DEFINE_MACHINE("orangepi-pc"

Re: [PATCH v3 02/17] hw/arm: add Xunlong Orange Pi PC machine

[Philippe Mathieu-Daudé]

On 1/8/20 9:00 PM, 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 
Tested-by: KONRAD Frederic 
---
  hw/arm/orangepi.c| 93 
  MAINTAINERS  |  1 +
  hw/arm/Makefile.objs |  2 +-
  3 files changed, 95 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 00..051184f14f
--- /dev/null
+++ b/hw/arm/orangepi.c
@@ -0,0 +1,93 @@
+/*
+ * Orange Pi emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * 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 .
+ */
+
+#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"
+#include "sysemu/sysemu.h"
+
+static struct arm_boot_info orangepi_binfo = {
+.board_id = -1,


Since you use a dtb, I suppose Linux ignores this board_id.

Nitpick, you can set this here:

   .nb_cpus = AW_H3_NUM_CPUS,


+};
+
+typedef struct OrangePiState {
+AwH3State *h3;
+MemoryRegion sdram;
+} OrangePiState;
+
+static void orangepi_init(MachineState *machine)
+{
+OrangePiState *s = g_new(OrangePiState, 1);
+


I'd do the bios check first, as it is cheap and avoid allocating RAM + H3:

   /* Load target kernel or start using BootROM */
   if (bios_name) {
   error_report("BIOS not supported for this machine");
   exit(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), 32768, "clk0-freq",
+&error_abort);
+object_property_set_int(OBJECT(s->h3), 2400, "clk1-freq",


I find 24 * 1000 * 1000 easier to review.


+&error_abort);
+
+/* Mark H3 object realized */
+object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort);
+
+/* SDRAM */
+if (machine->ram_size != 1 * GiB) {
+error_report("Requested ram size is not supported for this machine: "
+ "restricted to 1GiB");


Maybe easier as "This machine can only be used with 1GiB of RAM"


+exit(1);
+}
+memory_region_allocate_system_memory(&s->sdram, NULL, "sdram",
+ machine->ram_size);
+memory_region_add_subregion(get_system_memory(), 
s->h3->memmap[AW_H3_SDRAM],
+&s->sdram);
+
+/* Load target kernel or start using BootROM */
+if (bios_name) {
+error_report("BIOS not supported for this machine");
+exit(1);
+}
+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);
+}
+
+static void orangepi_machine_init(MachineClass *mc)
+{
+mc->desc = "Orange Pi PC";
+mc->init = orangepi_init;
+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");
+mc->default_ram_size = 1 * GiB;
+}
+
+DEFINE_MACHINE("orangepi-pc", orangepi_machine_init)
diff --git a/MAINTAINERS b/MAINTAINERS
index dc2d7991bf..6e1b92b5fa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -487,6 +487,7 @@ L: qemu-...@nongnu.org
  S: Maintained
  F: hw/*/allwinner-h3*
  F: include/hw/*/allwinner-h3*
+F: hw/arm/orangepi.c
  
  ARM PrimeCell and CMSDK devices

  M: Peter Maydell 
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 omap

[PATCH v3 02/17] hw/arm: add Xunlong Orange Pi PC machine

[Niek Linnenbank]

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 
Tested-by: KONRAD Frederic 
---
 hw/arm/orangepi.c| 93 
 MAINTAINERS  |  1 +
 hw/arm/Makefile.objs |  2 +-
 3 files changed, 95 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 00..051184f14f
--- /dev/null
+++ b/hw/arm/orangepi.c
@@ -0,0 +1,93 @@
+/*
+ * Orange Pi emulation
+ *
+ * Copyright (C) 2019 Niek Linnenbank 
+ *
+ * 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 .
+ */
+
+#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"
+#include "sysemu/sysemu.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), 32768, "clk0-freq",
+&error_abort);
+object_property_set_int(OBJECT(s->h3), 2400, "clk1-freq",
+&error_abort);
+
+/* Mark H3 object realized */
+object_property_set_bool(OBJECT(s->h3), true, "realized", &error_abort);
+
+/* SDRAM */
+if (machine->ram_size != 1 * GiB) {
+error_report("Requested ram size is not supported for this machine: "
+ "restricted to 1GiB");
+exit(1);
+}
+memory_region_allocate_system_memory(&s->sdram, NULL, "sdram",
+ machine->ram_size);
+memory_region_add_subregion(get_system_memory(), 
s->h3->memmap[AW_H3_SDRAM],
+&s->sdram);
+
+/* Load target kernel or start using BootROM */
+if (bios_name) {
+error_report("BIOS not supported for this machine");
+exit(1);
+}
+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);
+}
+
+static void orangepi_machine_init(MachineClass *mc)
+{
+mc->desc = "Orange Pi PC";
+mc->init = orangepi_init;
+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");
+mc->default_ram_size = 1 * GiB;
+}
+
+DEFINE_MACHINE("orangepi-pc", orangepi_machine_init)
diff --git a/MAINTAINERS b/MAINTAINERS
index dc2d7991bf..6e1b92b5fa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -487,6 +487,7 @@ L: qemu-...@nongnu.org
 S: Maintained
 F: hw/*/allwinner-h3*
 F: include/hw/*/allwinner-h3*
+F: hw/arm/orangepi.c
 
 ARM PrimeCell and CMSDK devices
 M: Peter Maydell 
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
-- 
2.17.1