| Message ID | 1382011392-18199-1-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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, > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,