All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Jander <david.jander@protonic.nl>
To: u-boot@lists.denx.de
Subject: [U-Boot] i.MX51: FEC: Cache coherency problem?
Date: Wed, 20 Jul 2011 11:21:13 +0200	[thread overview]
Message-ID: <20110720112113.575c96fa@archvile> (raw)
In-Reply-To: <4E269827.7040804@aribaud.net>

On Wed, 20 Jul 2011 10:56:07 +0200
Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:

> Hi David,
> 
> Le 20/07/2011 08:29, David Jander a ?crit :
> > On Tue, 19 Jul 2011 14:10:48 +0200
> > David Jander<david.jander@protonic.nl>  wrote:
> >
> >> On Tue, 19 Jul 2011 13:20:26 +0200
> >> Wolfgang Denk<wd@denx.de>  wrote:
> >>
> >>> Dear David Jander,
> >>>
> >>> In message<20110719131744.403a81e6@archvile>  you wrote:
> >>>>
> >>>> Now I finally know what's wrong and am working on a proposed fix to make
> >>>> this one driver cache-aware.
> >>>
> >>> I would just like to point out that these efforts are highly
> >>> appreciated!
> >>
> >> Thanks.
> >> I'll try my best, but I am running out of time, and it is still not as it
> >> should. I still have trouble identifying the right place where receive
> >> buffer descriptors should be cache-invalidated. At one point, a magic
> >> flush_dcache_all() at a certain place in code made it work, but that
> >> can't be entirely correct, and if I replace it with only the necessary
> >> ranges, it doesn't work correctly anymore, so I guess this flush is also
> >> invalidating something I need elsewhere :-(
> >> I keep searching.... through the IMO fairly convoluted network code.
> >
> > Ok, that was not such a problem. I found few nice spots, where
> > cache-maintenance code should go IMO:
> >
> > In fec_rx_task_enable() cleaned RX BD's should be flushed.
> > In fec_tx_task_enable() dirty TX BD's should be flushed, as well as data
> > buffers.
> > In fec_send() Invalidate BD status fields before reading, also in the while
> > loop.
> > In fec_recv() Invalidate BD and data buffers just before every read.
> >
> > And of course align all buffers to cache-line size and pad around the end
> > of each buffer to cache-line size.
> >
> > Problem: It doesn't work (tftp transfer stops after a few packets)! :-(
> > I am giving up!
> >
> > Plan B: I'll now try to allocate all buffers in internal RAM (which is not
> > cached AFAIK).
> >
> > Any ideas?
> 
> Yes, one: I had issues with the Marvell Ethernet adapter, which has DMA 
> as well, not because of cache (it was not active at the time) but 
> because of instruction reordering done by the compiler when optimizing, 
> which resulted in out-of-order accesses to the descritpors and DMA 
> registered, so that the DMA start was written before the descriptors 
> were set up. The (proper and clean) solution was to introduce memory 
> barriers at strategic points of the driver to ensure that specific DMA 
> starts were done only after all writes to the descriptors (and possibly 
> buffers).

Hmmm. I know that issue, but AFAICS, the driver already uses readX() and
writeX() macros when accessing register and DMA memory. Those macros have
compiler barriers included.... and armv7 is still in-order execution ;-)
Thanks for the suggestion anyway :-)

Best regards,

-- 
David Jander
Protonic Holland.

  reply	other threads:[~2011-07-20  9:21 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-18 15:18 [U-Boot] i.MX51: FEC: Cache coherency problem? David Jander
2011-07-18 16:16 ` Aneesh V
2011-07-19  7:26   ` David Jander
2011-07-19 11:07   ` Matthias Weißer
2011-07-19 11:17     ` David Jander
2011-07-19 11:20       ` Wolfgang Denk
2011-07-19 12:10         ` David Jander
2011-07-20  6:29           ` David Jander
2011-07-20  8:56             ` Albert ARIBAUD
2011-07-20  9:21               ` David Jander [this message]
2011-07-20 10:29                 ` Aneesh V
2011-07-20 11:31                   ` David Jander
2011-07-20 12:05                     ` Aneesh V
2011-07-19 11:19     ` Wolfgang Denk
2011-07-19 14:31       ` Matthias Weißer
2011-07-19 11:51     ` Aneesh V
2011-07-18 16:55 ` Stefano Babic
2011-07-19  7:44   ` David Jander
2011-07-19  8:21     ` Albert ARIBAUD
2011-07-19  8:37       ` David Jander
2011-07-19  8:43         ` Aneesh V
2011-07-19  8:58           ` David Jander
2011-07-19  9:11             ` Albert ARIBAUD
2011-07-19 11:50               ` Aneesh V
2011-07-19 11:42             ` Aneesh V
2011-07-19  9:05           ` Albert ARIBAUD
2011-07-19 14:36             ` J. William Campbell
2011-07-19 15:17               ` David Jander
2011-07-19 18:14               ` Anton Staaf
2011-07-19 20:11                 ` J. William Campbell
2011-07-20 13:02                   ` Albert ARIBAUD
     [not found]                     ` <4E26DF9D.5070709@comcast.net>
     [not found]                       ` <4E26E7AA.9070001@aribaud.net>
2011-07-20 15:36                         ` J. William Campbell
2011-07-21  6:48                           ` David Jander
2011-07-23 13:04                             ` Albert ARIBAUD
2011-07-23 15:35                               ` J. William Campbell
2011-07-20  8:37                 ` Aneesh V

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=20110720112113.575c96fa@archvile \
    --to=david.jander@protonic.nl \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.