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

[Marek Vasut <marex@denx.de>]

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?

Best regards,
Marek Vasut