Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <21eff2e7-1ebc-1d98-6e5e-025f5cf7b822@linaro.org>
Date:   Sun, 16 Apr 2023 20:00:44 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v2 03/12] ARM: exynos: Re-introduce Exynos4212 support
Content-Language: en-US
To:     Artur Weber <aweber.kernel@gmail.com>
Cc:     Alim Akhtar <alim.akhtar@samsung.com>,
        Rob Herring <robh+dt@kernel.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Tomasz Figa <tomasz.figa@gmail.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Olof Johansson <olof@lixom.net>, soc@kernel.org,
        Russell King <linux@armlinux.org.uk>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Kukjin Kim <kgene@kernel.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Vinod Koul <vkoul@kernel.org>,
        Kishon Vijay Abraham I <kishon@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-pm@vger.kernel.org, linux-media@vger.kernel.org,
        linux-phy@lists.infradead.org,
        ~postmarketos/upstreaming@lists.sr.ht
References: <20230416133422.1949-1-aweber.kernel@gmail.com>
 <20230416133422.1949-4-aweber.kernel@gmail.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20230416133422.1949-4-aweber.kernel@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 16/04/2023 15:34, Artur Weber wrote:
> The platform was originally dropped in commit bca9085e0ae9 ("ARM:
> dts: exynos: remove Exynos4212 support (dead code)"), as there were
> no boards using it.

This is not accurate. Platform was not dropped with that commit. This
was only DTS. I propose to drop it and focus on commit which you are
reverting.

> 
> We will be adding a device that uses it, so add it back.
> 
> This effectively reverts commit 9e43eca3c874 ("ARM: EXYNOS: Remove
> Exynos4212 related dead code").
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
>  arch/arm/mach-exynos/Kconfig    | 5 +++++
>  arch/arm/mach-exynos/common.h   | 8 ++++++++
>  arch/arm/mach-exynos/exynos.c   | 2 ++
>  arch/arm/mach-exynos/firmware.c | 8 +++++++-
>  arch/arm/mach-exynos/pm.c       | 2 +-
>  arch/arm/mach-exynos/suspend.c  | 4 ++++
>  6 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 4d3b40e4049a..b3d5df5225fe 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -78,6 +78,11 @@ config CPU_EXYNOS4210
>  	default y
>  	depends on ARCH_EXYNOS4
>  
> +config SOC_EXYNOS4212
> +	bool "Samsung Exynos4212"
> +	default y
> +	depends on ARCH_EXYNOS4
> +
>  config SOC_EXYNOS4412
>  	bool "Samsung Exynos4412"
>  	default y
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 29eb075b24a4..c9e85d33c309 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -15,6 +15,7 @@
>  #define EXYNOS3_SOC_MASK	0xFFFFF000
>  
>  #define EXYNOS4210_CPU_ID	0x43210000
> +#define EXYNOS4212_CPU_ID	0x43220000
>  #define EXYNOS4412_CPU_ID	0xE4412200
>  #define EXYNOS4_CPU_MASK	0xFFFE0000
>  
> @@ -34,6 +35,7 @@ static inline int is_samsung_##name(void)	\
>  
>  IS_SAMSUNG_CPU(exynos3250, EXYNOS3250_SOC_ID, EXYNOS3_SOC_MASK)
>  IS_SAMSUNG_CPU(exynos4210, EXYNOS4210_CPU_ID, EXYNOS4_CPU_MASK)
> +IS_SAMSUNG_CPU(exynos4212, EXYNOS4212_CPU_ID, EXYNOS4_CPU_MASK)
>  IS_SAMSUNG_CPU(exynos4412, EXYNOS4412_CPU_ID, EXYNOS4_CPU_MASK)
>  IS_SAMSUNG_CPU(exynos5250, EXYNOS5250_SOC_ID, EXYNOS5_SOC_MASK)
>  IS_SAMSUNG_CPU(exynos5410, EXYNOS5410_SOC_ID, EXYNOS5_SOC_MASK)
> @@ -52,6 +54,12 @@ IS_SAMSUNG_CPU(exynos5800, EXYNOS5800_SOC_ID, EXYNOS5_SOC_MASK)
>  # define soc_is_exynos4210()	0
>  #endif
>  
> +#if defined(CONFIG_SOC_EXYNOS4212)
> +# define soc_is_exynos4212()	is_samsung_exynos4212()
> +#else
> +# define soc_is_exynos4212()	0
> +#endif
> +
>  #if defined(CONFIG_SOC_EXYNOS4412)
>  # define soc_is_exynos4412()	is_samsung_exynos4412()
>  #else
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 51a247ca4da8..5671621f1661 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -178,6 +178,7 @@ static void __init exynos_dt_machine_init(void)
>  		exynos_cpuidle.dev.platform_data = &cpuidle_coupled_exynos_data;
>  #endif
>  	if (of_machine_is_compatible("samsung,exynos4210") ||
> +	    of_machine_is_compatible("samsung,exynos4212") ||
>  	    (of_machine_is_compatible("samsung,exynos4412") &&
>  	     (of_machine_is_compatible("samsung,trats2") ||
>  		  of_machine_is_compatible("samsung,midas") ||
> @@ -192,6 +193,7 @@ static char const *const exynos_dt_compat[] __initconst = {
>  	"samsung,exynos3250",
>  	"samsung,exynos4",
>  	"samsung,exynos4210",
> +	"samsung,exynos4212",
>  	"samsung,exynos4412",
>  	"samsung,exynos5",
>  	"samsung,exynos5250",
> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> index 2da5b60b59e2..110c8064ee64 100644
> --- a/arch/arm/mach-exynos/firmware.c
> +++ b/arch/arm/mach-exynos/firmware.c
> @@ -63,12 +63,18 @@ static int exynos_cpu_boot(int cpu)
>  	 *
>  	 * On Exynos5 devices the call is ignored by trustzone firmware.
>  	 */
> -	if (!soc_is_exynos4210() && !soc_is_exynos4412())
> +	if (!soc_is_exynos4210() && !soc_is_exynos4412() &&
> +	    !soc_is_exynos4212())

Keep them ordered, so 4210, 4212 and 4412.

>  		return 0;
>  
>  	/*
>  	 * The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
> +	 * But, Exynos4212 has only one secondary CPU so second parameter
> +	 * isn't used for informing secure firmware about CPU id.
>  	 */
> +	if (soc_is_exynos4212())
> +		cpu = 0;
> +
>  	exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>  	return 0;
>  }
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 30f4e55bf39e..9b6db04e4e34 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -161,7 +161,7 @@ void exynos_enter_aftr(void)
>  
>  	exynos_pm_central_suspend();
>  
> -	if (soc_is_exynos4412()) {
> +	if (soc_is_exynos4412() || soc_is_exynos4212()) {

Ditto

Best regards,
Krzysztof