Hi Philippe,
Thanks again for your quick and helpful feedback! :-)
On Tue, Dec 17, 2019 at 8:31 AM Philippe Mathieu-Daudé
<address@hidden <mailto:address@hidden>> 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 <address@hidden
<mailto:address@hidden>>
> Tested-by: KONRAD Frederic <address@hidden
<mailto: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
<mailto: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);
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?