On 19.06.24 15:19, Ilias Apalodimas wrote:
On Wed, 19 Jun 2024 at 16:05, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
On Wed, 19 Jun 2024 at 15:36, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
On 19.06.24 14:23, Ilias Apalodimas wrote:
On Sun, 16 Jun 2024 at 20:31, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
If we have multiple weak implementations of functions, the linker might
choose any of these. ARM and RISC-V already provide a weak implementation
of flush_dcache_all().
Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
cmd/cache.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/cmd/cache.c b/cmd/cache.c
index 0254ff17f9b..16fa0f7c652 100644
--- a/cmd/cache.c
+++ b/cmd/cache.c
@@ -52,11 +52,14 @@ static int do_icache(struct cmd_tbl *cmdtp, int flag, int
argc,
return 0;
}
+/* ARM and RISC-V define a weak flush_dcache_all() themselves. */
+#if !defined(CONFIG_ARM) && !defined(CONFIG_RISCV)
void __weak flush_dcache_all(void)
{
puts("No arch specific flush_dcache_all available!\n");
/* please define arch specific flush_dcache_all */
}
Aren't we supposed to add a single __weak function so the linker can
replace it? IOW why is the declaration for Arm/riscv a weak one?
Some sub-architectures override the architecture specific weak
implementation, e.g.
arch/riscv/cpu/andes/cache.c:44:
void flush_dcache_all(void)
arch/arm/cpu/arm926ejs/cache.c:17:
void flush_dcache_all(void)
Ok, this does fix a problem, but afaict this is a band-aid and the
cache management is a mess overall -- e.g invalidate_icache_all() will
suffer from the same issue if a sub-architecture decides to define its
own in the future.
Would it be less bad to define
static inline __weak flush_dcache_all(void)
{
}
static inline __weak flush_icache_all(void)
{
}
ugh, this but without the inline!
Thanks
/Ilias
GCC does not allow both a weak and a strong implementation in the same
module:
CC board/sandbox/sandbox.o
arch/sandbox/cpu/cache.c:15:6: error: redefinition of
‘invalidate_icache_all’
15 | void invalidate_icache_all(void)
| ^~~~~~~~~~~~~~~~~~~~~
In file included from arch/sandbox/cpu/cache.c:6:
include/cpu_func.h:74:13: note: previous definition of
‘invalidate_icache_all’ with type ‘void(void)’
74 | void __weak invalidate_icache_all(void)
| ^~~~~~~~~~~~~~~~~~~~~
Best regards
Heinrich
in include/cpu_func.h instead of a random cmd file?
We can only define those if !CONFIG_ARM && !!CONFIG_RISCV and add a
comment on why.
It's kind of putting lipstick on a pig, but at least we'll have them
gathered in a single header file and know what we have to fix.
Cheers
/Ilias
Best regards
Heinrich
Thanks
/Ilias
+#endif
static int do_dcache(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
--
2.43.0
