[PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Sachin Kamat]

L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
Thus move this function to Exynos4 early init call to avoid non-Exynos4
SoCs from calling this function in multi-platform support.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
---
Tested on Origen(4210), Arndale(5250) and 5420 boards.
---
 arch/arm/mach-exynos/common.c          |    3 +--
 arch/arm/mach-exynos/common.h          |    2 +-
 arch/arm/mach-exynos/mach-exynos4-dt.c |    9 ++++++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 1510436..f822b226 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -396,7 +396,7 @@ static int __init exynos_core_init(void)
 }
 core_initcall(exynos_core_init);
 
-static int __init exynos4_l2x0_cache_init(void)
+int __init exynos4_l2x0_cache_init(void)
 {
 	int ret;
 
@@ -408,7 +408,6 @@ static int __init exynos4_l2x0_cache_init(void)
 	clean_dcache_area(&l2x0_regs_phys, sizeof(unsigned long));
 	return 0;
 }
-early_initcall(exynos4_l2x0_cache_init);
 
 static int __init exynos_init(void)
 {
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 3f03334..3031321 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -24,7 +24,7 @@ void exynos5_restart(enum reboot_mode mode, const char *cmd);
 void exynos_cpuidle_init(void);
 void exynos_cpufreq_init(void);
 void exynos_init_late(void);
-
+int exynos4_l2x0_cache_init(void);
 void exynos_firmware_init(void);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
index ffd19a0..2e6f51c 100644
--- a/arch/arm/mach-exynos/mach-exynos4-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
@@ -49,11 +49,18 @@ static void __init exynos4_reserve(void)
 				mfc_mem.lsize);
 #endif
 }
+
+static void __init exynos4_early_init(void)
+{
+	exynos4_l2x0_cache_init();
+	exynos_firmware_init();
+}
+
 DT_MACHINE_START(EXYNOS4210_DT, "Samsung Exynos4 (Flattened Device Tree)")
 	/* Maintainer: Thomas Abraham <thomas.abraham@linaro.org> */
 	.smp		= smp_ops(exynos_smp_ops),
 	.map_io		= exynos_init_io,
-	.init_early	= exynos_firmware_init,
+	.init_early	= exynos4_early_init,
 	.init_machine	= exynos4_dt_machine_init,
 	.init_late	= exynos_init_late,
 	.dt_compat	= exynos4_dt_compat,
-- 
1.7.9.5

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Yadwinder Singh Brar]

Hi Sachin,


> --- a/arch/arm/mach-exynos/mach-exynos4-dt.c
> +++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
> @@ -49,11 +49,18 @@ static void __init exynos4_reserve(void)
>                                 mfc_mem.lsize);
>  #endif
>  }
> +
> +static void __init exynos4_early_init(void)
> +{
> +       exynos4_l2x0_cache_init();
> +       exynos_firmware_init();

I think, exynos4_l2xO_cache_init() should be called after
exynos_firmware_init(),
though support for l2x0_cache_init through firmware is not their yet
but firmware_init should be done earlier that that.

Regards,
Yadwinder

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Tomasz Figa]

Hi Sachin,

On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
> Thus move this function to Exynos4 early init call to avoid non-Exynos4
> SoCs from calling this function in multi-platform support.

I believe that at the time .init_early() is called there is no ioremap()
infrastructure working yet and so L2 cache initialization which needs
it will cause a kernel panic. This patch worked only because currently
there is no L2X0 controller defined in device tree sources of Exynos 4.

Other than that, it will be required to register firmware operations
before L2X0 initialization, because on some boards it is needed to
initialize the L2 cache through secure firmware.

Please let me handle this by the way of patches adding firmware based
L2X0 initialization.

Best regards,
Tomasz

> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> ---
> Tested on Origen(4210), Arndale(5250) and 5420 boards.
> ---
>  arch/arm/mach-exynos/common.c          |    3 +--
>  arch/arm/mach-exynos/common.h          |    2 +-
>  arch/arm/mach-exynos/mach-exynos4-dt.c |    9 ++++++++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index 1510436..f822b226 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -396,7 +396,7 @@ static int __init exynos_core_init(void)
>  }
>  core_initcall(exynos_core_init);
>  
> -static int __init exynos4_l2x0_cache_init(void)
> +int __init exynos4_l2x0_cache_init(void)
>  {
>  	int ret;
>  
> @@ -408,7 +408,6 @@ static int __init exynos4_l2x0_cache_init(void)
>  	clean_dcache_area(&l2x0_regs_phys, sizeof(unsigned long));
>  	return 0;
>  }
> -early_initcall(exynos4_l2x0_cache_init);
>  
>  static int __init exynos_init(void)
>  {
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index 3f03334..3031321 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -24,7 +24,7 @@ void exynos5_restart(enum reboot_mode mode, const char *cmd);
>  void exynos_cpuidle_init(void);
>  void exynos_cpufreq_init(void);
>  void exynos_init_late(void);
> -
> +int exynos4_l2x0_cache_init(void);
>  void exynos_firmware_init(void);
>  
>  #ifdef CONFIG_PM_GENERIC_DOMAINS
> diff --git a/arch/arm/mach-exynos/mach-exynos4-dt.c b/arch/arm/mach-exynos/mach-exynos4-dt.c
> index ffd19a0..2e6f51c 100644
> --- a/arch/arm/mach-exynos/mach-exynos4-dt.c
> +++ b/arch/arm/mach-exynos/mach-exynos4-dt.c
> @@ -49,11 +49,18 @@ static void __init exynos4_reserve(void)
>  				mfc_mem.lsize);
>  #endif
>  }
> +
> +static void __init exynos4_early_init(void)
> +{
> +	exynos4_l2x0_cache_init();
> +	exynos_firmware_init();
> +}
> +
>  DT_MACHINE_START(EXYNOS4210_DT, "Samsung Exynos4 (Flattened Device Tree)")
>  	/* Maintainer: Thomas Abraham <thomas.abraham@linaro.org> */
>  	.smp		= smp_ops(exynos_smp_ops),
>  	.map_io		= exynos_init_io,
> -	.init_early	= exynos_firmware_init,
> +	.init_early	= exynos4_early_init,
>  	.init_machine	= exynos4_dt_machine_init,
>  	.init_late	= exynos_init_late,
>  	.dt_compat	= exynos4_dt_compat,
> 

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Sachin Kamat]

Hi Tomasz,

On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Sachin,
>
> On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
>> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
>> Thus move this function to Exynos4 early init call to avoid non-Exynos4
>> SoCs from calling this function in multi-platform support.
>
> I believe that at the time .init_early() is called there is no ioremap()
> infrastructure working yet and so L2 cache initialization which needs
> it will cause a kernel panic. This patch worked only because currently
> there is no L2X0 controller defined in device tree sources of Exynos 4.

It is not clear to me as to why device tree entry is required for this
to work in current form
as we call the  L2X0 init function explicitly.

>
> Other than that, it will be required to register firmware operations
> before L2X0 initialization, because on some boards it is needed to
> initialize the L2 cache through secure firmware.
>
> Please let me handle this by the way of patches adding firmware based
> L2X0 initialization.

Sure, please go ahead. BTW, any rough estimate about when you plan to post them?

-- 
With warm regards,
Sachin

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Tomasz Figa]

Hi Sachin,

[adding more people on Cc for some discussion]

On Saturday 19 of October 2013 18:09:15 Sachin Kamat wrote:
> Hi Tomasz,
> 
> On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
> > Hi Sachin,
> > 
> > On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
> >> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
> >> Thus move this function to Exynos4 early init call to avoid non-Exynos4
> >> SoCs from calling this function in multi-platform support.
> > 
> > I believe that at the time .init_early() is called there is no ioremap()
> > infrastructure working yet and so L2 cache initialization which needs
> > it will cause a kernel panic. This patch worked only because currently
> > there is no L2X0 controller defined in device tree sources of Exynos 4.
> 
> It is not clear to me as to why device tree entry is required for this
> to work in current form
> as we call the  L2X0 init function explicitly.

