[PATCH 03/18] crypto: marvell: add flag to determine algorithm endianness

2015-10-18 Thread Russell King 
Rather than determining whether we're using a MD5 hash by looking at
the digest size, switch to a cleaner solution using a per-request flag
initialised by the method type.

Signed-off-by: Russell King 
---
 drivers/crypto/marvell/cesa.h |  1 +
 drivers/crypto/marvell/hash.c | 17 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
index e19302c9dec9..5d5b66ea2ceb 100644
--- a/drivers/crypto/marvell/cesa.h
+++ b/drivers/crypto/marvell/cesa.h
@@ -612,6 +612,7 @@ struct mv_cesa_ahash_req {
u64 len;
int src_nents;
bool last_req;
+   bool algo_le;
u32 state[8];
 };
 
diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index f59faabcd34f..aa12274608ab 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -351,7 +351,7 @@ static int mv_cesa_ahash_process(struct 
crypto_async_request *req, u32 status)
 * Hardware's MD5 digest is in little endian format, but
 * SHA in big endian format
 */
-   if (digsize == MD5_DIGEST_SIZE) {
+   if (creq->algo_le) {
__le32 *result = (void *)ahashreq->result;
 
for (i = 0; i < digsize / 4; i++)
@@ -407,7 +407,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops = 
{
 };
 
 static int mv_cesa_ahash_init(struct ahash_request *req,
- struct mv_cesa_op_ctx *tmpl)
+ struct mv_cesa_op_ctx *tmpl, bool algo_le)
 {
struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
 
@@ -421,6 +421,7 @@ static int mv_cesa_ahash_init(struct ahash_request *req,
mv_cesa_set_mac_op_frag_len(tmpl, 0);
creq->op_tmpl = *tmpl;
creq->len = 0;
+   creq->algo_le = algo_le;
 
return 0;
 }
@@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
 
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, true);
 
return 0;
 }
@@ -924,7 +925,7 @@ static int mv_cesa_sha1_init(struct ahash_request *req)
 
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA1);
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, false);
 
return 0;
 }
@@ -987,7 +988,7 @@ static int mv_cesa_sha256_init(struct ahash_request *req)
 
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_SHA256);
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, false);
 
return 0;
 }
@@ -1218,7 +1219,7 @@ static int mv_cesa_ahmac_md5_init(struct ahash_request 
*req)
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_HMAC_MD5);
memcpy(tmpl.ctx.hash.iv, ctx->iv, sizeof(ctx->iv));
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, true);
 
return 0;
 }
@@ -1288,7 +1289,7 @@ static int mv_cesa_ahmac_sha1_init(struct ahash_request 
*req)
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_HMAC_SHA1);
memcpy(tmpl.ctx.hash.iv, ctx->iv, sizeof(ctx->iv));
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, false);
 
return 0;
 }
@@ -1378,7 +1379,7 @@ static int mv_cesa_ahmac_sha256_init(struct ahash_request 
*req)
mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_HMAC_SHA256);
memcpy(tmpl.ctx.hash.iv, ctx->iv, sizeof(ctx->iv));
 
-   mv_cesa_ahash_init(req, &tmpl);
+   mv_cesa_ahash_init(req, &tmpl, false);
 
return 0;
 }
-- 
2.1.0

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

Re: [PATCH 03/18] crypto: marvell: add flag to determine algorithm endianness

2015-10-19 Thread Jason Cooper 
Hey Russell,

On Sun, Oct 18, 2015 at 05:23:40PM +0100, Russell King wrote:
> Rather than determining whether we're using a MD5 hash by looking at
> the digest size, switch to a cleaner solution using a per-request flag
> initialised by the method type.
> 
> Signed-off-by: Russell King 
> ---
>  drivers/crypto/marvell/cesa.h |  1 +
>  drivers/crypto/marvell/hash.c | 17 +
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
...
> diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
> index f59faabcd34f..aa12274608ab 100644
> --- a/drivers/crypto/marvell/hash.c
> +++ b/drivers/crypto/marvell/hash.c
> @@ -351,7 +351,7 @@ static int mv_cesa_ahash_process(struct 
> crypto_async_request *req, u32 status)
>* Hardware's MD5 digest is in little endian format, but
>* SHA in big endian format
>*/
> - if (digsize == MD5_DIGEST_SIZE) {
> + if (creq->algo_le) {
>   __le32 *result = (void *)ahashreq->result;
>  
>   for (i = 0; i < digsize / 4; i++)
> @@ -407,7 +407,7 @@ static const struct mv_cesa_req_ops mv_cesa_ahash_req_ops 
> = {
>  };
>  
>  static int mv_cesa_ahash_init(struct ahash_request *req,
> -   struct mv_cesa_op_ctx *tmpl)
> +   struct mv_cesa_op_ctx *tmpl, bool algo_le)

nit: Would it make more sense the do bool algo_endian, and then ...

> @@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
>  
>   mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
>  
> - mv_cesa_ahash_init(req, &tmpl);
> + mv_cesa_ahash_init(req, &tmpl, true);

mv_cesa_ahash_init(req, &tmpl, ALGO_ENDIAN_LITTLE);

'true' doesn't seem as obvious.  But again, nit-picky.

thx,

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

Re: [PATCH 03/18] crypto: marvell: add flag to determine algorithm endianness

2015-10-19 Thread Russell King - ARM Linux 
On Mon, Oct 19, 2015 at 03:04:51PM +, Jason Cooper wrote:
> Hey Russell,
> 
> On Sun, Oct 18, 2015 at 05:23:40PM +0100, Russell King wrote:
> >  static int mv_cesa_ahash_init(struct ahash_request *req,
> > - struct mv_cesa_op_ctx *tmpl)
> > + struct mv_cesa_op_ctx *tmpl, bool algo_le)
> 
> nit: Would it make more sense the do bool algo_endian, and then ...

