Hi Pascal,
On Fri, Jul 26, 2019 at 12:57:21PM +0000, Pascal Van Leeuwen wrote:
> > On Fri, Jul 05, 2019 at 08:49:22AM +0200, Pascal van Leeuwen wrote:
> > > Signed-off-by: Pascal van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>
> >
> > Could you provide a commit message, explaining briefly what the patch is
> > doing?
> >
> I initially figured that to be redundant if the subject already covered it completely.
> But now that I think of it, it's possible the subject does not end up in the commit
> at all ... if that is the case, would it work if I just copy-paste the relevant part of the
> subject message? Or do I need to be more verbose?
The subject will be the commit title. I know sometimes the commit
message is trivial or redundant, but it's still a good practice to
always have one (and many maintainers will ask for one). Even if it's
only two lines :)
> > > @@ -199,6 +201,15 @@ static int safexcel_aead_aes_setkey(struct crypto_aead *ctfm, const u8 *key,
> > > goto badkey;
> > >
> > > /* Encryption key */
> > > + if (ctx->alg == SAFEXCEL_3DES) {
> > > + flags = crypto_aead_get_flags(ctfm);
> > > + err = __des3_verify_key(&flags, keys.enckey);
> > > + crypto_aead_set_flags(ctfm, flags);
> >
> > You could use directly des3_verify_key() which does exactly this.
> >
> Actually, I couldn't due to des3_verify_key expecting a struct crypto_skcipher as input,
> and not a struct crypto_aead, that's why I had to do it this way ...
I see. Maybe a good way would be to provide a function taking
'struct crypto_aead' as an argument so that not every single driver
reimplement the same logic. But this can come later if needed.
> > > +struct safexcel_alg_template safexcel_alg_authenc_hmac_sha1_cbc_des3_ede = {
> > > + .type = SAFEXCEL_ALG_TYPE_AEAD,
> >
> > You either missed to fill .engines member of this struct, or this series
> > is based on another one not merged yet.
> >
> Yes, that happened in the patchset of which v2 did not make it to the mailing list ...
:)
So in general if there's a dependency you should say so in the cover
letter.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