L2x0 controller is just another IP block on the SoC. In addition its
registers are memory mapped and the driver needs some way to get their
location. The function being called explicitly simply scans device tree
for applicable node and performs rest of the setup only if it finds one.

Ideally there should be no need to call this function explicitly, as it
should be called by some code based on presence of applicable device tree
node.

> 
> > Other than that, it will be required to register firmware operations
> > before L2X0 initialization, because on some boards it is needed to
> > initialize the L2 cache through secure firmware.
> > 
> > Please let me handle this by the way of patches adding firmware based
> > L2X0 initialization.
> 
> Sure, please go ahead. BTW, any rough estimate about when you plan to post
> them?

They might not make it for 3.13, as during next week I'm on LCE/KS/ELCE
in Edinburgh and I will be able to continue my work on this only after
getting back.

If this is something of high priority, then here's my idea:

- the l2x0_of_init() function itself should be an early_initcall, without
  any arguments. It would carry on with appropriate initialization only
  if there is an l2x0 node specified in device tree,
- the aux_val and aux_mask parameters should rather come from device tree,
  if there are no other means of obtaining them automatically.

This is something that still needs a bit more thought, as there are some
more interesting cases (as mach-ux500) which do more things than just
simple call to l2x0_of_init(). I guess this should be discussed with
people responsible for such platforms and l2x0 driver in general.

Best regards,
Tomasz

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Will Deacon]

On Sat, Oct 19, 2013 at 03:04:22PM +0100, Tomasz Figa wrote:
> On Saturday 19 of October 2013 18:09:15 Sachin Kamat wrote:
> > Hi Tomasz,
> > 
> > On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
> > > Hi Sachin,
> > > 
> > > On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
> > >> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
> > >> Thus move this function to Exynos4 early init call to avoid non-Exynos4
> > >> SoCs from calling this function in multi-platform support.
> > > 
> > > I believe that at the time .init_early() is called there is no ioremap()
> > > infrastructure working yet and so L2 cache initialization which needs
> > > it will cause a kernel panic. This patch worked only because currently
> > > there is no L2X0 controller defined in device tree sources of Exynos 4.
> > 
> > It is not clear to me as to why device tree entry is required for this
> > to work in current form
> > as we call the  L2X0 init function explicitly.
> 
> L2x0 controller is just another IP block on the SoC. In addition its
> registers are memory mapped and the driver needs some way to get their
> location. The function being called explicitly simply scans device tree
> for applicable node and performs rest of the setup only if it finds one.
> 
> Ideally there should be no need to call this function explicitly, as it
> should be called by some code based on presence of applicable device tree
> node.

It's a nice idea, but you need to be careful if you start deferring this
stuff. Aside from the potential performance loss during early boot, I'm not
even sure that the l2x0_init function is safe to run with IRQs enabled.

Will

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Tomasz Figa]

Hi Will,

