From: Radu Alexe <[email protected]>
This patch-set introduces a new DMA memcpy driver based on the DMA
capabilities of the CAAM crypto engine. Because of this dependency the
included commits target various parts of the kernel tree.
Patch 1.
Since the CAAM DMA driver is a platform driver it is enabled by a new node
in the device tree. This commit adds documentation for the device tree
bindings.
Patch 2.
This patch adds the "caam-dma" node in the fsl-ls1012a.dtsi file.
Patch 3.
This commit adds various capabilities in the JR driver of the CAAM that is
used by the CAAM DMA driver.
Patch 4.
Adds the CAAM DMA memcpy driver.
Patch 1 and 3 should be ack-ed by the crypto maintainers, patch 2 by
devicetree maintainers and patch 4 by the DMA maintainers.
The intent is to go withh all the patches through the dmaengine tree.
Radu Alexe (4):
crypto: caam: add caam-dma node to SEC4.0 device tree binding
arm64: dts: ls1012a: add caam-dma node
crypto: caam: add functionality used by the caam_dma driver
dma: caam: add dma memcpy driver
.../devicetree/bindings/crypto/fsl-sec4.txt | 21 +
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 6 +
drivers/crypto/caam/desc.h | 3 +
drivers/crypto/caam/jr.c | 42 ++
drivers/crypto/caam/jr.h | 2 +
drivers/dma/Kconfig | 17 +
drivers/dma/Makefile | 1 +
drivers/dma/caam_dma.c | 444 +++++++++++++++++++++
8 files changed, 536 insertions(+)
create mode 100644 drivers/dma/caam_dma.c
--
2.14.2.606.g7451fcd
From: Radu Alexe <[email protected]>
Signed-off-by: Radu Alexe <[email protected]>
---
.../devicetree/bindings/crypto/fsl-sec4.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
index 7aef0eae58d4..97b37c15d793 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -6,6 +6,7 @@ Copyright (C) 2008-2011 Freescale Semiconductor Inc.
-Overview
-SEC 4 Node
-Job Ring Node
+ -CAAM DMA Node
-Run Time Integrity Check (RTIC) Node
-Run Time Integrity Check (RTIC) Memory Node
-Secure Non-Volatile Storage (SNVS) Node
@@ -215,6 +216,26 @@ EXAMPLE
interrupts = <88 2>;
};
+=====================================================================
+CAAM DMA Node
+
+ Child node of the crypto node that enables the use of the DMA capabilities
+ of the CAAM by a stand-alone driver. The only required property is the
+ "compatible" property. All the other properties are determined from
+ the job rings on which the CAAM DMA driver depends (ex: the number of
+ dma-channels is equal to the number of defined job rings).
+
+ - compatible
+ Usage: required
+ Value type: <string>
+ Definition: Must include "fsl,sec-v4.0-dma"
+
+EXAMPLE
+ caam-dma {
+ compatible = "fsl,sec-v5.4-dma",
+ "fsl,sec-v5.0-dma",
+ "fsl,sec-v4.0-dma";
+ }
=====================================================================
Run Time Integrity Check (RTIC) Node
--
2.14.2.606.g7451fcd
From: Radu Alexe <[email protected]>
Signed-off-by: Radu Alexe <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
index df83915d6ea6..f92ecf381cb1 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
@@ -197,6 +197,12 @@
interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
};
+ caam-dma {
+ compatible = "fsl,sec-v5.4-dma",
+ "fsl,sec-v5.0-dma",
+ "fsl,sec-v4.0-dma";
+ };
+
rtic@60000 {
compatible = "fsl,sec-v5.4-rtic",
"fsl,sec-v5.0-rtic",
--
2.14.2.606.g7451fcd
On Mon, 30 Oct 2017 15:46:51 +0200
Horia Geantă <[email protected]> wrote:
> +=====================================================================
> +CAAM DMA Node
> +
> + Child node of the crypto node that enables the use of the DMA capabilities
> + of the CAAM by a stand-alone driver. The only required property is the
> + "compatible" property. All the other properties are determined from
> + the job rings on which the CAAM DMA driver depends (ex: the number of
> + dma-channels is equal to the number of defined job rings).
> +
> + - compatible
> + Usage: required
> + Value type: <string>
> + Definition: Must include "fsl,sec-v4.0-dma"
> +
> +EXAMPLE
> + caam-dma {
> + compatible = "fsl,sec-v5.4-dma",
> + "fsl,sec-v5.0-dma",
> + "fsl,sec-v4.0-dma";
> + }
If this isn't describing an aspect of the hardware, then what is it
doing in the device tree?
Kim
On 10/30/2017 4:24 PM, Kim Phillips wrote:
> On Mon, 30 Oct 2017 15:46:51 +0200
> Horia Geant? <[email protected]> wrote:
>
>> +=====================================================================
>> +CAAM DMA Node
>> +
>> + Child node of the crypto node that enables the use of the DMA capabilities
>> + of the CAAM by a stand-alone driver. The only required property is the
>> + "compatible" property. All the other properties are determined from
>> + the job rings on which the CAAM DMA driver depends (ex: the number of
>> + dma-channels is equal to the number of defined job rings).
>> +
>> + - compatible
>> + Usage: required
>> + Value type: <string>
>> + Definition: Must include "fsl,sec-v4.0-dma"
>> +
>> +EXAMPLE
>> + caam-dma {
>> + compatible = "fsl,sec-v5.4-dma",
>> + "fsl,sec-v5.0-dma",
>> + "fsl,sec-v4.0-dma";
>> + }
>
> If this isn't describing an aspect of the hardware, then what is it
> doing in the device tree?
>
> Kim
>
Because the caam_dma driver is a platform driver I needed to create a
platform device to activate the driver. My thought was that it was
simpler to implement it using device tree.
The next patch version will create the platform device dynamically at
run time.
BR,
Radu
On Thu, 9 Nov 2017 11:54:13 +0000
Radu Andrei Alexe <[email protected]> wrote:
> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> > On Mon, 30 Oct 2017 15:46:51 +0200
> > Horia Geantă <[email protected]> wrote:
> >
> >> +=====================================================================
> >> +CAAM DMA Node
> >> +
> >> + Child node of the crypto node that enables the use of the DMA capabilities
> >> + of the CAAM by a stand-alone driver. The only required property is the
> >> + "compatible" property. All the other properties are determined from
> >> + the job rings on which the CAAM DMA driver depends (ex: the number of
> >> + dma-channels is equal to the number of defined job rings).
> >> +
> >> + - compatible
> >> + Usage: required
> >> + Value type: <string>
> >> + Definition: Must include "fsl,sec-v4.0-dma"
> >> +
> >> +EXAMPLE
> >> + caam-dma {
> >> + compatible = "fsl,sec-v5.4-dma",
> >> + "fsl,sec-v5.0-dma",
> >> + "fsl,sec-v4.0-dma";
> >> + }
> >
> > If this isn't describing an aspect of the hardware, then what is it
> > doing in the device tree?
> >
> > Kim
> >
>
> Because the caam_dma driver is a platform driver I needed to create a
> platform device to activate the driver. My thought was that it was
> simpler to implement it using device tree.
> 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?
Kim
On 11/9/2017 6:34 PM, Kim Phillips wrote:
> On Thu, 9 Nov 2017 11:54:13 +0000
> Radu Andrei Alexe <[email protected]> wrote:
>
>> On 10/30/2017 4:24 PM, Kim Phillips wrote:
>>> On Mon, 30 Oct 2017 15:46:51 +0200
>>> Horia Geant? <[email protected]> wrote:
>>>
>>>> +=====================================================================
>>>> +CAAM DMA Node
>>>> +
>>>> + Child node of the crypto node that enables the use of the DMA capabilities
>>>> + of the CAAM by a stand-alone driver. The only required property is the
>>>> + "compatible" property. All the other properties are determined from
>>>> + the job rings on which the CAAM DMA driver depends (ex: the number of
>>>> + dma-channels is equal to the number of defined job rings).
>>>> +
>>>> + - compatible
>>>> + Usage: required
>>>> + Value type: <string>
>>>> + Definition: Must include "fsl,sec-v4.0-dma"
>>>> +
>>>> +EXAMPLE
>>>> + caam-dma {
>>>> + compatible = "fsl,sec-v5.4-dma",
>>>> + "fsl,sec-v5.0-dma",
>>>> + "fsl,sec-v4.0-dma";
>>>> + }
>>>
>>> If this isn't describing an aspect of the hardware, then what is it
>>> doing in the device tree?
>>>
>>> Kim
>>>
>>
>> Because the caam_dma driver is a platform driver I needed to create a
>> platform device to activate the driver. My thought was that it was
>> simpler to implement it using device tree.
>> 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?
>
> Kim
>
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. Placing the code in the same directory as
the caam subsystem would only create confusion for the reader of an
already complex driver.
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.
BR,
Radu
On Fri, 10 Nov 2017 08:02:01 +0000
Radu Andrei Alexe <[email protected]> wrote:
> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > On Thu, 9 Nov 2017 11:54:13 +0000
> > Radu Andrei Alexe <[email protected]> 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.
What is the rationale for using the crypto h/w as a dma engine anyway?
Are there supporting performance figures?
Kim
On 11/10/2017 6:44 PM, Kim Phillips wrote:
> On Fri, 10 Nov 2017 08:02:01 +0000
> Radu Andrei Alexe <[email protected]> wrote:
[snip]>> 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.
>
Please see previous discussion with Vinod:
https://www.mail-archive.com/[email protected]/msg21468.html
> What is the rationale for using the crypto h/w as a dma engine anyway?
SoCs that don't have a system DMA, for e.g. LS1012A.
Horia
On 11/10/2017 6:44 PM, Kim Phillips wrote:
> On Fri, 10 Nov 2017 08:02:01 +0000
> Radu Andrei Alexe <[email protected]> wrote:
>
>> On 11/9/2017 6:34 PM, Kim Phillips wrote:
>>> On Thu, 9 Nov 2017 11:54:13 +0000
>>> Radu Andrei Alexe <[email protected]> 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
On Mon, 13 Nov 2017 08:32:24 +0000
Horia Geantă <[email protected]> wrote:
> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +0000
> > Radu Andrei Alexe <[email protected]> wrote:
> [snip]>> 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.
> >
> Please see previous discussion with Vinod:
> https://www.mail-archive.com/[email protected]/msg21468.html
Vinod says: "If the dma controller is internal to crypto, then it might
be okay to be inside the crypto driver."
Is that the case for the CCP driver? Isn't it the case here?
In any case, I don't care that much about that, this all begat from new
*devices* coming out of nowhere.
> > What is the rationale for using the crypto h/w as a dma engine anyway?
> SoCs that don't have a system DMA, for e.g. LS1012A.
OK.
Kim
On Mon, 13 Nov 2017 09:44:06 +0000
Radu Andrei Alexe <[email protected]> wrote:
> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +0000
> > Radu Andrei Alexe <[email protected]> wrote:
> >
> >> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> >>> On Thu, 9 Nov 2017 11:54:13 +0000
> >>> Radu Andrei Alexe <[email protected]> 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?
Don't see how that's relevant here, but people applying patches should
solicit acks from the appropriate sources, esp. if a patch is across
multiple subsystems.
> 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?
Why would they want to do that?
In any case, I suspect you're thinking about cross-subsystem Kconfig
entries, which is common, but something like that can be a module
parameter, too.
I would say that maybe CRYPTO_DEV_FSL_CAAM should be made to not depend
on CRYPTO_HW, but I think that's overkill for the addition of this
minor feature.
> > 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
OK, please mention that next time.
> use-case where we need to issue large transfers (hundred of MBs)
> asynchronously, without using the core.
Curious: what subsystem does that?
Thanks,
Kim
On Fri, Nov 10, 2017 at 08:02:01AM +0000, Radu Andrei Alexe wrote:
> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > On Thu, 9 Nov 2017 11:54:13 +0000
> > Radu Andrei Alexe <[email protected]> wrote:
> >
> >> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> >>> On Mon, 30 Oct 2017 15:46:51 +0200
> >>> Horia Geantă <[email protected]> wrote:
> >>>
> >>>> +=====================================================================
> >>>> +CAAM DMA Node
> >>>> +
> >>>> + Child node of the crypto node that enables the use of the DMA capabilities
> >>>> + of the CAAM by a stand-alone driver. The only required property is the
> >>>> + "compatible" property. All the other properties are determined from
> >>>> + the job rings on which the CAAM DMA driver depends (ex: the number of
> >>>> + dma-channels is equal to the number of defined job rings).
> >>>> +
> >>>> + - compatible
> >>>> + Usage: required
> >>>> + Value type: <string>
> >>>> + Definition: Must include "fsl,sec-v4.0-dma"
> >>>> +
> >>>> +EXAMPLE
> >>>> + caam-dma {
> >>>> + compatible = "fsl,sec-v5.4-dma",
> >>>> + "fsl,sec-v5.0-dma",
> >>>> + "fsl,sec-v4.0-dma";
> >>>> + }
> >>>
> >>> If this isn't describing an aspect of the hardware, then what is it
> >>> doing in the device tree?
> >>>
> >>> Kim
> >>>
> >>
> >> Because the caam_dma driver is a platform driver I needed to create a
> >> platform device to activate the driver. My thought was that it was
> >> simpler to implement it using device tree.
> >> 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?
> >
> > Kim
> >
>
>
> 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. Placing the code in the same directory as
> the caam subsystem would only create confusion for the reader of an
> already complex driver.
>
> 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.
These are very good reasons.
We already have one DMA implementation drivers/crypto/ccp/ccp-dmaengine.c
which was sneaked past us and put into kernel with proper review!
On the other hand we have *bunch* of dmaengine users in crypto subsystem
which use dmaengine drivers and APIs and are good citizens. It allows folks
to share code with other usages of dmaengine and the usual arguments for
common code and frameworks...
If there are enough users we can add up a crypto-dmaengine lib which
programs dma controller for crypto users and avoid open coding for everyone.
for example sound-dmaengine layers does that...
HTH
--
~Vinod
On Fri, Nov 10, 2017 at 10:44:30AM -0600, Kim Phillips wrote:
> On Fri, 10 Nov 2017 08:02:01 +0000
> Radu Andrei Alexe <[email protected]> wrote:
>
> > On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > > On Thu, 9 Nov 2017 11:54:13 +0000
> > > Radu Andrei Alexe <[email protected]> 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.
which is a shame as it was sneaked past the dmaengine community!!
This is the *only* example and there and other examples where people use
dmaengine:
$ grep -rl dmaengine_prep* drivers/crypto/* |uniq
drivers/crypto/atmel-aes.c
drivers/crypto/atmel-sha.c
drivers/crypto/atmel-tdes.c
drivers/crypto/img-hash.c
drivers/crypto/omap-aes.c
drivers/crypto/omap-des.c
drivers/crypto/omap-sham.c
drivers/crypto/qce/dma.c
drivers/crypto/stm32/stm32-hash.c
drivers/crypto/ux500/cryp/cryp_core.c
drivers/crypto/ux500/hash/hash_core.c
>
> I also don't see how that complicates things much further.
>
> What is the rationale for using the crypto h/w as a dma engine anyway?
> Are there supporting performance figures?
that is a very good question, perf does matter. Given that we have many
folks using it, I think it would help, but yes nothing better than numbers
speak for themselves.
--
~Vinod