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 > 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 > > >> > >