From mboxrd@z Thu Jan  1 00:00:00 1970
From: marex@denx.de (Marek Vasut)
Date: Sun, 25 May 2014 16:30:09 +0200
Subject: [PATCH 3/3] crypto: Add Allwinner Security System crypto
 accelerator
In-Reply-To: <5381DAEF.20007@gmail.com>
References: <1400771396-9686-1-git-send-email-clabbe.montjoie@gmail.com>
 <201405241400.03456.marex@denx.de> <5381DAEF.20007@gmail.com>
Message-ID: <201405251630.09753.marex@denx.de>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

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