public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [RFC][MIPS] Remove mips_cache_lock
Date: Fri, 21 Mar 2008 22:45:48 +0900	[thread overview]
Message-ID: <47E3BC0C.4060406@ruby.dti.ne.jp> (raw)
In-Reply-To: <47DFC7FA.4040200@ruby.dti.ne.jp>

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 <skuribay@ruby.dti.ne.jp>
> 
> 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 <skuribay@ruby.dti.ne.jp>
> ---
> 
>  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 <skuribay@ruby.dti.ne.jp>
---

 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)

  reply	other threads:[~2008-03-21 13:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-17 14:42 [U-Boot-Users] [RFC][MIPS] Remove mips_cache_lock Shinya Kuribayashi
2008-03-17 21:04 ` Wolfgang Denk
2008-03-17 23:55   ` Andrew Dyer
2008-03-18  0:28     ` Wolfgang Denk
2008-03-18 12:56       ` Shinya Kuribayashi
2008-03-18 13:47         ` Shinya Kuribayashi
2008-03-21 13:45           ` Shinya Kuribayashi [this message]
2008-03-24  2:29             ` Andrew Dyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47E3BC0C.4060406@ruby.dti.ne.jp \
    --to=skuribay@ruby.dti.ne.jp \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox