Re: [PATCH cryptodev 1/4] crypto: caam - remove error propagation handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/19/2014 9:01 PM, Marek Vasut wrote:
On Wednesday, March 19, 2014 at 06:25:48 PM, Horia Geantă wrote:
On 3/17/2014 8:23 PM, Marek Vasut wrote:
On Friday, March 14, 2014 at 04:46:49 PM, Horia Geanta wrote:
Commit 61bb86bba169507a5f223b94b9176c32c84b4721
("crypto: caam - set descriptor sharing type to SERIAL")
changed the descriptor sharing mode from SHARE_WAIT to SHARE_SERIAL.

All descriptor commands that handle the "ok to share" and
"error propagation" settings should also go away, since they have no
meaning for SHARE_SERIAL.

[...]

@@ -253,7 +236,7 @@ static int aead_set_sh_desc(struct crypto_aead
*aead)

   	/* assoclen + cryptlen = seqinlen - ivsize */
   	append_math_sub_imm_u32(desc, REG2, SEQINLEN, IMM, tfm->ivsize);

-	/* assoclen + cryptlen = (assoclen + cryptlen) - cryptlen */
+	/* assoclen = (assoclen + cryptlen) - cryptlen */

This comment basically says 'x = x' , but it doesn't explain anything to
uninformed observer. Can you fix such comments please ?

The line under the comment is:
append_math_sub(desc, VARSEQINLEN, REG2, REG3, CAAM_CMD_SZ);

which translates to:
VARSEQINLEN = REG2 - REG3

The comment basically says that VARSEQINLEN gets assoclen by
substracting REG3 = cryptlen from REG2 = assoclen + cryptlen.

If you still think this is "cryptic", that's perfectly fine - I'll
respin the patch.

OK, I don't get it anyway. But that's OK, I am sure the next Marek that comes
across this code won't get it either. So I'd suggest you produce a patch
afterwards, which cleans up the documentation ugliness in this driver. Would
that work for you?

VARSEQINLEN, REG2, REG3 are registers of the crypto engine.
aead_set_sh_desc() is meant to generate microprograms / descriptors to be executed by the engine.

Due to inherent complexity of the engine, it's difficult to explain these microprograms in a few lines of comments.
Having a basic understanding of the HW block is mandatory.

I'll consider your suggestion of improving documentation in a subsequent patch.


Best regards,
Marek Vasut







--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux