crypto: Add Allwinner Security System crypto accelerator

crypto: Add Allwinner Security System crypto accelerator

[LABBE Corentin]

Hello

This is the driver for the Security System included in Allwinner SoC A20.
The Security System (SS for short) is a hardware cryptographic accelerator that support AES/MD5/SHA1/DES/3DES/PRNG algorithms.
It could be found on others Allwinner SoC: 
- A10s, A10 and A31 diagram speak about it
- A13 manual give the same datasheet for SS than A20
But I do not have access on any of those hardware, tests are welcome.

This driver currently supports:
- MD5 and SHA1 hash algorithms
- AES block cipher in CBC mode with 128/196/256bits keys.
- DES and 3DES block cipher in CBC mode
- PRNG
The driver exposes all those algorithms through the kernel cryptographic API.

The driver support only CPU driven (aka poll mode) transfer mode since the DMA engine of the A20 does not have a driver yet.


Since it is my first cryptographic driver, I have some questions with it:
- When setting .cra_priority to 300 (so more priority than krng), the PRNG seems not to be used (any of the dev_* message was never seen).
  But something use/lock it, because I cannot unload the module.
  So for the moment I have set its .cra_priority to 100 and need to investigate more.

- Some values of crypto_alg struct are not well documented, what exactly cra_alignmask involves ?

- Since SS could only process one request at a time, I have use shash and blkcipher (vs ahash/ablkcipher) does it is a good choice ?
  I think ahash/ablkcipher would be good if I add an interrupt driven driver, does I am right ?


Now speak about performance.
The performance are very good, up to x2 for hashing and 30/50% for AES:
(r/s means requests per second)
- MD5 without SS
10000 requests of 16 bytes in 813409us (0.813409s) 12293.938477r/s
10000 requests of 32 bytes in 824489us (0.824489s) 12128.724609r/s
10000 requests of 64 bytes in 834231us (0.834231s) 11987.087891r/s
10000 requests of 128 bytes in 851992us (0.851992s) 11737.199219r/s
10000 requests of 256 bytes in 866311us (0.866311s) 11543.198242r/s
10000 requests of 512 bytes in 909070us (0.909070s) 11000.252930r/s
10000 requests of 1024 bytes in 956643us (0.956643s) 10453.220703r/s
10000 requests of 2048 bytes in 1086405us (1.086405s) 9204.670898r/s
10000 requests of 4096 bytes in 1338719us (1.338719s) 7469.827637r/s
10000 requests of 8192 bytes in 1867788us (1.867788s) 5353.926758r/s
10000 requests of 16384 bytes in 2972250us (2.972250s) 3364.454590r/s
10000 requests of 32768 bytes in 4945905us (4.945905s) 2021.874634r/s
10000 requests of 65536 bytes in 8971740us (8.971740s) 1114.610962r/s
1000 requests of 131072 bytes in 1705265us (1.705265s) 586.419128r/s
1000 requests of 262144 bytes in 3340201us (3.340201s) 299.383179r/s
1000 requests of 524288 bytes in 6552412us (6.552412s) 152.615555r/s
1000 requests of 1048576 bytes in 13027299us (13.027299s) 76.761887r/s
1000 requests of 2097152 bytes in 25890814us (25.890814s) 38.623737r/s

- MD5 with SS
10000 requests of 16 bytes in 833828us (0.833828s) 11992.880859r/s
10000 requests of 32 bytes in 839807us (0.839807s) 11907.498047r/s
10000 requests of 64 bytes in 849816us (0.849816s) 11767.252930r/s
10000 requests of 128 bytes in 859020us (0.859020s) 11641.172852r/s
10000 requests of 256 bytes in 867209us (0.867209s) 11531.246094r/s
10000 requests of 512 bytes in 886111us (0.886111s) 11285.267578r/s
10000 requests of 1024 bytes in 934183us (0.934183s) 10704.541016r/s
10000 requests of 2048 bytes in 1008961us (1.008961s) 9911.185547r/s
10000 requests of 4096 bytes in 1171796us (1.171796s) 8533.908203r/s
10000 requests of 8192 bytes in 1518018us (1.518018s) 6587.537109r/s
10000 requests of 16384 bytes in 2665293us (2.665293s) 3751.932617r/s
10000 requests of 32768 bytes in 3580903us (3.580903s) 2792.591797r/s
10000 requests of 65536 bytes in 6260570us (6.260570s) 1597.298706r/s
1000 requests of 131072 bytes in 1164345us (1.164345s) 858.851990r/s
1000 requests of 262144 bytes in 2243004us (2.243004s) 445.830688r/s
1000 requests of 524288 bytes in 4404737us (4.404737s) 227.028305r/s
1000 requests of 1048576 bytes in 8720956us (8.720956s) 114.666328r/s
1000 requests of 2097152 bytes in 17383040us (17.383039s) 57.527336r/s

- SHA1 without SS
10000 requests of 16 bytes in 875042us (0.875042s) 11428.022461r/s
10000 requests of 32 bytes in 823671us (0.823671s) 12140.769531r/s
10000 requests of 64 bytes in 871748us (0.871748s) 11471.205078r/s
10000 requests of 128 bytes in 862865us (0.862865s) 11589.298828r/s
10000 requests of 256 bytes in 881417us (0.881417s) 11345.368164r/s
10000 requests of 512 bytes in 950922us (0.950922s) 10516.109375r/s
10000 requests of 1024 bytes in 1050256us (1.050256s) 9521.488281r/s
10000 requests of 2048 bytes in 1258961us (1.258961s) 7943.057617r/s
10000 requests of 4096 bytes in 1683097us (1.683097s) 5941.428223r/s
10000 requests of 8192 bytes in 2526498us (2.526498s) 3958.047852r/s
10000 requests of 16384 bytes in 4269592us (4.269592s) 2342.144287r/s
10000 requests of 32768 bytes in 7602177us (7.602177s) 1315.412720r/s
10000 requests of 65536 bytes in 14377450us (14.377450s) 695.533630r/s
1000 requests of 131072 bytes in 2765845us (2.765845s) 361.553162r/s
1000 requests of 262144 bytes in 5445735us (5.445735s) 183.629944r/s
1000 requests of 524288 bytes in 10789435us (10.789435s) 92.683258r/s
1000 requests of 1048576 bytes in 21505394us (21.505394s) 46.499962r/s
1000 requests of 2097152 bytes in 42825792us (42.825790s) 23.350414r/s

- SHA1 with SS
10000 requests of 16 bytes in 889208us (0.889208s) 11245.962891r/s
10000 requests of 32 bytes in 842736us (0.842736s) 11866.112305r/s
10000 requests of 64 bytes in 852888us (0.852888s) 11724.869141r/s
10000 requests of 128 bytes in 861924us (0.861924s) 11601.951172r/s
10000 requests of 256 bytes in 869454us (0.869454s) 11501.470703r/s
10000 requests of 512 bytes in 885505us (0.885505s) 11292.991211r/s
10000 requests of 1024 bytes in 936839us (0.936839s) 10674.192383r/s
10000 requests of 2048 bytes in 1012891us (1.012891s) 9872.730469r/s
10000 requests of 4096 bytes in 1185065us (1.185065s) 8438.355469r/s
10000 requests of 8192 bytes in 1521115us (1.521115s) 6574.125000r/s
10000 requests of 16384 bytes in 2234033us (2.234033s) 4476.209473r/s
10000 requests of 32768 bytes in 3571667us (3.571667s) 2799.812988r/s
10000 requests of 65536 bytes in 6266210us (6.266210s) 1595.860962r/s
1000 requests of 131072 bytes in 1164847us (1.164847s) 858.481873r/s
1000 requests of 262144 bytes in 2246109us (2.246109s) 445.214355r/s
1000 requests of 524288 bytes in 4454817us (4.454817s) 224.476105r/s
1000 requests of 1048576 bytes in 8807469us (8.807469s) 113.539993r/s
1000 requests of 2097152 bytes in 17339978us (17.339977s) 57.670200r/s

- AES 128 without SS
1000000 requests of 16 bytes in 12796270us (12.796270s) 78147.773438r/s
100000 requests of 32 bytes in 1354404us (1.354404s) 73833.210938r/s
100000 requests of 64 bytes in 1513001us (1.513001s) 66093.804688r/s
100000 requests of 128 bytes in 1827315us (1.827315s) 54725.101562r/s
10000 requests of 256 bytes in 243519us (0.243519s) 41064.558594r/s
10000 requests of 512 bytes in 364327us (0.364327s) 27447.869141r/s
10000 requests of 1024 bytes in 622588us (0.622588s) 16061.986328r/s
10000 requests of 2048 bytes in 1097997us (1.097997s) 9107.493164r/s
10000 requests of 4096 bytes in 2219479us (2.219479s) 4505.562012r/s
10000 requests of 8192 bytes in 4315617us (4.315617s) 2317.165771r/s
10000 requests of 16384 bytes in 8392507us (8.392507s) 1191.539062r/s
10000 requests of 32768 bytes in 16723522us (16.723522s) 597.960144r/s
10000 requests of 65536 bytes in 33682600us (33.682598s) 296.889191r/s
10000 requests of 131072 bytes in 66817472us (66.817474s) 149.661453r/s
5000 requests of 262144 bytes in 66956952us (66.956955s) 74.674843r/s
5000 requests of 524288 bytes in 130631872us (130.631866s) 38.275497r/s
1000 requests of 1048576 bytes in 52419776us (52.419777s) 19.076769r/s

- AES 128 with SS
1000000 requests of 16 bytes in 13789299us (13.789299s) 72520r/s
100000 requests of 32 bytes in 1408070us (1.408070s) 71019.195312r/s
100000 requests of 64 bytes in 1460338us (1.460338s) 68477.296875r/s
100000 requests of 128 bytes in 1665112us (1.665112s) 60056.019531r/s
10000 requests of 256 bytes in 209169us (0.209169s) 47808.230469r/s
10000 requests of 512 bytes in 288330us (0.288330s) 34682.480469r/s
10000 requests of 1024 bytes in 450088us (0.450088s) 22217.876953r/s
10000 requests of 2048 bytes in 779608us (0.779608s) 12826.958984r/s
10000 requests of 4096 bytes in 1459871us (1.459871s) 6849.920410r/s
10000 requests of 8192 bytes in 2795100us (2.795100s) 3577.689453r/s
10000 requests of 16384 bytes in 5465166us (5.465166s) 1829.770630r/s
10000 requests of 32768 bytes in 10775686us (10.775686s) 928.015137r/s
10000 requests of 65536 bytes in 21437566us (21.437567s) 466.470856r/s
10000 requests of 131072 bytes in 42685016us (42.685017s) 234.274246r/s
5000 requests of 262144 bytes in 42684292us (42.684292s) 117.139114r/s
5000 requests of 524288 bytes in 85359792us (85.359795s) 58.575588r/s
1000 requests of 1048576 bytes in 34153296us (34.153297s) 29.279751r/s

- AES 192 without SS
1000000 requests of 16 bytes in 20133432us (20.133432s) 49668.628906r/s
100000 requests of 32 bytes in 2094374us (2.094374s) 47746.964844r/s
100000 requests of 64 bytes in 2337254us (2.337254s) 42785.250000r/s
100000 requests of 128 bytes in 2633498us (2.633498s) 37972.308594r/s
10000 requests of 256 bytes in 331907us (0.331907s) 30128.921875r/s
10000 requests of 512 bytes in 469994us (0.469994s) 21276.867188r/s
10000 requests of 1024 bytes in 754699us (0.754699s) 13250.315430r/s
10000 requests of 2048 bytes in 1303181us (1.303181s) 7673.531250r/s
10000 requests of 4096 bytes in 2533219us (2.533219s) 3947.546631r/s
10000 requests of 8192 bytes in 4812353us (4.812353s) 2077.985596r/s
10000 requests of 16384 bytes in 9409004us (9.409004s) 1062.811768r/s
10000 requests of 32768 bytes in 18517712us (18.517712s) 540.023499r/s
10000 requests of 65536 bytes in 36765264us (36.765263s) 271.995880r/s
10000 requests of 131072 bytes in 74134720us (74.134720s) 134.889557r/s
5000 requests of 262144 bytes in 73354224us (73.354225s) 68.162399r/s
5000 requests of 524288 bytes in 146576640us (146.576645s) 34.111847r/s
1000 requests of 1048576 bytes in 59421648us (59.421646s) 16.828884r/s

- AES 192 with SS
1000000 requests of 16 bytes in 13977035us (13.977035s) 71545.929688r/s
100000 requests of 32 bytes in 1415502us (1.415502s) 70646.312500r/s
100000 requests of 64 bytes in 1520800us (1.520800s) 65754.867188r/s
100000 requests of 128 bytes in 1747678us (1.747678s) 57218.777344r/s
10000 requests of 256 bytes in 215039us (0.215039s) 46503.191406r/s
10000 requests of 512 bytes in 296430us (0.296430s) 33734.777344r/s
10000 requests of 1024 bytes in 461102us (0.461102s) 21687.175781r/s
10000 requests of 2048 bytes in 785948us (0.785948s) 12723.488281r/s
10000 requests of 4096 bytes in 1470694us (1.470694s) 6799.510742r/s
10000 requests of 8192 bytes in 2796490us (2.796490s) 3575.911133r/s
10000 requests of 16384 bytes in 5456299us (5.456299s) 1832.744141r/s
10000 requests of 32768 bytes in 11207501us (11.207501s) 892.259583r/s
10000 requests of 65536 bytes in 21422548us (21.422548s) 466.797882r/s
10000 requests of 131072 bytes in 42843424us (42.843426s) 233.408051r/s
5000 requests of 262144 bytes in 43383952us (43.383953s) 115.249992r/s
5000 requests of 524288 bytes in 86114304us (86.114304s) 58.062363r/s
1000 requests of 1048576 bytes in 34595356us (34.595356s) 28.905613r/s

- AES 256 without SS
1000000 requests of 16 bytes in 13021451us (13.021451s) 76796.359375r/s
100000 requests of 32 bytes in 1403426us (1.403426s) 71254.203125r/s
100000 requests of 64 bytes in 1613680us (1.613680s) 61970.152344r/s
100000 requests of 128 bytes in 2016162us (2.016162s) 49599.187500r/s
10000 requests of 256 bytes in 279614us (0.279614s) 35763.589844r/s
10000 requests of 512 bytes in 437451us (0.437451s) 22859.703125r/s
10000 requests of 1024 bytes in 749297us (0.749297s) 13345.842773r/s
10000 requests of 2048 bytes in 1382859us (1.382859s) 7231.395020r/s
10000 requests of 4096 bytes in 2718771us (2.718771s) 3678.132568r/s
10000 requests of 8192 bytes in 5290314us (5.290314s) 1890.246948r/s
10000 requests of 16384 bytes in 10419478us (10.419478s) 959.740967r/s
10000 requests of 32768 bytes in 20698064us (20.698065s) 483.136963r/s
10000 requests of 65536 bytes in 41186012us (41.186012s) 242.800888r/s
10000 requests of 131072 bytes in 82355632us (82.355629s) 121.424606r/s
5000 requests of 262144 bytes in 82577168us (82.577171s) 60.549423r/s
5000 requests of 524288 bytes in 164405808us (164.405807s) 30.412550r/s
1000 requests of 1048576 bytes in 66188452us (66.188454s) 15.108376r/s

- AES 256 with SS
1000000 requests of 16 bytes in 14093180us (14.093180s) 70956.304688r/s
100000 requests of 32 bytes in 1483984us (1.483984s) 67386.171875r/s
100000 requests of 64 bytes in 1588778us (1.588778s) 62941.453125r/s
100000 requests of 128 bytes in 1802914us (1.802914s) 55465.761719r/s
10000 requests of 256 bytes in 222742us (0.222742s) 44894.992188r/s
10000 requests of 512 bytes in 310578us (0.310578s) 32198.031250r/s
10000 requests of 1024 bytes in 480900us (0.480900s) 20794.343750r/s
10000 requests of 2048 bytes in 822032us (0.822032s) 12164.976562r/s
10000 requests of 4096 bytes in 1548041us (1.548041s) 6459.777344r/s
10000 requests of 8192 bytes in 2938495us (2.938495s) 3403.102539r/s
10000 requests of 16384 bytes in 5730902us (5.730902s) 1744.926025r/s
10000 requests of 32768 bytes in 11309177us (11.309177s) 884.237671r/s
10000 requests of 65536 bytes in 22495560us (22.495560s) 444.532166r/s
10000 requests of 131072 bytes in 45363720us (45.363720s) 220.440475r/s
5000 requests of 262144 bytes in 44884176us (44.884174s) 111.397835r/s
5000 requests of 524288 bytes in 89794592us (89.794594s) 55.682640r/s
1000 requests of 1048576 bytes in 35895056us (35.895058s) 27.858990r/s

Bests regards

[PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[LABBE Corentin]

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt

diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
new file mode 100644
index 0000000..356563b
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
@@ -0,0 +1,9 @@
+* Allwinner Security System found on A20 SoC
+
+Required properties:
+- compatible : Should be "allwinner,sun7i-a20-crypto".
+- reg: Should contain the SS register location and length.
+- interrupts: Should contain the IRQ line for the SS.
+- clocks : A phandle to the functional clock node of the SS module
+- clock-names : Name of the functional clock, should be "ahb" and "mod".
+
-- 
1.8.5.5

[PATCH 2/3] ARM: sun7i: dt: Add Security System to A20 SoC DTS

[LABBE Corentin]

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index f4b00a5..ea56552 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -523,6 +523,14 @@
 			status = "disabled";
 		};
 
