From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Tue, 18 Oct 2016 13:10:19 -0600 Subject: [U-Boot] [PATCH 1/2] armv8: add hooks for all cache-wide operations In-Reply-To: References: <20161017213540.5984-1-swarren@wwwdotorg.org> <2eb98062-bf22-2a49-ea13-701f3dd65c4e@wwwdotorg.org> Message-ID: <3b4e7932-ea89-5c5f-9fb1-6ad64e5466b5@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 10/18/2016 01:03 PM, Simon Glass wrote: > Hi Stephen, > > On 18 October 2016 at 12:58, Stephen Warren wrote: >> On 10/18/2016 10:23 AM, Simon Glass wrote: >>> >>> Hi Stephen, >>> >>> On 17 October 2016 at 15:35, Stephen Warren wrote: >>>> >>>> From: Stephen Warren >>>> >>>> SoC-specific logic may be required for all forms of cache-wide >>>> operations; invalidate and flush of both dcache and icache (note that >>>> only 3 of the 4 possible combinations make sense, since the icache never >>>> contains dirty lines). This patch adds an optional hook for all >>>> implemented cache-wide operations, and renames the one existing hook to >>>> better represent exactly which operation it is implementing. A dummy >>>> no-op implementation of each hook is provided. These dummy >>>> implementations are moved into C code, since there's no need to >>>> implement them in assembly. >>>> >>>> Signed-off-by: Stephen Warren >>>> --- >>>> arch/arm/cpu/armv8/cache.S | 6 ------ >>>> arch/arm/cpu/armv8/cache_v8.c | 23 >>>> ++++++++++++++++++++--- >>>> arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S | 4 ++-- >>>> arch/arm/include/asm/system.h | 5 ++++- >>>> arch/arm/mach-tegra/tegra186/cache.c | 2 +- >>>> 5 files changed, 27 insertions(+), 13 deletions(-) >>>> >>> >>> I think we should have a proper interface for this stuff rather than >>> weak functions. Maybe we need a linker-list approach, or a cache >>> uclass? >> >> >> What's improper about this interface? Presumably we could argue that no >> function in the entire of U-Boot be called by symbol name, which doesn't >> seem useful. >> >> I'm not sure exactly what you envisage by a linker-list approach. Can you >> provide some background? I understand how the linker can construct list of >> objects/implementations/..., but that doesn't seem useful here since there's >> a known-ahead-of-time single implementation of these functions in a single >> build of U-Boot. > > Your own commit messages says that this is SoC-specific. I'm > suggesting that we define an interface (which I think you have already > done with your header file additions), and allow SoCs to implement it > via a linker list. > > IMO the cache code in U-Boot is starting to get a bit creaky. > >> A cache uclass seems like /massive/ overkill, especially since I'd expect >> these very low-level functions to be required well before any higher level >> code like DM/classes/... to be available. > > DM is available very early. But it's not clear from your patch when > this code is actually called. I believe that weak functions are a perfectly acceptable approach here. Yes, the implementation of these functions is SoC-specific. The Makefiles will pull in the appropriate implementation for that SoC whenever U-Boot is built, just like every other board- or SoC-specific function in the entire of U-Boot. There's no need for linker lists since there is only ever one implementation.