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

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

[...]

quoted

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.

quoted

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.

quoted

[...]

quoted

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.

[...]

quoted

quoted

+		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.
[...]