linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: okaya@codeaurora.org
To: "Vinod Koul" <vinod.koul@intel.com>
Cc: "Sinan Kaya" <okaya@codeaurora.org>,
	"Andy Shevchenko" <andy.shevchenko@gmail.com>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	timur@codeaurora.org, cov@codeaurora.org, jcm@redhat.com,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2 2/3] dmaselftest: add memcpy selftest support functions
Date: Fri, 13 Nov 2015 20:20:57 -0000	[thread overview]
Message-ID: <0cbe4b9bd72dad1336178e1d7cd94267.squirrel@www.codeaurora.org> (raw)
In-Reply-To: <20151108135352.GA22709@localhost>

>>
>> I looked at this. IMO, merging selftest code and dmatest code is not
>> a good idea.
>>
>> Dmatest code has been well written and structured so that multiple
>> DMA capabilities (XOR, PQ, MEMCPY) can be tested with the same code.
>>
>> It supports threads and user space interaction.
>>
>> The code I want to change (dmatest_func) is 3 levels deep in
>> structure. My refactored code looked really ugly compared to the
>> original code.
>
> dmatest_func is still a bigger fn specific to dmatest. I was thinking that
> we
> should have rather have two common functions
> 1) dmatest_do_dma() which does buffer allocation, invoking dmaengine APIs
> and
> checking results, part of dmatest_func today
>
> 2) request_channels() would also be common along with cleanup routines
>

Tried again,

It still doesn't look right. Depending on the used API, the source and
destination buffers are different.

For instance, when you use dma_map_page, your source addresses are pages.
If you use scatter_sg, you are working on the scatter-gather list. If you
use dma_coherent, then the source cannot be a kmalloc API. Source comes
from the DMA API. If you use dma_map_single, it is a kmalloc buffer. This
is causing havoc in the compare APIs.

IMO, in the goals of sharing code, we are making it unmanageable.

I'm leaning towards dropping this patch altogether (which I'm not very
happy about) or pushing it this way and have somebody do the
reorganization in another iteration.





  reply	other threads:[~2015-11-13 20:21 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1446444460-21600-1-git-send-email-okaya@codeaurora.org>
2015-11-02  6:07 ` [PATCH V2 1/3] dma: add Qualcomm Technologies HIDMA management driver Sinan Kaya
2015-11-02 15:57   ` Rob Herring
2015-11-02 16:20     ` Sinan Kaya
2015-11-02 17:26       ` Timur Tabi
2015-11-02 17:42         ` Rob Herring
2015-11-02 17:48           ` Timur Tabi
2015-11-02 18:25             ` Rob Herring
2015-11-02 18:30               ` Timur Tabi
2015-11-05 14:31                 ` Rob Herring
2015-11-05 14:43                   ` Timur Tabi
2015-11-02 18:49           ` Sinan Kaya
2015-11-02 22:00             ` Arnd Bergmann
2015-11-03  5:18     ` Sinan Kaya
2015-11-03 10:22   ` Andy Shevchenko
2015-11-04  0:47     ` Sinan Kaya
2015-11-02  6:07 ` [PATCH V2 2/3] dmaselftest: add memcpy selftest support functions Sinan Kaya
2015-11-03  4:15   ` Vinod Koul
2015-11-03  4:18     ` Sinan Kaya
2015-11-03  6:30       ` Vinod Koul
2015-11-03  7:44         ` Dan Williams
2015-11-03  8:22           ` Andy Shevchenko
2015-11-03 16:08             ` Vinod Koul
2015-11-05  2:42               ` Sinan Kaya
2015-11-05 12:05                 ` Vinod Koul
2015-11-05 16:17                   ` Sinan Kaya
2015-11-07  6:23                     ` Sinan Kaya
2015-11-08 13:53                       ` Vinod Koul
2015-11-13 20:20                         ` okaya [this message]
2015-11-03 15:51           ` Sinan Kaya
2015-11-03 16:06           ` Vinod Koul
2015-11-03 14:31       ` Timur Tabi
2015-11-03 16:10         ` Vinod Koul
2015-11-03 16:28           ` Sinan Kaya
2015-11-03 16:46             ` Timur Tabi
2015-11-03 16:57               ` Sinan Kaya
2015-11-03 16:48           ` Timur Tabi
2015-11-02  6:07 ` [PATCH V2 3/3] dma: add Qualcomm Technologies HIDMA channel driver Sinan Kaya
2015-11-03 10:10   ` Andy Shevchenko
2015-11-04  0:07     ` Sinan Kaya
2015-11-04 17:44       ` Andy Shevchenko
2015-11-05  2:22         ` Sinan Kaya

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=0cbe4b9bd72dad1336178e1d7cd94267.squirrel@www.codeaurora.org \
    --to=okaya@codeaurora.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=cov@codeaurora.org \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=jcm@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=timur@codeaurora.org \
    --cc=vinod.koul@intel.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;
as well as URLs for NNTP newsgroup(s).