public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/4] net: round up before calling flush_cache
Date: Mon, 2 Apr 2012 05:34:28 +0200	[thread overview]
Message-ID: <201204020534.28361.marek.vasut@gmail.com> (raw)
In-Reply-To: <201204012306.48991.vapier@gentoo.org>

Dear Mike Frysinger,

> On Sunday 01 April 2012 21:44:39 Marek Vasut wrote:
> > Dear Mike Frysinger,
> > 
> > > On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
> > > > Dear Mike Frysinger,
> > > > 
> > > > > On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
> > > > > > If the range passed to flush_cache is not multiple
> > > > > > of ARCH_DMA_MINALIGN, a warning due to mislaignment
> > > > > > is printed.
> > > > > > Detected with fec_mxc, mx35 boards:
> > > > > > 
> > > > > > CACHE: Misaligned operation at range [80800000, 8083c310]
> > > > > 
> > > > > warning on flushing is broken.  the arch/arm/cpu/arm926ejs/cache.c
> > > > > code should probably be fixed instead.
> > > > 
> > > > Why exactly?
> > > 
> > > the flush isn't harmful (ignoring the fact that a few extra bytes might
> > > get written back to external memory), and the data isn't evicted from
> > > cache. after all, we aren't talking about invalidate here, we're
> > > talking about flush.
> > 
> > Right ... and can you be sure nothing important is overwritten in RAM?
> 
> except i'd bet money you're already running dcache in writethrough mode, so
> the flush is largely irrelevant to this line of reasoning

Sure, but you can write your data, then the DMA happens and then you flush your 
stuff again. This is when you're screwed (all right, it's 5:30am here, I'm not 
confident anymore).

> 
> > > plus, no other arch (linux or u-boot) does this.
> > > 
> > > so the better question is, why exactly should you be warning ?  you
> > > should provide justification when doing something unusual ...
> > 
> > Because you can destroy data in DRAM that arrived there by DMA transfer
> > for example?
> 
> that isn't the problem of the flush functions.  there would have been an
> invalidate call at some point with misaligned addresses, iff it actually
> mattered.  you could argue for invalidation triggering a warning, but that
> isn't what we're talking about.  and still, linux doesn't trigger warnings,
> and i think only one other arm soc does atm in u-boot.

Ain't that because linux uses aligned buffers ?

> 
> as already shown here, the flush call was perfectly fine, and adding
> roundup to that call site is a waste of code space to "fix" something that
> isn't a problem.

I believe unaligned flush in uboot should always trigger a warning, not do 
alignment for the programmer. The programmer knows the best how the buffer looks 
(aka. if you embed the buffer in some structure, it might be problem).

> -mike

Best regards,
Marek Vasut

  reply	other threads:[~2012-04-02  3:34 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-30 14:02 [U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations Anatolij Gustschin
2012-03-30 14:20 ` Stefano Babic
2012-03-30 14:35   ` Anatolij Gustschin
2012-03-30 15:04     ` Stefano Babic
2012-03-30 15:28       ` Marek Vasut
2012-03-30 15:42         ` Anatolij Gustschin
2012-03-30 15:58         ` Stefano Babic
2012-03-30 16:05           ` Marek Vasut
2012-03-30 16:16             ` Stefano Babic
2012-04-01 13:22 ` [U-Boot] [PATCH 1/4] " Stefano Babic
2012-04-01 13:22   ` [U-Boot] [PATCH 2/4] net: round up before calling flush_cache Stefano Babic
2012-04-01 13:46     ` Marek Vasut
2012-04-01 14:56       ` Stefano Babic
2012-04-01 15:35         ` Marek Vasut
2012-04-01 19:23     ` Mike Frysinger
2012-04-01 21:00       ` Marek Vasut
2012-04-02  1:38         ` Mike Frysinger
2012-04-02  1:44           ` Marek Vasut
2012-04-02  3:06             ` Mike Frysinger
2012-04-02  3:34               ` Marek Vasut [this message]
2012-04-02  5:56                 ` Mike Frysinger
2012-04-02  7:13       ` Stefano Babic
2012-04-02 14:03         ` Marek Vasut
2012-04-02 14:38           ` Stefano Babic
2012-04-01 13:23   ` [U-Boot] [PATCH 3/4] mx35: flea3: fix when cache functions are linked Stefano Babic
2012-04-01 13:23   ` [U-Boot] [PATCH 4/4] mx35: mx35pdk: " Stefano Babic
2012-04-02 16:18   ` [U-Boot] [PATCH V3 1/4] ARM1136: add cache flush and invalidate operations Stefano Babic
2012-04-02 16:29     ` Marek Vasut
2012-04-02 16:51     ` Stefano Babic
2012-04-02 16:18   ` [U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses Stefano Babic
2012-04-02 16:29     ` Marek Vasut
2012-04-02 18:23     ` Mike Frysinger
2012-04-02 18:42       ` Marek Vasut
2012-04-02 19:07         ` Mike Frysinger

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=201204020534.28361.marek.vasut@gmail.com \
    --to=marek.vasut@gmail.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