diff mbox series

[v2,02/10] hw: arm: add Xunlong Orange Pi PC machine

Message ID 20191216233519.29030-3-nieklinnenbank@gmail.com
State New
Headers show
Series Add Allwinner H3 SoC and Orange Pi PC Machine | expand

Commit Message

Niek Linnenbank Dec. 16, 2019, 11:35 p.m. UTC
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 <nieklinnenbank@gmail.com>
Tested-by: KONRAD Frederic <frederic.konrad@adacore.com>
---
 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

Comments

Philippe Mathieu-Daudé Dec. 17, 2019, 7:31 a.m. UTC | #1
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 <nieklinnenbank@gmail.com>
> Tested-by: KONRAD Frederic <frederic.konrad@adacore.com>
> ---
>   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 <nieklinnenbank@gmail.com>
> + *
> + * 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: qemu-arm@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 <peter.maydell@linaro.org>
> 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
>
Niek Linnenbank Dec. 18, 2019, 8:14 p.m. UTC | #2
Hi Philippe,

Thanks again for your quick and helpful feedback! :-)

On Tue, Dec 17, 2019 at 8:31 AM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> 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 <nieklinnenbank@gmail.com>
> > Tested-by: KONRAD Frederic <frederic.konrad@adacore.com>
> > ---
> >   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 <nieklinnenbank@gmail.com>
> > + *
> > + * 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);
>
Good point. I shall rework that part using your suggestion.
Actually, I copied the timer support code from the existing cubieboard.c
that has
the Allwinner A10, so potentially the same problem is there.

While looking more closer at this part, I now also discovered that the
timer module from the Allwinner H3 is
mostly a stripped down version of the timer module in the Allwinner A10:

  Allwinner A10, 10.2 Timer Register List, page 85:
  https://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf

The A10 version has six timers, where the H3 has only two. That should be
fine I would say, the guest would simply
use those available on H3 and ignore the rest. There is however one
conflicting difference: the WDOG0 registers in the Allwinner H3 start
at a different offset and are also different. The current A10 timer does
not currently implement the watchdog part.

The watchdog part of this timer is relevant for the 'reset' command in
U-Boot: that does not work right now, because
U-Boot implements the reset for the Allwinner H3 boards by letting this
watchdog expire (and we dont emulate it).
Also, this timer module is required for booting Linux, without it the
kernel crashes using the sunxi_defconfig:

[    0.000000] PC is at sun4i_timer_init+0x34/0x168
[    0.000000] LR is at sun4i_timer_init+0x2c/0x168
[    0.000000] pc : [<c07fa634>]    lr : [<c07fa62c>]    psr: 600000d3
[    0.000000] sp : c0a03f70  ip : eec00188  fp : ef7ed040
...
[    0.000000] [<c07fa634>] (sun4i_timer_init) from [<c07fa4e8>]
(timer_probe+0x74/0xe4)
[    0.000000] [<c07fa4e8>] (timer_probe) from [<c07d9c10>]
(start_kernel+0x2e0/0x440)
[    0.000000] [<c07d9c10>] (start_kernel) from [<00000000>] (0x0)


So in my opinion its a bit of a trade off here: we can keep it like this
and re-use the A10 timer for now, and perhaps
attempt to generalize that module for proper use in both SoCs. Or we can
introduce a new H3 specific timer module.
What do you think?


>
> 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.
>
> Ah ok, I'll remove them.


> > +
> > +    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.
>

OK sure, I'll change it to a enforcing 1GiB. I do recall we briefly
discussed this
in v1. Then we agreed to make it an upper limit for use cases where
resources are
limited which is why I changed it like this.


> 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:
>

Indeed! By the way, this is also the case for U-Boot: it freezes when using
128MiB.
Actually when working on the initial code I searched a bit for a way
to set a default ram size, but could not find it at that time. But now I
see in your comment below,
it can be done simply with mc->default_ram_size. Thanks a lot, I will
surely add that!


>
> [ ***  ] (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".
>

OK!


> > +                                         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.
>

Good suggestion, its not handled, at least not anywhere in the code I added
for H3 support.
Shall I make it an error_report followed by exit(1), similar to the 1GiB
check?


>
> > +}
> > +
> > +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".
>
True, it should be in patch 9 indeed. I overlooked this when separating the
work in individual patches.
Now I am also wondering if I actually need this setting. Without it, the SD
device still works fine.
I did some greps in the code to discover what it is used for, but its not
very clear to me yet. Is this ment to
restrict machines to only one harddisk (or SD card)? If I try to supply
multiple SD cards with multiple -sd arguments,
this error is printed, regardless of having units_per_default_bus=1 or not:
   qemu-system-arm: -sd test3.ext2: machine type does not support