On Friday 25 of October 2013 14:48:52 Will Deacon wrote:
> On Sat, Oct 19, 2013 at 03:04:22PM +0100, Tomasz Figa wrote:
> > On Saturday 19 of October 2013 18:09:15 Sachin Kamat wrote:
> > > Hi Tomasz,
> > > 
> > > On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
> > > > Hi Sachin,
> > > > 
> > > > On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
> > > >> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
> > > >> Thus move this function to Exynos4 early init call to avoid non-Exynos4
> > > >> SoCs from calling this function in multi-platform support.
> > > > 
> > > > I believe that at the time .init_early() is called there is no ioremap()
> > > > infrastructure working yet and so L2 cache initialization which needs
> > > > it will cause a kernel panic. This patch worked only because currently
> > > > there is no L2X0 controller defined in device tree sources of Exynos 4.
> > > 
> > > It is not clear to me as to why device tree entry is required for this
> > > to work in current form
> > > as we call the  L2X0 init function explicitly.
> > 
> > L2x0 controller is just another IP block on the SoC. In addition its
> > registers are memory mapped and the driver needs some way to get their
> > location. The function being called explicitly simply scans device tree
> > for applicable node and performs rest of the setup only if it finds one.
> > 
> > Ideally there should be no need to call this function explicitly, as it
> > should be called by some code based on presence of applicable device tree
> > node.
> 
> It's a nice idea, but you need to be careful if you start deferring this
> stuff. Aside from the potential performance loss during early boot, I'm not
> even sure that the l2x0_init function is safe to run with IRQs enabled.
> 
> Will

I just realised that I have not replied to your message, sorry for that.

So, my intention was not deferring this, but rather making this more
generic. In case of Exynos, we call l2x0_of_init() from an early_initcall
which is long time already after enabling interrupts, but still before
SMP is initialized.

My idea was to create a generic early_initcall in arch/arm/mm/cache-l2x0.c
that would scan DT for applicable node, parse aux_val and aux_mask from
such node (if found) and then initialize L2X0 without any need for
having platform-specific code.

However my concern here is whether on some platforms such platform code
wouldn't be needed anyway. Unfortunately I don't really know enough
about L2X0 to be able to answer this myself, at least at the moment.

Best regards,
Tomasz

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Rob Herring]

On 11/13/2013 06:41 PM, Tomasz Figa wrote:
> Hi Will,
> 
> On Friday 25 of October 2013 14:48:52 Will Deacon wrote:
>> On Sat, Oct 19, 2013 at 03:04:22PM +0100, Tomasz Figa wrote:
>>> On Saturday 19 of October 2013 18:09:15 Sachin Kamat wrote:
>>>> Hi Tomasz,
>>>>
>>>> On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
>>>>> Hi Sachin,
>>>>>
>>>>> On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
>>>>>> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
>>>>>> Thus move this function to Exynos4 early init call to avoid non-Exynos4
>>>>>> SoCs from calling this function in multi-platform support.
>>>>>
>>>>> I believe that at the time .init_early() is called there is no ioremap()
>>>>> infrastructure working yet and so L2 cache initialization which needs
>>>>> it will cause a kernel panic. This patch worked only because currently
>>>>> there is no L2X0 controller defined in device tree sources of Exynos 4.
>>>>
>>>> It is not clear to me as to why device tree entry is required for this
>>>> to work in current form
>>>> as we call the  L2X0 init function explicitly.
>>>
>>> L2x0 controller is just another IP block on the SoC. In addition its
>>> registers are memory mapped and the driver needs some way to get their
>>> location. The function being called explicitly simply scans device tree
>>> for applicable node and performs rest of the setup only if it finds one.
>>>
>>> Ideally there should be no need to call this function explicitly, as it
>>> should be called by some code based on presence of applicable device tree
>>> node.
>>
>> It's a nice idea, but you need to be careful if you start deferring this
>> stuff. Aside from the potential performance loss during early boot, I'm not
>> even sure that the l2x0_init function is safe to run with IRQs enabled.
>>
>> Will
> 
> I just realised that I have not replied to your message, sorry for that.
> 
> So, my intention was not deferring this, but rather making this more
> generic. In case of Exynos, we call l2x0_of_init() from an early_initcall
> which is long time already after enabling interrupts, but still before
> SMP is initialized.

I believe SMP is enabled before any initcalls.

> My idea was to create a generic early_initcall in arch/arm/mm/cache-l2x0.c
> that would scan DT for applicable node, parse aux_val and aux_mask from
> such node (if found) and then initialize L2X0 without any need for
> having platform-specific code.
> 
> However my concern here is whether on some platforms such platform code
> wouldn't be needed anyway. Unfortunately I don't really know enough
> about L2X0 to be able to answer this myself, at least at the moment.

