On Thu, 16 Jun 2016 14:02:42 +0200
Romain Perier <romain.perier@xxxxxxxxxxxxxxxxxx> wrote:
> > 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 ?
Sounds good.
> >
> >> 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 ?
And that's exactly my point :-). When these fields are NULL the request
is a STD request...
>
> >
> >> 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)
... and here you're testing if it's a DMA request, which will always be
false, since mv_cesa_ahash_dma_req_init() is the function supposed to
fill the ->first and ->last fields.
> >
> > 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"...
As explained above, it's not ;).
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel 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
