Re: [Xen-devel] [PATCH v2 1/2] xen/arm64: Add Support for Allwinner H5 (sun50i)

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 09/26/2017 10:37 AM, Awais Masood wrote:
> This patch adds support for Allwinner H5/sun50i SoC.
>
> Makefile updated to enable ARM64 compilation for sunxi.c.
>
> sunxi.c updates include:
>    - Addition of H5/sun50i dt compatibility string.
>    - Handling of different Watchdog timer base addresses on sun7i
>      and sun50i.
>
> Tested on Orange Pi PC2
>
> Signed-off-by: Awais Masood <awais.masood@xxxxxxxxxx>
>
> ---
> Changes since v1:
>    - Improved patch description
> ---
>   xen/arch/arm/platforms/Makefile |  1 +
>   xen/arch/arm/platforms/sunxi.c  | 40 +++++++++++++++++++++++++++++++---------
>   2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 49fa683..722897a 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_ARM_32) += omap5.o
>   obj-$(CONFIG_ARM_32) += rcar2.o
>   obj-$(CONFIG_ARM_64) += seattle.o
>   obj-$(CONFIG_ARM_32) += sunxi.o
> +obj-$(CONFIG_ARM_64) += sunxi.o
Please use obj-y += sunxi.o as the platform is now supported by both 
Arm32 and Arm64.
>   obj-$(CONFIG_ARM_64) += xgene-storm.o
>   obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> diff --git a/xen/arch/arm/platforms/sunxi.c b/xen/arch/arm/platforms/sunxi.c
> index 0ba7b3d..06d62e7 100644
> --- a/xen/arch/arm/platforms/sunxi.c
> +++ b/xen/arch/arm/platforms/sunxi.c
> @@ -22,18 +22,18 @@
>   #include <asm/io.h>
>   
>   /* Watchdog constants: */
> -#define SUNXI_WDT_BASE            0x01c20c90
> +#define SUNXI_WDT_A20_BASE        0x01c20c90
> +#define SUNXI_WDT_H5_BASE         0x01c20cA0
I know that we hardcoded this value for the A20. However, I am wondering 
if we could find this address from the Device-Tree?
>   #define SUNXI_WDT_MODE            0x04
> -#define SUNXI_WDT_MODEADDR        (SUNXI_WDT_BASE + SUNXI_WDT_MODE)
>   #define SUNXI_WDT_MODE_EN         (1 << 0)
>   #define SUNXI_WDT_MODE_RST_EN     (1 << 1)
>   
>   
> -static void sunxi_reset(void)
> +static void sunxi_reset(u32 base)
>   {
>       void __iomem *wdt;
>   
> -    wdt = ioremap_nocache(SUNXI_WDT_MODEADDR & PAGE_MASK, PAGE_SIZE);
> +    wdt = ioremap_nocache((base + SUNXI_WDT_MODE) & PAGE_MASK, PAGE_SIZE);
>       if ( !wdt )
>       {
>           dprintk(XENLOG_ERR, "Unable to map watchdog register!\n");
> @@ -42,19 +42,35 @@ static void sunxi_reset(void)
>   
>       /* Enable watchdog to trigger a reset after 500 ms: */
>       writel(SUNXI_WDT_MODE_EN | SUNXI_WDT_MODE_RST_EN,
> -      wdt + (SUNXI_WDT_MODEADDR & ~PAGE_MASK));
> +      wdt + ((base + SUNXI_WDT_MODE) & ~PAGE_MASK));
>       iounmap(wdt); >
>       for (;;)
>           wfi();
>   }
>
> -static const char * const sunxi_dt_compat[] __initconst =
> +static void sunxi_a20_reset(void)
> +{
> +    sunxi_reset(SUNXI_WDT_A20_BASE);
> +}
> +
> +static void sunxi_h5_reset(void)
> +{
> +    sunxi_reset(SUNXI_WDT_H5_BASE);
If I read correctly the Device-Tree for 
(linux/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi), the firmware is 
supporting PSCI 0.2.
PSCI 0.2 provides call for power-off/reset, so implementation the reset 
callback should not be necessary.
Similarly the cubietrucks we have in osstest are using PSCI 0.2 and 
should not need the reset. Andre do you know if it is the case for all 
the A20?
For H5, I would impose PSCI 0.2 as the way to reset the platform. I am 
leaning towards the same for A20 given that it would just be a matter of 
upgrading the bootloader. Most likely you would have already done that 
to get fixes.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel