On Monday 21 April 2008, Wolfgang Denk wrote:
In message 1208530299-9817-1-git-send-email-sr@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_enabledcache_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.
So what do you suggest instead? Removing these functions completely for 440? This would result in bigger changes to common code currently using those functions (especially dcache_disable). Probably by using more #ifdefs there which I would really like not to see.
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?
OK, I removed this patch from my custodian repository. But I assume that you are you asking for additional changes too. Are you asking me to remove (a) all dummy cache entries or (b) to support *real* cache support functions for 440? (a) would lead as explained above to bigger code changes in the common code and (b) is extremely difficult and I just have no time for such a thing currently.
BTW: All the already existing 440 dummy cache entries were not introduced by myself.
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.
Understood. We should propably revert this patch then.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================