I don't like "bool"s that don't self-document what they mean.
What does "if (algo_endian)" mean?  It's pretty clear what
"if (algo_le)" means in the context of endianness, though I'll
give you that "if (algo_little_endian)" would be a lot better.

> 
> > @@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
> >  
> > mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
> >  
> > -   mv_cesa_ahash_init(req, &tmpl);
> > +   mv_cesa_ahash_init(req, &tmpl, true);
> 
>   mv_cesa_ahash_init(req, &tmpl, ALGO_ENDIAN_LITTLE);
> 
> 'true' doesn't seem as obvious.  But again, nit-picky.

I did think about:

enum {
ALGO_LITTLE_ENDIAN,
ALGO_BIG_ENDIAN,
};

and passing an int algo_endian around, but that seemed to be like too
much bloat... though if you want to insist, I could make that change.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/18] crypto: marvell: add flag to determine algorithm endianness

2015-10-19 Thread Herbert Xu 
On Mon, Oct 19, 2015 at 04:25:07PM +0100, Russell King - ARM Linux wrote:
>
> > > @@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
> > >  
> > >   mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
> > >  
> > > - mv_cesa_ahash_init(req, &tmpl);
> > > + mv_cesa_ahash_init(req, &tmpl, true);
> > 
> > mv_cesa_ahash_init(req, &tmpl, ALGO_ENDIAN_LITTLE);
> > 
> > 'true' doesn't seem as obvious.  But again, nit-picky.
> 
> I did think about:
> 
> enum {
>   ALGO_LITTLE_ENDIAN,
>   ALGO_BIG_ENDIAN,
> };
> 
> and passing an int algo_endian around, but that seemed to be like too
> much bloat... though if you want to insist, I could make that change.

I think the patch is fine as it is.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/18] crypto: marvell: add flag to determine algorithm endianness

2015-10-19 Thread Jason Cooper 
On Mon, Oct 19, 2015 at 04:25:07PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 19, 2015 at 03:04:51PM +, Jason Cooper wrote:
> > Hey Russell,
> > 
> > On Sun, Oct 18, 2015 at 05:23:40PM +0100, Russell King wrote:
> > >  static int mv_cesa_ahash_init(struct ahash_request *req,
> > > -   struct mv_cesa_op_ctx *tmpl)
> > > +   struct mv_cesa_op_ctx *tmpl, bool algo_le)
> > 
> > nit: Would it make more sense the do bool algo_endian, and then ...
> 
> I don't like "bool"s that don't self-document what they mean.
> What does "if (algo_endian)" mean?  It's pretty clear what

That's where I would go with an enum.  "if (algo_endian ==
ALGO_ENDIAN_LITTLE) ..."

> "if (algo_le)" means in the context of endianness, though I'll
> give you that "if (algo_little_endian)" would be a lot better.

Right, it's really a question of where do we want readability?  I was
focusing on the call site, since the context isn't there for newcomers to
easily grok 'true'.

> > > @@ -861,7 +862,7 @@ static int mv_cesa_md5_init(struct ahash_request *req)
> > >  
> > >   mv_cesa_set_op_cfg(&tmpl, CESA_SA_DESC_CFG_MACM_MD5);
> > >  
> > > - mv_cesa_ahash_init(req, &tmpl);
> > > + mv_cesa_ahash_init(req, &tmpl, true);
> > 
> > mv_cesa_ahash_init(req, &tmpl, ALGO_ENDIAN_LITTLE);
> > 
> > 'true' doesn't seem as obvious.  But again, nit-picky.
> 
> I did think about:
> 
> enum {
>   ALGO_LITTLE_ENDIAN,
>   ALGO_BIG_ENDIAN,
> };
> 
> and passing an int algo_endian around, but that seemed to be like too
> much bloat... though if you want to insist, I could make that change.

Like I said, it's a nit.  Either a self-documenting bool, or an enum
will work.


thx,

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