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

[Tomasz Figa <tomasz.figa@gmail.com>]

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