public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] nios2: convert cache flush to use dm cpu data
Date: Mon, 12 Oct 2015 15:29:45 +0200	[thread overview]
Message-ID: <201510121529.45730.marex@denx.de> (raw)
In-Reply-To: <561BB1B2.3020607@wytron.com.tw>

On Monday, October 12, 2015 at 03:12:18 PM, Thomas Chou wrote:
> Hi Marek,
> 
> On 10/12/2015 06:30 PM, Marek Vasut wrote:
> > There are also DEFINE_CACHE_ALIGN_BUFFER() and ALLOC_CACHE_ALIGN_BUFFER()
> > macros which can be used to allocate such stuff on stack. And you
> > sometimes do want to allocate things on stack instead of using malloc().
> 
> Thanks for sharing this.
> 
> > Sometimes you might want to allocate DMA buffers on stack, for example if
> > you don't have mallocator running yet or if it's more convenient for some
> > reason. So forcing everyone to allocate DMA buffers using malloc is not
> > gonna slide I'm afraid.
> 
> The same rule can be applied to buffer allocated on stack, with the
> macro you mentioned above. In all, cache line aware allocation on heap
> or on stack must be used for DMA buffer.

That's correct, they must be used. But sadly, this is not yet the case in
all the drivers, which we need to rectify. And how best to rectify this
than to scream when someone does such a thing, right ?

> > The cache flush ops is the best place to scream death and murder if
> > someone tries such unaligned cache operation, so maybe you should even
> > do a printf() there to weed such crappy drivers out for the 2016.01
> > release.
> > 
> > I agree it's the responsibility of the driver, so if the driver doesn't
> > do things right, it's a bug and the behavior of cache ops is undefined,
> > which might as well be that we do the safer thing here and flush
> > nothing.
> 
> It won't be safer to flush nothing. Sooner or later the cache will be
> flushed due to data access, even if the cache flush ops is skip.

That is bad bad bad, that's even nastier. We really need to fix the drivers,
not paper over it in the cache ops.

> To solve problem like this, the only solution is to enforce the rule to
> allocate DMA buffer. It is wrong to skip the flush.

I absolutelly agree we need aligned allocations for DMA memory areas. But,
we also shouldn't hide bugs. And I believe aligning the incorrect arguments
to cache functions is not the way to go. We should check the arguments and
if someone tries an unaligned cache op, we should scream. What do you think?

btw. I think you won't get way too many cache warnings nowadays and we can
fix those few remaining way before the 2016.01 is out.

Best regards,
Marek Vasut

  reply	other threads:[~2015-10-12 13:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-06  8:20 [U-Boot] [PATCH] nios2: convert cache flush to use dm cpu data Thomas Chou
2015-10-08 21:39 ` Marek Vasut
2015-10-09  2:49   ` Ley Foon Tan
2015-10-09  8:00     ` Thomas Chou
2015-10-09 14:42       ` Marek Vasut
2015-10-10  5:55         ` Thomas Chou
2015-10-10  6:32           ` Thomas Chou
2015-10-10 18:18             ` Marek Vasut
2015-10-11  0:38               ` Thomas Chou
2015-10-11 12:15                 ` Marek Vasut
2015-10-12  0:34                   ` Thomas Chou
2015-10-12 10:30                     ` Marek Vasut
2015-10-12 13:12                       ` Thomas Chou
2015-10-12 13:29                         ` Marek Vasut [this message]
2015-10-12 13:49                           ` Wolfgang Denk
2015-10-13  1:04                           ` Thomas Chou
2015-10-16 23:03                             ` Marek Vasut
2015-10-17  3:22                               ` Thomas Chou
2015-10-17 11:44                                 ` Marek Vasut
2015-10-10 18:12           ` Marek Vasut
2015-10-09 14:40     ` Marek Vasut
2015-10-09 12:58 ` [U-Boot] [PATCH v2] " Thomas Chou

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=201510121529.45730.marex@denx.de \
    --to=marex@denx.de \
    --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