2015-08-21 16:17:08

by Fabio Estevam

[permalink] [raw]
Subject: [PATCH v2 1/3] crypto: caam: ctrl: Fix the error handling

From: Fabio Estevam <[email protected]>

In the error path we should disable the resources that were previously
acquired, so fix the error handling accordingly.

Signed-off-by: Fabio Estevam <[email protected]>
---
Changes since v1:
- None

drivers/crypto/caam/ctrl.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 81b552d..9c5ca46 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -474,27 +474,27 @@ static int caam_probe(struct platform_device *pdev)
ret = clk_prepare_enable(ctrlpriv->caam_ipg);
if (ret < 0) {
dev_err(&pdev->dev, "can't enable CAAM ipg clock: %d\n", ret);
- return -ENODEV;
+ return ret;
}

ret = clk_prepare_enable(ctrlpriv->caam_mem);
if (ret < 0) {
dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
ret);
- return -ENODEV;
+ goto disable_caam_ipg;
}

ret = clk_prepare_enable(ctrlpriv->caam_aclk);
if (ret < 0) {
dev_err(&pdev->dev, "can't enable CAAM aclk clock: %d\n", ret);
- return -ENODEV;
+ goto disable_caam_mem;
}

ret = clk_prepare_enable(ctrlpriv->caam_emi_slow);
if (ret < 0) {
dev_err(&pdev->dev, "can't enable CAAM emi slow clock: %d\n",
ret);
- return -ENODEV;
+ goto disable_caam_aclk;
}

/* Get configuration properties from device tree */
@@ -502,7 +502,7 @@ static int caam_probe(struct platform_device *pdev)
ctrl = of_iomap(nprop, 0);
if (ctrl == NULL) {
dev_err(dev, "caam: of_iomap() failed\n");
- return -ENOMEM;
+ goto disable_caam_emi_slow;
}
/* Finding the page size for using the CTPR_MS register */
comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ms);
@@ -586,8 +586,8 @@ static int caam_probe(struct platform_device *pdev)
sizeof(struct platform_device *) * rspec,
GFP_KERNEL);
if (ctrlpriv->jrpdev == NULL) {
- iounmap(ctrl);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto iounmap_ctrl;
}

ring = 0;
@@ -627,8 +627,8 @@ static int caam_probe(struct platform_device *pdev)
/* If no QI and no rings specified, quit and go home */
if ((!ctrlpriv->qi_present) && (!ctrlpriv->total_jobrs)) {
dev_err(dev, "no queues configured, terminating\n");
- caam_remove(pdev);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto caam_remove;
}

