Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Biju Das <biju.das@bp.renesas.com>
To:     Oleksandr Tyshchenko <olekstysh@gmail.com>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "julien.grall@arm.com" <julien.grall@arm.com>,
        "horms@verge.net.au" <horms@verge.net.au>,
        "magnus.damm@gmail.com" <magnus.damm@gmail.com>,
        "linux@armlinux.org.uk" <linux@armlinux.org.uk>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Subject: RE: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI is
 enabled
Thread-Topic: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI
 is enabled
Thread-Index: AQHU9UCJJrXLep/mxUmrfpQDMFE9GKZBf8rw
Date:   Thu, 18 Apr 2019 07:14:43 +0000
Message-ID: <OSBPR01MB21036296606B40DB2FE22CD2B8260@OSBPR01MB2103.jpnprd01.prod.outlook.com>
References: <1555521040-16706-1-git-send-email-olekstysh@gmail.com>
In-Reply-To: <1555521040-16706-1-git-send-email-olekstysh@gmail.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: bp.renesas.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: a98b62ae-99ca-4826-f040-08d6c3cd87bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2019 07:14:43.1691
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OSBPR01MB3063
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Oleksandr,

Thanks for the patch.

> Subject: [PATCH] ARM: mach-shmobile: Don't configure ARCH timer if PSCI i=
s
> enabled
>=20
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>=20
> 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.
>=20
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>=20
> ---
> A bit of context here...
>=20
> We are highly interested in Renesas "Stout" board support (r8a7790) in Xe=
n
> hypervisor. The reason is to have fully supported HW for performing
> "OSSTEST" (Xen automatic test system) on ARM32.
>=20
> 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.
>=20
> 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
>=20
> You can find corresponding patches for Xen hypervisor here:
> https://www.mail-archive.com/xen-
> devel@lists.xenproject.org/msg43332.html
>=20
> To sumarize:
> Together with enabling CONFIG_PSCI in shmobile_defconfig, current patch i=
s
> 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 insertin=
g
> proper "enable-method" strings in CPU nodes.
>=20
> ---
>  arch/arm/mach-shmobile/setup-rcar-gen2.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>=20
> 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 =3D {
>  	{ /* sentinel */ }
>  };
>=20
> -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)
>=20
>  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>
  /*                                                                      =
=20
 87          * Update the timer if it is either not running, or is not at t=
he       =20
 88          * right frequency. The timer is only configurable in secure mo=
de       =20
 89          * so this avoids an abort if the loader started the timer and =
         =20
 90          * entered the kernel in non-secure mode.                      =
         =20
 91          */                                                            =
         =20
 92                                                                        =
         =20
 93         if ((ioread32(base + CNTCR) & 1) =3D=3D 0 ||                   =
             =20
 94             ioread32(base + CNTFID0) !=3D freq) {                      =
           =20
 95                 /* Update registers with correct frequency */          =
         =20
 96                 iowrite32(freq, base + CNTFID0);                       =
         =20
 97                 asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq=
));      =20
 98                                                                        =
         =20
 99                 /* make sure arch timer is started by setting bit 0 of =
CNTCR */ =20
100                 iowrite32(1, base + CNTCR);                            =
         =20
101         }    =20


Regards,
Biju

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