public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [v3] command/cache: Add flush command
Date: Wed, 10 Apr 2013 14:42:03 -0500	[thread overview]
Message-ID: <1365622923.8381.10@snotra> (raw)
In-Reply-To: <20130410115425.5316520146E@gemini.denx.de> (from wd@denx.de on Wed Apr 10 06:54:25 2013)

On 04/10/2013 06:54:25 AM, Wolfgang Denk wrote:
> Dear Scott,
> 
> In message <1365555490.31043.20@snotra> you wrote:
> >
> > If that means you have some other reason for objecting that you *do*
> > want to go into, could you elaborate?
> 
> I explained that before: we already have commands to operate with the
> caches, and we should rather use the existingones and extend these as
> needed instead of adding arbitrary new ones.

The existing ones have semantics that are mismatched to what we want to  
expose.
	
> > This is just one implementation of flush_cache().  Here are all of  
> them:
> 
> Thanks for the list.  But that was not my question.  I asked (ok, in
> the other message):

It was my answer to your question.  If you don't like my answer, fine.

> | ...  Can you please point me to
> | the code (ideally in mainline U-Boot) which would cause problems  
> with
> | the suggested separation of invalidating the IC and flushing the DC
> | into subcommands of the "icache" resp. "dcache" commands?
> 
> Yes, I did have a look at all these implementations, and I think none
> of them will cause any issues.

Needing to touch them at all is a big issue.  We don't know the details  
of all these architectures, and cannot test them.  If there were a good  
*reason* for it we could engage the maintainers of those architectures,  
but I've yet to hear a reason that justifies the hassle.  We'd probably  
be better off just keeping the patch in our internal tree, which is the  
tree that the user who ran into this problem is using.

> Actually quite a number of them are actually just combiing calls like  
> that:

Many have the instruction/data sequence inside the loop so we'd need to  
pick it apart (higher risk of introducing a bug, so more need for  
testing that we cannot do).  Blackfin is weird -- if we did a simple  
split at the C-code level it looks like we'd have two dummy loops  
executing.

> > This is what we're trying to expose as a command.  If you want it  
> split
> > into icache and dcache, it needs to be rewritten for every  
> architecture
> 
> Actually it appears to be already split quite naturally for all
> currently supported architectures (at least to the extend these
> implement this functinality at all).  flush_cache() is just a
> convenience, and if you think twice not even a very lucky one.
> The openrisc example above shows this pretty clearly.

The openrisc example does not show any great difficulty implementing  
flush_cache().

> > the actual need for splitting it?  The semantics we're trying to  
> expose
> > as a command are the same as this function is meant to implement,
> 
> You defend this stupid function  flush_cache()  as if it was of major
> achievement of the development of software in the last decades?

"Major achievement"?  No, I just said it was a useful (but poorly  
named) abstraction, which it is.  What do we gain by replacing every  
caller of flush_cache() with two function calls instead?  When would we  
ever call one but not the other, and if there is such a case, what harm  
would come out of doing both anyway?

-Scott

  reply	other threads:[~2013-04-10 19:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-05 20:50 [U-Boot] [v3] command/cache: Add flush command York Sun
2013-04-05 21:00 ` Tom Rini
2013-04-05 21:09   ` York Sun
2013-04-05 22:09 ` Wolfgang Denk
2013-04-05 23:02   ` York Sun
2013-04-06  7:01     ` Wolfgang Denk
2013-04-07  3:31       ` sun york-R58495
2013-04-07  8:29         ` Wolfgang Denk
2013-04-08 17:45           ` sun york-R58495
2013-04-08 18:35             ` Wolfgang Denk
2013-04-08 19:05               ` Scott Wood
2013-04-08 19:18                 ` Wolfgang Denk
2013-04-08 19:31                   ` Scott Wood
2013-04-09 17:45                     ` Wolfgang Denk
2013-04-10  0:58                       ` Scott Wood
2013-04-10  2:07                         ` [U-Boot] DWMMC / DWMCI question TigerLiu at viatech.com.cn
2013-04-10 11:58                           ` Wolfgang Denk
2013-04-11  1:43                             ` TigerLiu at viatech.com.cn
2013-04-10 11:54                         ` [U-Boot] [v3] command/cache: Add flush command Wolfgang Denk
2013-04-10 19:42                           ` Scott Wood [this message]
2013-04-10 21:04                             ` Wolfgang Denk
2013-04-10 21:10                               ` Scott Wood
2013-04-10 22:50                                 ` Wolfgang Denk
2013-04-10 23:00                                   ` Scott Wood
2013-04-11 11:56                                     ` Wolfgang Denk
2013-04-08 19:50           ` Scott Wood
2013-04-09 17:48             ` Wolfgang Denk

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=1365622923.8381.10@snotra \
    --to=scottwood@freescale.com \
    --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