RE: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI is enabled

[Biju Das <biju.das@bp.renesas.com>]

Hi Oleksandr,

Thanks for the patch.

> Subject: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI is
> enabled
> 
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> If CONFIG_PSCI is enabled then most likely we are running on PSCI-enabled
> U-Boot which, we assume, has already taken care of configuring ARCH timer
> stuff before switching to non-secure mode.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> ---
> A bit of context here...
> 
> We are highly interested in Renesas "Stout" board support (r8a7790) in Xen
> hypervisor. The reason is to have fully supported HW for performing
> "OSSTEST" (Xen automatic test system) on ARM32.
> 
> To reach that target we need a "generic way" for the secondary CPU cores
> bring up and switching them to non-secure hyp mode.
> So, the PSCI as a generic well-known way to bring up CPUs, was chosen for
> that purpose.
> 
> You can find corresponding patches for U-Boot here:
> http://u-boot.10912.n7.nabble.com/PATCH-0-3-PSCI-support-for-r8a7790-
> SoC-Lager-Stout-boards-td357352.html
> 
> You can find corresponding patches for Xen hypervisor here:
> https://www.mail-archive.com/xen-
> devel@lists.xenproject.org/msg43332.html
> 
> To sumarize:
> Together with enabling CONFIG_PSCI in shmobile_defconfig, current patch is
> a minimal required change needed to run mainline Linux on top of PSCI-
> enabled U-Boot.
> There is no need to modify device tree. U-Boot will take care of inserting
> proper "enable-method" strings in CPU nodes.
> 
> ---
>  arch/arm/mach-shmobile/setup-rcar-gen2.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-
> shmobile/setup-rcar-gen2.c
> index eea60b2..bac4490 100644
> --- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
> @@ -32,7 +32,7 @@ static const struct of_device_id cpg_matches[]
> __initconst = {
>  	{ /* sentinel */ }
>  };
> 
> -static unsigned int __init get_extal_freq(void)
> +static unsigned int __init __maybe_unused get_extal_freq(void)
>  {
>  	const struct of_device_id *match;
>  	struct device_node *cpg, *extal;
> @@ -60,6 +60,12 @@ static unsigned int __init get_extal_freq(void)
> 
>  void __init rcar_gen2_timer_init(void)
>  {
> +/*
> + * If CONFIG_PSCI is enabled then most likely we are running on
> +PSCI-enabled
> + * U-Boot which, we assume, has already taken care of configuring ARCH
> +timer
> + * stuff before switching to non-secure mode.
> + */
> +#if !defined(CONFIG_ARM_PSCI)
Is it required?  If you see the below comment, it is already taken care
by the below code.

Is your code entering into this block, when booting  the kernel in NS mode?

<snip>
  /*                                                                       
 87          * Update the timer if it is either not running, or is not at the        
 88          * right frequency. The timer is only configurable in secure mode        
 89          * so this avoids an abort if the loader started the timer and           
 90          * entered the kernel in non-secure mode.                                
 91          */                                                                      
 92                                                                                  
 93         if ((ioread32(base + CNTCR) & 1) == 0 ||                                 
 94             ioread32(base + CNTFID0) != freq) {                                  
 95                 /* Update registers with correct frequency */                    
 96                 iowrite32(freq, base + CNTFID0);                                 
 97                 asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));       
 98                                                                                  
 99                 /* make sure arch timer is started by setting bit 0 of CNTCR */  
100                 iowrite32(1, base + CNTCR);                                      
101         }     


Regards,
Biju

>  	void __iomem *base;
>  	u32 freq;
> 
> @@ -101,6 +107,7 @@ void __init rcar_gen2_timer_init(void)
>  	}
> 
>  	iounmap(base);
> +#endif /* #if !defined(CONFIG_ARM_PSCI) */
> 
>  	of_clk_init(NULL);
>  	timer_probe();
> --
> 2.7.4