Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding

From: Radu Andrei Alexe <radu.alexe@nxp.com>
To: Kim Phillips <kim.phillips@arm.com>
Cc: "Horia Geantă" <horia.geanta@nxp.com>,
	"Vinod Koul" <vinod.koul@intel.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	"Dan Douglass" <dan.douglass@nxp.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
Date: Mon, 13 Nov 2017 09:44:06 +0000	[thread overview]
Message-ID: <VI1PR04MB132502335EEB864169926F4E8A2B0@VI1PR04MB1325.eurprd04.prod.outlook.com> (raw)
In-Reply-To: 20171110104430.28b2f56f910b7514d778c4b2@arm.com

On 11/10/2017 6:44 PM, Kim Phillips wrote:
> On Fri, 10 Nov 2017 08:02:01 +0000
> Radu Andrei Alexe <radu.alexe@nxp.com> wrote:
> 
>> On 11/9/2017 6:34 PM, Kim Phillips wrote:
>>> On Thu, 9 Nov 2017 11:54:13 +0000
>>> Radu Andrei Alexe <radu.alexe@nxp.com> wrote:
>>>> The next patch version will create the platform device dynamically at
>>>> run time.
>>>
>>> Why create a new device when that h/w already has one?
>>>
>>> Why doesn't the existing crypto driver register dma capabilities with
>>> the dma driver subsystem?
>>>
>> I can think of two reasons:
>>
>> 1. The code that this driver introduces has nothing to do with crypto
>> and everything to do with dma.
> 
> I would think that at least a crypto "null" algorithm implementation
> would share code.
>
>> Placing the code in the same directory as
>> the caam subsystem would only create confusion for the reader of an
>> already complex driver.
> 
> this different directory argument seems to be identical to your 2 below:
> 
>> 2. I wanted this driver to be tracked by the dma engine team. They have
>> the right expertise to provide adequate feedback. If all the code was in
>> the crypto directory they wouldn't know about this driver or any
>> subsequent changes to it.
> 
> dma subsystem bits could still be put in the dma area if deemed
> necessary but I don't think it is: I see
> drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for
> example.
> 
> I also don't see how that complicates things much further.
> 

So who made their review? The guys from crypto?

If someone wants to enable only the DMA functionality of the CCP and not 
the crypto part how do they do it? Look for it in the crypto submenu?

> What is the rationale for using the crypto h/w as a dma engine anyway?
> Are there supporting performance figures?

We have a platform that doesn't have a dedicated DMA controller but has 
the CAAM hardware block that can perform dma transfers. We have a 
use-case where we need to issue large transfers (hundred of MBs) 
asynchronously, without using the core.

> 
> Kim
> 

BR,
Radu