[U-Boot-Users] [PATCH] ppc4xx: Add dcache_enable() for 440

[Wolfgang Denk]

In message <1208530299-9817-1-git-send-email-sr at denx.de> you wrote:
> dcache_enable() was missing for 440 and the patch
> 017e9b7925f74878d0e9475388cca9bda5ef9482 ["allow ports to override bootelf
> "] behavior uses this function.
> 
> Note: Currently the cache handling functions like
> d/icache_disable/enable() are NOP's on 440. This may be changed in the
> future.

Sorry, but I don't think this is a good idea. Actually I tend to
reject this patch.

> --- a/cpu/ppc4xx/cache.S
> +++ b/cpu/ppc4xx/cache.S
> @@ -166,9 +166,11 @@ _GLOBAL(invalidate_dcache)
>  #ifdef CONFIG_440
>  
>         .globl  dcache_disable
> +       .globl  dcache_enable
>         .globl  icache_disable
>         .globl  icache_enable
>  dcache_disable:
> +dcache_enable:
>  icache_disable:
>  icache_enable:
>  	blr

I was not are that we have such code in U-Boot; I think it  is  plain
unacceptable.  Either  we  implement these functions correctly, i. e.
such that they perform the actions their name suggests, or we do  not
define these functions at all.

But providing a function that promises to enable  or  disable  the  I
resp.  D  cache, and which then actually does nothing, is *extremely*
misleading. I see all kind of regressions and lost  time  because  of
people debug completely different parts of the code just because they
don't know that these functions are dummies.


Please: let's not do this. May I please ask to clean this up?

Yes,  I  am  aware  that  the   current   (new)   implementation   of
do_bootelf_exec()  needs  to  be fixed for this, too - and maybe some
other places as well. But this is important enough to me.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Q:  What's a light-year?
A:  One-third less calories than a regular year.