From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Tue, 20 Jan 2015 15:32:34 +0100 Subject: [U-Boot] [PATCH 05/12] sunxi: Move setting of CPU system control register SMP bit to save_boot_params In-Reply-To: <20150120112206.0737bbe6@lilith> References: <1421333554-29822-1-git-send-email-hdegoede@redhat.com> <1421333554-29822-6-git-send-email-hdegoede@redhat.com> <1421535080.13341.18.camel@hellion.org.uk> <54BD555A.5010205@redhat.com> <20150120112206.0737bbe6@lilith> Message-ID: <54BE6702.1040307@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 20-01-15 11:22, Albert ARIBAUD wrote: > Hello Hans, > > On Mon, 19 Jan 2015 20:04:58 +0100, Hans de Goede > wrote: >> Hi, >> >> On 17-01-15 23:51, Ian Campbell wrote: >>> On Thu, 2015-01-15 at 15:52 +0100, Hans de Goede wrote: >>>> According to the "Cortex-A7 MPCore Technical Reference Manual": >>>> >>>> "You must ensure this bit is set to 1 before the caches and MMU are enabled, >>>> or any cache and TLB maintenance operations are performed." >>> >>> Given that this is a feature of the Cortex-A7 (actually, I believe it >>> applies to at least Cortex-A15 too) and not really specific to sunxi, >>> perhaps we can make this more generic? >> >> Strange enough the bit is different between the A7 and A15, for the A7 the docs >> say it must be set before doing anything with caches, on the A15 it only needs >> to be set for the core to accept cache management operations from other cpu >> cores (or so the docs say), which is likely why it is not in the standard >> init sequence yet, as for u-boot it seems to only be necessary to do this on >> a Cortex A7. I agree that it would be good to move this to the generic start.S >> though, Albert ? > > [...] > >>>> Since arch/arm/cpu/armv7/start.S: cpu_init_cp15 does several cache operations, >>>> we should thus enable the SMP bit earlier, and the only chance to do that is >>>> to do it at save_boot_params time. >>> >>> Would it be so terrible to add an ifdef CORTEX_A7 here, or to call out >>> to (or call as a macro) a soc_init_cp15? >> >> From my pov no that would not be too terrible, but ... >>> >>> I'm cc-ing Albert for input these questions. >> >> That indeed is Albert's call. > > I don't like the idea of #ifdef'ing that much. > > OTOH, if we do introduce soc_init_cp15, then we end up with two CP15 > init functions, soc_init_cp15 and cpu_init_cp15, with all sorts of > questions on which should be called first and whether one could break > the other's work, and what happens in-between, etc. > > Either way, setting CP15 registers is something that all ARM CPUs, and > SoCs require, not just armv7. It's just that other start.S files touch > cp15 directly. > > I'm leaning toward grouping all CP15 inits (including cache(s) > and TLB disabling and maybe VBAR setting) in a single CP15 call to > a single soc_init_cp15 function. > > Now, SoCs with the same CPU will have a common CP15 init part, and > that part could go into a _init_cp15 function which soc_init_cp15 > would call. Of course, since we're doing this way before we have any > stack, we will have to handle nested calls by saving and restoring LR > in intermediate function contexts. > >> Note that solving this still leaves the A80 magic sram controller poke which >> also needs to happen really really early or otherwise the entire SoC just >> resets as if the watchdog has triggered, I'm fine with using save_boot_params >> for that, it is not its intended purpose, but it works fine for it, so >> I see no reason to complicate things with yet another callback. > > Maybe we could turn soc_init_cp15 into a more general soc_init function > which would do whatever is needed, on cp15 or otherwise. > > (I see there is one soc_init defined, for spear600, but it is actually > empty and could/should be removed. Patch anyone?) Hmm, so if I'm reading the above correctly, then I think you want to do the following: 1) Rename cpu_init_cp15 to cpu_init_cp15_common 2) Add a new soc_init function, with a weak default which just calls cpu_init_cp15_common 3) Add a a7_init_cp15 which sets the smp bit 4) Have Cortex A7 SoCs override soc_init with one which first calls a7_init_cp15 and then calls cpu_init_cp15_common 5) And on SoC's which need to do something special before or after cp15 init, they can do so by overriding soc_init and do what ever they need to do there before *or* after calling cpu_init_cp15_common Have I got that right ? If so I can try to write a patch-set for this, my arm asm is a bit weak, but I should be able to cobble this together using existing code as an example. Regards, Hans