+		crypto: crypto-engine@01c15000 {
+			compatible = "allwinner,sun7i-a20-crypto";
+			reg = <0x01c15000 0x1000>;
+			interrupts = <0 86 4>;
+			clocks = <&ahb_gates 5>, <&ss_clk>;
+			clock-names = "ahb", "mod";
+		};
+
 		spi2: spi@01c17000 {
 			compatible = "allwinner,sun4i-a10-spi";
 			reg = <0x01c17000 0x1000>;
-- 
1.8.5.5

[PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[LABBE Corentin]

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
---
 drivers/crypto/Kconfig    |   49 ++
 drivers/crypto/Makefile   |    1 +
 drivers/crypto/sunxi-ss.c | 1476 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1526 insertions(+)
 create mode 100644 drivers/crypto/sunxi-ss.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 03ccdb0..5ea0922 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
 	  To compile this driver as a module, choose M here: the module
 	  will be called mxs-dcp.
 
+config CRYPTO_DEV_SUNXI_SS
+        tristate "Support for Allwinner Security System cryptographic accelerator"
+        depends on ARCH_SUNXI
+        help
+          Some Allwinner processors have a crypto accelerator named
+          Security System. Select this if you want to use it.
+
+          To compile this driver as a module, choose M here: the module
+          will be called sunxi-ss.
+
+if CRYPTO_DEV_SUNXI_SS
+config CRYPTO_DEV_SUNXI_SS_PRNG
+	bool "Security System PRNG"
+	select CRYPTO_RNG2
+	help
+	  If you enable this option, the SS will provide a pseudo random
+	  number generator.
+config CRYPTO_DEV_SUNXI_SS_MD5
+	bool "Security System MD5"
+	select CRYPTO_MD5
+	help
+	  If you enable this option, the SS will provide MD5 hardware
+	  acceleration.
+config CRYPTO_DEV_SUNXI_SS_SHA1
+	bool "Security System SHA1"
+	select CRYPTO_SHA1
+	help
+	  If you enable this option, the SS will provide SHA1 hardware
+	  acceleration.
+config CRYPTO_DEV_SUNXI_SS_AES
+	bool "Security System AES"
+	select CRYPTO_AES
+	help
+	  If you enable this option, the SS will provide AES hardware
+	  acceleration.
+config CRYPTO_DEV_SUNXI_SS_DES
+	bool "Security System DES"
+	select CRYPTO_DES
+	help
+	  If you enable this option, the SS will provide DES hardware
+	  acceleration.
+config CRYPTO_DEV_SUNXI_SS_3DES
+	bool "Security System 3DES"
+	select CRYPTO_DES
+	help
+	  If you enable this option, the SS will provide 3DES hardware
+	  acceleration.
+endif #CRYPTO_DEV_SUNXI_SS
+
 endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 482f090..490dae5 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_CRYPTO_DEV_S5P) += s5p-sss.o
 obj-$(CONFIG_CRYPTO_DEV_SAHARA) += sahara.o
 obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
 obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
+obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss.o
diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
new file mode 100644
index 0000000..bbf57bc
--- /dev/null
+++ b/drivers/crypto/sunxi-ss.c
@@ -0,0 +1,1476 @@
+/*
+ * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC
+ *
+ * Copyright (C) 2013-2014 Corentin LABBE <clabbe.montjoie@gmail.com>
+ *
+ * Support AES cipher with 128,192,256 bits keysize.
+ * Support MD5 and SHA1 hash algorithms.
+ * Support DES and 3DES
+ * Support PRNG
+ *
+ * You could find the datasheet at
+ * http://dl.linux-sunxi.org/A20/A20%20User%20Manual%202013-03-22.pdf
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation version 2 of the License
+ */
+
+#include <linux/clk.h>
+#include <linux/crypto.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <crypto/scatterwalk.h>
+#include <linux/scatterlist.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+#include <crypto/md5.h>
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+#include <crypto/sha.h>
+#define SUNXI_SS_HASH_COMMON
+#endif
+#ifdef SUNXI_SS_HASH_COMMON
+#include <crypto/hash.h>
+#include <crypto/internal/hash.h>
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+#include <crypto/aes.h>
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+#define SUNXI_SS_DES
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+#define SUNXI_SS_DES
+#endif
+#ifdef SUNXI_SS_DES
+#include <crypto/des.h>
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+#include <crypto/internal/rng.h>
+
+struct prng_context {
+	u8 seed[192/8];
+	unsigned int slen;
+};
+#endif
+
+#define SUNXI_SS_CTL            0x00
+#define SUNXI_SS_KEY0           0x04
+#define SUNXI_SS_KEY1           0x08
+#define SUNXI_SS_KEY2           0x0C
+#define SUNXI_SS_KEY3           0x10
+#define SUNXI_SS_KEY4           0x14
+#define SUNXI_SS_KEY5           0x18
+#define SUNXI_SS_KEY6           0x1C
+#define SUNXI_SS_KEY7           0x20
+
+#define SUNXI_SS_IV0            0x24
+#define SUNXI_SS_IV1            0x28
+#define SUNXI_SS_IV2            0x2C
+#define SUNXI_SS_IV3            0x30
+
+#define SUNXI_SS_CNT0           0x34
+#define SUNXI_SS_CNT1           0x38
+#define SUNXI_SS_CNT2           0x3C
+#define SUNXI_SS_CNT3           0x40
+
+#define SUNXI_SS_FCSR           0x44
+#define SUNXI_SS_ICSR           0x48
+
+#define SUNXI_SS_MD0            0x4C
+#define SUNXI_SS_MD1            0x50
+#define SUNXI_SS_MD2            0x54
+#define SUNXI_SS_MD3            0x58
+#define SUNXI_SS_MD4            0x5C
+
+#define SS_RXFIFO         0x200
+#define SS_TXFIFO         0x204
+
+/* SUNXI_SS_CTL configuration values */
+
+/* AES/DES/3DES key select - bits 24-27 */
+#define SUNXI_SS_KEYSELECT_KEYN		(0 << 24)
+
+/* PRNG generator mode - bit 15 */
+#define SUNXI_PRNG_ONESHOT              (0 << 15)
+#define SUNXI_PRNG_CONTINUE             (1 << 15)
+
+/* IV Steady of SHA-1/MD5 constants - bit 14 */
+#define SUNXI_SS_IV_CONSTANTS              (0 << 14)
+#define SUNXI_IV_ARBITRARY              (1 << 14)
+
+/* SS operation mode - bits 12-13 */
+#define SUNXI_SS_ECB                    (0 << 12)
+#define SUNXI_SS_CBC                    (1 << 12)
+#define SUNXI_SS_CNT                    (2 << 12)
+
+/* Counter width for CNT mode - bits 10-11 */
+#define SUNXI_CNT_16BITS                (0 << 10)
+#define SUNXI_CNT_32BITS                (1 << 10)
+#define SUNXI_CNT_64BITS                (2 << 10)
+
+/* Key size for AES - bits 8-9 */
+#define SUNXI_AES_128BITS               (0 << 8)
+#define SUNXI_AES_192BITS               (1 << 8)
+#define SUNXI_AES_256BITS               (2 << 8)
+
+/* Operation direction - bit 7 */
+#define SUNXI_SS_ENCRYPTION             (0 << 7)
+#define SUNXI_SS_DECRYPTION             (1 << 7)
+
+/* SS Method - bits 4-6 */
+#define SUNXI_OP_AES                    (0 << 4)
+#define SUNXI_OP_DES                    (1 << 4)
+#define SUNXI_OP_3DES                   (2 << 4)
+#define SUNXI_OP_SHA1                   (3 << 4)
+#define SUNXI_OP_MD5                    (4 << 4)
+#define SUNXI_OP_PRNG                   (5 << 4)
+
+/* Data end bit - bit 2 */
+#define SUNXI_SS_DATA_END               BIT(2)
+
+/* PRNG start bit - bit 1 */
+#define SUNXI_PRNG_START                BIT(1)
+
+/* SS Enable bit - bit 0 */
+#define SUNXI_SS_DISABLED               (0 << 0)
+#define SUNXI_SS_ENABLED                (1 << 0)
+
+/* RX FIFO status - bit 30 */
+#define SS_RXFIFO_FREE               BIT(30)
+
+/* RX FIFO empty spaces - bits 24-29 */
+#define SS_RXFIFO_SPACES(val)        (((val) >> 24) & 0x3f)
+
+/* TX FIFO status - bit 22 */
+#define SS_TXFIFO_AVAILABLE          BIT(22)
+
+/* TX FIFO available spaces - bits 16-21 */
+#define SS_TXFIFO_SPACES(val)        (((val) >> 16) & 0x3f)
+
+#define SUNXI_RXFIFO_EMP_INT_PENDING    BIT(10)
+#define SUNXI_TXFIFO_AVA_INT_PENDING    BIT(8)
+#define SUNXI_RXFIFO_EMP_INT_ENABLE     BIT(2)
+#define SUNXI_TXFIFO_AVA_INT_ENABLE     BIT(0)
+
+#define SUNXI_SS_ICS_DRQ_ENABLE         BIT(4)
+
+/* General notes:
+ * I cannot use a key/IV cache because each time one of these change ALL stuff
+ * need to be re-writed.
+ * And for example, with dm-crypt IV changes on each request.
+ *
+ * After each request the device must be disabled.
+ *
+ * For performance reason, we use writel_relaxed/read_relaxed for all
+ * operations on RX and TX FIFO.
+ * For all other registers, we use writel.
+ * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
+ * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
+ * */
+
+static struct sunxi_ss_ctx {
+	void *base;
+	int irq;
+	struct clk *busclk;
+	struct clk *ssclk;
+	struct device *dev;
+	struct resource *res;
+	void *buf_in; /* pointer to data to be uploaded to the device */
+	size_t buf_in_size; /* size of buf_in */
+	void *buf_out;
+	size_t buf_out_size;
+} _ss_ctx, *ss_ctx = &_ss_ctx;
+
+static DEFINE_MUTEX(lock);
+static DEFINE_MUTEX(bufout_lock);
+static DEFINE_MUTEX(bufin_lock);
+
+struct sunxi_req_ctx {
+	u8 key[AES_MAX_KEY_SIZE * 8];
+	u32 keylen;
+	u32 mode;
+	u64 byte_count; /* number of bytes "uploaded" to the device */
+	u32 waitbuf; /* a partial word waiting to be completed and
+			uploaded to the device */
+	/* number of bytes to be uploaded in the waitbuf word */
+	unsigned int nbwait;
+};
+
+#ifdef SUNXI_SS_HASH_COMMON
+/*============================================================================*/
+/*============================================================================*/
+/* sunxi_hash_init: initialize request context
+ * Activate the SS, and configure it for MD5 or SHA1
+ */
+static int sunxi_shash_init(struct shash_desc *desc)
+{
+	const char *hash_type;
+	struct sunxi_req_ctx *op = crypto_shash_ctx(desc->tfm);
+	u32 tmp = SUNXI_SS_ENABLED | SUNXI_SS_IV_CONSTANTS;
+
+	mutex_lock(&lock);
+
+	hash_type = crypto_tfm_alg_name(crypto_shash_tfm(desc->tfm));
+
+	op->byte_count = 0;
+	op->nbwait = 0;
+	op->waitbuf = 0;
+
+	/* Enable and configure SS for MD5 or SHA1 */
+	if (strcmp(hash_type, "sha1") == 0) {
+		tmp |= SUNXI_OP_SHA1;
+		op->mode = SUNXI_OP_SHA1;
+	} else {
+		tmp |= SUNXI_OP_MD5;
+		op->mode = SUNXI_OP_MD5;
+	}
+
+	writel(tmp, ss_ctx->base + SUNXI_SS_CTL);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/*
+ * sunxi_hash_update: update hash engine
+ *
+ * Could be used for both SHA1 and MD5
+ * Write data by step of 32bits and put then in the SS.
+ * The remaining data is stored (nbwait bytes) in op->waitbuf
+ * As an optimisation, we do not check RXFIFO_SPACES, since SS handle
+ * the FIFO faster than our writes
+ */
+static int sunxi_shash_update(struct shash_desc *desc,
+		const u8 *data, unsigned int length)
+{
+	u32 v;
+	unsigned int i = 0;
+	struct sunxi_req_ctx *op = crypto_shash_ctx(desc->tfm);
+
+	u8 *waitbuf = (u8 *)(&op->waitbuf);
+
+	if (length == 0)
+		return 0;
+
+	if (op->nbwait > 0) {
+		for (; op->nbwait < 4 && i < length; op->nbwait++) {
+			waitbuf[op->nbwait] = *(data + i);
+			i++;
+		}
+		if (op->nbwait == 4) {
+			writel(op->waitbuf, ss_ctx->base + SS_RXFIFO);
+			op->byte_count += 4;
+			op->nbwait = 0;
+			op->waitbuf = 0;
+		}
+	}
+	/* TODO bench this optim */
+	if (i == 0 && ((length - i) % 4) == 0) {
+		u32 *src32 = (u32 *)(data + i);
+		i = (length - i) / 4;
+		while (i > 0) {
+			writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
+			i--;
+		}
+		op->byte_count += length;
+		return 0;
+	}
+	while (length - i >= 4) {
+		v = *(u32 *)(data + i);
+		writel_relaxed(v, ss_ctx->base + SS_RXFIFO);
+		i += 4;
+		op->byte_count += 4;
+	}
+	/* if we have less than 4 bytes, copy them in waitbuf */
+	if (i < length && length - i < 4) {
+		do {
+			waitbuf[op->nbwait] = *(data + i + op->nbwait);
+			op->nbwait++;
+		} while (i + op->nbwait < length);
+	}
+
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/*
+ * sunxi_hash_final: finalize hashing operation
+ *
+ * If we have some remaining bytes, send it.
+ * Then ask the SS for finalizing the hash
+ */
+static int sunxi_shash_final(struct shash_desc *desc, u8 *out)
+{
+	u32 v;
+	unsigned int i;
+	int zeros;
+	unsigned int index, padlen;
+	__be64 bits;
+	struct sunxi_req_ctx *op = crypto_shash_ctx(desc->tfm);
+
+	if (op->nbwait > 0) {
+		op->waitbuf |= ((1 << 7) << (op->nbwait * 8));
+		writel(op->waitbuf, ss_ctx->base + SS_RXFIFO);
+	} else {
+		writel((1 << 7), ss_ctx->base + SS_RXFIFO);
+	}
+
+	/* number of space to pad to obtain 64o minus 8(size) minus 4 (final 1)
+	 * example len=0
+	 * example len=56
+	 * */
+
+	/* we have already send 4 more byte of which nbwait data */
+	if (op->mode == SUNXI_OP_MD5) {
+		index = (op->byte_count + 4) & 0x3f;
+		op->byte_count += op->nbwait;
+		if (index > 56)
+			zeros = (120 - index) / 4;
+		else
+			zeros = (56 - index) / 4;
+	} else {
+		op->byte_count += op->nbwait;
+		index = op->byte_count & 0x3f;
+		padlen = (index < 56) ? (56 - index) : ((64+56) - index);
+		zeros = (padlen - 1) / 4;
+	}
+#ifdef DEBUG
+	/* This should not happen, TODO set a unlikely() ? */
+	if (zeros > 64 || zeros < 0) {
+		dev_err(ss_ctx->dev, "ERROR: too many zeros len=%llu\n",
+			op->byte_count);
+		zeros = 0;
+	}
+#endif
+	for (i = 0; i < zeros; i++)
+		writel(0, ss_ctx->base + SS_RXFIFO);
+
+	/* write the lenght */
+	if (op->mode == SUNXI_OP_SHA1) {
+		bits = cpu_to_be64(op->byte_count << 3);
+		writel(bits & 0xffffffff, ss_ctx->base + SS_RXFIFO);
+		writel((bits >> 32) & 0xffffffff,
+		       ss_ctx->base + SS_RXFIFO);
+	} else {
+		writel((op->byte_count << 3) & 0xffffffff,
+				ss_ctx->base + SS_RXFIFO);
+		writel((op->byte_count >> 29) & 0xffffffff,
+				ss_ctx->base + SS_RXFIFO);
+	}
+
+	/* stop the hashing */
+	v = readl(ss_ctx->base + SUNXI_SS_CTL);
+	v |= SUNXI_SS_DATA_END;
+	writel(v, ss_ctx->base + SUNXI_SS_CTL);
+
+	/* check the end */
+	/* The timeout could happend only in case of bad overcloking */
+#define SUNXI_SS_TIMEOUT 100
+	i = 0;
+	do {
+		v = readl(ss_ctx->base + SUNXI_SS_CTL);
+		i++;
+	} while (i < SUNXI_SS_TIMEOUT && (v & SUNXI_SS_DATA_END) > 0);
+	if (i >= SUNXI_SS_TIMEOUT) {
+		dev_err(ss_ctx->dev, "ERROR: hash end timeout %d>%d\n",
+				i, SUNXI_SS_TIMEOUT);
+		writel(0, ss_ctx->base + SUNXI_SS_CTL);
+		mutex_unlock(&lock);
+		return -1;
+	}
+
+	if (op->mode == SUNXI_OP_SHA1) {
+		for (i = 0; i < 5; i++) {
+			v = cpu_to_be32(readl(ss_ctx->base +
+						SUNXI_SS_MD0 + i * 4));
+			memcpy(out + i * 4, &v, 4);
+		}
+	} else {
+		for (i = 0; i < 4; i++) {
+			v = readl(ss_ctx->base + SUNXI_SS_MD0 + i * 4);
+			memcpy(out + i * 4, &v, 4);
+		}
+	}
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+	mutex_unlock(&lock);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/* sunxi_hash_finup: finalize hashing operation after an update */
+static int sunxi_shash_finup(struct shash_desc *desc, const u8 *in,
+		unsigned int count, u8 *out)
+{
+	int err;
+
+	err = sunxi_shash_update(desc, in, count);
+	if (err != 0)
+		return err;
+
+	return sunxi_shash_final(desc, out);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/* combo of init/update/final functions */
+static int sunxi_shash_digest(struct shash_desc *desc, const u8 *in,
+		unsigned int count, u8 *out)
+{
+	int err;
+
+	err = sunxi_shash_init(desc);
+	if (err != 0)
+		return err;
+
+	err = sunxi_shash_update(desc, in, count);
+	if (err != 0)
+		return err;
+
+	return sunxi_shash_final(desc, out);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static struct shash_alg sunxi_md5_alg = {
+	.init = sunxi_shash_init,
+	.update = sunxi_shash_update,
+	.final = sunxi_shash_final,
+	.finup = sunxi_shash_finup,
+	.digest = sunxi_shash_digest,
+	.digestsize = MD5_DIGEST_SIZE,
+	.base = {
+		.cra_name = "md5",
+		.cra_driver_name = "md5-sunxi-ss",
+		.cra_priority = 300,
+		.cra_alignmask = 3,
+		.cra_flags = CRYPTO_ALG_TYPE_SHASH,
+		.cra_blocksize = MD5_HMAC_BLOCK_SIZE,
+		.cra_ctxsize = sizeof(struct sunxi_req_ctx),
+		.cra_module = THIS_MODULE,
+	}
+};
+
+static struct shash_alg sunxi_sha1_alg = {
+	.init = sunxi_shash_init,
+	.update = sunxi_shash_update,
+	.final = sunxi_shash_final,
+	.finup = sunxi_shash_finup,
+	.digest = sunxi_shash_digest,
+	.digestsize = SHA1_DIGEST_SIZE,
+	.base = {
+		.cra_name = "sha1",
+		.cra_driver_name = "sha1-sunxi-ss",
+		.cra_priority = 300,
+		.cra_alignmask = 3,
+		.cra_flags = CRYPTO_ALG_TYPE_SHASH,
+		.cra_ctxsize = sizeof(struct sunxi_req_ctx),
+		.cra_module = THIS_MODULE,
+	}
+};
+#endif /* ifdef SUNXI_SS_HASH_COMMON */
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_aes_poll(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		const unsigned int nbytes, const u32 flag)
+{
+	u32 tmp;
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	u32 rx_cnt = 32; /* when activating SS, the default FIFO space is 32 */
+	u32 tx_cnt = 0;
+	u32 v;
+	int i;
+	struct scatterlist *in_sg;
+	struct scatterlist *out_sg;
+	void *src_addr;
+	void *dst_addr;
+	unsigned int ileft = nbytes;
+	unsigned int oleft = nbytes;
+	unsigned int sgileft = src->length;
+	unsigned int sgoleft = dst->length;
+	unsigned int todo;
+	u32 *src32;
+	u32 *dst32;
+
+	tmp = flag;
+	tmp |= SUNXI_SS_KEYSELECT_KEYN;
+	tmp |= SUNXI_SS_ENABLED;
+
+	in_sg = src;
+	out_sg = dst;
+	if (src == NULL || dst == NULL) {
+		dev_err(ss_ctx->dev, "ERROR: Some SGs are NULL\n");
+		return -1;
+	}
+	mutex_lock(&lock);
+	if (desc->info != NULL) {
+		for (i = 0; i < op->keylen; i += 4) {
+			v = *(u32 *)(op->key + i);
+			writel(v, ss_ctx->base + SUNXI_SS_KEY0 + i);
+		}
+		for (i = 0; i < 4; i++) {
+			v = *(u32 *)(desc->info + i * 4);
+			writel(v, ss_ctx->base + SUNXI_SS_IV0 + i * 4);
+		}
+	}
+	writel(tmp, ss_ctx->base + SUNXI_SS_CTL);
+
+	/* If we have only one SG, we can use kmap_atomic */
+	if (sg_next(in_sg) == NULL && sg_next(out_sg) == NULL) {
+		src_addr = kmap_atomic(sg_page(in_sg)) + in_sg->offset;
+		if (src_addr == NULL) {
+			dev_err(ss_ctx->dev, "kmap_atomic error for src SG\n");
+			writel(0, ss_ctx->base + SUNXI_SS_CTL);
+			mutex_unlock(&lock);
+			return -1;
+		}
+		dst_addr = kmap_atomic(sg_page(out_sg)) + out_sg->offset;
+		if (dst_addr == NULL) {
+			dev_err(ss_ctx->dev, "kmap_atomic error for dst SG\n");
+			writel(0, ss_ctx->base + SUNXI_SS_CTL);
+			mutex_unlock(&lock);
+			kunmap_atomic(src_addr);
+			return -1;
+		}
+		src32 = (u32 *)src_addr;
+		dst32 = (u32 *)dst_addr;
+		ileft = nbytes / 4;
+		oleft = nbytes / 4;
+		do {
+			if (ileft > 0 && rx_cnt > 0) {
+				todo = min(rx_cnt, ileft);
+				ileft -= todo;
+				do {
+					writel_relaxed(*src32++,
+						       ss_ctx->base +
+						       SS_RXFIFO);
+					todo--;
+				} while (todo > 0);
+			}
+			if (tx_cnt > 0) {
+				todo = min(tx_cnt, oleft);
+				oleft -= todo;
+				do {
+					*dst32++ = readl_relaxed(ss_ctx->base +
+								SS_TXFIFO);
+					todo--;
+				} while (todo > 0);
+			}
+			tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
+			rx_cnt = SS_RXFIFO_SPACES(tmp);
+			tx_cnt = SS_TXFIFO_SPACES(tmp);
+		} while (oleft > 0);
+		writel(0, ss_ctx->base + SUNXI_SS_CTL);
+		mutex_unlock(&lock);
+		kunmap_atomic(src_addr);
+		kunmap_atomic(dst_addr);
+		return 0;
+	}
+
+	/* If we have more than one SG, we cannot use kmap_atomic since
+	 * we hold the mapping too long*/
+	src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
+	if (src_addr == NULL) {
+		dev_err(ss_ctx->dev, "KMAP error for src SG\n");
+		return -1;
+	}
+	dst_addr = kmap(sg_page(out_sg)) + out_sg->offset;
+	if (dst_addr == NULL) {
+		kunmap(src_addr);
+		dev_err(ss_ctx->dev, "KMAP error for dst SG\n");
+		return -1;
+	}
+	src32 = (u32 *)src_addr;
+	dst32 = (u32 *)dst_addr;
+	ileft = nbytes / 4;
+	oleft = nbytes / 4;
+	sgileft = in_sg->length / 4;
+	sgoleft = out_sg->length / 4;
+	do {
+		tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
+		rx_cnt = SS_RXFIFO_SPACES(tmp);
+		tx_cnt = SS_TXFIFO_SPACES(tmp);
+		todo = min3(rx_cnt, ileft, sgileft);
+		if (todo > 0) {
+			ileft -= todo;
+			sgileft -= todo;
+		}
+		while (todo > 0) {
+			writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
+			todo--;
+		}
+		if (in_sg != NULL && sgileft == 0) {
+			kunmap(sg_page(in_sg));
+			in_sg = sg_next(in_sg);
+			if (in_sg != NULL && ileft > 0) {
+				src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
+				if (src_addr == NULL) {
+					dev_err(ss_ctx->dev, "KMAP error for src SG\n");
+					return -1;
+				}
+				src32 = src_addr;
+				sgileft = in_sg->length / 4;
+			}
+		}
+		/* do not test oleft since when oleft == 0 we have finished */
+		todo = min3(tx_cnt, oleft, sgoleft);
+		if (todo > 0) {
+			oleft -= todo;
+			sgoleft -= todo;
+		}
+		while (todo > 0) {
+			*dst32++ = readl_relaxed(ss_ctx->base + SS_TXFIFO);
+			todo--;
+		}
+		if (out_sg != NULL && sgoleft == 0) {
+			kunmap(sg_page(out_sg));
+			out_sg = sg_next(out_sg);
+			if (out_sg != NULL) {
+				dst_addr = kmap(sg_page(out_sg)) +
+					out_sg->offset;
+				if (dst_addr == NULL) {
+					dev_err(ss_ctx->dev, "KMAP error\n");
+					return -1;
+				}
+				dst32 = dst_addr;
+				sgoleft = out_sg->length / 4;
+			}
+		}
+	} while (oleft > 0);
+
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+	mutex_unlock(&lock);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_aes_cbc_encrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (unlikely(ivsize < 4)) {
+		dev_err(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_err(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_ENCRYPTION;
+	op->mode |= SUNXI_OP_AES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_aes_poll(desc, dst, src, nbytes, op->mode);
+}
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_aes_cbc_decrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (unlikely(ivsize < 4)) {
+		dev_err(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_err(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_DECRYPTION;
+	op->mode |= SUNXI_OP_AES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_aes_poll(desc, dst, src, nbytes, op->mode);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_aes_init(struct crypto_tfm *tfm)
+{
+	struct sunxi_req_ctx *op = crypto_tfm_ctx(tfm);
+	memset(op, 0, sizeof(struct sunxi_req_ctx));
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static void sunxi_aes_exit(struct crypto_tfm *tfm)
+{
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/* check and set the AES key, prepare the mode to be used */
+static int sunxi_aes_setkey(struct crypto_tfm *tfm, const u8 *key,
+		unsigned int keylen)
+{
+	struct sunxi_req_ctx *op = crypto_tfm_ctx(tfm);
+	switch (keylen) {
+	case 128 / 8:
+		op->mode = SUNXI_AES_128BITS;
+		break;
+	case 192 / 8:
+		op->mode = SUNXI_AES_192BITS;
+		break;
+	case 256 / 8:
+		op->mode = SUNXI_AES_256BITS;
+		break;
+	default:
+		dev_err(ss_ctx->dev, "Invalid keylen %u\n", keylen);
+		crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	op->keylen = keylen;
+	memcpy(op->key, key, keylen);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static struct crypto_alg sunxi_aes_alg = {
+	.cra_name = "cbc(aes)",
+	.cra_driver_name = "cbc-aes-sunxi-ss",
+	.cra_priority = 300,
+	.cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
+	.cra_blocksize   = AES_BLOCK_SIZE,
+	.cra_ctxsize = sizeof(struct sunxi_req_ctx),
+	.cra_module = THIS_MODULE,
+	.cra_alignmask = 3,
+	.cra_type = &crypto_blkcipher_type,
+	.cra_init = sunxi_aes_init,
+	.cra_exit = sunxi_aes_exit,
+	.cra_u = {
+		.blkcipher = {
+			.min_keysize    = AES_MIN_KEY_SIZE,
+			.max_keysize    = AES_MAX_KEY_SIZE,
+			.ivsize         = AES_BLOCK_SIZE,
+			.setkey         = sunxi_aes_setkey,
+			.encrypt        = sunxi_aes_cbc_encrypt,
+			.decrypt        = sunxi_aes_cbc_decrypt,
+		}
+	}
+};
+
+#endif /* CONFIG_CRYPTO_DEV_SUNXI_SS_AES */
+
+#ifdef SUNXI_SS_DES
+/*============================================================================*/
+/*============================================================================*/
+/* common for DES/3DES */
+static int sunxi_des_init(struct crypto_tfm *tfm)
+{
+	struct sunxi_req_ctx *op = crypto_tfm_ctx(tfm);
+	memset(op, 0, sizeof(struct sunxi_req_ctx));
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+/* common for DES/3DES */
+static void sunxi_des_exit(struct crypto_tfm *tfm)
+{
+}
+/*============================================================================*/
+/*============================================================================*/
+/* Pure CPU way of doing DES/3DES with SS
+ * Since DES and 3DES SGs could be smaller than 4 bytes, I use sg_copy_to_buffer
+ * for "linearize" them.
+ * The only problem with that is that I alloc (2 x nbytes) for buf_in/buf_out
+ * TODO change this system
+ * SGsrc -> buf_in -> SS -> buf_out -> SGdst */
+static int sunxi_des_poll(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		const unsigned int nbytes, const u32 flag)
+{
+	u32 tmp, value;
+	size_t nb_in_sg_tx, nb_in_sg_rx;
+	size_t ir, it;
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	u32 tx_cnt = 0;
+	u32 rx_cnt = 0;
+	u32 v;
+	int i;
+	int no_chunk = 1;
+
+	/* if we have only SGs with size multiple of 4,
+	 * we can use the SS AES function */
+	struct scatterlist *in_sg;
+	struct scatterlist *out_sg;
+	in_sg = src;
+	out_sg = dst;
+
+	while (in_sg != NULL && no_chunk == 1) {
+		if ((in_sg->length % 4) != 0)
+			no_chunk = 0;
+		in_sg = sg_next(in_sg);
+	}
+	while (out_sg != NULL && no_chunk == 1) {
+		if ((out_sg->length % 4) != 0)
+			no_chunk = 0;
+		out_sg = sg_next(out_sg);
+	}
+
+	if (no_chunk == 1)
+		return sunxi_aes_poll(desc, dst, src, nbytes, flag);
+
+	tmp = flag;
+	tmp |= SUNXI_SS_KEYSELECT_KEYN;
+	tmp |= SUNXI_SS_ENABLED;
+
+	nb_in_sg_rx = sg_nents(src);
+	nb_in_sg_tx = sg_nents(dst);
+
+	mutex_lock(&bufin_lock);
+	if (ss_ctx->buf_in == NULL) {
+		ss_ctx->buf_in = kmalloc(nbytes, GFP_KERNEL);
+		ss_ctx->buf_in_size = nbytes;
+	} else {
+		if (nbytes > ss_ctx->buf_in_size) {
+			kfree(ss_ctx->buf_in);
+			ss_ctx->buf_in = kmalloc(nbytes, GFP_KERNEL);
+			ss_ctx->buf_in_size = nbytes;
+		}
+	}
+	if (ss_ctx->buf_in == NULL) {
+		ss_ctx->buf_in_size = 0;
+		mutex_unlock(&bufin_lock);
+		dev_err(ss_ctx->dev, "Unable to allocate pages.\n");
+		return -ENOMEM;
+	}
+	if (ss_ctx->buf_out == NULL) {
+		mutex_lock(&bufout_lock);
+		ss_ctx->buf_out = kmalloc(nbytes, GFP_KERNEL);
+		if (ss_ctx->buf_out == NULL) {
+			ss_ctx->buf_out_size = 0;
+			mutex_unlock(&bufout_lock);
+			dev_err(ss_ctx->dev, "Unable to allocate pages.\n");
+			return -ENOMEM;
+		}
+		ss_ctx->buf_out_size = nbytes;
+		mutex_unlock(&bufout_lock);
+	} else {
+		if (nbytes > ss_ctx->buf_out_size) {
+			mutex_lock(&bufout_lock);
+			kfree(ss_ctx->buf_out);
+			ss_ctx->buf_out = kmalloc(nbytes, GFP_KERNEL);
+			if (ss_ctx->buf_out == NULL) {
+				ss_ctx->buf_out_size = 0;
+				mutex_unlock(&bufout_lock);
+				dev_err(ss_ctx->dev, "Unable to allocate pages.\n");
+				return -ENOMEM;
+			}
+			ss_ctx->buf_out_size = nbytes;
+			mutex_unlock(&bufout_lock);
+		}
+	}
+
+	sg_copy_to_buffer(src, nb_in_sg_rx, ss_ctx->buf_in, nbytes);
+
+	ir = 0;
+	it = 0;
+	mutex_lock(&lock);
+	if (desc->info != NULL) {
+		for (i = 0; i < op->keylen; i += 4) {
+			v = *(u32 *)(op->key + i);
+			writel(v, ss_ctx->base + SUNXI_SS_KEY0 + i);
+		}
+		for (i = 0; i < 4; i++) {
+			v = *(u32 *)(desc->info + i * 4);
+			writel(v, ss_ctx->base + SUNXI_SS_IV0 + i * 4);
+		}
+	}
+	writel(tmp, ss_ctx->base + SUNXI_SS_CTL);
+
+	do {
+		if (rx_cnt == 0 || tx_cnt == 0) {
+			tmp = readl(ss_ctx->base + SUNXI_SS_FCSR);
+			rx_cnt = SS_RXFIFO_SPACES(tmp);
+			tx_cnt = SS_TXFIFO_SPACES(tmp);
+		}
+		if (rx_cnt > 0 && ir < nbytes) {
+			do {
+				value = *(u32 *)(ss_ctx->buf_in + ir);
+				writel(value, ss_ctx->base + SS_RXFIFO);
+				ir += 4;
+				rx_cnt--;
+			} while (rx_cnt > 0 && ir < nbytes);
+		}
+		if (tx_cnt > 0 && it < nbytes) {
+			do {
+				if (ir <= it)
+					dev_warn(ss_ctx->dev, "ANORMAL %u %u\n",
+							ir, it);
+				value = readl(ss_ctx->base + SS_TXFIFO);
+				*(u32 *)(ss_ctx->buf_out + it) = value;
+				it += 4;
+				tx_cnt--;
+			} while (tx_cnt > 0 && it < nbytes);
+		}
+		if (ir == nbytes) {
+			mutex_unlock(&bufin_lock);
+			ir++;
+		}
+	} while (it < nbytes);
+
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+	mutex_unlock(&lock);
+
+	/* a simple optimization, since we dont need the hardware for this copy
+	 * we release the lock and do the copy. With that we gain 5/10% perf */
+	mutex_lock(&bufout_lock);
+	sg_copy_from_buffer(dst, nb_in_sg_tx, ss_ctx->buf_out, nbytes);
+
+	mutex_unlock(&bufout_lock);
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+/*============================================================================*/
+/*============================================================================*/
+/* check and set the DES key, prepare the mode to be used */
+static int sunxi_des_setkey(struct crypto_tfm *tfm, const u8 *key,
+		unsigned int keylen)
+{
+	struct sunxi_req_ctx *op = crypto_tfm_ctx(tfm);
+	if (keylen != DES_KEY_SIZE) {
+		dev_err(ss_ctx->dev, "Invalid keylen %u\n", keylen);
+		crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	op->keylen = keylen;
+	memcpy(op->key, key, keylen);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_des_cbc_encrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (ivsize < 4) {
+		dev_info(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_info(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_ENCRYPTION;
+	op->mode |= SUNXI_OP_DES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_des_poll(desc, dst, src, nbytes, op->mode);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_des_cbc_decrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (ivsize < 4) {
+		dev_info(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_info(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_DECRYPTION;
+	op->mode |= SUNXI_OP_DES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_des_poll(desc, dst, src, nbytes, op->mode);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static struct crypto_alg sunxi_des_alg = {
+	.cra_name = "cbc(des)",
+	.cra_driver_name = "cbc-des-sunxi-ss",
+	.cra_priority = 300,
+	.cra_blocksize = DES_BLOCK_SIZE,
+	.cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
+	.cra_ctxsize = sizeof(struct sunxi_req_ctx),
+	.cra_module = THIS_MODULE,
+	.cra_type = &crypto_blkcipher_type,
+	.cra_init = sunxi_des_init,
+	.cra_exit = sunxi_des_exit,
+	.cra_alignmask = 3,
+	.cra_u.blkcipher = {
+		.min_keysize    = DES_KEY_SIZE,
+		.max_keysize    = DES_KEY_SIZE,
+		.ivsize         = 8,
+		.setkey         = sunxi_des_setkey,
+		.encrypt        = sunxi_des_cbc_encrypt,
+		.decrypt        = sunxi_des_cbc_decrypt,
+	}
+};
+
+#endif /* CONFIG_CRYPTO_DEV_SUNXI_SS_DES */
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+/*============================================================================*/
+/*============================================================================*/
+/* check and set the 3DES key, prepare the mode to be used */
+static int sunxi_des3_setkey(struct crypto_tfm *tfm, const u8 *key,
+		unsigned int keylen)
+{
+	struct sunxi_req_ctx *op = crypto_tfm_ctx(tfm);
+	if (keylen != 3 * DES_KEY_SIZE) {
+		dev_err(ss_ctx->dev, "Invalid keylen %u\n", keylen);
+		crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	op->keylen = keylen;
+	memcpy(op->key, key, keylen);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_des3_cbc_encrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (ivsize < 4) {
+		dev_info(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_info(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_ENCRYPTION;
+	op->mode |= SUNXI_OP_3DES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_des_poll(desc, dst, src, nbytes, op->mode);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_des3_cbc_decrypt(struct blkcipher_desc *desc,
+		struct scatterlist *dst, struct scatterlist *src,
+		unsigned int nbytes)
+{
+	struct sunxi_req_ctx *op = crypto_blkcipher_ctx(desc->tfm);
+	unsigned int ivsize = crypto_blkcipher_ivsize(desc->tfm);
+
+	if (ivsize < 4) {
+		dev_info(ss_ctx->dev, "Bad IV size %u\n", ivsize);
+		return -1;
+	}
+
+	if (desc->info == NULL) {
+		dev_info(ss_ctx->dev, "Empty IV\n");
+		return -1;
+	}
+
+	op->mode |= SUNXI_SS_DECRYPTION;
+	op->mode |= SUNXI_OP_3DES;
+	op->mode |= SUNXI_SS_CBC;
+
+	return sunxi_des_poll(desc, dst, src, nbytes, op->mode);
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static struct crypto_alg sunxi_des3_alg = {
+	.cra_name = "cbc(des3_ede)",
+	.cra_driver_name = "cbc-des3-sunxi-ss",
+	.cra_priority = 300,
+	.cra_blocksize = DES3_EDE_BLOCK_SIZE,
+	.cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
+	.cra_ctxsize = sizeof(struct sunxi_req_ctx),
+	.cra_module = THIS_MODULE,
+	.cra_type = &crypto_blkcipher_type,
+	.cra_init = sunxi_des_init,
+	.cra_exit = sunxi_des_exit,
+	.cra_alignmask = 3,
+	.cra_u.blkcipher = {
+		.min_keysize    = DES3_EDE_KEY_SIZE,
+		.max_keysize    = DES3_EDE_KEY_SIZE,
+		.ivsize         = 8,
+		.setkey         = sunxi_des3_setkey,
+		.encrypt        = sunxi_des3_cbc_encrypt,
+		.decrypt        = sunxi_des3_cbc_decrypt,
+	}
+};
+
+#endif /* CONFIG_CRYPTO_DEV_SUNXI_SS_3DES */
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_ss_rng_get_random(struct crypto_rng *tfm, u8 *rdata,
+		unsigned int dlen)
+{
+	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);
+	unsigned int i;
+	u32 mode = 0;
+	u32 v;
+
+	dev_dbg(ss_ctx->dev, "DEBUG %s dlen=%u\n", __func__, dlen);
+
+	if (dlen == 0 || rdata == NULL)
+		return 0;
+
+	mode |= SUNXI_OP_PRNG;
+	mode |= SUNXI_PRNG_ONESHOT;
+	mode |= SUNXI_SS_ENABLED;
+
+	mutex_lock(&lock);
+	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
+
+	for (i = 0; i < ctx->slen; i += 4) {
+		v = *(u32 *)(ctx->seed + i);
+		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
+	}
+
+	for (i = 0; i < ctx->slen && i < 192/8 && i < 16; i += 4) {
+		v = *(u32 *)(ctx->seed + i);
+		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
+		writel(v, ss_ctx->base + SUNXI_SS_KEY0 + i);
+	}
+
+	mode |= SUNXI_PRNG_START;
+	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
+	for (i = 0; i < 4; i++) {
+		v = readl(ss_ctx->base + SUNXI_SS_CTL);
+		dev_dbg(ss_ctx->dev, "DEBUG CTL %x %x\n", mode, v);
+	}
+	for (i = 0; i < dlen && i < 160 / 8; i += 4) {
+		v = readl(ss_ctx->base + SUNXI_SS_MD0 + i);
+		*(u32 *)(rdata + i) = v;
+		dev_dbg(ss_ctx->dev, "DEBUG MD%d %x\n", i, v);
+	}
+
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+	mutex_unlock(&lock);
+	return dlen;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_ss_rng_reset(struct crypto_rng *tfm, u8 *seed,
+		unsigned int slen)
+{
+	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);
+
+	dev_dbg(ss_ctx->dev, "DEBUG %s slen=%u\n", __func__, slen);
+	memcpy(ctx->seed, seed, slen);
+	ctx->slen = slen;
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static struct crypto_alg sunxi_ss_prng = {
+	.cra_name = "stdrng",
+	.cra_driver_name = "rng-sunxi-ss",
+	.cra_priority = 100,
+	.cra_flags = CRYPTO_ALG_TYPE_RNG,
+	.cra_ctxsize = sizeof(struct prng_context),
+	.cra_module = THIS_MODULE,
+	.cra_type = &crypto_rng_type,
+	.cra_u.rng = {
+		.rng_make_random = sunxi_ss_rng_get_random,
+		.rng_reset = sunxi_ss_rng_reset,
+		.seedsize = 192/8
+	}
+};
+#endif /* CRYPTO_DEV_SUNXI_SS_PRNG */
+
+/*============================================================================*/
+/*============================================================================*/
+static int sunxi_ss_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	u32 v;
+	int err;
+	unsigned long cr;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	memset(ss_ctx, 0, sizeof(struct sunxi_ss_ctx));
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "Cannot get the MMIO ressource\n");
+		/* TODO PTR_ERR ? */
+		return -ENXIO;
+	}
+	ss_ctx->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ss_ctx->base)) {
+		dev_err(&pdev->dev, "Cannot request MMIO\n");
+		return PTR_ERR(ss_ctx->base);
+	}
+
+	/* TODO Does this information could be usefull ? */
+	writel(SUNXI_SS_ENABLED, ss_ctx->base + SUNXI_SS_CTL);
+	v = readl(ss_ctx->base + SUNXI_SS_CTL);
+	v >>= 16;
+	v &= 0x07;
+	dev_info(&pdev->dev, "Die ID %d\n", v);
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+
+	ss_ctx->ssclk = devm_clk_get(&pdev->dev, "mod");
+	if (IS_ERR(ss_ctx->ssclk)) {
+		err = PTR_ERR(ss_ctx->ssclk);
+		dev_err(&pdev->dev, "Cannot get SS clock err=%d\n", err);
+		return err;
+	}
+	dev_dbg(&pdev->dev, "clock ss acquired\n");
+
+	ss_ctx->busclk = devm_clk_get(&pdev->dev, "ahb");
+	if (IS_ERR(ss_ctx->busclk)) {
+		err = PTR_ERR(ss_ctx->busclk);
+		dev_err(&pdev->dev, "Cannot get AHB SS clock err=%d\n", err);
+		return err;
+	}
+	dev_dbg(&pdev->dev, "clock ahb_ss acquired\n");
+
+
+	/* Enable the clocks */
+	err = clk_prepare_enable(ss_ctx->busclk);
+	if (err != 0) {
+		dev_err(&pdev->dev, "Cannot prepare_enable busclk\n");
+		return err;
+	}
+	err = clk_prepare_enable(ss_ctx->ssclk);
+	if (err != 0) {
+		dev_err(&pdev->dev, "Cannot prepare_enable ssclk\n");
+		clk_disable_unprepare(ss_ctx->busclk);
+		return err;
+	}
+
+#define	SUNXI_SS_CLOCK_RATE_BUS (24 * 1000 * 1000)
+#define	SUNXI_SS_CLOCK_RATE_SS (150 * 1000 * 1000)
+
+	/* Check that clock have the correct rates gived in the datasheet */
+	cr = clk_get_rate(ss_ctx->busclk);
+	if (cr >= SUNXI_SS_CLOCK_RATE_BUS)
+		dev_dbg(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= %u)\n",
+				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
+	else
+		dev_warn(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= %u)\n",
+				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
+	cr = clk_get_rate(ss_ctx->ssclk);
+	if (cr == SUNXI_SS_CLOCK_RATE_SS)
+		dev_dbg(&pdev->dev, "Clock ss %lu (%lu MHz) (must be <= %u)\n",
+				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
+	else {
+		dev_warn(&pdev->dev, "Clock ss is at %lu (%lu MHz) (must be <= %u)\n",
+				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
+		/* Try to set the clock to the maximum allowed */
+		err = clk_set_rate(ss_ctx->ssclk, SUNXI_SS_CLOCK_RATE_SS);
+		if (err != 0) {
+			dev_err(&pdev->dev, "Cannot set clock rate to ssclk\n");
+			goto label_error_clock;
+		}
+		cr = clk_get_rate(ss_ctx->ssclk);
+		dev_info(&pdev->dev, "Clock ss set to %lu (%lu MHz) (must be >= %u)\n",
+				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
+	}
+
+	ss_ctx->buf_in = NULL;
+	ss_ctx->buf_in_size = 0;
+	ss_ctx->buf_out = NULL;
+	ss_ctx->buf_out_size = 0;
+	ss_ctx->dev = &pdev->dev;
+
+	mutex_init(&lock);
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+	err = crypto_register_alg(&sunxi_ss_prng);
+	if (err) {
+		dev_err(&pdev->dev, "crypto_register_alg error\n");
+		goto label_error_prng;
+	} else {
+		dev_info(&pdev->dev, "Registred PRNG\n");
+	}
+#endif
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+	err = crypto_register_shash(&sunxi_md5_alg);
+	if (err) {
+		dev_err(&pdev->dev, "Fail to register MD5\n");
+		goto label_error_md5;
+	} else {
+		dev_info(&pdev->dev, "Registred MD5\n");
+	}
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+	err = crypto_register_shash(&sunxi_sha1_alg);
+	if (err) {
+		dev_err(&pdev->dev, "Fail to register SHA1\n");
+		goto label_error_sha1;
+	} else {
+		dev_info(&pdev->dev, "Registred SHA1\n");
+	}
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+	err = crypto_register_alg(&sunxi_aes_alg);
+	if (err) {
+		dev_err(&pdev->dev, "crypto_register_alg error for AES\n");
+		goto label_error_aes;
+	} else {
+		dev_info(&pdev->dev, "Registred AES\n");
+	}
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+	err = crypto_register_alg(&sunxi_des_alg);
+	if (err) {
+		dev_err(&pdev->dev, "crypto_register_alg error for DES\n");
+		goto label_error_des;
+	} else {
+		dev_info(&pdev->dev, "Registred DES\n");
+	}
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+	err = crypto_register_alg(&sunxi_des3_alg);
+	if (err) {
+		dev_err(&pdev->dev, "crypto_register_alg error for 3DES\n");
+		goto label_error_des3;
+	} else {
+		dev_info(&pdev->dev, "Registred 3DES\n");
+	}
+#endif
+	return 0;
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+label_error_des3:
+	crypto_unregister_alg(&sunxi_des3_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+label_error_des:
+	crypto_unregister_alg(&sunxi_des_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+label_error_aes:
+	crypto_unregister_alg(&sunxi_aes_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+label_error_sha1:
+	crypto_unregister_shash(&sunxi_sha1_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+label_error_md5:
+	crypto_unregister_shash(&sunxi_md5_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+label_error_prng:
+	crypto_unregister_alg(&sunxi_ss_prng);
+#endif
+label_error_clock:
+	if (ss_ctx->ssclk != NULL)
+		clk_disable_unprepare(ss_ctx->ssclk);
+	if (ss_ctx->busclk != NULL)
+		clk_disable_unprepare(ss_ctx->busclk);
+
+	return err;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static int __exit sunxi_ss_remove(struct platform_device *pdev)
+{
+	if (!pdev->dev.of_node)
+		return 0;
+
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
+	crypto_unregister_shash(&sunxi_md5_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
+	crypto_unregister_shash(&sunxi_sha1_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
+	crypto_unregister_alg(&sunxi_ss_prng);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
+	crypto_unregister_alg(&sunxi_aes_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
+	crypto_unregister_alg(&sunxi_des3_alg);
+#endif
+#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
+	crypto_unregister_alg(&sunxi_des_alg);
+#endif
+	/* TODO devm_kmalloc / devm_kfree */
+	if (ss_ctx->buf_in != NULL)
+		kfree(ss_ctx->buf_in);
+	if (ss_ctx->buf_out != NULL)
+		kfree(ss_ctx->buf_out);
+
+	writel(0, ss_ctx->base + SUNXI_SS_CTL);
+	clk_disable_unprepare(ss_ctx->busclk);
+	clk_disable_unprepare(ss_ctx->ssclk);
+	return 0;
+}
+
+/*============================================================================*/
+/*============================================================================*/
+static const struct of_device_id a20ss_crypto_of_match_table[] = {
+	{ .compatible = "allwinner,sun7i-a20-crypto" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, a20ss_crypto_of_match_table);
+
+static struct platform_driver sunxi_ss_driver = {
+	.probe          = sunxi_ss_probe,
+	.remove         = sunxi_ss_remove,
+	.driver         = {
+		.owner          = THIS_MODULE,
+		.name           = "sunxi-ss",
+		.of_match_table	= a20ss_crypto_of_match_table,
+	},
+};
+
+module_platform_driver(sunxi_ss_driver);
+
+MODULE_DESCRIPTION("Allwinner Security System crypto accelerator");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Corentin LABBE <clabbe.montjoie@gmail.com>");
-- 
1.8.5.5

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Arnd Bergmann]

On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/crypto/Kconfig    |   49 ++
>  drivers/crypto/Makefile   |    1 +
>  drivers/crypto/sunxi-ss.c | 1476 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1526 insertions(+)
>  create mode 100644 drivers/crypto/sunxi-ss.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 03ccdb0..5ea0922 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mxs-dcp.
>  
> +config CRYPTO_DEV_SUNXI_SS
> +        tristate "Support for Allwinner Security System cryptographic accelerator"
> +        depends on ARCH_SUNXI
> +        help
> +          Some Allwinner processors have a crypto accelerator named
> +          Security System. Select this if you want to use it.
> +
> +          To compile this driver as a module, choose M here: the module
> +          will be called sunxi-ss.
> +
> +if CRYPTO_DEV_SUNXI_SS
> +config CRYPTO_DEV_SUNXI_SS_PRNG
> +	bool "Security System PRNG"
> +	select CRYPTO_RNG2
> +	help
> +	  If you enable this option, the SS will provide a pseudo random
> +	  number generator.
> +config CRYPTO_DEV_SUNXI_SS_MD5
> +	bool "Security System MD5"
> +	select CRYPTO_MD5
> +	help
> +	  If you enable this option, the SS will provide MD5 hardware
> +	  acceleration.

My feeling is that this should either be one driver that provides
all five algorithms unconditionally, or five drivers that are each
separate loadable modules and based on top of a common module
that only exports functions but has no active logic itself

> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> +#include <crypto/md5.h>
> +#define SUNXI_SS_HASH_COMMON
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
> +#include <crypto/sha.h>
> +#define SUNXI_SS_HASH_COMMON
> +#endif
> +#ifdef SUNXI_SS_HASH_COMMON
> +#include <crypto/hash.h>
> +#include <crypto/internal/hash.h>
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
> +#include <crypto/aes.h>
> +#endif
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
> +#define SUNXI_SS_DES
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
> +#define SUNXI_SS_DES
> +#endif
> +#ifdef SUNXI_SS_DES
> +#include <crypto/des.h>
> +#endif
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
> +#include <crypto/internal/rng.h>

That would cleanup this #ifdef chain either way.

> +/* General notes:
> + * I cannot use a key/IV cache because each time one of these change ALL stuff
> + * need to be re-writed.
> + * And for example, with dm-crypt IV changes on each request.
> + *
> + * After each request the device must be disabled.
> + *
> + * For performance reason, we use writel_relaxed/read_relaxed for all
> + * operations on RX and TX FIFO.
> + * For all other registers, we use writel.
> + * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
> + * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
> + * */
> +
> +static struct sunxi_ss_ctx {
> +	void *base;

__iomem ?

> +	int irq;
> +	struct clk *busclk;
> +	struct clk *ssclk;
> +	struct device *dev;
> +	struct resource *res;
> +	void *buf_in; /* pointer to data to be uploaded to the device */
> +	size_t buf_in_size; /* size of buf_in */
> +	void *buf_out;
> +	size_t buf_out_size;
> +} _ss_ctx, *ss_ctx = &_ss_ctx;
> +
> +static DEFINE_MUTEX(lock);
> +static DEFINE_MUTEX(bufout_lock);
> +static DEFINE_MUTEX(bufin_lock);

I guess the mutexes should be part of sunxi_ss_ctx.

Are you sure you need the global _ss_ctx structure?
Normally you should get a pointer from whoever calls you.

> +struct sunxi_req_ctx {
> +	u8 key[AES_MAX_KEY_SIZE * 8];
> +	u32 keylen;
> +	u32 mode;
> +	u64 byte_count; /* number of bytes "uploaded" to the device */
> +	u32 waitbuf; /* a partial word waiting to be completed and
> +			uploaded to the device */
> +	/* number of bytes to be uploaded in the waitbuf word */
> +	unsigned int nbwait;
> +};
> +
> +#ifdef SUNXI_SS_HASH_COMMON
> +/*============================================================================*/
> +/*============================================================================*/

Please remove all the ======== lines.

> +	/* If we have more than one SG, we cannot use kmap_atomic since
> +	 * we hold the mapping too long*/
> +	src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
> +	if (src_addr == NULL) {
> +		dev_err(ss_ctx->dev, "KMAP error for src SG\n");
> +		return -1;
> +	}
> +	dst_addr = kmap(sg_page(out_sg)) + out_sg->offset;
> +	if (dst_addr == NULL) {
> +		kunmap(src_addr);
> +		dev_err(ss_ctx->dev, "KMAP error for dst SG\n");
> +		return -1;
> +	}
> +	src32 = (u32 *)src_addr;
> +	dst32 = (u32 *)dst_addr;
> +	ileft = nbytes / 4;
> +	oleft = nbytes / 4;
> +	sgileft = in_sg->length / 4;
> +	sgoleft = out_sg->length / 4;
> +	do {
> +		tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
> +		rx_cnt = SS_RXFIFO_SPACES(tmp);
> +		tx_cnt = SS_TXFIFO_SPACES(tmp);
> +		todo = min3(rx_cnt, ileft, sgileft);
> +		if (todo > 0) {
> +			ileft -= todo;
> +			sgileft -= todo;
> +		}
> +		while (todo > 0) {
> +			writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
> +			todo--;
> +		}

I wonder if this is meant to be used in combination with a dma engine
rather than accessed with writel/readl.

How does the original driver do it?

	Arnd

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Corentin LABBE]

Le 22/05/2014 17:28, Arnd Bergmann a écrit :
> On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>>  drivers/crypto/Kconfig    |   49 ++
>>  drivers/crypto/Makefile   |    1 +
>>  drivers/crypto/sunxi-ss.c | 1476 +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1526 insertions(+)
>>  create mode 100644 drivers/crypto/sunxi-ss.c
>>
>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>> index 03ccdb0..5ea0922 100644
>> --- a/drivers/crypto/Kconfig
>> +++ b/drivers/crypto/Kconfig
>> @@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called mxs-dcp.
>>  
>> +config CRYPTO_DEV_SUNXI_SS
>> +        tristate "Support for Allwinner Security System cryptographic accelerator"
>> +        depends on ARCH_SUNXI
>> +        help
>> +          Some Allwinner processors have a crypto accelerator named
>> +          Security System. Select this if you want to use it.
>> +
>> +          To compile this driver as a module, choose M here: the module
>> +          will be called sunxi-ss.
>> +
>> +if CRYPTO_DEV_SUNXI_SS
>> +config CRYPTO_DEV_SUNXI_SS_PRNG
>> +	bool "Security System PRNG"
>> +	select CRYPTO_RNG2
>> +	help
>> +	  If you enable this option, the SS will provide a pseudo random
>> +	  number generator.
>> +config CRYPTO_DEV_SUNXI_SS_MD5
>> +	bool "Security System MD5"
>> +	select CRYPTO_MD5
>> +	help
>> +	  If you enable this option, the SS will provide MD5 hardware
>> +	  acceleration.
> 
> My feeling is that this should either be one driver that provides
> all five algorithms unconditionally, or five drivers that are each
> separate loadable modules and based on top of a common module
> that only exports functions but has no active logic itself

I agree for the split.
It was my first intention but I feared to add too many files.
So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c, sunxi-ss-aes.c, sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h
Does can I add a sunxi-ss directory in drivers/crypto ?

> 
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
>> +#include <crypto/md5.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
>> +#include <crypto/sha.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef SUNXI_SS_HASH_COMMON
>> +#include <crypto/hash.h>
>> +#include <crypto/internal/hash.h>
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
>> +#include <crypto/aes.h>
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef SUNXI_SS_DES
>> +#include <crypto/des.h>
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
>> +#include <crypto/internal/rng.h>
> 
> That would cleanup this #ifdef chain either way.
> 
>> +/* General notes:
>> + * I cannot use a key/IV cache because each time one of these change ALL stuff
>> + * need to be re-writed.
>> + * And for example, with dm-crypt IV changes on each request.
>> + *
>> + * After each request the device must be disabled.
>> + *
>> + * For performance reason, we use writel_relaxed/read_relaxed for all
>> + * operations on RX and TX FIFO.
>> + * For all other registers, we use writel.
>> + * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
>> + * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
>> + * */
>> +
>> +static struct sunxi_ss_ctx {
>> +	void *base;
> 
> __iomem ?

ok

> 
>> +	int irq;
>> +	struct clk *busclk;
>> +	struct clk *ssclk;
>> +	struct device *dev;
>> +	struct resource *res;
>> +	void *buf_in; /* pointer to data to be uploaded to the device */
>> +	size_t buf_in_size; /* size of buf_in */
>> +	void *buf_out;
>> +	size_t buf_out_size;
>> +} _ss_ctx, *ss_ctx = &_ss_ctx;
>> +
>> +static DEFINE_MUTEX(lock);
>> +static DEFINE_MUTEX(bufout_lock);
>> +static DEFINE_MUTEX(bufin_lock);
> 
> I guess the mutexes should be part of sunxi_ss_ctx.
> 
> Are you sure you need the global _ss_ctx structure?
> Normally you should get a pointer from whoever calls you.

I will use platform_get_drvdata/platform_set_drvdata for cleaning all thoses global structures..

> 
>> +struct sunxi_req_ctx {
>> +	u8 key[AES_MAX_KEY_SIZE * 8];
>> +	u32 keylen;
>> +	u32 mode;
>> +	u64 byte_count; /* number of bytes "uploaded" to the device */
>> +	u32 waitbuf; /* a partial word waiting to be completed and
>> +			uploaded to the device */
>> +	/* number of bytes to be uploaded in the waitbuf word */
>> +	unsigned int nbwait;
>> +};
>> +
>> +#ifdef SUNXI_SS_HASH_COMMON
>> +/*============================================================================*/
>> +/*============================================================================*/
> 
> Please remove all the ======== lines.

ok

> 
>> +	/* If we have more than one SG, we cannot use kmap_atomic since
>> +	 * we hold the mapping too long*/
>> +	src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
>> +	if (src_addr == NULL) {
>> +		dev_err(ss_ctx->dev, "KMAP error for src SG\n");
>> +		return -1;
>> +	}
>> +	dst_addr = kmap(sg_page(out_sg)) + out_sg->offset;
>> +	if (dst_addr == NULL) {
>> +		kunmap(src_addr);
>> +		dev_err(ss_ctx->dev, "KMAP error for dst SG\n");
>> +		return -1;
>> +	}
>> +	src32 = (u32 *)src_addr;
>> +	dst32 = (u32 *)dst_addr;
>> +	ileft = nbytes / 4;
>> +	oleft = nbytes / 4;
>> +	sgileft = in_sg->length / 4;
>> +	sgoleft = out_sg->length / 4;
>> +	do {
>> +		tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
>> +		rx_cnt = SS_RXFIFO_SPACES(tmp);
>> +		tx_cnt = SS_TXFIFO_SPACES(tmp);
>> +		todo = min3(rx_cnt, ileft, sgileft);
>> +		if (todo > 0) {
>> +			ileft -= todo;
>> +			sgileft -= todo;
>> +		}
>> +		while (todo > 0) {
>> +			writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
>> +			todo--;
>> +		}
> 
> I wonder if this is meant to be used in combination with a dma engine
> rather than accessed with writel/readl.

You could do both, but the dmaengine driver is under development.
When it will be ready, I will add DMA support.
But my intention is to keep both mode, since poll mode is better than DMA for small request.

> 
> How does the original driver do it?

There are no original driver, this driver is the first for the Security System.

> 
> 	Arnd
> 

Thanks for your review.

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Arnd Bergmann]

On Thursday 22 May 2014, Corentin LABBE wrote:
> Le 22/05/2014 17:28, Arnd Bergmann a écrit :
> > On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
> >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > 
> > My feeling is that this should either be one driver that provides
> > all five algorithms unconditionally, or five drivers that are each
> > separate loadable modules and based on top of a common module
> > that only exports functions but has no active logic itself
> 
> I agree for the split.
> It was my first intention but I feared to add too many files.
> So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c, sunxi-ss-aes.c, sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h
> Does can I add a sunxi-ss directory in drivers/crypto ?

Yes, I think a subdirectory would be best.


> >> +	/* If we have more than one SG, we cannot use kmap_atomic since
> >> +	 * we hold the mapping too long*/
> >> +	src_addr = kmap(sg_page(in_sg)) + in_sg->offset;
> >> +	if (src_addr == NULL) {
> >> +		dev_err(ss_ctx->dev, "KMAP error for src SG\n");
> >> +		return -1;
> >> +	}
> >> +	dst_addr = kmap(sg_page(out_sg)) + out_sg->offset;
> >> +	if (dst_addr == NULL) {
> >> +		kunmap(src_addr);
> >> +		dev_err(ss_ctx->dev, "KMAP error for dst SG\n");
> >> +		return -1;
> >> +	}
> >> +	src32 = (u32 *)src_addr;
> >> +	dst32 = (u32 *)dst_addr;
> >> +	ileft = nbytes / 4;
> >> +	oleft = nbytes / 4;
> >> +	sgileft = in_sg->length / 4;
> >> +	sgoleft = out_sg->length / 4;
> >> +	do {
> >> +		tmp = readl_relaxed(ss_ctx->base + SUNXI_SS_FCSR);
> >> +		rx_cnt = SS_RXFIFO_SPACES(tmp);
> >> +		tx_cnt = SS_TXFIFO_SPACES(tmp);
> >> +		todo = min3(rx_cnt, ileft, sgileft);
> >> +		if (todo > 0) {
> >> +			ileft -= todo;
> >> +			sgileft -= todo;
> >> +		}
> >> +		while (todo > 0) {
> >> +			writel_relaxed(*src32++, ss_ctx->base + SS_RXFIFO);
> >> +			todo--;
> >> +		}
> > 
> > I wonder if this is meant to be used in combination with a dma engine
> > rather than accessed with writel/readl.
> 
> You could do both, but the dmaengine driver is under development.
> When it will be ready, I will add DMA support.
> But my intention is to keep both mode, since poll mode is better than DMA for small request.

Ok, I see.

> > How does the original driver do it?
> 
> There are no original driver, this driver is the first for the Security System.

Ah, I thought there was one in the allwinner BSP kernel.

	Arnd

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Marek Vasut]

On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:

Missing commit message. Please fix this and send a V2.

> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> 100644
> index 0000000..356563b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> @@ -0,0 +1,9 @@
> +* Allwinner Security System found on A20 SoC
> +
> +Required properties:
> +- compatible : Should be "allwinner,sun7i-a20-crypto".

Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can that not 
be described by DT props ?

> +- reg: Should contain the SS register location and length.

SS? What is that? Fix this text to be actually descriptive please.

> +- interrupts: Should contain the IRQ line for the SS.
> +- clocks : A phandle to the functional clock node of the SS module
> +- clock-names : Name of the functional clock, should be "ahb" and "mod".
> +

Best regards,
Marek Vasut

Re: [PATCH 2/3] ARM: sun7i: dt: Add Security System to A20 SoC DTS

[Marek Vasut]

On Thursday, May 22, 2014 at 05:09:55 PM, LABBE Corentin wrote:

Commit message ... damn, there is a reason why you _should_ write the commit 
message, even if the $subject is descriptive enough. You should elaborate on 
what you did here and _why_ you did it. The _why_ part is also really important 
since when you read this patch 5 years from now, you won't be looking at this in 
a confused fashion.

> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi
> b/arch/arm/boot/dts/sun7i-a20.dtsi index f4b00a5..ea56552 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -523,6 +523,14 @@
>  			status = "disabled";
>  		};
> 
> +		crypto: crypto-engine@01c15000 {
> +			compatible = "allwinner,sun7i-a20-crypto";
> +			reg = <0x01c15000 0x1000>;
> +			interrupts = <0 86 4>;
> +			clocks = <&ahb_gates 5>, <&ss_clk>;
> +			clock-names = "ahb", "mod";
> +		};
> +
>  		spi2: spi@01c17000 {
>  			compatible = "allwinner,sun4i-a10-spi";
>  			reg = <0x01c17000 0x1000>;

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Friday, May 23, 2014 at 12:46:10 PM, Arnd Bergmann wrote:
> On Thursday 22 May 2014, Corentin LABBE wrote:
> > Le 22/05/2014 17:28, Arnd Bergmann a écrit :
> > > On Thursday 22 May 2014 17:09:56 LABBE Corentin wrote:
> > >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > > 
> > > My feeling is that this should either be one driver that provides
> > > all five algorithms unconditionally, or five drivers that are each
> > > separate loadable modules and based on top of a common module
> > > that only exports functions but has no active logic itself
> > 
> > I agree for the split.
> > It was my first intention but I feared to add too many files.
> > So I propose to split in 6, sunxi-ss-hash.c, sunxi-ss-des.c,
> > sunxi-ss-aes.c, sunxi-ss-rng.c, sunxi-ss-common.c and sunxi-ss.h Does
> > can I add a sunxi-ss directory in drivers/crypto ?
> 
> Yes, I think a subdirectory would be best.

Full ACK on this one. Use drivers/crypto/sunxi-ss/ .
[...]

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Thursday, May 22, 2014 at 05:09:56 PM, LABBE Corentin wrote:

Do I have to repeat myself ? :)

> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> ---
>  drivers/crypto/Kconfig    |   49 ++
>  drivers/crypto/Makefile   |    1 +
>  drivers/crypto/sunxi-ss.c | 1476
> +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1526
> insertions(+)
>  create mode 100644 drivers/crypto/sunxi-ss.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 03ccdb0..5ea0922 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mxs-dcp.
> 
> +config CRYPTO_DEV_SUNXI_SS
> +        tristate "Support for Allwinner Security System cryptographic
> accelerator" +        depends on ARCH_SUNXI
> +        help
> +          Some Allwinner processors have a crypto accelerator named
> +          Security System. Select this if you want to use it.
> +
> +          To compile this driver as a module, choose M here: the module
> +          will be called sunxi-ss.
> +
> +if CRYPTO_DEV_SUNXI_SS
> +config CRYPTO_DEV_SUNXI_SS_PRNG
> +	bool "Security System PRNG"
> +	select CRYPTO_RNG2
> +	help
> +	  If you enable this option, the SS will provide a pseudo random
> +	  number generator.
> +config CRYPTO_DEV_SUNXI_SS_MD5
> +	bool "Security System MD5"
> +	select CRYPTO_MD5
> +	help
> +	  If you enable this option, the SS will provide MD5 hardware
> +	  acceleration.

This is one IP block and it provides 5 algorithms. Put it under one config 
option please.

Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the name 
is useless.

[...]

> diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
> new file mode 100644
> index 0000000..bbf57bc
> --- /dev/null
> +++ b/drivers/crypto/sunxi-ss.c
> @@ -0,0 +1,1476 @@
> +/*
> + * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC

Why can this not be generic Allwinner-all-chips driver ? Does the IP block 
change that dramatically between the chips?

[...]

> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> +#include <crypto/md5.h>
> +#define SUNXI_SS_HASH_COMMON
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
> +#include <crypto/sha.h>
> +#define SUNXI_SS_HASH_COMMON
> +#endif
> +#ifdef SUNXI_SS_HASH_COMMON
> +#include <crypto/hash.h>
> +#include <crypto/internal/hash.h>
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
> +#include <crypto/aes.h>
> +#endif
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
> +#define SUNXI_SS_DES
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
> +#define SUNXI_SS_DES
> +#endif
> +#ifdef SUNXI_SS_DES
> +#include <crypto/des.h>
> +#endif

Please discard this mayhem when getting rid of all the configuration option.

> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
> +#include <crypto/internal/rng.h>
> +
> +struct prng_context {
> +	u8 seed[192/8];
> +	unsigned int slen;
> +};
> +#endif
> +
> +#define SUNXI_SS_CTL            0x00
> +#define SUNXI_SS_KEY0           0x04
> +#define SUNXI_SS_KEY1           0x08
> +#define SUNXI_SS_KEY2           0x0C
> +#define SUNXI_SS_KEY3           0x10
> +#define SUNXI_SS_KEY4           0x14
> +#define SUNXI_SS_KEY5           0x18
> +#define SUNXI_SS_KEY6           0x1C
> +#define SUNXI_SS_KEY7           0x20
> +
> +#define SUNXI_SS_IV0            0x24
> +#define SUNXI_SS_IV1            0x28
> +#define SUNXI_SS_IV2            0x2C
> +#define SUNXI_SS_IV3            0x30
> +
> +#define SUNXI_SS_CNT0           0x34
> +#define SUNXI_SS_CNT1           0x38
> +#define SUNXI_SS_CNT2           0x3C
> +#define SUNXI_SS_CNT3           0x40
> +
> +#define SUNXI_SS_FCSR           0x44
> +#define SUNXI_SS_ICSR           0x48
> +
> +#define SUNXI_SS_MD0            0x4C
> +#define SUNXI_SS_MD1            0x50
> +#define SUNXI_SS_MD2            0x54
> +#define SUNXI_SS_MD3            0x58
> +#define SUNXI_SS_MD4            0x5C
> +
> +#define SS_RXFIFO         0x200
> +#define SS_TXFIFO         0x204

You don't have much consistency in the register naming scheme, please fix this 
somehow. I suppose renaming the SS_[RT]XFIFO registers to SUNXI_SS_[RT]XFIFO is 
a good idea.

> +/* SUNXI_SS_CTL configuration values */
> +
> +/* AES/DES/3DES key select - bits 24-27 */
> +#define SUNXI_SS_KEYSELECT_KEYN		(0 << 24)

Uh oh , you ORR some values with this flag, which is always zero. This seems 
broken.

[...]

> +/* SS Method - bits 4-6 */
> +#define SUNXI_OP_AES                    (0 << 4)
> +#define SUNXI_OP_DES                    (1 << 4)
> +#define SUNXI_OP_3DES                   (2 << 4)
> +#define SUNXI_OP_SHA1                   (3 << 4)
> +#define SUNXI_OP_MD5                    (4 << 4)
> +#define SUNXI_OP_PRNG                   (5 << 4)
> +
> +/* Data end bit - bit 2 */
> +#define SUNXI_SS_DATA_END               BIT(2)

Please use the BIT() macros in consistent fashion. Either use it or don't use it 
(I'd much rather see it not used), but don't mix the stuff :-(

[...]

> +/* General notes:
> + * I cannot use a key/IV cache because each time one of these change ALL
> stuff + * need to be re-writed.
> + * And for example, with dm-crypt IV changes on each request.
> + *
> + * After each request the device must be disabled.

This really isn't quite clear, can you please expand the comment so it's more 
understandtable ?

> + * For performance reason, we use writel_relaxed/read_relaxed for all
> + * operations on RX and TX FIFO.
> + * For all other registers, we use writel.
> + * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
> + * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
> + * */

I will not review the algos, they need to be AHASH/ABLKCIPHER algos. See the 
reasoning further down below.

[...]

> +/*========================================================================
> ====*/
> +/*=======================================================================
> =====*/ +static struct crypto_alg sunxi_des3_alg = {
> +	.cra_name = "cbc(des3_ede)",
> +	.cra_driver_name = "cbc-des3-sunxi-ss",
> +	.cra_priority = 300,
> +	.cra_blocksize = DES3_EDE_BLOCK_SIZE,
> +	.cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,

This must be rewritten to be ABLKCIPHER.

> +	.cra_ctxsize = sizeof(struct sunxi_req_ctx),
> +	.cra_module = THIS_MODULE,
> +	.cra_type = &crypto_blkcipher_type,
> +	.cra_init = sunxi_des_init,
> +	.cra_exit = sunxi_des_exit,
> +	.cra_alignmask = 3,
> +	.cra_u.blkcipher = {
> +		.min_keysize    = DES3_EDE_KEY_SIZE,
> +		.max_keysize    = DES3_EDE_KEY_SIZE,
> +		.ivsize         = 8,
> +		.setkey         = sunxi_des3_setkey,
> +		.encrypt        = sunxi_des3_cbc_encrypt,
> +		.decrypt        = sunxi_des3_cbc_decrypt,
> +	}
> +};
> +
> +#endif /* CONFIG_CRYPTO_DEV_SUNXI_SS_3DES */
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
> +/*========================================================================
> ====*/
> +/*=======================================================================
> =====*/ +static int sunxi_ss_rng_get_random(struct crypto_rng *tfm, u8
> *rdata, +		unsigned int dlen)
> +{
> +	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);

Use crypto_rng_ctx() here . The cast is plain wrong. I know it works due to how 
the structures are defined, but this is not right.

> +	unsigned int i;
> +	u32 mode = 0;
> +	u32 v;
> +
> +	dev_dbg(ss_ctx->dev, "DEBUG %s dlen=%u\n", __func__, dlen);
> +
> +	if (dlen == 0 || rdata == NULL)
> +		return 0;

Return -EINVAL or somesuch here, no?

> +	mode |= SUNXI_OP_PRNG;
> +	mode |= SUNXI_PRNG_ONESHOT;
> +	mode |= SUNXI_SS_ENABLED;
> +
> +	mutex_lock(&lock);
> +	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
> +
> +	for (i = 0; i < ctx->slen; i += 4) {
> +		v = *(u32 *)(ctx->seed + i);

Define the .seed field as u32 if you actually want to access it as u32. You are 
very lucky you didn't trigger alignment faults with this yet.

> +		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
> +	}

But this debug instrumentation looks quite useless anyway.

> +	for (i = 0; i < ctx->slen && i < 192/8 && i < 16; i += 4) {
> +		v = *(u32 *)(ctx->seed + i);
> +		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
> +		writel(v, ss_ctx->base + SUNXI_SS_KEY0 + i);
> +	}
> +
> +	mode |= SUNXI_PRNG_START;
> +	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
> +	for (i = 0; i < 4; i++) {
> +		v = readl(ss_ctx->base + SUNXI_SS_CTL);
> +		dev_dbg(ss_ctx->dev, "DEBUG CTL %x %x\n", mode, v);
> +	}
> +	for (i = 0; i < dlen && i < 160 / 8; i += 4) {
> +		v = readl(ss_ctx->base + SUNXI_SS_MD0 + i);
> +		*(u32 *)(rdata + i) = v;
> +		dev_dbg(ss_ctx->dev, "DEBUG MD%d %x\n", i, v);
> +	}

Drop the debug instrumentation and define the seed buffer as u32.

> +	writel(0, ss_ctx->base + SUNXI_SS_CTL);
> +	mutex_unlock(&lock);
> +	return dlen;
> +}
> +
> +/*========================================================================
> ====*/
> +/*=======================================================================
> =====*/ +static int sunxi_ss_rng_reset(struct crypto_rng *tfm, u8 *seed,
> +		unsigned int slen)
> +{
> +	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);

Use crypto_rng_ctx() here . The cast is plain wrong.

> +	dev_dbg(ss_ctx->dev, "DEBUG %s slen=%u\n", __func__, slen);
> +	memcpy(ctx->seed, seed, slen);
> +	ctx->slen = slen;
> +	return 0;
> +}

[...]

> +static int sunxi_ss_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	u32 v;
> +	int err;
> +	unsigned long cr;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	memset(ss_ctx, 0, sizeof(struct sunxi_ss_ctx));

Static data are always zeroed out by default, no ?

If you just so accidentally happen to probe() this driver twice, you will get 
some seriously ugly surprise here, since you zero the data out without checking 
if the device might have already been probed. A much better idea would be to
define a static struct sunxi_ss_ctx *ss_ctx; and then

 if (ss_ctx) {
  dev_err(&pdev->dev, "Only one instance allowed");
  return -ENODEV;
 }
 ss_ctx = devm_kzalloc(&pdev->dev, sizeof(*ss_ctx), GFP_KERNEL);

Also note the use of sizeof(*ss_ctx), this will allow for this line to not be 
changed in case the type of *ss_ctx ever changes in the future. So use it.

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (res == NULL) {
> +		dev_err(&pdev->dev, "Cannot get the MMIO ressource\n");
> +		/* TODO PTR_ERR ? */

Fix the TODOs .

> +		return -ENXIO;
> +	}
> +	ss_ctx->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ss_ctx->base)) {
> +		dev_err(&pdev->dev, "Cannot request MMIO\n");
> +		return PTR_ERR(ss_ctx->base);
> +	}

Actually, this is the pattern. You don't need to check for IS_ERR(res) before 
calling devm_ioremap_resource().

res = platform_get_resource();
base = devm_ioremap_resource(&pdev->dev, res)
if (IS_ERR(base))
 return PTR_ERR(base);

> +	/* TODO Does this information could be usefull ? */
> +	writel(SUNXI_SS_ENABLED, ss_ctx->base + SUNXI_SS_CTL);
> +	v = readl(ss_ctx->base + SUNXI_SS_CTL);
> +	v >>= 16;
> +	v &= 0x07;
> +	dev_info(&pdev->dev, "Die ID %d\n", v);
> +	writel(0, ss_ctx->base + SUNXI_SS_CTL);

You are configuring hardware before enabling clock for that hardware. That does 
not seem right.

> +	ss_ctx->ssclk = devm_clk_get(&pdev->dev, "mod");
> +	if (IS_ERR(ss_ctx->ssclk)) {
> +		err = PTR_ERR(ss_ctx->ssclk);
> +		dev_err(&pdev->dev, "Cannot get SS clock err=%d\n", err);
> +		return err;
> +	}
> +	dev_dbg(&pdev->dev, "clock ss acquired\n");
> +
> +	ss_ctx->busclk = devm_clk_get(&pdev->dev, "ahb");
> +	if (IS_ERR(ss_ctx->busclk)) {
> +		err = PTR_ERR(ss_ctx->busclk);
> +		dev_err(&pdev->dev, "Cannot get AHB SS clock err=%d\n", err);
> +		return err;
> +	}
> +	dev_dbg(&pdev->dev, "clock ahb_ss acquired\n");

These dev_dbg() aren't really useful.

> +	/* Enable the clocks */
> +	err = clk_prepare_enable(ss_ctx->busclk);
> +	if (err != 0) {
> +		dev_err(&pdev->dev, "Cannot prepare_enable busclk\n");
> +		return err;
> +	}
> +	err = clk_prepare_enable(ss_ctx->ssclk);
> +	if (err != 0) {
> +		dev_err(&pdev->dev, "Cannot prepare_enable ssclk\n");
> +		clk_disable_unprepare(ss_ctx->busclk);
> +		return err;
> +	}
> +
> +#define	SUNXI_SS_CLOCK_RATE_BUS (24 * 1000 * 1000)
> +#define	SUNXI_SS_CLOCK_RATE_SS (150 * 1000 * 1000)

Make this const unsigned long or somesuch, the define is horrid and not 
typesafe.

> +	/* Check that clock have the correct rates gived in the datasheet */
> +	cr = clk_get_rate(ss_ctx->busclk);
> +	if (cr >= SUNXI_SS_CLOCK_RATE_BUS)
> +		dev_dbg(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= %u)\n",
> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
> +	else
> +		dev_warn(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= 
%u)\n",
> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
> +	cr = clk_get_rate(ss_ctx->ssclk);
> +	if (cr == SUNXI_SS_CLOCK_RATE_SS)
> +		dev_dbg(&pdev->dev, "Clock ss %lu (%lu MHz) (must be <= %u)\n",
> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
> +	else {
> +		dev_warn(&pdev->dev, "Clock ss is at %lu (%lu MHz) (must be <= 
%u)\n",
> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
> +		/* Try to set the clock to the maximum allowed */
> +		err = clk_set_rate(ss_ctx->ssclk, SUNXI_SS_CLOCK_RATE_SS);
> +		if (err != 0) {
> +			dev_err(&pdev->dev, "Cannot set clock rate to ssclk\n");
> +			goto label_error_clock;
> +		}
> +		cr = clk_get_rate(ss_ctx->ssclk);
> +		dev_info(&pdev->dev, "Clock ss set to %lu (%lu MHz) (must be >= 
%u)\n",
> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
> +	}

Just run clk_set_rate() for both and be done with it.

> +	ss_ctx->buf_in = NULL;
> +	ss_ctx->buf_in_size = 0;
> +	ss_ctx->buf_out = NULL;
> +	ss_ctx->buf_out_size = 0;

This is useless, you already did that with devm_kzalloc() above.

> +	ss_ctx->dev = &pdev->dev;
> +
> +	mutex_init(&lock);
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
> +	err = crypto_register_alg(&sunxi_ss_prng);

Use crypto_register_algs() when you get rid of those useless #defines for each 
algo.

> +	if (err) {
> +		dev_err(&pdev->dev, "crypto_register_alg error\n");
> +		goto label_error_prng;
> +	} else {
> +		dev_info(&pdev->dev, "Registred PRNG\n");

This is useless, you can always check that the thing was probed via /proc/crypto 
. Please remove this dev_into().

> +	}
> +#endif
> +
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> +	err = crypto_register_shash(&sunxi_md5_alg);

Do not use shash for such device. This is clearly and ahash (and async in 
general) device. The rule of a thumb here is that you use sync algos only for 
devices which have dedicated instructions for computing the transformation. For 
devices which are attached to some kind of bus, you use async algos (ahash etc).

> +	if (err) {
> +		dev_err(&pdev->dev, "Fail to register MD5\n");
> +		goto label_error_md5;
> +	} else {
> +		dev_info(&pdev->dev, "Registred MD5\n");
> +	}
> +#endif
> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
> +	err = crypto_register_shash(&sunxi_sha1_alg);

DTTO.

I am preparing a document for the crypto API, drop me a PM if you want the 
preliminary version. I don't want to release it until it's more complete as it 
will only produce more confusion throughout the crypto API than there already 
is.

[...]

> +static const struct of_device_id a20ss_crypto_of_match_table[] = {
> +	{ .compatible = "allwinner,sun7i-a20-crypto" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, a20ss_crypto_of_match_table);

If possible, scrub the a20 nonsense.

> +static struct platform_driver sunxi_ss_driver = {
> +	.probe          = sunxi_ss_probe,
> +	.remove         = sunxi_ss_remove,
> +	.driver         = {
> +		.owner          = THIS_MODULE,
> +		.name           = "sunxi-ss",
> +		.of_match_table	= a20ss_crypto_of_match_table,
> +	},
> +};
> +
> +module_platform_driver(sunxi_ss_driver);
> +
> +MODULE_DESCRIPTION("Allwinner Security System crypto accelerator");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Corentin LABBE <clabbe.montjoie@gmail.com>");

MODULE_ALIAS is missing I think.

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Tomasz Figa]

Hi Marek,

On 24.05.2014 13:21, Marek Vasut wrote:
> On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> 
> Missing commit message. Please fix this and send a V2.
> 
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
>> 100644
>> index 0000000..356563b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>> @@ -0,0 +1,9 @@
>> +* Allwinner Security System found on A20 SoC
>> +
>> +Required properties:
>> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> 
> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can that not 
> be described by DT props ?

A widely used convention is to define compatible strings after first
SoCs on which particular IP blocks appear. It is quite common among IP
blocks for which there is no well defined versioning scheme.

Best regards,
Tomasz

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Marek Vasut]

On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
> Hi Marek,
> 
> On 24.05.2014 13:21, Marek Vasut wrote:
> > On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> > 
> > Missing commit message. Please fix this and send a V2.
> > 
> >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> >> ---
> >> 
> >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> >> 100644
> >> index 0000000..356563b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> >> @@ -0,0 +1,9 @@
> >> +* Allwinner Security System found on A20 SoC
> >> +
> >> +Required properties:
> >> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> > 
> > Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > that not be described by DT props ?
> 
> A widely used convention is to define compatible strings after first
> SoCs on which particular IP blocks appear. It is quite common among IP
> blocks for which there is no well defined versioning scheme.

Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
manufactured by AW. I find that information redundant, the "allwinner,a20-
crypto" would suffice. But I wonder if that IP block might have appeared even 
earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
be a better string ?

Best regards,
Marek Vasut

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Tomasz Figa]

On 24.05.2014 21:43, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
>> Hi Marek,
>>
>> On 24.05.2014 13:21, Marek Vasut wrote:
>>> On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
>>>
>>> Missing commit message. Please fix this and send a V2.
>>>
>>>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>>>> ---
>>>>
>>>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>  create mode 100644
>>>>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
>>>> 100644
>>>> index 0000000..356563b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
>>>> @@ -0,0 +1,9 @@
>>>> +* Allwinner Security System found on A20 SoC
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be "allwinner,sun7i-a20-crypto".
>>>
>>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
>>> that not be described by DT props ?
>>
>> A widely used convention is to define compatible strings after first
>> SoCs on which particular IP blocks appear. It is quite common among IP
>> blocks for which there is no well defined versioning scheme.
> 
> Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
> manufactured by AW. I find that information redundant, the "allwinner,a20-
> crypto" would suffice. But I wonder if that IP block might have appeared even 
> earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
> be a better string ?

I'm not aware of Allwinner naming schemes too much, so please correct me
if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
would be better indeed.

Whether it was really the first SoC is another thing. Obviously this
needs to be checked, although it isn't really that important. For this
particular naming scheme you need to specify all the SoCs for which
given compatible string can be used for this IP anyway, because there is
usually no other source of information about this available (except
directly comparing two datasheets...).

Best regards,
Tomasz

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Marek Vasut]

On Saturday, May 24, 2014 at 09:51:59 PM, Tomasz Figa wrote:
[...]
> >>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> >>> that not be described by DT props ?
> >> 
> >> A widely used convention is to define compatible strings after first
> >> SoCs on which particular IP blocks appear. It is quite common among IP
> >> blocks for which there is no well defined versioning scheme.
> > 
> > Well yeah, that's fine. But in this case, "sun7i" is the entire group of
> > CPUs manufactured by AW. I find that information redundant, the
> > "allwinner,a20- crypto" would suffice. But I wonder if that IP block
> > might have appeared even earlier ? Or if it is CPU family specific, thus
> > "allwinner,sun7i-crypto" would be a better string ?
> 
> I'm not aware of Allwinner naming schemes too much, so please correct me
> if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
> would be better indeed.

True.

> Whether it was really the first SoC is another thing. Obviously this
> needs to be checked, although it isn't really that important. For this
> particular naming scheme you need to specify all the SoCs for which
> given compatible string can be used for this IP anyway, because there is
> usually no other source of information about this available (except
> directly comparing two datasheets...).

Better get the DT stuff correctly right from the start. That's why I'm asking 
what chips contains the IP block, so we can guess the right name.

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Corentin LABBE]

Le 24/05/2014 14:00, Marek Vasut a écrit :
> On Thursday, May 22, 2014 at 05:09:56 PM, LABBE Corentin wrote:
> 
> Do I have to repeat myself ? :)

No, sorry for the commit message, I begin to use git
Generally I agree all your remarks with some comments below.

> 
>> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
>> ---
>>  drivers/crypto/Kconfig    |   49 ++
>>  drivers/crypto/Makefile   |    1 +
>>  drivers/crypto/sunxi-ss.c | 1476
>> +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1526
>> insertions(+)
>>  create mode 100644 drivers/crypto/sunxi-ss.c
>>
>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>> index 03ccdb0..5ea0922 100644
>> --- a/drivers/crypto/Kconfig
>> +++ b/drivers/crypto/Kconfig
>> @@ -418,4 +418,53 @@ config CRYPTO_DEV_MXS_DCP
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called mxs-dcp.
>>
>> +config CRYPTO_DEV_SUNXI_SS
>> +        tristate "Support for Allwinner Security System cryptographic
>> accelerator" +        depends on ARCH_SUNXI
>> +        help
>> +          Some Allwinner processors have a crypto accelerator named
>> +          Security System. Select this if you want to use it.
>> +
>> +          To compile this driver as a module, choose M here: the module
>> +          will be called sunxi-ss.
>> +
>> +if CRYPTO_DEV_SUNXI_SS
>> +config CRYPTO_DEV_SUNXI_SS_PRNG
>> +	bool "Security System PRNG"
>> +	select CRYPTO_RNG2
>> +	help
>> +	  If you enable this option, the SS will provide a pseudo random
>> +	  number generator.
>> +config CRYPTO_DEV_SUNXI_SS_MD5
>> +	bool "Security System MD5"
>> +	select CRYPTO_MD5
>> +	help
>> +	  If you enable this option, the SS will provide MD5 hardware
>> +	  acceleration.
> 
> This is one IP block and it provides 5 algorithms. Put it under one config 
> option please.

I want to let the user choose what it want to be used. Someone could want only to accelerate md5 and to not use the PRNG.
Lots of other hw crypto driver do the same.

> 
> Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the name 
> is useless.

I think not, most of cryptographic hardware driver begin with CRYPTO_DEV (CRYPTO_DEV_PADLOCK, CRYPTO_DEV_GEODE, CRYPTO_DEV_TALITOS etc...), only S390 does not have a _DEV.

,
> 
> [...]
> 
>> diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
>> new file mode 100644
>> index 0000000..bbf57bc
>> --- /dev/null
>> +++ b/drivers/crypto/sunxi-ss.c
>> @@ -0,0 +1,1476 @@
>> +/*
>> + * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20 SoC
> 
> Why can this not be generic Allwinner-all-chips driver ? Does the IP block 
> change that dramatically between the chips?

As I said in my introduction email, lots of allwinner chips seems to have the same crypto device.
But since I do not own any of those hardware, and in most case does not have a datasheet, I only assume support for A20.
I will add this comment in the header of the driver.

> 
> [...]
> 
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
>> +#include <crypto/md5.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
>> +#include <crypto/sha.h>
>> +#define SUNXI_SS_HASH_COMMON
>> +#endif
>> +#ifdef SUNXI_SS_HASH_COMMON
>> +#include <crypto/hash.h>
>> +#include <crypto/internal/hash.h>
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_AES
>> +#include <crypto/aes.h>
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_3DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_DES
>> +#define SUNXI_SS_DES
>> +#endif
>> +#ifdef SUNXI_SS_DES
>> +#include <crypto/des.h>
>> +#endif
> 
> Please discard this mayhem when getting rid of all the configuration option.
> 
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
>> +#include <crypto/internal/rng.h>
>> +
>> +struct prng_context {
>> +	u8 seed[192/8];
>> +	unsigned int slen;
>> +};
>> +#endif
>> +
>> +#define SUNXI_SS_CTL            0x00
>> +#define SUNXI_SS_KEY0           0x04
>> +#define SUNXI_SS_KEY1           0x08
>> +#define SUNXI_SS_KEY2           0x0C
>> +#define SUNXI_SS_KEY3           0x10
>> +#define SUNXI_SS_KEY4           0x14
>> +#define SUNXI_SS_KEY5           0x18
>> +#define SUNXI_SS_KEY6           0x1C
>> +#define SUNXI_SS_KEY7           0x20
>> +
>> +#define SUNXI_SS_IV0            0x24
>> +#define SUNXI_SS_IV1            0x28
>> +#define SUNXI_SS_IV2            0x2C
>> +#define SUNXI_SS_IV3            0x30
>> +
>> +#define SUNXI_SS_CNT0           0x34
>> +#define SUNXI_SS_CNT1           0x38
>> +#define SUNXI_SS_CNT2           0x3C
>> +#define SUNXI_SS_CNT3           0x40
>> +
>> +#define SUNXI_SS_FCSR           0x44
>> +#define SUNXI_SS_ICSR           0x48
>> +
>> +#define SUNXI_SS_MD0            0x4C
>> +#define SUNXI_SS_MD1            0x50
>> +#define SUNXI_SS_MD2            0x54
>> +#define SUNXI_SS_MD3            0x58
>> +#define SUNXI_SS_MD4            0x5C
>> +
>> +#define SS_RXFIFO         0x200
>> +#define SS_TXFIFO         0x204
> 
> You don't have much consistency in the register naming scheme, please fix this 
> somehow. I suppose renaming the SS_[RT]XFIFO registers to SUNXI_SS_[RT]XFIFO is 
> a good idea.
> 
>> +/* SUNXI_SS_CTL configuration values */
>> +
>> +/* AES/DES/3DES key select - bits 24-27 */
>> +#define SUNXI_SS_KEYSELECT_KEYN		(0 << 24)
> 
> Uh oh , you ORR some values with this flag, which is always zero. This seems 
> broken.
> 
> [...]
> 
>> +/* SS Method - bits 4-6 */
>> +#define SUNXI_OP_AES                    (0 << 4)
>> +#define SUNXI_OP_DES                    (1 << 4)
>> +#define SUNXI_OP_3DES                   (2 << 4)
>> +#define SUNXI_OP_SHA1                   (3 << 4)
>> +#define SUNXI_OP_MD5                    (4 << 4)
>> +#define SUNXI_OP_PRNG                   (5 << 4)
>> +
>> +/* Data end bit - bit 2 */
>> +#define SUNXI_SS_DATA_END               BIT(2)
> 
> Please use the BIT() macros in consistent fashion. Either use it or don't use it 
> (I'd much rather see it not used), but don't mix the stuff :-(
> 
> [...]
> 
>> +/* General notes:
>> + * I cannot use a key/IV cache because each time one of these change ALL
>> stuff + * need to be re-writed.
>> + * And for example, with dm-crypt IV changes on each request.
>> + *
>> + * After each request the device must be disabled.
> 
> This really isn't quite clear, can you please expand the comment so it's more 
> understandtable ?
> 
>> + * For performance reason, we use writel_relaxed/read_relaxed for all
>> + * operations on RX and TX FIFO.
>> + * For all other registers, we use writel.
>> + * See http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117644
>> + * and http://permalink.gmane.org/gmane.linux.ports.arm.kernel/117640
>> + * */
> 
> I will not review the algos, they need to be AHASH/ABLKCIPHER algos. See the 
> reasoning further down below.
> 
> [...]
> 
>> +/*========================================================================
>> ====*/
>> +/*=======================================================================
>> =====*/ +static struct crypto_alg sunxi_des3_alg = {
>> +	.cra_name = "cbc(des3_ede)",
>> +	.cra_driver_name = "cbc-des3-sunxi-ss",
>> +	.cra_priority = 300,
>> +	.cra_blocksize = DES3_EDE_BLOCK_SIZE,
>> +	.cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
> 
> This must be rewritten to be ABLKCIPHER.
> 
>> +	.cra_ctxsize = sizeof(struct sunxi_req_ctx),
>> +	.cra_module = THIS_MODULE,
>> +	.cra_type = &crypto_blkcipher_type,
>> +	.cra_init = sunxi_des_init,
>> +	.cra_exit = sunxi_des_exit,
>> +	.cra_alignmask = 3,
>> +	.cra_u.blkcipher = {
>> +		.min_keysize    = DES3_EDE_KEY_SIZE,
>> +		.max_keysize    = DES3_EDE_KEY_SIZE,
>> +		.ivsize         = 8,
>> +		.setkey         = sunxi_des3_setkey,
>> +		.encrypt        = sunxi_des3_cbc_encrypt,
>> +		.decrypt        = sunxi_des3_cbc_decrypt,
>> +	}
>> +};
>> +
>> +#endif /* CONFIG_CRYPTO_DEV_SUNXI_SS_3DES */
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
>> +/*========================================================================
>> ====*/
>> +/*=======================================================================
>> =====*/ +static int sunxi_ss_rng_get_random(struct crypto_rng *tfm, u8
>> *rdata, +		unsigned int dlen)
>> +{
>> +	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);
> 
> Use crypto_rng_ctx() here . The cast is plain wrong. I know it works due to how 
> the structures are defined, but this is not right.
> 
>> +	unsigned int i;
>> +	u32 mode = 0;
>> +	u32 v;
>> +
>> +	dev_dbg(ss_ctx->dev, "DEBUG %s dlen=%u\n", __func__, dlen);
>> +
>> +	if (dlen == 0 || rdata == NULL)
>> +		return 0;
> 
> Return -EINVAL or somesuch here, no?
> 
>> +	mode |= SUNXI_OP_PRNG;
>> +	mode |= SUNXI_PRNG_ONESHOT;
>> +	mode |= SUNXI_SS_ENABLED;
>> +
>> +	mutex_lock(&lock);
>> +	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
>> +
>> +	for (i = 0; i < ctx->slen; i += 4) {
>> +		v = *(u32 *)(ctx->seed + i);
> 
> Define the .seed field as u32 if you actually want to access it as u32. You are 
> very lucky you didn't trigger alignment faults with this yet.
> 
>> +		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
>> +	}
> 
> But this debug instrumentation looks quite useless anyway.

As I said in my introduction mail, I cannot get PRNG to work, so it is the reason of lots of dev_dbg()
Anyway, I will remove them.

> 
>> +	for (i = 0; i < ctx->slen && i < 192/8 && i < 16; i += 4) {
>> +		v = *(u32 *)(ctx->seed + i);
>> +		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
>> +		writel(v, ss_ctx->base + SUNXI_SS_KEY0 + i);
>> +	}
>> +
>> +	mode |= SUNXI_PRNG_START;
>> +	writel(mode, ss_ctx->base + SUNXI_SS_CTL);
>> +	for (i = 0; i < 4; i++) {
>> +		v = readl(ss_ctx->base + SUNXI_SS_CTL);
>> +		dev_dbg(ss_ctx->dev, "DEBUG CTL %x %x\n", mode, v);
>> +	}
>> +	for (i = 0; i < dlen && i < 160 / 8; i += 4) {
>> +		v = readl(ss_ctx->base + SUNXI_SS_MD0 + i);
>> +		*(u32 *)(rdata + i) = v;
>> +		dev_dbg(ss_ctx->dev, "DEBUG MD%d %x\n", i, v);
>> +	}
> 
> Drop the debug instrumentation and define the seed buffer as u32.
> 
>> +	writel(0, ss_ctx->base + SUNXI_SS_CTL);
>> +	mutex_unlock(&lock);
>> +	return dlen;
>> +}
>> +
>> +/*========================================================================
>> ====*/
>> +/*=======================================================================
>> =====*/ +static int sunxi_ss_rng_reset(struct crypto_rng *tfm, u8 *seed,
>> +		unsigned int slen)
>> +{
>> +	struct prng_context *ctx = crypto_tfm_ctx((struct crypto_tfm *)tfm);
> 
> Use crypto_rng_ctx() here . The cast is plain wrong.
> 
>> +	dev_dbg(ss_ctx->dev, "DEBUG %s slen=%u\n", __func__, slen);
>> +	memcpy(ctx->seed, seed, slen);
>> +	ctx->slen = slen;
>> +	return 0;
>> +}
> 
> [...]
> 
>> +static int sunxi_ss_probe(struct platform_device *pdev)
>> +{
>> +	struct resource *res;
>> +	u32 v;
>> +	int err;
>> +	unsigned long cr;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	memset(ss_ctx, 0, sizeof(struct sunxi_ss_ctx));
> 
> Static data are always zeroed out by default, no ?
> 
> If you just so accidentally happen to probe() this driver twice, you will get 
> some seriously ugly surprise here, since you zero the data out without checking 
> if the device might have already been probed. A much better idea would be to
> define a static struct sunxi_ss_ctx *ss_ctx; and then
> 
>  if (ss_ctx) {
>   dev_err(&pdev->dev, "Only one instance allowed");
>   return -ENODEV;
>  }
>  ss_ctx = devm_kzalloc(&pdev->dev, sizeof(*ss_ctx), GFP_KERNEL);
> 
> Also note the use of sizeof(*ss_ctx), this will allow for this line to not be 
> changed in case the type of *ss_ctx ever changes in the future. So use it.
> 
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (res == NULL) {
>> +		dev_err(&pdev->dev, "Cannot get the MMIO ressource\n");
>> +		/* TODO PTR_ERR ? */
> 
> Fix the TODOs .
> 
>> +		return -ENXIO;
>> +	}
>> +	ss_ctx->base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(ss_ctx->base)) {
>> +		dev_err(&pdev->dev, "Cannot request MMIO\n");
>> +		return PTR_ERR(ss_ctx->base);
>> +	}
> 
> Actually, this is the pattern. You don't need to check for IS_ERR(res) before 
> calling devm_ioremap_resource().
> 
> res = platform_get_resource();
> base = devm_ioremap_resource(&pdev->dev, res)
> if (IS_ERR(base))
>  return PTR_ERR(base);
> 
>> +	/* TODO Does this information could be usefull ? */
>> +	writel(SUNXI_SS_ENABLED, ss_ctx->base + SUNXI_SS_CTL);
>> +	v = readl(ss_ctx->base + SUNXI_SS_CTL);
>> +	v >>= 16;
>> +	v &= 0x07;
>> +	dev_info(&pdev->dev, "Die ID %d\n", v);
>> +	writel(0, ss_ctx->base + SUNXI_SS_CTL);
> 
> You are configuring hardware before enabling clock for that hardware. That does 
> not seem right.
> 
>> +	ss_ctx->ssclk = devm_clk_get(&pdev->dev, "mod");
>> +	if (IS_ERR(ss_ctx->ssclk)) {
>> +		err = PTR_ERR(ss_ctx->ssclk);
>> +		dev_err(&pdev->dev, "Cannot get SS clock err=%d\n", err);
>> +		return err;
>> +	}
>> +	dev_dbg(&pdev->dev, "clock ss acquired\n");
>> +
>> +	ss_ctx->busclk = devm_clk_get(&pdev->dev, "ahb");
>> +	if (IS_ERR(ss_ctx->busclk)) {
>> +		err = PTR_ERR(ss_ctx->busclk);
>> +		dev_err(&pdev->dev, "Cannot get AHB SS clock err=%d\n", err);
>> +		return err;
>> +	}
>> +	dev_dbg(&pdev->dev, "clock ahb_ss acquired\n");
> 
> These dev_dbg() aren't really useful.
> 
>> +	/* Enable the clocks */
>> +	err = clk_prepare_enable(ss_ctx->busclk);
>> +	if (err != 0) {
>> +		dev_err(&pdev->dev, "Cannot prepare_enable busclk\n");
>> +		return err;
>> +	}
>> +	err = clk_prepare_enable(ss_ctx->ssclk);
>> +	if (err != 0) {
>> +		dev_err(&pdev->dev, "Cannot prepare_enable ssclk\n");
>> +		clk_disable_unprepare(ss_ctx->busclk);
>> +		return err;
>> +	}
>> +
>> +#define	SUNXI_SS_CLOCK_RATE_BUS (24 * 1000 * 1000)
>> +#define	SUNXI_SS_CLOCK_RATE_SS (150 * 1000 * 1000)
> 
> Make this const unsigned long or somesuch, the define is horrid and not 
> typesafe.
> 
>> +	/* Check that clock have the correct rates gived in the datasheet */
>> +	cr = clk_get_rate(ss_ctx->busclk);
>> +	if (cr >= SUNXI_SS_CLOCK_RATE_BUS)
>> +		dev_dbg(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= %u)\n",
>> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
>> +	else
>> +		dev_warn(&pdev->dev, "Clock bus %lu (%lu MHz) (must be >= 
> %u)\n",
>> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
>> +	cr = clk_get_rate(ss_ctx->ssclk);
>> +	if (cr == SUNXI_SS_CLOCK_RATE_SS)
>> +		dev_dbg(&pdev->dev, "Clock ss %lu (%lu MHz) (must be <= %u)\n",
>> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
>> +	else {
>> +		dev_warn(&pdev->dev, "Clock ss is at %lu (%lu MHz) (must be <= 
> %u)\n",
>> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_SS);
>> +		/* Try to set the clock to the maximum allowed */
>> +		err = clk_set_rate(ss_ctx->ssclk, SUNXI_SS_CLOCK_RATE_SS);
>> +		if (err != 0) {
>> +			dev_err(&pdev->dev, "Cannot set clock rate to ssclk\n");
>> +			goto label_error_clock;
>> +		}
>> +		cr = clk_get_rate(ss_ctx->ssclk);
>> +		dev_info(&pdev->dev, "Clock ss set to %lu (%lu MHz) (must be >= 
> %u)\n",
>> +				cr, cr / 1000000, SUNXI_SS_CLOCK_RATE_BUS);
>> +	}
> 
> Just run clk_set_rate() for both and be done with it.
> 
>> +	ss_ctx->buf_in = NULL;
>> +	ss_ctx->buf_in_size = 0;
>> +	ss_ctx->buf_out = NULL;
>> +	ss_ctx->buf_out_size = 0;
> 
> This is useless, you already did that with devm_kzalloc() above.
> 
>> +	ss_ctx->dev = &pdev->dev;
>> +
>> +	mutex_init(&lock);
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_PRNG
>> +	err = crypto_register_alg(&sunxi_ss_prng);
> 
> Use crypto_register_algs() when you get rid of those useless #defines for each 
> algo.
> 
>> +	if (err) {
>> +		dev_err(&pdev->dev, "crypto_register_alg error\n");
>> +		goto label_error_prng;
>> +	} else {
>> +		dev_info(&pdev->dev, "Registred PRNG\n");
> 
> This is useless, you can always check that the thing was probed via /proc/crypto 
> . Please remove this dev_into().
> 
>> +	}
>> +#endif
>> +
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
>> +	err = crypto_register_shash(&sunxi_md5_alg);
> 
> Do not use shash for such device. This is clearly and ahash (and async in 
> general) device. The rule of a thumb here is that you use sync algos only for 
> devices which have dedicated instructions for computing the transformation. For 
> devices which are attached to some kind of bus, you use async algos (ahash etc).
> 
>> +	if (err) {
>> +		dev_err(&pdev->dev, "Fail to register MD5\n");
>> +		goto label_error_md5;
>> +	} else {
>> +		dev_info(&pdev->dev, "Registred MD5\n");
>> +	}
>> +#endif
>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_SHA1
>> +	err = crypto_register_shash(&sunxi_sha1_alg);
> 
> DTTO.
> 
> I am preparing a document for the crypto API, drop me a PM if you want the 
> preliminary version. I don't want to release it until it's more complete as it 
> will only produce more confusion throughout the crypto API than there already 
> is.
> 
> [...]
> 
>> +static const struct of_device_id a20ss_crypto_of_match_table[] = {
>> +	{ .compatible = "allwinner,sun7i-a20-crypto" },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, a20ss_crypto_of_match_table);
> 
> If possible, scrub the a20 nonsense.
> 
>> +static struct platform_driver sunxi_ss_driver = {
>> +	.probe          = sunxi_ss_probe,
>> +	.remove         = sunxi_ss_remove,
>> +	.driver         = {
>> +		.owner          = THIS_MODULE,
>> +		.name           = "sunxi-ss",
>> +		.of_match_table	= a20ss_crypto_of_match_table,
>> +	},
>> +};
>> +
>> +module_platform_driver(sunxi_ss_driver);
>> +
>> +MODULE_DESCRIPTION("Allwinner Security System crypto accelerator");
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Corentin LABBE <clabbe.montjoie@gmail.com>");
> 
> MODULE_ALIAS is missing I think.
> 

Thanks for your review.

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]

On Sat, May 24, 2014 at 09:43:42PM +0200, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:20:03 PM, Tomasz Figa wrote:
> > Hi Marek,
> > 
> > On 24.05.2014 13:21, Marek Vasut wrote:
> > > On Thursday, May 22, 2014 at 05:09:54 PM, LABBE Corentin wrote:
> > > 
> > > Missing commit message. Please fix this and send a V2.
> > > 
> > >> Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
> > >> ---
> > >> 
> > >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +++++++++
> > >>  1 file changed, 9 insertions(+)
> > >>  create mode 100644
> > >>  Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> 
> > >> diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt new file mode
> > >> 100644
> > >> index 0000000..356563b
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
> > >> @@ -0,0 +1,9 @@
> > >> +* Allwinner Security System found on A20 SoC
> > >> +
> > >> +Required properties:
> > >> +- compatible : Should be "allwinner,sun7i-a20-crypto".
> > > 
> > > Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > > that not be described by DT props ?
> > 
> > A widely used convention is to define compatible strings after first
> > SoCs on which particular IP blocks appear. It is quite common among IP
> > blocks for which there is no well defined versioning scheme.
> 
> Well yeah, that's fine. But in this case, "sun7i" is the entire group of CPUs 
> manufactured by AW. I find that information redundant, the "allwinner,a20-
> crypto" would suffice. But I wonder if that IP block might have appeared even 
> earlier ? Or if it is CPU family specific, thus "allwinner,sun7i-crypto" would 
> be a better string ?

No. sun7i-a20-crypto is perfectly fine, and the pattern is used for
all the IPs. sun7i is the SoC family, A20 the actual SoC. In the A20
case, they're equivalent, it's not the case for other Allwinner
SoCs. And I definitely prefer consistency over plain mess. You might
see it differently, but that's not going to change.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/3] dt: Add DT bindings documentation for SUNXI Security System

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]

On Sat, May 24, 2014 at 09:59:30PM +0200, Marek Vasut wrote:
> On Saturday, May 24, 2014 at 09:51:59 PM, Tomasz Figa wrote:
> [...]
> > >>> Why sun7i-a20 ? Is the crypto unit different in other sunxi chips ? Can
> > >>> that not be described by DT props ?
> > >> 
> > >> A widely used convention is to define compatible strings after first
> > >> SoCs on which particular IP blocks appear. It is quite common among IP
> > >> blocks for which there is no well defined versioning scheme.
> > > 
> > > Well yeah, that's fine. But in this case, "sun7i" is the entire group of
> > > CPUs manufactured by AW. I find that information redundant, the
> > > "allwinner,a20- crypto" would suffice. But I wonder if that IP block
> > > might have appeared even earlier ? Or if it is CPU family specific, thus
> > > "allwinner,sun7i-crypto" would be a better string ?
> > 
> > I'm not aware of Allwinner naming schemes too much, so please correct me
> > if I'm wrong, but if A20 implies sun7i, then "allwinner,a20-crypto"
> > would be better indeed.
> 
> True.
> 
> > Whether it was really the first SoC is another thing. Obviously this
> > needs to be checked, although it isn't really that important. For this
> > particular naming scheme you need to specify all the SoCs for which
> > given compatible string can be used for this IP anyway, because there is
> > usually no other source of information about this available (except
> > directly comparing two datasheets...).
> 
> Better get the DT stuff correctly right from the start. That's why I'm asking 
> what chips contains the IP block, so we can guess the right name.

The name is fine, please stop this bikeshedding.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Sunday, May 25, 2014 at 01:58:39 PM, Corentin LABBE wrote:

[...]

> > This is one IP block and it provides 5 algorithms. Put it under one
> > config option please.
> 
> I want to let the user choose what it want to be used. Someone could want
> only to accelerate md5 and to not use the PRNG. Lots of other hw crypto
> driver do the same.

I don't find this useful, most users will enable all of them anyway.

> > Also, just shorted this to CONFIG_CRYPTO_SUNXI_SS , the _DEV stuff in the
> > name is useless.
> 
> I think not, most of cryptographic hardware driver begin with CRYPTO_DEV
> (CRYPTO_DEV_PADLOCK, CRYPTO_DEV_GEODE, CRYPTO_DEV_TALITOS etc...), only
> S390 does not have a _DEV.

OK. I don't mind either way.

> > [...]
> > 
> >> diff --git a/drivers/crypto/sunxi-ss.c b/drivers/crypto/sunxi-ss.c
> >> new file mode 100644
> >> index 0000000..bbf57bc
> >> --- /dev/null
> >> +++ b/drivers/crypto/sunxi-ss.c
> >> @@ -0,0 +1,1476 @@
> >> +/*
> >> + * sunxi-ss.c - hardware cryptographic accelerator for Allwinner A20
> >> SoC
> > 
> > Why can this not be generic Allwinner-all-chips driver ? Does the IP
> > block change that dramatically between the chips?
> 
> As I said in my introduction email, lots of allwinner chips seems to have
> the same crypto device. But since I do not own any of those hardware, and
> in most case does not have a datasheet, I only assume support for A20. I
> will add this comment in the header of the driver.

Can you ask others to test with other chips? Surely, you can easily prepare some 
kind of a test for others to verify on their chips.

[...]

> >> +		dev_dbg(ss_ctx->dev, "DEBUG Seed %d %x\n", i, v);
> >> +	}
> > 
> > But this debug instrumentation looks quite useless anyway.
> 
> As I said in my introduction mail, I cannot get PRNG to work, so it is the
> reason of lots of dev_dbg() Anyway, I will remove them.

Then please don't submit non-functional code for inclusion into kernel. Just 
discard the PRNG part completely until it's operational. Submit just the 
portions of code that are working please.
[...]

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Herbert Xu]

On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
>
> > +	}
> > +#endif
> > +
> > +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> > +	err = crypto_register_shash(&sunxi_md5_alg);
> 
> Do not use shash for such device. This is clearly and ahash (and async in 
> general) device. The rule of a thumb here is that you use sync algos only for 
> devices which have dedicated instructions for computing the transformation. For 
> devices which are attached to some kind of bus, you use async algos (ahash etc).

I'm sorry that I didn't catch this earlier but there is no such
rule.

Unless you need the async interface you should stick to the sync
interfaces for the sake of simplicity.

We have a number of existing drivers that are synchronous but
using the async interface.  They should either be converted
over to the sync interface or made interrupt-driven if possible.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
> On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
> > > +	}
> > > +#endif
> > > +
> > > +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> > > +	err = crypto_register_shash(&sunxi_md5_alg);
> > 
> > Do not use shash for such device. This is clearly and ahash (and async in
> > general) device. The rule of a thumb here is that you use sync algos only
> > for devices which have dedicated instructions for computing the
> > transformation. For devices which are attached to some kind of bus, you
> > use async algos (ahash etc).
> 
> I'm sorry that I didn't catch this earlier but there is no such
> rule.
> 
> Unless you need the async interface you should stick to the sync
> interfaces for the sake of simplicity.
> 
> We have a number of existing drivers that are synchronous but
> using the async interface.  They should either be converted
> over to the sync interface or made interrupt-driven if possible.

Sure, but this device is interrupt driven and uses DMA to feed the crypto 
engine, therefore async, right ?

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Herbert Xu]

On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote:
> On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
> > On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
> > > > +	}
> > > > +#endif
> > > > +
> > > > +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> > > > +	err = crypto_register_shash(&sunxi_md5_alg);
> > > 
> > > Do not use shash for such device. This is clearly and ahash (and async in
> > > general) device. The rule of a thumb here is that you use sync algos only
> > > for devices which have dedicated instructions for computing the
> > > transformation. For devices which are attached to some kind of bus, you
> > > use async algos (ahash etc).
> > 
> > I'm sorry that I didn't catch this earlier but there is no such
> > rule.
> > 
> > Unless you need the async interface you should stick to the sync
> > interfaces for the sake of simplicity.
> > 
> > We have a number of existing drivers that are synchronous but
> > using the async interface.  They should either be converted
> > over to the sync interface or made interrupt-driven if possible.
> 
> Sure, but this device is interrupt driven and uses DMA to feed the crypto 
> engine, therefore async, right ?

If it's interrupt-driven, then yes it would certainly make sense to
be async.  But all I see is polling in the latest posting, was the
first version different?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote:
> On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote:
> > On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
> > > On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
> > > > > +	}
> > > > > +#endif
> > > > > +
> > > > > +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> > > > > +	err = crypto_register_shash(&sunxi_md5_alg);
> > > > 
> > > > Do not use shash for such device. This is clearly and ahash (and
> > > > async in general) device. The rule of a thumb here is that you use
> > > > sync algos only for devices which have dedicated instructions for
> > > > computing the transformation. For devices which are attached to some
> > > > kind of bus, you use async algos (ahash etc).
> > > 
> > > I'm sorry that I didn't catch this earlier but there is no such
> > > rule.
> > > 
> > > Unless you need the async interface you should stick to the sync
> > > interfaces for the sake of simplicity.
> > > 
> > > We have a number of existing drivers that are synchronous but
> > > using the async interface.  They should either be converted
> > > over to the sync interface or made interrupt-driven if possible.
> > 
> > Sure, but this device is interrupt driven and uses DMA to feed the crypto
> > engine, therefore async, right ?
> 
> If it's interrupt-driven, then yes it would certainly make sense to
> be async.  But all I see is polling in the latest posting, was the
> first version different?

I stand corrected then, sorry.

Is it possible to use DMA to feed the crypto accelerator, Corentin?

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Corentin LABBE]

Le 23/07/2014 17:51, Marek Vasut a écrit :
> On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote:
>> On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote:
>>> On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
>>>> On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
>>>>>> +	}
>>>>>> +#endif
>>>>>> +
>>>>>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
>>>>>> +	err = crypto_register_shash(&sunxi_md5_alg);
>>>>>
>>>>> Do not use shash for such device. This is clearly and ahash (and
>>>>> async in general) device. The rule of a thumb here is that you use
>>>>> sync algos only for devices which have dedicated instructions for
>>>>> computing the transformation. For devices which are attached to some
>>>>> kind of bus, you use async algos (ahash etc).
>>>>
>>>> I'm sorry that I didn't catch this earlier but there is no such
>>>> rule.
>>>>
>>>> Unless you need the async interface you should stick to the sync
>>>> interfaces for the sake of simplicity.
>>>>
>>>> We have a number of existing drivers that are synchronous but
>>>> using the async interface.  They should either be converted
>>>> over to the sync interface or made interrupt-driven if possible.
>>>
>>> Sure, but this device is interrupt driven and uses DMA to feed the crypto
>>> engine, therefore async, right ?
>>
>> If it's interrupt-driven, then yes it would certainly make sense to
>> be async.  But all I see is polling in the latest posting, was the
>> first version different?
> 
> I stand corrected then, sorry.
> 
> Is it possible to use DMA to feed the crypto accelerator, Corentin?
> 
> Best regards,
> Marek Vasut
> 

Yes, DMA is possible and will be implemented soon.
So if I have well understood, I keep using async interface.

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Marek Vasut]

On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote:
> Le 23/07/2014 17:51, Marek Vasut a écrit :
> > On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote:
> >> On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote:
> >>> On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
> >>>> On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
> >>>>>> +	}
> >>>>>> +#endif
> >>>>>> +
> >>>>>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> >>>>>> +	err = crypto_register_shash(&sunxi_md5_alg);
> >>>>> 
> >>>>> Do not use shash for such device. This is clearly and ahash (and
> >>>>> async in general) device. The rule of a thumb here is that you use
> >>>>> sync algos only for devices which have dedicated instructions for
> >>>>> computing the transformation. For devices which are attached to some
> >>>>> kind of bus, you use async algos (ahash etc).
> >>>> 
> >>>> I'm sorry that I didn't catch this earlier but there is no such
> >>>> rule.
> >>>> 
> >>>> Unless you need the async interface you should stick to the sync
> >>>> interfaces for the sake of simplicity.
> >>>> 
> >>>> We have a number of existing drivers that are synchronous but
> >>>> using the async interface.  They should either be converted
> >>>> over to the sync interface or made interrupt-driven if possible.
> >>> 
> >>> Sure, but this device is interrupt driven and uses DMA to feed the
> >>> crypto engine, therefore async, right ?
> >> 
> >> If it's interrupt-driven, then yes it would certainly make sense to
> >> be async.  But all I see is polling in the latest posting, was the
> >> first version different?
> > 
> > I stand corrected then, sorry.
> > 
> > Is it possible to use DMA to feed the crypto accelerator, Corentin?
> > 
> > Best regards,
> > Marek Vasut
> 
> Yes, DMA is possible and will be implemented soon.
> So if I have well understood, I keep using async interface.

Yeah, in this case, using DMA and async interface combo is the way to go.

Best regards,
Marek Vasut

Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator

[Herbert Xu]

On Wed, Jul 23, 2014 at 09:38:35PM +0200, Marek Vasut wrote:
> On Wednesday, July 23, 2014 at 08:52:12 PM, Corentin LABBE wrote:
> > Le 23/07/2014 17:51, Marek Vasut a écrit :
> > > On Wednesday, July 23, 2014 at 04:13:09 PM, Herbert Xu wrote:
> > >> On Wed, Jul 23, 2014 at 04:07:20PM +0200, Marek Vasut wrote:
> > >>> On Wednesday, July 23, 2014 at 03:57:35 PM, Herbert Xu wrote:
> > >>>> On Sat, May 24, 2014 at 02:00:03PM +0200, Marek Vasut wrote:
> > >>>>>> +	}
> > >>>>>> +#endif
> > >>>>>> +
> > >>>>>> +#ifdef CONFIG_CRYPTO_DEV_SUNXI_SS_MD5
> > >>>>>> +	err = crypto_register_shash(&sunxi_md5_alg);
> > >>>>> 
> > >>>>> Do not use shash for such device. This is clearly and ahash (and
> > >>>>> async in general) device. The rule of a thumb here is that you use
> > >>>>> sync algos only for devices which have dedicated instructions for
> > >>>>> computing the transformation. For devices which are attached to some
> > >>>>> kind of bus, you use async algos (ahash etc).
> > >>>> 
> > >>>> I'm sorry that I didn't catch this earlier but there is no such
> > >>>> rule.
> > >>>> 
> > >>>> Unless you need the async interface you should stick to the sync
> > >>>> interfaces for the sake of simplicity.
> > >>>> 
> > >>>> We have a number of existing drivers that are synchronous but
> > >>>> using the async interface.  They should either be converted
> > >>>> over to the sync interface or made interrupt-driven if possible.
> > >>> 
> > >>> Sure, but this device is interrupt driven and uses DMA to feed the
> > >>> crypto engine, therefore async, right ?
> > >> 
> > >> If it's interrupt-driven, then yes it would certainly make sense to
> > >> be async.  But all I see is polling in the latest posting, was the
> > >> first version different?
> > > 
> > > I stand corrected then, sorry.
> > > 
> > > Is it possible to use DMA to feed the crypto accelerator, Corentin?
> > > 
> > > Best regards,
> > > Marek Vasut
> > 
> > Yes, DMA is possible and will be implemented soon.
> > So if I have well understood, I keep using async interface.
> 
> Yeah, in this case, using DMA and async interface combo is the way to go.

OK fair enough.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt