From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shinya Kuribayashi Date: Fri, 21 Mar 2008 22:45:48 +0900 Subject: [U-Boot-Users] [RFC][MIPS] Remove mips_cache_lock In-Reply-To: <47DFC7FA.4040200@ruby.dti.ne.jp> References: <20080318002849.224C3246C5@gemini.denx.de> <47DFBBEE.1090006@ruby.dti.ne.jp> <47DFC7FA.4040200@ruby.dti.ne.jp> Message-ID: <47E3BC0C.4060406@ruby.dti.ne.jp> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Shinya Kuribayashi wrote: > Shinya Kuribayashi wrote: >>> But simply deleting it is definitely not a good idea, as it would most >>> probably break existing board support. >> I still think this removal will not break any existing targets, but yes >> agreed. I'll try to introduce a #ifdef alternative. > > Patch attached. Any comments are still appreciated. Thanks. > > ================> > [MIPS] mips_cache_lock: Introduce CFG_INIT_RAM_LOCK > > From: Shinya Kuribayashi > > We don't have to lock cache lines for stack use. Meanwhile, once U-Boot > locks cache, but never gets them unlocked. So the code relies on whatever > gets loaded after U-Boot to re-initialize the cache and clear the locks. > For these reasons, I proposed the removal of mips_cache_lock() from the > global start-up routines. > > This patch makes mips_cache_lock() user-selectable so that we don't break > any existing board support. Let's see how things go for a while. > > Signed-off-by: Shinya Kuribayashi > --- > > cpu/mips/cache.S | 2 ++ > cpu/mips/start.S | 2 ++ > include/configs/dbau1x00.h | 1 + > include/configs/gth2.h | 1 + > include/configs/incaip.h | 1 + > include/configs/pb1x00.h | 1 + > include/configs/purple.h | 2 ++ > include/configs/qemu-mips.h | 1 + > include/configs/tb0229.h | 1 + > 9 files changed, 12 insertions(+), 0 deletions(-) After careful thought, I changed my mind. I'd like to change the default behavior. The one who makes no regression does none of the work B-) I hope the patche below is acceptable for the next release. ================> [MIPS] Request for the 'mips_cache_lock()' removal The initial intension of having mips_cache_lock() was to use the cache as memory for temporary stack use so that a C environment can be set up as early as possible. But now mips_cache_lock() follow lowlevel_init(). We've already have the real memory initilaized at this point, therefore we could/should use it. No reason to lock at all. Other problems: Cache locking is not consistent across MIPS implementaions. Some imple- mentations don't support locking at all. The style of locking varies - some support per line locking, others per way, etc. Some parts use bits in status registers instead of cache ops. Current mips_cache_lock() is not necessarily general-purpose. And this is worthy of special mention; once U-Boot/MIPS locks the lines, they are never get unlocked, so the code relies on whatever gets loaded after U-Boot to re-initialize the cache and clear the locks. We're sup- posed to have CFG_INIT_RAM_LOCK and unlock_ram_in_cache() implemented, but leave the situation as it is for a long time. For these reasons, I proposed the removal of mips_cache_lock() from the global start-up code. This patch adds CFG_INIT_RAM_LOCK_MIPS to make existing users aware that *things have changed*. If he wants the same behavior as before, he needs to have CFG_INIT_RAM_LOCK_MIPS in his config file. If we don't have any regression report through several releases, then we'll remove codes entirely. Signed-off-by: Shinya Kuribayashi --- cpu/mips/cache.S | 2 ++ cpu/mips/start.S | 2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/cpu/mips/cache.S b/cpu/mips/cache.S index 443240e..67ee19f 100644 --- a/cpu/mips/cache.S +++ b/cpu/mips/cache.S @@ -238,6 +238,7 @@ dcache_disable: .end dcache_disable +#ifdef CFG_INIT_RAM_LOCK_MIPS /******************************************************************************* * * mips_cache_lock - lock RAM area pointed to by a0 in cache. @@ -263,3 +264,4 @@ mips_cache_lock: j ra .end mips_cache_lock +#endif /* CFG_INIT_RAM_LOCK_MIPS */ diff --git a/cpu/mips/start.S b/cpu/mips/start.S index c92b162..930f9b3 100644 --- a/cpu/mips/start.S +++ b/cpu/mips/start.S @@ -267,10 +267,12 @@ reset: /* Set up temporary stack. */ +#ifdef CFG_INIT_RAM_LOCK_MIPS li a0, CFG_INIT_SP_OFFSET la t9, mips_cache_lock jalr t9 nop +#endif li t0, CFG_SDRAM_BASE + CFG_INIT_SP_OFFSET la sp, 0(t0)