Re: [PATCH 4/7] crypto: marvell: Moving the tdma chain out of mv_cesa_tdma_req

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

 



Hello,

Le 15/06/2016 22:42, Boris Brezillon a écrit :
On Wed, 15 Jun 2016 21:15:31 +0200
Romain Perier <romain.perier@xxxxxxxxxxxxxxxxxx> wrote:

Actually the only way to access the tdma chain is to use the 'req' union

Currently, ...

ok

Now that the dma specific fields are part of the base request there's no
reason to keep this union.

You can just put struct mv_cesa_req base; directly under struct
mv_cesa_ablkcipher_req, and move mv_cesa_ablkcipher_std_req fields in
mv_cesa_ablkcipher_req.


Well, I think that I might keep the changes related to mv_cesa_tdma_req in this commit (+ put struct mv_cesa_req base; direct under struct mv_cesa_ablkcipher_req) and move the changes related to mv_cesa_ablkcipher_std_req into another commit. What do you think ?


Initialize basereq earlier and pass it as the first argument of
mv_cesa_dma_process().

ok



@@ -174,9 +174,9 @@ static inline void
  mv_cesa_ablkcipher_dma_prepare(struct ablkcipher_request *req)
  {
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma;
+	struct mv_cesa_req *dreq = &creq->req.base;

You named it basereq in mv_cesa_ablkcipher_step(). Try to be
consistent, no matter the name.

ack



-	mv_cesa_dma_prepare(dreq, dreq->base.engine);
+	mv_cesa_dma_prepare(dreq, dreq->engine);
  }

  static inline void
@@ -199,7 +199,7 @@ static inline void mv_cesa_ablkcipher_prepare(struct crypto_async_request *req,
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(ablkreq);
  	creq->req.base.engine = engine;

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
  		mv_cesa_ablkcipher_dma_prepare(ablkreq);
  	else
  		mv_cesa_ablkcipher_std_prepare(ablkreq);
@@ -302,14 +302,13 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
  	struct mv_cesa_ablkcipher_req *creq = ablkcipher_request_ctx(req);
  	gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
  		      GFP_KERNEL : GFP_ATOMIC;
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

ack


@@ -256,9 +256,9 @@ static int mv_cesa_ahash_std_process(struct ahash_request *req, u32 status)
  static inline void mv_cesa_ahash_dma_prepare(struct ahash_request *req)
  {
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
-	struct mv_cesa_tdma_req *dreq = &creq->req.dma.base;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

ack

@@ -340,7 +340,7 @@ static void mv_cesa_ahash_prepare(struct crypto_async_request *req,

  	creq->req.base.engine = engine;

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)
  		mv_cesa_ahash_dma_prepare(ahashreq);
  	else
  		mv_cesa_ahash_std_prepare(ahashreq);
@@ -555,8 +555,7 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
  	gfp_t flags = (req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP) ?
  		      GFP_KERNEL : GFP_ATOMIC;
-	struct mv_cesa_ahash_dma_req *ahashdreq = &creq->req.dma;
-	struct mv_cesa_tdma_req *dreq = &ahashdreq->base;
+	struct mv_cesa_req *dreq = &creq->req.base;

Ditto.

ack


  	struct mv_cesa_ahash_dma_iter iter;
  	struct mv_cesa_op_ctx *op = NULL;
  	unsigned int frag_len;
@@ -662,11 +661,6 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
  	struct mv_cesa_ahash_req *creq = ahash_request_ctx(req);
  	int ret;

-	if (cesa_dev->caps->has_tdma)
-		creq->req.base.type = CESA_DMA_REQ;
-	else
-		creq->req.base.type = CESA_STD_REQ;
-

Hm, where is it decided now? I mean, I don't see this test anywhere
else in your patch, which means you're now always using standard mode.

It has been replaced by mv_cesa_req_get_type() + initializing chain.first to NULL in std_init. So, that's the same thing, no ?


  	creq->src_nents = sg_nents_for_len(req->src, req->nbytes);
  	if (creq->src_nents < 0) {
  		dev_err(cesa_dev->dev, "Invalid number of src SG");
@@ -680,7 +674,7 @@ static int mv_cesa_ahash_req_init(struct ahash_request *req, bool *cached)
  	if (*cached)
  		return 0;

-	if (creq->req.base.type == CESA_DMA_REQ)
+	if (mv_cesa_req_get_type(&creq->req.base) == CESA_DMA_REQ)

Should be

	if (cesa_dev->caps->has_tdma)

  		ret = mv_cesa_ahash_dma_req_init(req);

Why ? mv_cesa_req_get_type() tests mv_cesa_req->chain and returns a code depending on its value. This value is initialized according to what is set un "has_tdma"...


Thanks,
Regards,
Romain
--
Romain Perier, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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