cha_vid_ls = rd_reg32(&ctrl->perfmon.cha_id_ls);
@@ -685,8 +685,7 @@ static int caam_probe(struct platform_device *pdev)
} while ((ret == -EAGAIN) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
if (ret) {
dev_err(dev, "failed to instantiate RNG");
- caam_remove(pdev);
- return ret;
+ goto caam_remove;
}
/*
* Set handles init'ed by this module as the complement of the
@@ -790,6 +789,20 @@ static int caam_probe(struct platform_device *pdev)
&ctrlpriv->ctl_tdsk_wrap);
#endif
return 0;
+
+caam_remove:
+ caam_remove(pdev);
+iounmap_ctrl:
+ iounmap(ctrl);
+disable_caam_emi_slow:
+ clk_disable_unprepare(ctrlpriv->caam_emi_slow);
+disable_caam_aclk:
+ clk_disable_unprepare(ctrlpriv->caam_aclk);
+disable_caam_mem:
+ clk_disable_unprepare(ctrlpriv->caam_mem);
+disable_caam_ipg:
+ clk_disable_unprepare(ctrlpriv->caam_ipg);
+ return ret;
}

static struct of_device_id caam_match[] = {
--
1.9.1


2015-08-21 16:17:12

by Fabio Estevam

[permalink] [raw]
Subject: [PATCH v2 3/3] crypto: caam: Use the preferred style for memory allocations

From: Fabio Estevam <[email protected]>

From Documentation/CodingStyle:

"The preferred form for passing a size of a struct is the following:

p = kmalloc(sizeof(*p), ...);
....

The preferred form for allocating a zeroed array is the following:

p = kcalloc(n, sizeof(...), ...); "

,so do as suggested.

Signed-off-by: Fabio Estevam <[email protected]>
---
Changes since v1:
- Consolidate patches 3 and 4 into a single one (Horia Geantă)
- Fix all the occurrences inside drivers/crypto/caam (Horia Geantă)

drivers/crypto/caam/caamalg.c | 2 +-
drivers/crypto/caam/caamhash.c | 2 +-
drivers/crypto/caam/caamrng.c | 2 +-
drivers/crypto/caam/ctrl.c | 8 +++-----
drivers/crypto/caam/jr.c | 12 +++++-------
5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 83d2306..ba79d63 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -4314,7 +4314,7 @@ static struct caam_crypto_alg *caam_alg_alloc(struct caam_alg_template
struct caam_crypto_alg *t_alg;
struct crypto_alg *alg;

- t_alg = kzalloc(sizeof(struct caam_crypto_alg), GFP_KERNEL);
+ t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
pr_err("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 6c14261..94433b9 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1842,7 +1842,7 @@ caam_hash_alloc(struct caam_hash_template *template,
struct ahash_alg *halg;
struct crypto_alg *alg;

- t_alg = kzalloc(sizeof(struct caam_hash_alg), GFP_KERNEL);
+ t_alg = kzalloc(sizeof(*t_alg), GFP_KERNEL);
if (!t_alg) {
pr_err("failed to allocate t_alg\n");
return ERR_PTR(-ENOMEM);
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index fb0cc54..9b92af2 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -351,7 +351,7 @@ static int __init caam_rng_init(void)
pr_err("Job Ring Device allocation for transform failed\n");
return PTR_ERR(dev);
}
- rng_ctx = kmalloc(sizeof(struct caam_rng_ctx), GFP_DMA);
+ rng_ctx = kmalloc(sizeof(*rng_ctx), GFP_DMA);
if (!rng_ctx) {
err = -ENOMEM;
goto free_caam_alloc;
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 1153417..c9c5892 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -424,8 +424,7 @@ static int caam_probe(struct platform_device *pdev)
int pg_size;
int BLOCK_OFFSET = 0;

- ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(struct caam_drv_private),
- GFP_KERNEL);
+ ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL);
if (!ctrlpriv)
return -ENOMEM;

@@ -582,9 +581,8 @@ static int caam_probe(struct platform_device *pdev)
of_device_is_compatible(np, "fsl,sec4.0-job-ring"))
rspec++;

- ctrlpriv->jrpdev = devm_kzalloc(&pdev->dev,
- sizeof(struct platform_device *) * rspec,
- GFP_KERNEL);
+ ctrlpriv->jrpdev = devm_kcalloc(&pdev->dev, rspec,
+ sizeof(*ctrlpriv->jrpdev), GFP_KERNEL);
if (ctrlpriv->jrpdev == NULL) {
ret = -ENOMEM;
goto iounmap_ctrl;
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index b7ec1ad..f7e0d8d 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -410,18 +410,17 @@ static int caam_jr_init(struct device *dev)
goto out_free_irq;

error = -ENOMEM;
- jrp->inpring = dma_alloc_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
- &inpbusaddr, GFP_KERNEL);
+ jrp->inpring = dma_alloc_coherent(dev, sizeof(*jrp->inpring) *
+ JOBR_DEPTH, &inpbusaddr, GFP_KERNEL);
if (!jrp->inpring)
goto out_free_irq;

- jrp->outring = dma_alloc_coherent(dev, sizeof(struct jr_outentry) *
+ jrp->outring = dma_alloc_coherent(dev, sizeof(*jrp->outring) *
JOBR_DEPTH, &outbusaddr, GFP_KERNEL);
if (!jrp->outring)
goto out_free_inpring;

- jrp->entinfo = kzalloc(sizeof(struct caam_jrentry_info) * JOBR_DEPTH,
- GFP_KERNEL);
+ jrp->entinfo = kcalloc(JOBR_DEPTH, sizeof(*jrp->entinfo), GFP_KERNEL);
if (!jrp->entinfo)
goto out_free_outring;

@@ -479,8 +478,7 @@ static int caam_jr_probe(struct platform_device *pdev)
int error;

jrdev = &pdev->dev;
- jrpriv = devm_kmalloc(jrdev, sizeof(struct caam_drv_private_jr),
- GFP_KERNEL);
+ jrpriv = devm_kmalloc(jrdev, sizeof(*jrpriv), GFP_KERNEL);
if (!jrpriv)
return -ENOMEM;

--
1.9.1

2015-08-21 16:17:10

by Fabio Estevam

[permalink] [raw]
Subject: [PATCH v2 2/3] crypto: caam: ctrl: Propagate the real error code

From: Fabio Estevam <[email protected]>

Instead of propagating a 'fake' error code, just propagate the real
one in the case of caam_drv_identify_clk() failure.

Signed-off-by: Fabio Estevam <[email protected]>
---
Changes since v1:
- None

drivers/crypto/caam/ctrl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 9c5ca46..1153417 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -440,7 +440,7 @@ static int caam_probe(struct platform_device *pdev)
ret = PTR_ERR(clk);
dev_err(&pdev->dev,
"can't identify CAAM ipg clk: %d\n", ret);
- return -ENODEV;
+ return ret;
}
ctrlpriv->caam_ipg = clk;

@@ -449,7 +449,7 @@ static int caam_probe(struct platform_device *pdev)
ret = PTR_ERR(clk);
dev_err(&pdev->dev,
"can't identify CAAM mem clk: %d\n", ret);
- return -ENODEV;
+ return ret;
}
ctrlpriv->caam_mem = clk;

@@ -458,7 +458,7 @@ static int caam_probe(struct platform_device *pdev)
ret = PTR_ERR(clk);
dev_err(&pdev->dev,
"can't identify CAAM aclk clk: %d\n", ret);
- return -ENODEV;
+ return ret;
}
ctrlpriv->caam_aclk = clk;

@@ -467,7 +467,7 @@ static int caam_probe(struct platform_device *pdev)
ret = PTR_ERR(clk);
dev_err(&pdev->dev,
"can't identify CAAM emi_slow clk: %d\n", ret);
- return -ENODEV;
+ return ret;
}
ctrlpriv->caam_emi_slow = clk;

--
1.9.1

2015-08-21 16:38:18

by Horia Geantă

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] crypto: caam: ctrl: Fix the error handling

On 8/21/2015 7:16 PM, Fabio Estevam wrote:
> From: Fabio Estevam <[email protected]>
>
> In the error path we should disable the resources that were previously
> acquired, so fix the error handling accordingly.
>
> Signed-off-by: Fabio Estevam <[email protected]>

For the series:
Reviewed-by: Horia Geantă <[email protected]>

Thanks!
Horia

2015-08-21 16:43:17

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] crypto: caam: ctrl: Fix the error handling

On Fri, Aug 21, 2015 at 1:16 PM, Fabio Estevam <[email protected]> wrote:
> From: Fabio Estevam <[email protected]>
>
> In the error path we should disable the resources that were previously
> acquired, so fix the error handling accordingly.
>
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> Changes since v1:
> - None
>
> drivers/crypto/caam/ctrl.c | 35 ++++++++++++++++++++++++-----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index 81b552d..9c5ca46 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -474,27 +474,27 @@ static int caam_probe(struct platform_device *pdev)
> ret = clk_prepare_enable(ctrlpriv->caam_ipg);
> if (ret < 0) {
> dev_err(&pdev->dev, "can't enable CAAM ipg clock: %d\n", ret);
> - return -ENODEV;
> + return ret;
> }
>
> ret = clk_prepare_enable(ctrlpriv->caam_mem);
> if (ret < 0) {
> dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
> ret);
> - return -ENODEV;
> + goto disable_caam_ipg;
> }
>
> ret = clk_prepare_enable(ctrlpriv->caam_aclk);
> if (ret < 0) {
> dev_err(&pdev->dev, "can't enable CAAM aclk clock: %d\n", ret);
> - return -ENODEV;
> + goto disable_caam_mem;
> }
>
> ret = clk_prepare_enable(ctrlpriv->caam_emi_slow);
> if (ret < 0) {
> dev_err(&pdev->dev, "can't enable CAAM emi slow clock: %d\n",
> ret);
> - return -ENODEV;
> + goto disable_caam_aclk;
> }
>
> /* Get configuration properties from device tree */
> @@ -502,7 +502,7 @@ static int caam_probe(struct platform_device *pdev)
> ctrl = of_iomap(nprop, 0);
> if (ctrl == NULL) {
> dev_err(dev, "caam: of_iomap() failed\n");
> - return -ENOMEM;
> + goto disable_caam_emi_slow;

Ops, I missed a 'ret = -ENOMEM' here.

Will send a v3.