[U-Boot] [PATCH 1/2] armv8: add hooks for all cache-wide operations

[Stephen Warren]

On 10/18/2016 01:03 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 18 October 2016 at 12:58, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 10/18/2016 10:23 AM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 17 October 2016 at 15:35, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>
>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> 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 <swarren at nvidia.com>
>>>> ---
>>>>  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.