On 11/10/2017 6:44 PM, Kim Phillips wrote: > On Fri, 10 Nov 2017 08:02:01 +0000 > Radu Andrei Alexe <radu.alexe@xxxxxxx> 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@xxxxxxx> 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
