[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200811201224.15493.david-b@pacbell.net>
Date: Thu, 20 Nov 2008 12:24:15 -0800
From: David Brownell <david-b@...bell.net>
To: Bryan Wu <cooloney@...nel.org>
Cc: spi-devel-general@...ts.sourceforge.net,
linux-kernel@...r.kernel.org,
Vitja Makarov <vitja.makarov@...il.com>
Subject: Re: [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA
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?
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.
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?
> +
> /* clear tx reg soformer data is not shifted out */
> write_TDBR(drv_data, 0xFFFF);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists