Thread (27 messages) 27 messages, 7 authors, 2014-11-16

[linux-sunxi] Re: [PATCH v5 4/4] crypto: Add Allwinner Security System crypto accelerator

From: clabbe.montjoie@gmail.com (Corentin LABBE)
Date: 2014-10-24 18:50:38
Also in: linux-crypto, linux-devicetree, lkml

Le 22/10/2014 11:00, Arnd Bergmann a ?crit :
On Sunday 19 October 2014 16:16:22 LABBE Corentin wrote:
quoted
Add support for the Security System included in Allwinner SoC A20.
The Security System is a hardware cryptographic accelerator that support AES/MD5/SHA1/DES/3DES/PRNG algorithms.

Signed-off-by: LABBE Corentin <clabbe.montjoie@gmail.com>
Please wrap lines in the changelog after about 70 characters.
Oups I just see the corresponding part in submittingpatches.txt
Sorry
quoted
--- /dev/null
+++ b/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
@@ -0,0 +1,489 @@
quoted
+#include "sunxi-ss.h"
+
+extern struct sunxi_ss_ctx *ss;
'extern' declarations belong into header files, not .c files. It would
be even better to avoid this completely and carry the pointer to the
context in an object that gets passed around. In general we want drivers
to be written in a way that allows having multiple instances of the
device, which the global pointer prevents.
As I already said I think the driver will never be used with multiple instance.
But since many people want this pointer dead, I will work on it.
quoted
+
+	src32 = (u32 *)src_addr;
+	dst32 = (u32 *)dst_addr;

You appear to be missing '__iomem' annotations for the mmio pointers.
Please always run your code through the 'sparse' checker using 'make C=1'
to catch and fix this and other erros.
Ok, but with which version of sparse do you have such a warning. I use the 0.5.0 version and I got no warning at all.
quoted
+	ileft = areq->nbytes / 4;
+	oleft = areq->nbytes / 4;
+	i = 0;
+	do {
+		if (ileft > 0 && rx_cnt > 0) {
+			todo = min(rx_cnt, ileft);
+			ileft -= todo;
+			do {
+				writel_relaxed(*src32++,
+						ss->base +
+						SS_RXFIFO);
+				todo--;
+			} while (todo > 0);
+		}
This looks like it should be using writesl() instead of the 
writel_relaxed() loop. That should not only be faster but it will
also change the byte ordering if you are running a big-endian
kernel.

Since this is a FIFO register, the ordering that writesl uses
is likely the correct one.
Great, the code is much cleaner with it. (with up to 10% speed gain)

Thanks

Corentin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help