Re: [PATCH V2 2/3] dmaselftest: add memcpy selftest support functions

Sinan Kaya
2015-11-07T06:23:42Z

On 11/5/2015 11:17 AM, Sinan Kaya wrote:
>
>
> On 11/5/2015 7:05 AM, Vinod Koul wrote:
>> On Wed, Nov 04, 2015 at 09:42:46PM -0500, Sinan Kaya wrote:
>>> Here is what I proposed.
>>>
>>> - a common file that gets compiled into a module that wants to use
>>> self-test with a public API. It can be called from driver's probe
>>> routine.
>>> - the test is independent of my implementation. It uses dmaengine
>>> API and should be portable to most drivers.
>>> - there *are* still drivers in the kernel that has selftest code
>>> embedded inside them. I followed the design pattern from other
>>> drivers thinking this must have been a good idea and it paid off for
>>> me.
>>>
>>> As far as I understand, there is interest in doing more than this
>>> and reusing the dmatest code for code duplication.
>>
>> the code that selftest uses to test will be very similar to dmatest code,
>> both of these _should_ share this common code so that fixes get done for
>> both!
>>
>
> OK, I can move the code around and try to combine it if possible.
>

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.

>>> Facts:
>>> - Dmatest can be actually configured to run during boot.
>>> - Nobody besides the dma driver developer uses dmatest. This leaves
>>> holes for regressions that are really hard to debug due to
>>> interaction with the rest of the system.
>>> - Dmatest doesn't exist in most distribution kernels.
>>
>> That doesn't mean it is not useful. This line of thought is not quite
>> right.
>> You are trying to say dmatest in not important and selftest is. Sorry but
>> you are wrong, both are equally important and since both try to test
>> and use
>> similar routines (dmaengien API) they need to share the code and not
>> duplicate it
>>
>>> If we want to do something else, I need clear directions. I can
>>> remove the self test code completely from my driver. But, I need an
>>> equivalent functionality.
>>
>> Add selftest to dmatest, we need both!
>>
>
> OK, do you have any objections to compiling dmatest along with hidma in
> the same module and calling a function from there ? or do you have
> something else in your mind ?
>

ping

>>>
>>>>
>>>> That part is tricky, you need to do so thru clients,
>>>> spi/audio/serial etc
>>>>
>>>
>>> My selftest code actually attaches to all slave devices and issues a
>>> memcpy command and then detaches from the slave devices.
>>
>> Not everyone supports memcpy!
>>
> Right, last time I checked; you can request a DMA channel that supports
> MEMCPY specifically.
>

--
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project