[U-Boot] [PATCH 1/4 v7] Exynos: Add hardware accelerated SHA256 and SHA1

[Akshay Saraswat]

Hi Kim,

>On Mon, 18 Mar 2013 02:06:15 -0400
>Akshay Saraswat <akshay.s at samsung.com> wrote:
>
>> SHA-256 and SHA-1 accelerated using ACE hardware.
>> 
>> Signed-off-by: ARUN MANKUZHI <arun.m at samsung.com>
>> Signed-off-by: Akshay Saraswat <akshay.s at samsung.com>
>> Acked-by: Simon Glass <sjg at chromium.org>
>> ---
>> +++ b/arch/arm/include/asm/arch-exynos/ace_sha.h
>
>ace_sha.h belongs in drivers/crypto/
>
>> +#define ACE_FC_SELBC_AES		(0 << 2)	/* AES */
>> +#define ACE_FC_SELBC_DES		(1 << 2)	/* DES */
>
>nit: comments seem a bit redundant

Few comments look same but they differ.

For example:
"Feed control - BTDMA control" and "Feed control - BRDMA control"

>
>> +#define ACE_FC_BRDMACARPROT_OFS		(2)
>> +#define ACE_FC_BRDMACARCACHE_OFS	(5)
>> +#define ACE_FC_BTDMACAWPROT_OFS		(2)
>> +#define ACE_FC_BTDMACAWCACHE_OFS	(5)
>> +#define ACE_FC_HRDMACARPROT_OFS		(2)
>> +#define ACE_FC_HRDMACARCACHE_OFS	(5)
>> +#define ACE_FC_SRAMOFFSET_MASK		(0xfff)
>
>no parens.
>
>> +/**
>> + * Computes hash value of input pbuf using ACE
>> + *
>> + * @param in_addr	A pointer to the input buffer
>> + * @param bufleni	Byte length of input buffer
>> + * @param out_addr	A pointer to the output buffer. When complete
>> + *			32 bytes are copied to pout[0]...pout[31]. Thus, a user
>> + *			should allocate at least 32 bytes at pOut in advance.
>> + * @param hash_type SHA1 or SHA256
>> + *
>> + * @return		0 on Success, -1 on Failure (Timeout)
>...
>> +	/* Check if status changes within given time limit */
>
>leftovers from timeout implementation?
>
>> +	while ((readl(&ace_sha_reg->hash_status) & ACE_HASH_MSGDONE_MASK) ==
>> +		ACE_HASH_MSGDONE_OFF) {
>> +		/*
>> +		 * PRNG error bit goes HIGH if a PRNG request occurs without
>> +		 * a complete seed setup. We are using this bit to check h/w
>> +		 * because proper setup is not expected in that case.
>> +		 */
>> +		if ((readl(&ace_sha_reg->hash_status)
>> +			& ACE_HASH_PRNGERROR_MASK) == ACE_HASH_PRNGERROR_ON)
>> +			break;
>> +	}
>
>so we have:
>
>whilst (not_done)
>	if (PRNGERROR_ON)
>		break;
>
>...and then success-assuming code flow continues after this.  What
>value is this check for PRNGERROR_ON, if the code isn't going to do
>anything differently?  And what does the status of the RNG have to
>do with keyless hashing?  It sounds like a check for proper
>initialization (RNG got seeded) is in order here, but only if it's
>required for the given hash operations (I doubt it, but the h/w
>might be fussy).
>
>Kim
>

PRNG ERROR means setup was not proper which should be the case
when h/w is faulty because driver does everything in order.
Earlier break was happening which was not correct so changing it to
"return -EBUSY" which means code flow should not be the same as
the one being successfull.


Regards,
Akshay Saraswat