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

[Awais Masood <awais.masood@xxxxxxxxxx>]

Hi,

On 09/29/2017 09:35 PM, Andre Przywara wrote:
> Hi,
> 
> On 09/28/2017 03:49 PM, Andre Przywara wrote:
>> Hi,
>>
>> On 09/28/2017 01:03 PM, Julien Grall wrote:
>>> 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.
> 
> ...
> 
>>>> --- 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?
>>
>> Yes, both sun7i-a20.dtsi and the H5 .dts have the WDT.
>> Its compatible strings are sun4i-a10-wdt and sun6i-a31-wdt, respectively. I 
>> have to check what the differences are, but I guess for our purposes these 
>> should be small.
>> That seems like a call to some proper DT driven timer/WDT driver?
> 
> Scratch that. I just see that this is solely used for the reset function. So 
> we should not need this for the H5 (and the A64 for that matter). We may need 
> this for the H3 (Cortex-A7) support, however, which seems quite popular on 
> cheap boards.
> 

Since reset routine will not be required with PSCI, I assume should revert the 
reset code changes for this H5 patch and leave the DT retrieval for another 
patch that adds H3 support. Or should I try that stuff for next version of this 
patch? 

Awais

> Cheers,
> Andre
> 
>>>>   #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.
>>
>> Yes, indeed, on the H5 PSCI 0.2 reset works via ATF.
>>
>>> 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?
>>
>> It claims 0.2, but in fact it seems not to be fully compliant, as (from 
>> looking at the code) U-Boot lacks the reset and poweroff calls. But it looks 
>> rather straight-forward to add them, as U-Boot knows how to reset and one 
>> would just need to wire up psci_system_reset to this.
>>
>>> For H5, I would impose PSCI 0.2 as the way to reset the platform.
>>
>> Yes.
>>
>>> 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.
>>
>> Not sure we should push people to upgrade U-Boot in general to be able to 
>> use Xen, but as even current mainline U-Boot doesn't seem to support it, I 
>> would rather leave the current reset support code in. Last time I checked 
>> Linux does the same.
>>
>> Cheers,
>> Andre.

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