All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mike Frysinger" <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "David Brownell" <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Vitja Makarov
	<vitja.makarov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
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-yBeKhBN/0LDR7s880joybQ@public.gmane.org>

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

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

WARNING: multiple messages have this Message-ID (diff)
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

  parent reply	other threads:[~2008-11-20 20:58 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-18  7:52 (no subject) Bryan Wu
2008-11-18  7:52 ` Bryan Wu
     [not found] ` <1226994760-4301-1-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-18  7:52   ` [PATCH 01/16] Blackfin SPI Driver: ensure cache coherency before doing DMA Bryan Wu
2008-11-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-2-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:24       ` David Brownell
2008-11-20 20:24         ` David Brownell
     [not found]         ` <200811201224.15493.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-11-20 20:58           ` Mike Frysinger [this message]
2008-11-20 20:58             ` Mike Frysinger
     [not found]             ` <8bd0f97a0811201258j14dfcf46vce1655632bb12e2-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-11-20 21:47               ` David Brownell
2008-11-20 21:47                 ` David Brownell
     [not found]                 ` <200811201347.46355.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2008-11-20 21:57                   ` Mike Frysinger
2008-11-20 21:57                     ` Mike Frysinger
     [not found]                     ` <8bd0f97a0811201357g1ffeb7d3pfad6e178fb20050e-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-11-20 22:05                       ` David Brownell
2008-11-20 22:05                         ` David Brownell
     [not found]                         ` <200811201405.06811.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2009-02-05  9:30                           ` Bryan Wu
2009-02-05  9:30                             ` Bryan Wu
2009-02-05 23:51                             ` David Brownell
     [not found]                               ` <200902051551.14146.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2009-02-06  2:31                                 ` Bryan Wu
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-3-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:25       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-4-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:26       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-5-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:25       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-6-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:29       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-7-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:32       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-8-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:34       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-9-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:35       ` David Brownell
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     ` Bryan Wu
2008-11-18  7:52   ` [PATCH 10/16] Blackfin SPI Driver: use the properl BIT_CTL_xxx defines Bryan Wu
2008-11-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-11-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:37       ` David Brownell
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     ` Bryan Wu
2008-11-18  7:52   ` [PATCH 12/16] Blackfin SPI Driver: get dma working for SPI flashes Bryan Wu
2008-11-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-13-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:43       ` David Brownell
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     ` 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     ` 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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-16-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:47       ` David Brownell
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-18  7:52     ` Bryan Wu
     [not found]     ` <1226994760-4301-17-git-send-email-cooloney-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2008-11-20 20:51       ` David Brownell
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-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    --cc=vitja.makarov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    /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.