Messages in this thread | | | From | Marek Vasut <> | Subject | Re: [PATCH 3/3] crypto: Add Allwinner Security System crypto accelerator | Date | Sun, 25 May 2014 16:30:09 +0200 |
| |
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. [...]
| |