Unfortunately, non-secure mode platforms like highbank and omap have to
make platform specific smc calls.

Rob

Re: [PATCH 1/1] ARM: EXYNOS: Initialize L2x0 cache controller for Exynos4 only

[Tomasz Figa]

Hi Rob,

On Wednesday 13 of November 2013 18:50:12 Rob Herring wrote:
> On 11/13/2013 06:41 PM, Tomasz Figa wrote:
> > Hi Will,
> > 
> > On Friday 25 of October 2013 14:48:52 Will Deacon wrote:
> >> On Sat, Oct 19, 2013 at 03:04:22PM +0100, Tomasz Figa wrote:
> >>> On Saturday 19 of October 2013 18:09:15 Sachin Kamat wrote:
> >>>> Hi Tomasz,
> >>>>
> >>>> On 17 October 2013 18:35, Tomasz Figa <t.figa@samsung.com> wrote:
> >>>>> Hi Sachin,
> >>>>>
> >>>>> On Thursday 17 of October 2013 17:33:12 Sachin Kamat wrote:
> >>>>>> L2x0 cache controller is present only in Cortex-A9 based Exynos4 SoCs.
> >>>>>> Thus move this function to Exynos4 early init call to avoid non-Exynos4
> >>>>>> SoCs from calling this function in multi-platform support.
> >>>>>
> >>>>> I believe that at the time .init_early() is called there is no ioremap()
> >>>>> infrastructure working yet and so L2 cache initialization which needs
> >>>>> it will cause a kernel panic. This patch worked only because currently
> >>>>> there is no L2X0 controller defined in device tree sources of Exynos 4.
> >>>>
> >>>> It is not clear to me as to why device tree entry is required for this
> >>>> to work in current form
> >>>> as we call the  L2X0 init function explicitly.
> >>>
> >>> L2x0 controller is just another IP block on the SoC. In addition its
> >>> registers are memory mapped and the driver needs some way to get their
> >>> location. The function being called explicitly simply scans device tree
> >>> for applicable node and performs rest of the setup only if it finds one.
> >>>
> >>> Ideally there should be no need to call this function explicitly, as it
> >>> should be called by some code based on presence of applicable device tree
> >>> node.
> >>
> >> It's a nice idea, but you need to be careful if you start deferring this
> >> stuff. Aside from the potential performance loss during early boot, I'm not
> >> even sure that the l2x0_init function is safe to run with IRQs enabled.
> >>
> >> Will
> > 
> > I just realised that I have not replied to your message, sorry for that.
> > 
> > So, my intention was not deferring this, but rather making this more
> > generic. In case of Exynos, we call l2x0_of_init() from an early_initcall
> > which is long time already after enabling interrupts, but still before
> > SMP is initialized.
> 
> I believe SMP is enabled before any initcalls.

You're almost right. Before any initcalls, except early_initcalls. This is
a misleading label in init/main.c in an array of initcall names, calling
initcall level 0 "early", but that should be "pure". early_initcalls are
called from a different place in code - see do_pre_smp_initcalls().

> > My idea was to create a generic early_initcall in arch/arm/mm/cache-l2x0.c
> > that would scan DT for applicable node, parse aux_val and aux_mask from
> > such node (if found) and then initialize L2X0 without any need for
> > having platform-specific code.
> > 
> > However my concern here is whether on some platforms such platform code
> > wouldn't be needed anyway. Unfortunately I don't really know enough
> > about L2X0 to be able to answer this myself, at least at the moment.
> 
> Unfortunately, non-secure mode platforms like highbank and omap have to
> make platform specific smc calls.

This is what I was afraid of. Still, I don't think there is no way we
could mitigate this. Maybe firmware_ops infrastructure could be used for
this?

I need to know more about requirements of such platforms w.r.t. L2 cache
initialization to be able to say anything more, though.

Best regards,
Tomasz