Hi Philippe, On Thu, Dec 19, 2019 at 7:51 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi, > > Niek added the H3 SoC in [1] and noticed in [2] the timer > controller is very similar (less timers, watchdog register > placed at different address). > > On 12/18/19 9:14 PM, Niek Linnenbank wrote: > > 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. > [...] > > 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? > > As an answer to his question, this series is to help him to > reuse the A10 timer controller instead of adding a new model > to the codebase. > Great!! This certainly answers my question indeed! I've applied this patch on top of the allwinner H3 v2 series to test it, and after changing the type from AwA10PITState to the new AllwinnerTmrCtrlState, the code compiled and ran linux/u-boot without any problems: diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h index 357bdfa711..fa0219fa1b 100644 --- a/include/hw/arm/allwinner-h3.h +++ b/include/hw/arm/allwinner-h3.h @@ -76,7 +76,7 @@ typedef struct AwH3State { ARMCPU cpus[AW_H3_NUM_CPUS]; const hwaddr *memmap; - AwA10PITState timer; + AllwinnerTmrCtrlState timer; AwH3ClockState ccu; AwH3CpuCfgState cpucfg; AwH3SysconState syscon; Also, I tested with the A10 cubieboard machine, and it also still works fine: ./arm-softmmu/qemu-system-arm -M cubieboard -kernel zImage -nographic -append 'console=ttyS0,115200 earlyprintk usbcore.nousb root=/dev/sda ro init=/sbin/init' -dtb sun4i-a10-cubieboard.dtb -m 512 -drive file=rootfs.ext2,if=none,id=drive-sata0-0-0,format=raw -device ide-hd,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 -nic user [ 0.000000] Booting Linux on physical CPU 0x0 [ 0.000000] Linux version 5.2.11 (me@host) (gcc version 5.4.0 20160609 (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.9)) #1 SMP Fri Sep 13 22:48:39 CEST 2019 [ 0.000000] CPU: ARMv7 Processor [410fc080] revision 0 (ARMv7), cr=10c5387d [ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing instruction cache [ 0.000000] OF: fdt: Machine model: Cubietech Cubieboard ... So for me this works with both the H3 and A10: Tested-by: Niek Linnenbank <nieklinnenb...@gmail.com> Regards, Niek > > [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg665532.html > [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg666304.html > > Philippe Mathieu-Daudé (13): > hw/timer/allwinner: Use the AW_A10_PIT_TIMER_NR definition > hw/timer/allwinner: Add AW_PIT_TIMER_MAX definition > hw/timer/allwinner: Remove unused definitions > hw/timer/allwinner: Move definitions from header to source > hw/timer/allwinner: Rename the ptimer field > hw/timer/allwinner: Rename 'timer_context' as 'timer' > hw/timer/allwinner: Move timer specific fields into AwA10TimerContext > hw/timer/allwinner: Add a timer_count field > hw/timer/allwinner: Rename AwA10TimerContext as AllwinnerTmrState > hw/timer/allwinner: Rename AwA10PITState as AllwinnerTmrCtrlState > hw/timer/allwinner: Introduce TYPE_AW_COMMON_PIT abstract device > hw/timer/allwinner: Rename AW_A10_PIT() as AW_TIMER_CTRL() > hw/timer/allwinner: Rename functions not specific to the A10 SoC > > include/hw/arm/allwinner-a10.h | 2 +- > include/hw/timer/allwinner-a10-pit.h | 54 ++---- > hw/timer/allwinner-a10-pit.c | 271 +++++++++++++++++---------- > 3 files changed, 192 insertions(+), 135 deletions(-) > > -- > 2.21.0 > > -- Niek Linnenbank
