| Message ID | 20230416133422.1949-4-aweber.kernel@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Re-introduce Exynos4212 support and add Samsung Galaxy Tab 3 8.0 boards | expand |
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
Hi Artur, On Sun, Apr 16, 2023 at 03:34:13PM +0200, 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. > > 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()) Seems more logical to have 4212 before 4412 here. > 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; Is it necessary to set cpu = 0? Are there any obvious issues without it (like second cpu not being brought up)? It seems vendor kernel does this for both exynos4210 and exynos4212 [1], but mainline has not done this for exynos4210 and there has been no reported issues on for that platform as far as I know. The assembly that handles SMC_CMD_CPU1BOOT in sboot.bin from firmware version T310XXSBQB2 is identical to what is found in sboot.bin from exynos4412-galaxy-s3, and it uses the cpu arg in both cases, so seems likely that we need this and I am mostly asking out of curiosity. [1] https://github.com/krzk/linux-vendor-backup/blob/mokee/android-3.4-samsung-galaxy-tab-s-10.5-sm-t805-exynos5420/arch/arm/mach-exynos/platsmp.c#L225-L229 Best regards, Henrik Grimler > 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()) { > /* Setting SEQ_OPTION register */ > pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0, > S5P_CENTRAL_SEQ_OPTION); > diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c > index 3bf14ca78b62..df1e10033f90 100644 > --- a/arch/arm/mach-exynos/suspend.c > +++ b/arch/arm/mach-exynos/suspend.c > @@ -231,6 +231,7 @@ static int __init exynos_pmu_irq_init(struct device_node *node, > > EXYNOS_PMU_IRQ(exynos3250_pmu_irq, "samsung,exynos3250-pmu"); > EXYNOS_PMU_IRQ(exynos4210_pmu_irq, "samsung,exynos4210-pmu"); > +EXYNOS_PMU_IRQ(exynos4212_pmu_irq, "samsung,exynos4212-pmu"); > EXYNOS_PMU_IRQ(exynos4412_pmu_irq, "samsung,exynos4412-pmu"); > EXYNOS_PMU_IRQ(exynos5250_pmu_irq, "samsung,exynos5250-pmu"); > EXYNOS_PMU_IRQ(exynos5420_pmu_irq, "samsung,exynos5420-pmu"); > @@ -640,6 +641,9 @@ static const struct of_device_id exynos_pmu_of_device_ids[] __initconst = { > }, { > .compatible = "samsung,exynos4210-pmu", > .data = &exynos4_pm_data, > + }, { > + .compatible = "samsung,exynos4212-pmu", > + .data = &exynos4_pm_data, > }, { > .compatible = "samsung,exynos4412-pmu", > .data = &exynos4_pm_data, > -- > 2.40.0 >
On 19/04/2023 22:24, Henrik Grimler wrote: >> /* >> * 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; > > Is it necessary to set cpu = 0? Are there any obvious issues without > it (like second cpu not being brought up)? Just tested this; looks like it's required here, without it the second core fails to start up: [ 0.064277] smp: Bringing up secondary CPUs ... [ 1.067163] CPU1: failed to boot: -110 [ 1.069683] smp: Brought up 1 node, 1 CPU [ 1.072470] SMP: Total of 1 processors activated (48.00 BogoMIPS). SBOOT on my tablet claims the version is T310XXSBQB2, so it should match with the one you checked. Best regards Artur Weber
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()) 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()) { /* Setting SEQ_OPTION register */ pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0, S5P_CENTRAL_SEQ_OPTION); diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c index 3bf14ca78b62..df1e10033f90 100644 --- a/arch/arm/mach-exynos/suspend.c +++ b/arch/arm/mach-exynos/suspend.c @@ -231,6 +231,7 @@ static int __init exynos_pmu_irq_init(struct device_node *node, EXYNOS_PMU_IRQ(exynos3250_pmu_irq, "samsung,exynos3250-pmu"); EXYNOS_PMU_IRQ(exynos4210_pmu_irq, "samsung,exynos4210-pmu"); +EXYNOS_PMU_IRQ(exynos4212_pmu_irq, "samsung,exynos4212-pmu"); EXYNOS_PMU_IRQ(exynos4412_pmu_irq, "samsung,exynos4412-pmu"); EXYNOS_PMU_IRQ(exynos5250_pmu_irq, "samsung,exynos5250-pmu"); EXYNOS_PMU_IRQ(exynos5420_pmu_irq, "samsung,exynos5420-pmu"); @@ -640,6 +641,9 @@ static const struct of_device_id exynos_pmu_of_device_ids[] __initconst = { }, { .compatible = "samsung,exynos4210-pmu", .data = &exynos4_pm_data, + }, { + .compatible = "samsung,exynos4212-pmu", + .data = &exynos4_pm_data, }, { .compatible = "samsung,exynos4412-pmu", .data = &exynos4_pm_data,
The platform was originally dropped in commit bca9085e0ae9 ("ARM: dts: exynos: remove Exynos4212 support (dead code)"), as there were no boards using it. 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(-)