From: "Mike Frysinger" <vapier.adi@gmail.com>
To: "David Brownell" <david-b@pacbell.net>
Cc: "Bryan Wu" <cooloney@kernel.org>,
spi-devel-general@lists.sourceforge.net,
linux-kernel@vger.kernel.org,
"Vitja Makarov" <vitja.makarov@gmail.com>
Subject: Re: [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA
Date: Thu, 20 Nov 2008 15:58:13 -0500 [thread overview]
Message-ID: <8bd0f97a0811201258j14dfcf46vce1655632bb12e2@mail.gmail.com> (raw)
In-Reply-To: <200811201224.15493.david-b@pacbell.net>
On Thu, Nov 20, 2008 at 15:24, David Brownell wrote:
> On Monday 17 November 2008, Bryan Wu wrote:
>> /* set transfer mode, and enable SPI */
>> dev_dbg(&drv_data->pdev->dev, "doing DMA in.\n");
>>
>> + /* invalidate caches, if needed */
>> + if (bfin_addr_dcachable((unsigned long) drv_data->rx))
>> + invalidate_dcache_range((unsigned long) drv_data->rx,
>> + (unsigned long) (drv_data->rx +
>> + drv_data->len));
>
> Could you explain why you're not using dma_map_*() calls
> or the rx_dma (and tx_dma) addresses the caller may pass?
i'm not familiar with said API. i worked with what was there already.
> As a rule, you should use the standard kernel interfaces
> for such stuff instead of platform-specific calls like
> those two. There are a LOT more developers who can find
> and fix bugs that way.
could you elaborate on the common calls that would replace these ?
> Also, this patch affects the "not full duplex" branch of
> this routine. DMA here seems unusually convoluted ... but
> if you didn't invalidate the cache (RX path) before
> flushing it (TX path) and instead did it the other way
> aroound, would you actually *need* separate branches?
it's convoluted because the hardware sucks. it cant do full duplex
with DMA. full duplex only works in the non-DMA case.
-mike
next prev parent reply other threads:[~2008-11-20 21:01 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-18 7:52 Bryan Wu
2008-11-18 7:52 ` [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA Bryan Wu
2008-11-20 20:24 ` David Brownell
2008-11-20 20:58 ` Mike Frysinger [this message]
2008-11-20 21:47 ` David Brownell
2008-11-20 21:57 ` Mike Frysinger
2008-11-20 22:05 ` David Brownell
2009-02-05 9:30 ` Bryan Wu
2009-02-05 23:51 ` David Brownell
2009-02-06 2:31 ` Bryan Wu
2008-11-18 7:52 ` [PATCH 02/16] Blackfin SPI Driver: Fix erroneous SPI Clock divisor calculation Bryan Wu
2008-11-20 20:25 ` David Brownell
2008-11-18 7:52 ` [PATCH 03/16] Blackfin SPI Driver: move bfin_addr_dcachable() and friends into the cacheflush header where it belongs Bryan Wu
2008-11-20 20:26 ` David Brownell
2008-11-18 7:52 ` [PATCH 04/16] Blackfin SPI Driver: use len_in_bytes when we care about the number of bytes transferred Bryan Wu
2008-11-20 20:25 ` David Brownell
2008-11-18 7:52 ` [PATCH 05/16] Blackfin SPI Driver: pass DMA overflow error to the higher level Bryan Wu
2008-11-20 20:29 ` David Brownell
2008-11-18 7:52 ` [PATCH 06/16] Blackfin SPI Driver: unify duplicated code in dma read/write paths Bryan Wu
2008-11-20 20:32 ` David Brownell
2008-11-18 7:52 ` [PATCH 07/16] Blackfin SPI Driver: drop bogus cast and touchup dma label Bryan Wu
2008-11-20 20:34 ` David Brownell
2008-11-18 7:52 ` [PATCH 08/16] Blackfin SPI Driver: add a few more debug messages in useful places Bryan Wu
2008-11-20 20:35 ` David Brownell
2008-11-18 7:52 ` [PATCH 09/16] Blackfin SPI Driver: do not check for SPI errors if DMA itself did not flag any Bryan Wu
2008-11-18 7:52 ` [PATCH 10/16] Blackfin SPI Driver: use the properl BIT_CTL_xxx defines Bryan Wu
2008-11-20 20:37 ` David Brownell
2008-11-18 7:52 ` [PATCH 11/16] Blackfin SPI Driver: remove duplicated MAX_SPI_SSEL and remove unnecessary array size Bryan Wu
2008-11-18 7:52 ` [PATCH 12/16] Blackfin SPI Driver: get dma working for SPI flashes Bryan Wu
2008-11-20 20:43 ` David Brownell
2008-11-18 7:52 ` [PATCH 13/16] Blackfin SPI Driver: add timeout while waiting for SPIF in dma irq handler Bryan Wu
2008-11-18 7:52 ` [PATCH 14/16] Blackfin SPI Driver: tweak magic spi dma sequence to get it working on BF54x Bryan Wu
2008-11-18 7:52 ` [PATCH 15/16] Blackfin SPI Driver: fix bug - spi controller driver does not assert/deassert CS correctly Bryan Wu
2008-11-20 20:47 ` David Brownell
2008-11-18 7:52 ` [PATCH 16/16] Blackfin SPI Driver: fix bug - correct usage of struct spi_transfer.cs_change Bryan Wu
2008-11-20 20:51 ` David Brownell
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=8bd0f97a0811201258j14dfcf46vce1655632bb12e2@mail.gmail.com \
--to=vapier.adi@gmail.com \
--cc=cooloney@kernel.org \
--cc=david-b@pacbell.net \
--cc=linux-kernel@vger.kernel.org \
--cc=spi-devel-general@lists.sourceforge.net \
--cc=vitja.makarov@gmail.com \
/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