if=sd,bus=1,unit=0


>
> > +    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;
>
Yes, thanks!


>
> > +}
> > +
> > +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: qemu-arm@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 <peter.maydell@linaro.org>
> > 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
> >
>
>
Regards,
Niek
Philippe Mathieu-Daudé Dec. 19, 2019, 7:06 p.m. UTC | #3
On 12/18/19 9:14 PM, Niek Linnenbank wrote:
> Hi Philippe,
> 
> Thanks again for your quick and helpful feedback! :-)
> 
> On Tue, Dec 17, 2019 at 8:31 AM Philippe Mathieu-Daudé 
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> 
>     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 <nieklinnenbank@gmail.com
>     <mailto:nieklinnenbank@gmail.com>>
>      > Tested-by: KONRAD Frederic <frederic.konrad@adacore.com
>     <mailto:frederic.konrad@adacore.com>>
>      > ---
>      >   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 <nieklinnenbank@gmail.com
>     <mailto:nieklinnenbank@gmail.com>>
>      > + *
>      > + * 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);
> 
> Good point. I shall rework that part using your suggestion.
> Actually, I copied the timer support code from the existing cubieboard.c 
> that has
> the Allwinner A10, so potentially the same problem is there.
> 
> While looking more closer at this part, I now also discovered that the 
> timer module from the Allwinner H3 is
> mostly a stripped down version of the timer module in the Allwinner A10:
> 
>    Allwinner A10, 10.2 Timer Register List, page 85:
> https://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf
> 
> The A10 version has six timers, where the H3 has only two. That should 
> be fine I would say, the guest would simply
> use those available on H3 and ignore the rest. There is however one 
> conflicting difference: the WDOG0 registers in the Allwinner H3 start
> at a different offset and are also different. The current A10 timer does 
> not currently implement the watchdog part.
> 
> The watchdog part of this timer is relevant for the 'reset' command in 
> U-Boot: that does not work right now, because
> U-Boot implements the reset for the Allwinner H3 boards by letting this 
> watchdog expire (and we dont emulate it).
> Also, this timer module is required for booting Linux, without it the 
> kernel crashes using the sunxi_defconfig:
> 
> [    0.000000] PC is at sun4i_timer_init+0x34/0x168
> [    0.000000] LR is at sun4i_timer_init+0x2c/0x168
> [    0.000000] pc : [<c07fa634>]    lr : [<c07fa62c>]    psr: 600000d3
> [    0.000000] sp : c0a03f70  ip : eec00188  fp : ef7ed040
> ...
> [    0.000000] [<c07fa634>] (sun4i_timer_init) from [<c07fa4e8>] (timer_probe+0x74/0xe4)
> [    0.000000] [<c07fa4e8>] (timer_probe) from [<c07d9c10>] (start_kernel+0x2e0/0x440)
> [    0.000000] [<c07d9c10>] (start_kernel) from [<00000000>] (0x0)
> 
> 
> So in my opinion its a bit of a trade off here: we can keep it like this 
> and re-use the A10 timer for now, and perhaps
> attempt to generalize that module for proper use in both SoCs. Or we can 
> introduce a new H3 specific timer module.
> What do you think?

See my answer about generalize/reuse here:
http://mid.mail-archive.com/20191219185127.24388-1-f4bug@amsat.org
diff mbox series

Patch

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 <nieklinnenbank@gmail.com>
+ *
+ * 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);
+    if (error_abort != NULL) {
+        error_reportf_err(error_abort, "Couldn't set clk0 frequency: ");
+        exit(1);
+    }
+
+    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);
+    }
+
+    /* 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);
+    }
+
+    /* RAM */
+    if (machine->ram_size > 1 * GiB) {
+        error_report("Requested ram size is too large for this machine: "
+                     "maximum is 1GB");
+        exit(1);
+    }
+    memory_region_allocate_system_memory(&s->sdram, NULL, "orangepi.ram",
+                                         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);
+}
+
+static void orangepi_machine_init(MachineClass *mc)
+{
+    mc->desc = "Orange Pi PC";
+    mc->init = orangepi_init;
+    mc->units_per_default_bus = 1;
+    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");
+}
+
+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: qemu-arm@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 <peter.maydell@linaro.org>
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