[PATCH v4 00/16] crypto: caam - Add i.MX8MQ support

[PATCH v4 00/16] crypto: caam - Add i.MX8MQ support

[Andrey Smirnov]

Everyone:

Picking up where Chris left off (I chatted with him privately
beforehead), this series adds support for i.MX8MQ to CAAM driver. Just
like [v1], this series is i.MX8MQ only.

Feedback is welcome!
Thanks,
Andrey Smirnov

Changes since [v3]:

  - Patchset changed to select DMA size at runtime in order to enable
    support for both i.MX8MQ and Layerscape at the same time. I only
    tested the patches on i.MX6,7 and 8MQ, since I don't have access
    to any of the Layerscape HW. Any help in that regard would be
    appareciated.

  - Bulk clocks and their number are now stored as a part of struct
    caam_drv_private to simplify allocation and cleanup code (no
    special context needed)
    
  - Renamed 'soc_attr' -> 'imx_soc_match' for clarity

Changes since [v2]:

  - Dropped "crypto: caam - do not initialise clocks on the i.MX8" and
    replaced it with "crypto: caam - simplfy clock initialization" and 
    "crypto: caam - add clock entry for i.MX8MQ"


Changes since [v1]

  - Series reworked to continue using register based interface for
    queueing RNG initialization job, dropping "crypto: caam - use job
    ring for RNG instantiation instead of DECO"

  - Added a patch to share DMA mask selection code

  - Added missing Signed-off-by for authors of original NXP tree
    commits that this sereis is based on

[v3] lore.kernel.org/r/20190617160339.29179-1-andrew.smirnov@gmail.com
[v2] lore.kernel.org/r/20190607200225.21419-1-andrew.smirnov@gmail.com
[v1] https://patchwork.kernel.org/cover/10825625/


Andrey Smirnov (16):
  crypto: caam - move DMA mask selection into a function
  crypto: caam - simplfy clock initialization
  crypto: caam - move tasklet_init() call down
  crypto: caam - use deveres to allocate 'entinfo'
  crypto: caam - use devres to allocate 'outring'
  crypto: caam - use devres to allocate 'inpring'
  crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64
  crypto: caam - use ioread64*_hi_lo in rd_reg64
  crypto: caam - drop 64-bit only wr/rd_reg64()
  crypto: caam - make CAAM_PTR_SZ dynamic
  crypto: caam - move cpu_to_caam_dma() selection to runtime
  crypto: caam - drop explicit usage of struct jr_outentry
  crypto: caam - don't hardcode inpentry size
  crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  crypto: caam - always select job ring via RSR on i.MX8MQ
  crypto: caam - add clock entry for i.MX8MQ

 drivers/crypto/caam/caamalg.c     |   2 +-
 drivers/crypto/caam/caamhash.c    |   2 +-
 drivers/crypto/caam/caampkc.c     |   8 +-
 drivers/crypto/caam/caamrng.c     |   2 +-
 drivers/crypto/caam/ctrl.c        | 224 ++++++++++++++----------------
 drivers/crypto/caam/desc_constr.h |  20 ++-
 drivers/crypto/caam/error.c       |   3 +
 drivers/crypto/caam/intern.h      |  32 ++++-
 drivers/crypto/caam/jr.c          |  67 +++------
 drivers/crypto/caam/pdb.h         |  16 ++-
 drivers/crypto/caam/pkc_desc.c    |   8 +-
 drivers/crypto/caam/regs.h        | 139 ++++++++++++------
 12 files changed, 294 insertions(+), 229 deletions(-)

-- 
2.21.0

[PATCH v4 01/16] crypto: caam - move DMA mask selection into a function

[Andrey Smirnov]

Exactly the same code to figure out DMA mask is repeated twice in the
driver code. To avoid repetition, move that logic into a standalone
subroutine in intern.h. While at it re-shuffle the code to make it
more readable with early returns.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c   | 11 +----------
 drivers/crypto/caam/intern.h | 20 ++++++++++++++++++++
 drivers/crypto/caam/jr.c     | 15 +--------------
 3 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 4e43ca4d3656..e674d8770cdb 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -688,16 +688,7 @@ static int caam_probe(struct platform_device *pdev)
 			      JRSTART_JR1_START | JRSTART_JR2_START |
 			      JRSTART_JR3_START);
 
-	if (sizeof(dma_addr_t) == sizeof(u64)) {
-		if (caam_dpaa2)
-			ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(49));
-		else if (of_device_is_compatible(nprop, "fsl,sec-v5.0"))
-			ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
-		else
-			ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
-	} else {
-		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-	}
+	ret = dma_set_mask_and_coherent(dev, caam_get_dma_mask(dev));
 	if (ret) {
 		dev_err(dev, "dma_set_mask_and_coherent failed (%d)\n", ret);
 		goto iounmap_ctrl;
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 6af84bbc612c..ec25d260fa40 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -10,6 +10,8 @@
 #ifndef INTERN_H
 #define INTERN_H
 
+#include "ctrl.h"
+
 /* Currently comes from Kconfig param as a ^2 (driver-required) */
 #define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE)
 
@@ -215,4 +217,22 @@ DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u32_ro, caam_debugfs_u32_get, NULL, "%llu\n");
 DEFINE_SIMPLE_ATTRIBUTE(caam_fops_u64_ro, caam_debugfs_u64_get, NULL, "%llu\n");
 #endif
 
+static inline u64 caam_get_dma_mask(struct device *dev)
+{
+	struct device_node *nprop = dev->of_node;
+
+	if (sizeof(dma_addr_t) != sizeof(u64))
+		return DMA_BIT_MASK(32);
+
+	if (caam_dpaa2)
+		return DMA_BIT_MASK(49);
+
+	if (of_device_is_compatible(nprop, "fsl,sec-v5.0-job-ring") ||
+	    of_device_is_compatible(nprop, "fsl,sec-v5.0"))
+		return DMA_BIT_MASK(40);
+
+	return DMA_BIT_MASK(36);
+}
+
+
 #endif /* INTERN_H */
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index cea811fed320..4b25b2fa3d02 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -543,20 +543,7 @@ static int caam_jr_probe(struct platform_device *pdev)
 
 	jrpriv->rregs = (struct caam_job_ring __iomem __force *)ctrl;
 
-	if (sizeof(dma_addr_t) == sizeof(u64)) {
-		if (caam_dpaa2)
-			error = dma_set_mask_and_coherent(jrdev,
-							  DMA_BIT_MASK(49));
-		else if (of_device_is_compatible(nprop,
-						 "fsl,sec-v5.0-job-ring"))
-			error = dma_set_mask_and_coherent(jrdev,
-							  DMA_BIT_MASK(40));
-		else
-			error = dma_set_mask_and_coherent(jrdev,
-							  DMA_BIT_MASK(36));
-	} else {
-		error = dma_set_mask_and_coherent(jrdev, DMA_BIT_MASK(32));
-	}
+	error = dma_set_mask_and_coherent(jrdev, caam_get_dma_mask(jrdev));
 	if (error) {
 		dev_err(jrdev, "dma_set_mask_and_coherent failed (%d)\n",
 			error);
-- 
2.21.0

[PATCH v4 02/16] crypto: caam - simplify clock initialization

[Andrey Smirnov]

Simplify clock initialization code by converting it to use clk-bulk,
devres and soc_device_match() match table. No functional change
intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c   | 203 +++++++++++++++++------------------
 drivers/crypto/caam/intern.h |   7 +-
 2 files changed, 98 insertions(+), 112 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e674d8770cdb..908d3ecf6d1c 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -25,16 +25,6 @@ EXPORT_SYMBOL(caam_dpaa2);
 #include "qi.h"
 #endif
 
-/*
- * i.MX targets tend to have clock control subsystems that can
- * enable/disable clocking to our device.
- */
-static inline struct clk *caam_drv_identify_clk(struct device *dev,
-						char *clk_name)
-{
-	return caam_imx ? devm_clk_get(dev, clk_name) : NULL;
-}
-
 /*
  * Descriptor to instantiate RNG State Handle 0 in normal mode and
  * load the JDKEK, TDKEK and TDSK registers
@@ -342,13 +332,6 @@ static int caam_remove(struct platform_device *pdev)
 	/* Unmap controller region */
 	iounmap(ctrl);
 
-	/* shut clocks off before finalizing shutdown */
-	clk_disable_unprepare(ctrlpriv->caam_ipg);
-	if (ctrlpriv->caam_mem)
-		clk_disable_unprepare(ctrlpriv->caam_mem);
-	clk_disable_unprepare(ctrlpriv->caam_aclk);
-	if (ctrlpriv->caam_emi_slow)
-		clk_disable_unprepare(ctrlpriv->caam_emi_slow);
 	return 0;
 }
 
@@ -497,20 +480,102 @@ static const struct of_device_id caam_match[] = {
 };
 MODULE_DEVICE_TABLE(of, caam_match);
 
+struct caam_imx_data {
+	const struct clk_bulk_data *clks;
+	int num_clks;
+};
+
+static const struct clk_bulk_data caam_imx6_clks[] = {
+	{ .id = "ipg" },
+	{ .id = "mem" },
+	{ .id = "aclk" },
+	{ .id = "emi_slow" },
+};
+
+static const struct caam_imx_data caam_imx6_data = {
+	.clks = caam_imx6_clks,
+	.num_clks = ARRAY_SIZE(caam_imx6_clks),
+};
+
+static const struct clk_bulk_data caam_imx7_clks[] = {
+	{ .id = "ipg" },
+	{ .id = "aclk" },
+};
+
+static const struct caam_imx_data caam_imx7_data = {
+	.clks = caam_imx7_clks,
+	.num_clks = ARRAY_SIZE(caam_imx7_clks),
+};
+
+static const struct clk_bulk_data caam_imx6ul_clks[] = {
+	{ .id = "ipg" },
+	{ .id = "mem" },
+	{ .id = "aclk" },
+};
+
+static const struct caam_imx_data caam_imx6ul_data = {
+	.clks = caam_imx6ul_clks,
+	.num_clks = ARRAY_SIZE(caam_imx6ul_clks),
+};
+
+static const struct soc_device_attribute caam_imx_soc_table[] = {
+	{ .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
+	{ .soc_id = "i.MX6*",  .data = &caam_imx6_data },
+	{ .soc_id = "i.MX7*",  .data = &caam_imx7_data },
+	{ .family = "Freescale i.MX" },
+};
+
+static void disable_clocks(void *data)
+{
+	struct caam_drv_private *ctrlpriv = data;
+
+	clk_bulk_disable_unprepare(ctrlpriv->num_clks, ctrlpriv->clks);
+}
+
+static int init_clocks(struct device *dev,
+		       struct caam_drv_private *ctrlpriv,
+		       const struct caam_imx_data *data)
+{
+	int ret;
+
+	ctrlpriv->num_clks = data->num_clks;
+	ctrlpriv->clks = devm_kmemdup(dev, data->clks,
+				      data->num_clks * sizeof(data->clks[0]),
+				      GFP_KERNEL);
+	if (!ctrlpriv->clks)
+		return -ENOMEM;
+
+	ret = devm_clk_bulk_get(dev, ctrlpriv->num_clks, ctrlpriv->clks);
+	if (ret) {
+		dev_err(dev,
+			"Failed to request all necessary clocks\n");
+		return ret;
+	}
+
+	ret = clk_bulk_prepare_enable(ctrlpriv->num_clks, ctrlpriv->clks);
+	if (ret) {
+		dev_err(dev,
+			"Failed to prepare/enable all necessary clocks\n");
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, disable_clocks, ctrlpriv);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
 	int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
 	u64 caam_id;
-	static const struct soc_device_attribute imx_soc[] = {
-		{.family = "Freescale i.MX"},
-		{},
-	};
+	const struct soc_device_attribute *imx_soc_match;
 	struct device *dev;
 	struct device_node *nprop, *np;
 	struct caam_ctrl __iomem *ctrl;
 	struct caam_drv_private *ctrlpriv;
-	struct clk *clk;
 #ifdef CONFIG_DEBUG_FS
 	struct caam_perfmon *perfmon;
 #endif
@@ -527,91 +592,25 @@ static int caam_probe(struct platform_device *pdev)
 	dev_set_drvdata(dev, ctrlpriv);
 	nprop = pdev->dev.of_node;
 
-	caam_imx = (bool)soc_device_match(imx_soc);
-
-	/* Enable clocking */
-	clk = caam_drv_identify_clk(&pdev->dev, "ipg");
-	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		dev_err(&pdev->dev,
-			"can't identify CAAM ipg clk: %d\n", ret);
-		return ret;
-	}
-	ctrlpriv->caam_ipg = clk;
-
-	if (!of_machine_is_compatible("fsl,imx7d") &&
-	    !of_machine_is_compatible("fsl,imx7s") &&
-	    !of_machine_is_compatible("fsl,imx7ulp")) {
-		clk = caam_drv_identify_clk(&pdev->dev, "mem");
-		if (IS_ERR(clk)) {
-			ret = PTR_ERR(clk);
-			dev_err(&pdev->dev,
-				"can't identify CAAM mem clk: %d\n", ret);
-			return ret;
+	imx_soc_match = soc_device_match(caam_imx_soc_table);
+	if (imx_soc_match) {
+		if (!imx_soc_match->data) {
+			dev_err(dev, "No clock data provided for i.MX SoC");
+			return -EINVAL;
 		}
-		ctrlpriv->caam_mem = clk;
-	}
 
-	clk = caam_drv_identify_clk(&pdev->dev, "aclk");
-	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		dev_err(&pdev->dev,
-			"can't identify CAAM aclk clk: %d\n", ret);
-		return ret;
-	}
-	ctrlpriv->caam_aclk = clk;
-
-	if (!of_machine_is_compatible("fsl,imx6ul") &&
-	    !of_machine_is_compatible("fsl,imx7d") &&
-	    !of_machine_is_compatible("fsl,imx7s") &&
-	    !of_machine_is_compatible("fsl,imx7ulp")) {
-		clk = caam_drv_identify_clk(&pdev->dev, "emi_slow");
-		if (IS_ERR(clk)) {
-			ret = PTR_ERR(clk);
-			dev_err(&pdev->dev,
-				"can't identify CAAM emi_slow clk: %d\n", ret);
+		ret = init_clocks(dev, ctrlpriv, imx_soc_match->data);
+		if (ret)
 			return ret;
-		}
-		ctrlpriv->caam_emi_slow = clk;
-	}
-
-	ret = clk_prepare_enable(ctrlpriv->caam_ipg);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "can't enable CAAM ipg clock: %d\n", ret);
-		return ret;
-	}
-
-	if (ctrlpriv->caam_mem) {
-		ret = clk_prepare_enable(ctrlpriv->caam_mem);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
-				ret);
-			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);
-		goto disable_caam_mem;
-	}
-
-	if (ctrlpriv->caam_emi_slow) {
-		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);
-			goto disable_caam_aclk;
-		}
 	}
+	caam_imx = (bool)imx_soc_match;
 
 	/* Get configuration properties from device tree */
 	/* First, get register page */
 	ctrl = of_iomap(nprop, 0);
 	if (ctrl == NULL) {
 		dev_err(dev, "caam: of_iomap() failed\n");
-		ret = -ENOMEM;
-		goto disable_caam_emi_slow;
+		return -ENOMEM;
 	}
 
 	caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
@@ -899,16 +898,6 @@ static int caam_probe(struct platform_device *pdev)
 #endif
 iounmap_ctrl:
 	iounmap(ctrl);
-disable_caam_emi_slow:
-	if (ctrlpriv->caam_emi_slow)
-		clk_disable_unprepare(ctrlpriv->caam_emi_slow);
-disable_caam_aclk:
-	clk_disable_unprepare(ctrlpriv->caam_aclk);
-disable_caam_mem:
-	if (ctrlpriv->caam_mem)
-		clk_disable_unprepare(ctrlpriv->caam_mem);
-disable_caam_ipg:
-	clk_disable_unprepare(ctrlpriv->caam_ipg);
 	return ret;
 }
 
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index ec25d260fa40..1f01703f510a 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -94,11 +94,8 @@ struct caam_drv_private {
 				   Handles of the RNG4 block are initialized
 				   by this driver */
 
-	struct clk *caam_ipg;
-	struct clk *caam_mem;
-	struct clk *caam_aclk;
-	struct clk *caam_emi_slow;
-
+	struct clk_bulk_data *clks;
+	int num_clks;
 	/*
 	 * debugfs entries for developer view into driver/device
 	 * variables at runtime.
-- 
2.21.0

Re: [PATCH v4 02/16] crypto: caam - simplify clock initialization

[Leonard Crestez]

On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
> Simplify clock initialization code by converting it to use clk-bulk,
> devres and soc_device_match() match table. No functional change
> intended.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>

This data-driven approach is much easier to understand.

Re: [PATCH v4 02/16] crypto: caam - simplify clock initialization

[Iuliana Prodan]

On 7/3/2019 11:15 AM, Andrey Smirnov wrote:
> Simplify clock initialization code by converting it to use clk-bulk,
> devres and soc_device_match() match table. No functional change
> intended.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Spencer <christopher.spencer@sea.co.uk>
> Cc: Cory Tusar <cory.tusar@zii.aero>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Horia Geantă <horia.geanta@nxp.com>
> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
> Cc: Leonard Crestez <leonard.crestez@nxp.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>   drivers/crypto/caam/ctrl.c   | 203 +++++++++++++++++------------------
>   drivers/crypto/caam/intern.h |   7 +-
>   2 files changed, 98 insertions(+), 112 deletions(-)
> 
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index e674d8770cdb..908d3ecf6d1c 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -25,16 +25,6 @@ EXPORT_SYMBOL(caam_dpaa2);
>   #include "qi.h"
>   #endif
>   
> -/*
> - * i.MX targets tend to have clock control subsystems that can
> - * enable/disable clocking to our device.
> - */
> -static inline struct clk *caam_drv_identify_clk(struct device *dev,
> -						char *clk_name)
> -{
> -	return caam_imx ? devm_clk_get(dev, clk_name) : NULL;
> -}
> -
>   /*
>    * Descriptor to instantiate RNG State Handle 0 in normal mode and
>    * load the JDKEK, TDKEK and TDSK registers
> @@ -342,13 +332,6 @@ static int caam_remove(struct platform_device *pdev)
>   	/* Unmap controller region */
>   	iounmap(ctrl);
>   
> -	/* shut clocks off before finalizing shutdown */
> -	clk_disable_unprepare(ctrlpriv->caam_ipg);
> -	if (ctrlpriv->caam_mem)
> -		clk_disable_unprepare(ctrlpriv->caam_mem);
> -	clk_disable_unprepare(ctrlpriv->caam_aclk);
> -	if (ctrlpriv->caam_emi_slow)
> -		clk_disable_unprepare(ctrlpriv->caam_emi_slow);
>   	return 0;
>   }
>   
> @@ -497,20 +480,102 @@ static const struct of_device_id caam_match[] = {
>   };
>   MODULE_DEVICE_TABLE(of, caam_match);
>   
> +struct caam_imx_data {
> +	const struct clk_bulk_data *clks;
> +	int num_clks;
> +};
> +
> +static const struct clk_bulk_data caam_imx6_clks[] = {
> +	{ .id = "ipg" },
> +	{ .id = "mem" },
> +	{ .id = "aclk" },
> +	{ .id = "emi_slow" },
> +};
> +
> +static const struct caam_imx_data caam_imx6_data = {
> +	.clks = caam_imx6_clks,
> +	.num_clks = ARRAY_SIZE(caam_imx6_clks),
> +};
> +
> +static const struct clk_bulk_data caam_imx7_clks[] = {
> +	{ .id = "ipg" },
> +	{ .id = "aclk" },
> +};
> +
> +static const struct caam_imx_data caam_imx7_data = {
> +	.clks = caam_imx7_clks,
> +	.num_clks = ARRAY_SIZE(caam_imx7_clks),
> +};
> +
> +static const struct clk_bulk_data caam_imx6ul_clks[] = {
> +	{ .id = "ipg" },
> +	{ .id = "mem" },
> +	{ .id = "aclk" },
> +};
> +
> +static const struct caam_imx_data caam_imx6ul_data = {
> +	.clks = caam_imx6ul_clks,
> +	.num_clks = ARRAY_SIZE(caam_imx6ul_clks),
> +};
> +
> +static const struct soc_device_attribute caam_imx_soc_table[] = {
> +	{ .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
> +	{ .soc_id = "i.MX6*",  .data = &caam_imx6_data },
> +	{ .soc_id = "i.MX7*",  .data = &caam_imx7_data },
> +	{ .family = "Freescale i.MX" },
> +};

You need to add a {/* sentinel */} in caam_imx_soc_table, otherwise will 
crash for other than i.MX targets, when trying to identify the SoC.

> +
> +static void disable_clocks(void *data)
> +{
> +	struct caam_drv_private *ctrlpriv = data;
> +
> +	clk_bulk_disable_unprepare(ctrlpriv->num_clks, ctrlpriv->clks);
> +}
> +
> +static int init_clocks(struct device *dev,
> +		       struct caam_drv_private *ctrlpriv,
> +		       const struct caam_imx_data *data)
> +{
> +	int ret;
> +
> +	ctrlpriv->num_clks = data->num_clks;
> +	ctrlpriv->clks = devm_kmemdup(dev, data->clks,
> +				      data->num_clks * sizeof(data->clks[0]),
> +				      GFP_KERNEL);
> +	if (!ctrlpriv->clks)
> +		return -ENOMEM;
> +
> +	ret = devm_clk_bulk_get(dev, ctrlpriv->num_clks, ctrlpriv->clks);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to request all necessary clocks\n");
> +		return ret;
> +	}
> +
> +	ret = clk_bulk_prepare_enable(ctrlpriv->num_clks, ctrlpriv->clks);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to prepare/enable all necessary clocks\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, disable_clocks, ctrlpriv);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>   /* Probe routine for CAAM top (controller) level */
>   static int caam_probe(struct platform_device *pdev)
>   {
>   	int ret, ring, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
>   	u64 caam_id;
> -	static const struct soc_device_attribute imx_soc[] = {
> -		{.family = "Freescale i.MX"},
> -		{},
> -	};
> +	const struct soc_device_attribute *imx_soc_match;
>   	struct device *dev;
>   	struct device_node *nprop, *np;
>   	struct caam_ctrl __iomem *ctrl;
>   	struct caam_drv_private *ctrlpriv;
> -	struct clk *clk;
>   #ifdef CONFIG_DEBUG_FS
>   	struct caam_perfmon *perfmon;
>   #endif
> @@ -527,91 +592,25 @@ static int caam_probe(struct platform_device *pdev)
>   	dev_set_drvdata(dev, ctrlpriv);
>   	nprop = pdev->dev.of_node;
>   
> -	caam_imx = (bool)soc_device_match(imx_soc);
> -
> -	/* Enable clocking */
> -	clk = caam_drv_identify_clk(&pdev->dev, "ipg");
> -	if (IS_ERR(clk)) {
> -		ret = PTR_ERR(clk);
> -		dev_err(&pdev->dev,
> -			"can't identify CAAM ipg clk: %d\n", ret);
> -		return ret;
> -	}
> -	ctrlpriv->caam_ipg = clk;
> -
> -	if (!of_machine_is_compatible("fsl,imx7d") &&
> -	    !of_machine_is_compatible("fsl,imx7s") &&
> -	    !of_machine_is_compatible("fsl,imx7ulp")) {
> -		clk = caam_drv_identify_clk(&pdev->dev, "mem");
> -		if (IS_ERR(clk)) {
> -			ret = PTR_ERR(clk);
> -			dev_err(&pdev->dev,
> -				"can't identify CAAM mem clk: %d\n", ret);
> -			return ret;
> +	imx_soc_match = soc_device_match(caam_imx_soc_table);
> +	if (imx_soc_match) {
> +		if (!imx_soc_match->data) {
> +			dev_err(dev, "No clock data provided for i.MX SoC");
> +			return -EINVAL;
>   		}
> -		ctrlpriv->caam_mem = clk;
> -	}
>   
> -	clk = caam_drv_identify_clk(&pdev->dev, "aclk");
> -	if (IS_ERR(clk)) {
> -		ret = PTR_ERR(clk);
> -		dev_err(&pdev->dev,
> -			"can't identify CAAM aclk clk: %d\n", ret);
> -		return ret;
> -	}
> -	ctrlpriv->caam_aclk = clk;
> -
> -	if (!of_machine_is_compatible("fsl,imx6ul") &&
> -	    !of_machine_is_compatible("fsl,imx7d") &&
> -	    !of_machine_is_compatible("fsl,imx7s") &&
> -	    !of_machine_is_compatible("fsl,imx7ulp")) {
> -		clk = caam_drv_identify_clk(&pdev->dev, "emi_slow");
> -		if (IS_ERR(clk)) {
> -			ret = PTR_ERR(clk);
> -			dev_err(&pdev->dev,
> -				"can't identify CAAM emi_slow clk: %d\n", ret);
> +		ret = init_clocks(dev, ctrlpriv, imx_soc_match->data);
> +		if (ret)
>   			return ret;
> -		}
> -		ctrlpriv->caam_emi_slow = clk;
> -	}
> -
> -	ret = clk_prepare_enable(ctrlpriv->caam_ipg);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "can't enable CAAM ipg clock: %d\n", ret);
> -		return ret;
> -	}
> -
> -	if (ctrlpriv->caam_mem) {
> -		ret = clk_prepare_enable(ctrlpriv->caam_mem);
> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "can't enable CAAM secure mem clock: %d\n",
> -				ret);
> -			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);
> -		goto disable_caam_mem;
> -	}
> -
> -	if (ctrlpriv->caam_emi_slow) {
> -		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);
> -			goto disable_caam_aclk;
> -		}
>   	}
> +	caam_imx = (bool)imx_soc_match;
>   
>   	/* Get configuration properties from device tree */
>   	/* First, get register page */
>   	ctrl = of_iomap(nprop, 0);
>   	if (ctrl == NULL) {
>   		dev_err(dev, "caam: of_iomap() failed\n");
> -		ret = -ENOMEM;
> -		goto disable_caam_emi_slow;
> +		return -ENOMEM;
>   	}
>   
>   	caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
> @@ -899,16 +898,6 @@ static int caam_probe(struct platform_device *pdev)
>   #endif
>   iounmap_ctrl:
>   	iounmap(ctrl);
> -disable_caam_emi_slow:
> -	if (ctrlpriv->caam_emi_slow)
> -		clk_disable_unprepare(ctrlpriv->caam_emi_slow);
> -disable_caam_aclk:
> -	clk_disable_unprepare(ctrlpriv->caam_aclk);
> -disable_caam_mem:
> -	if (ctrlpriv->caam_mem)
> -		clk_disable_unprepare(ctrlpriv->caam_mem);
> -disable_caam_ipg:
> -	clk_disable_unprepare(ctrlpriv->caam_ipg);
>   	return ret;
>   }
>   
> diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
> index ec25d260fa40..1f01703f510a 100644
> --- a/drivers/crypto/caam/intern.h
> +++ b/drivers/crypto/caam/intern.h
> @@ -94,11 +94,8 @@ struct caam_drv_private {
>   				   Handles of the RNG4 block are initialized
>   				   by this driver */
>   
> -	struct clk *caam_ipg;
> -	struct clk *caam_mem;
> -	struct clk *caam_aclk;
> -	struct clk *caam_emi_slow;
> -
> +	struct clk_bulk_data *clks;
> +	int num_clks;
>   	/*
>   	 * debugfs entries for developer view into driver/device
>   	 * variables at runtime.
> 

Iulia

Re: [PATCH v4 02/16] crypto: caam - simplify clock initialization

[Andrey Smirnov]

On Thu, Jul 4, 2019 at 8:43 AM Iuliana Prodan <iuliana.prodan@nxp.com> wrote:
>
> On 7/3/2019 11:15 AM, Andrey Smirnov wrote:
> > Simplify clock initialization code by converting it to use clk-bulk,
> > devres and soc_device_match() match table. No functional change
> > intended.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > Cc: Chris Spencer <christopher.spencer@sea.co.uk>
> > Cc: Cory Tusar <cory.tusar@zii.aero>
> > Cc: Chris Healy <cphealy@gmail.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Horia Geantă <horia.geanta@nxp.com>
> > Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
> > Cc: Leonard Crestez <leonard.crestez@nxp.com>
> > Cc: linux-crypto@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >   drivers/crypto/caam/ctrl.c   | 203 +++++++++++++++++------------------
> >   drivers/crypto/caam/intern.h |   7 +-
> >   2 files changed, 98 insertions(+), 112 deletions(-)
> >
> > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> > index e674d8770cdb..908d3ecf6d1c 100644
> > --- a/drivers/crypto/caam/ctrl.c
> > +++ b/drivers/crypto/caam/ctrl.c
> > @@ -25,16 +25,6 @@ EXPORT_SYMBOL(caam_dpaa2);
> >   #include "qi.h"
> >   #endif
> >
> > -/*
> > - * i.MX targets tend to have clock control subsystems that can
> > - * enable/disable clocking to our device.
> > - */
> > -static inline struct clk *caam_drv_identify_clk(struct device *dev,
> > -                                             char *clk_name)
> > -{
> > -     return caam_imx ? devm_clk_get(dev, clk_name) : NULL;
> > -}
> > -
> >   /*
> >    * Descriptor to instantiate RNG State Handle 0 in normal mode and
> >    * load the JDKEK, TDKEK and TDSK registers
> > @@ -342,13 +332,6 @@ static int caam_remove(struct platform_device *pdev)
> >       /* Unmap controller region */
> >       iounmap(ctrl);
> >
> > -     /* shut clocks off before finalizing shutdown */
> > -     clk_disable_unprepare(ctrlpriv->caam_ipg);
> > -     if (ctrlpriv->caam_mem)
> > -             clk_disable_unprepare(ctrlpriv->caam_mem);
> > -     clk_disable_unprepare(ctrlpriv->caam_aclk);
> > -     if (ctrlpriv->caam_emi_slow)
> > -             clk_disable_unprepare(ctrlpriv->caam_emi_slow);
> >       return 0;
> >   }
> >
> > @@ -497,20 +480,102 @@ static const struct of_device_id caam_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, caam_match);
> >
> > +struct caam_imx_data {
> > +     const struct clk_bulk_data *clks;
> > +     int num_clks;
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx6_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "mem" },
> > +     { .id = "aclk" },
> > +     { .id = "emi_slow" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx6_data = {
> > +     .clks = caam_imx6_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx6_clks),
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx7_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "aclk" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx7_data = {
> > +     .clks = caam_imx7_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx7_clks),
> > +};
> > +
> > +static const struct clk_bulk_data caam_imx6ul_clks[] = {
> > +     { .id = "ipg" },
> > +     { .id = "mem" },
> > +     { .id = "aclk" },
> > +};
> > +
> > +static const struct caam_imx_data caam_imx6ul_data = {
> > +     .clks = caam_imx6ul_clks,
> > +     .num_clks = ARRAY_SIZE(caam_imx6ul_clks),
> > +};
> > +
> > +static const struct soc_device_attribute caam_imx_soc_table[] = {
> > +     { .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
> > +     { .soc_id = "i.MX6*",  .data = &caam_imx6_data },
> > +     { .soc_id = "i.MX7*",  .data = &caam_imx7_data },
> > +     { .family = "Freescale i.MX" },
> > +};
>
> You need to add a {/* sentinel */} in caam_imx_soc_table, otherwise will
> crash for other than i.MX targets, when trying to identify the SoC.
>

Ugh, definitely, my bad. Thanks for catching this!

Thanks,
Andrey Smirnov

[PATCH v4 03/16] crypto: caam - move tasklet_init() call down

[Andrey Smirnov]

Move tasklet_init() call further down in order to simplify error path
cleanup. No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/jr.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 4b25b2fa3d02..a7ca2bbe243f 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
 
 	jrp = dev_get_drvdata(dev);
 
-	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
-
 	/* Connect job ring interrupt handler. */
 	error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
 			    dev_name(dev), dev);
 	if (error) {
 		dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
 			jrp->ridx, jrp->irq);
-		goto out_kill_deq;
+		return error;
 	}
 
 	error = caam_reset_hw_jr(dev);
@@ -471,6 +469,8 @@ static int caam_jr_init(struct device *dev)
 	if (!jrp->entinfo)
 		goto out_free_outring;
 
+	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
+
 	for (i = 0; i < JOBR_DEPTH; i++)
 		jrp->entinfo[i].desc_addr_dma = !0;
 
@@ -504,8 +504,6 @@ static int caam_jr_init(struct device *dev)
 	dev_err(dev, "can't allocate job rings for %d\n", jrp->ridx);
 out_free_irq:
 	free_irq(jrp->irq, dev);
-out_kill_deq:
-	tasklet_kill(&jrp->irqtask);
 	return error;
 }
 
-- 
2.21.0

Re: [PATCH v4 03/16] crypto: caam - move tasklet_init() call down

[Leonard Crestez]

On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
> Move tasklet_init() call further down in order to simplify error path
> cleanup. No functional change intended.
> 
> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> index 4b25b2fa3d02..a7ca2bbe243f 100644
> --- a/drivers/crypto/caam/jr.c
> +++ b/drivers/crypto/caam/jr.c
> @@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
>   
>   	jrp = dev_get_drvdata(dev);
>   
> -	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
> -
>   	/* Connect job ring interrupt handler. */
>   	error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
>   			    dev_name(dev), dev);
>   	if (error) {
>   		dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
>   			jrp->ridx, jrp->irq);
> -		goto out_kill_deq;
> +		return error;
>   	}

The caam_jr_interrupt handler can schedule the tasklet so it makes sense 
to have it be initialized ahead of request_irq. In theory it's possible 
for an interrupt to be triggered immediately when request_irq is called.

I'm not very familiar with the CAAM ip, can you ensure no interrupts are 
pending in HW at probe time? The "no functional change" part is not obvious.

--
Regards,
Leonard

Re: [PATCH v4 03/16] crypto: caam - move tasklet_init() call down

[Andrey Smirnov]

On Wed, Jul 3, 2019 at 6:51 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
> > Move tasklet_init() call further down in order to simplify error path
> > cleanup. No functional change intended.
> >
> > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> > index 4b25b2fa3d02..a7ca2bbe243f 100644
> > --- a/drivers/crypto/caam/jr.c
> > +++ b/drivers/crypto/caam/jr.c
> > @@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
> >
> >       jrp = dev_get_drvdata(dev);
> >
> > -     tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
> > -
> >       /* Connect job ring interrupt handler. */
> >       error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
> >                           dev_name(dev), dev);
> >       if (error) {
> >               dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
> >                       jrp->ridx, jrp->irq);
> > -             goto out_kill_deq;
> > +             return error;
> >       }
>
> The caam_jr_interrupt handler can schedule the tasklet so it makes sense
> to have it be initialized ahead of request_irq. In theory it's possible
> for an interrupt to be triggered immediately when request_irq is called.
>
> I'm not very familiar with the CAAM ip, can you ensure no interrupts are
> pending in HW at probe time? The "no functional change" part is not obvious.
>

Said tasklet will use both jrp->outring and jrp->entinfo array
initialized after IRQ request call in both versions of the code
(before/after this patch). AFAICT, the only case where this patch
would change initialization safety of the original code is if JR was
scheduled somehow while ORSFx is 0 (no jobs done), which I don't think
is possible.

Thanks,
Andrey Smirnov

Re: [PATCH v4 03/16] crypto: caam - move tasklet_init() call down

[Leonard Crestez]

On 7/3/2019 8:14 PM, Andrey Smirnov wrote:
> On Wed, Jul 3, 2019 at 6:51 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>> On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
>>> Move tasklet_init() call further down in order to simplify error path
>>> cleanup. No functional change intended.
>>>
>>> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
>>> index 4b25b2fa3d02..a7ca2bbe243f 100644
>>> --- a/drivers/crypto/caam/jr.c
>>> +++ b/drivers/crypto/caam/jr.c
>>> @@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
>>>
>>>        jrp = dev_get_drvdata(dev);
>>>
>>> -     tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
>>> -
>>>        /* Connect job ring interrupt handler. */
>>>        error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
>>>                            dev_name(dev), dev);
>>>        if (error) {
>>>                dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
>>>                        jrp->ridx, jrp->irq);
>>> -             goto out_kill_deq;
>>> +             return error;
>>>        }
>>
>> The caam_jr_interrupt handler can schedule the tasklet so it makes sense
>> to have it be initialized ahead of request_irq. In theory it's possible
>> for an interrupt to be triggered immediately when request_irq is called.
>>
>> I'm not very familiar with the CAAM ip, can you ensure no interrupts are
>> pending in HW at probe time? The "no functional change" part is not obvious.
>>
> 
> Said tasklet will use both jrp->outring and jrp->entinfo array
> initialized after IRQ request call in both versions of the code
> (before/after this patch). AFAICT, the only case where this patch
> would change initialization safety of the original code is if JR was
> scheduled somehow while ORSFx is 0 (no jobs done), which I don't think
> is possible.

I took a second look at caam_jr_init and there is apparently a whole 
bunch of other reset/init stuff done after request_irq. For example 
caam_reset_hw_jr is done after request_irq and masks interrupts?

What I'd expect is that request_irq is done last after all other 
initialization is performed. But I'm not familiar with how CAAM JRs work 
so feel free to ignore this.

--
Regards,
Leonard

Re: [PATCH v4 03/16] crypto: caam - move tasklet_init() call down

[Horia Geanta]

On 7/4/2019 2:45 AM, Leonard Crestez wrote:
> On 7/3/2019 8:14 PM, Andrey Smirnov wrote:
>> On Wed, Jul 3, 2019 at 6:51 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>>> On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
>>>> Move tasklet_init() call further down in order to simplify error path
>>>> cleanup. No functional change intended.
>>>>
>>>> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
>>>> index 4b25b2fa3d02..a7ca2bbe243f 100644
>>>> --- a/drivers/crypto/caam/jr.c
>>>> +++ b/drivers/crypto/caam/jr.c
>>>> @@ -441,15 +441,13 @@ static int caam_jr_init(struct device *dev)
>>>>
>>>>        jrp = dev_get_drvdata(dev);
>>>>
>>>> -     tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
>>>> -
>>>>        /* Connect job ring interrupt handler. */
>>>>        error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
>>>>                            dev_name(dev), dev);
>>>>        if (error) {
>>>>                dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
>>>>                        jrp->ridx, jrp->irq);
>>>> -             goto out_kill_deq;
>>>> +             return error;
>>>>        }
>>>
>>> The caam_jr_interrupt handler can schedule the tasklet so it makes sense
>>> to have it be initialized ahead of request_irq. In theory it's possible
>>> for an interrupt to be triggered immediately when request_irq is called.
>>>
>>> I'm not very familiar with the CAAM ip, can you ensure no interrupts are
>>> pending in HW at probe time? The "no functional change" part is not obvious.
>>>
Actually there is a previous report (in i.MX BSP) of CAAM job ring generating
an interrupt at probe time, between request_irq() and reset:
https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/crypto/caam?h=imx_4.14.98_2.0.0_ga&id=aa7b3f51971ec1f60f41fe8ea71870b215376b8a

So yes, there might be cases when interrupts are pending.

>>
>> Said tasklet will use both jrp->outring and jrp->entinfo array
>> initialized after IRQ request call in both versions of the code
>> (before/after this patch). AFAICT, the only case where this patch
>> would change initialization safety of the original code is if JR was
>> scheduled somehow while ORSFx is 0 (no jobs done), which I don't think
>> is possible.
> 
> I took a second look at caam_jr_init and there is apparently a whole 
> bunch of other reset/init stuff done after request_irq. For example 
> caam_reset_hw_jr is done after request_irq and masks interrupts?
> 
> What I'd expect is that request_irq is done last after all other 
> initialization is performed. But I'm not familiar with how CAAM JRs work 
> so feel free to ignore this.
> 
There's some history here... (which is in contradiction with above-mentioned
report).

Commit 9620fd959fb1 ("crypto: caam - handle interrupt lines shared across rings")
moved request_irq() before JR reset:
"
    - resetting a job ring triggers an interrupt, so move request_irq
    prior to jr_reset to avoid 'got IRQ but nobody cared' messages.
"

but IIUC that ("resetting a job ring triggers an interrupt") was actually
due to disabling the IRQ line using disable_irq() instead of masking
the interrupt in HW using JRCFGR_LS[IMSK].

The way JR reset sequence is implemented now - in caam_reset_hw_jr() - should not
trigger any interrupt.

Thus, it should be safe to move request_irq() after everything is set up,
at the end of probing.

My suggestion is to move both tasklet_init() and request_irq() at the bottom
of the probe callback.
However, I'd say this is a fix that should be marked accordingly and
should be posted either separately, or at the top of the patch set.

Thanks,
Horia

[PATCH v4 04/16] crypto: caam - use deveres to allocate 'entinfo'

[Andrey Smirnov]

Use deveres to allocate 'entinfo' and drop corresponding call to
kfree(). No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/jr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index a7ca2bbe243f..fc7deb445aa8 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -125,7 +125,6 @@ static int caam_jr_shutdown(struct device *dev)
 			  jrp->inpring, inpbusaddr);
 	dma_free_coherent(dev, sizeof(struct jr_outentry) * JOBR_DEPTH,
 			  jrp->outring, outbusaddr);
-	kfree(jrp->entinfo);
 
 	return ret;
 }
@@ -465,7 +464,8 @@ static int caam_jr_init(struct device *dev)
 	if (!jrp->outring)
 		goto out_free_inpring;
 
-	jrp->entinfo = kcalloc(JOBR_DEPTH, sizeof(*jrp->entinfo), GFP_KERNEL);
+	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
+				    GFP_KERNEL);
 	if (!jrp->entinfo)
 		goto out_free_outring;
 
-- 
2.21.0

[PATCH v4 05/16] crypto: caam - use devres to allocate 'outring'

[Andrey Smirnov]

Use devres to allocate 'outring' and drop corresponding call to
dma_free_coherent() as well as extra references to 'struct
jr_outentry' (needed in following commits). No functional change
inteded.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/jr.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index fc7deb445aa8..1eaa91dcc146 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -108,7 +108,7 @@ static int caam_reset_hw_jr(struct device *dev)
 static int caam_jr_shutdown(struct device *dev)
 {
 	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
-	dma_addr_t inpbusaddr, outbusaddr;
+	dma_addr_t inpbusaddr;
 	int ret;
 
 	ret = caam_reset_hw_jr(dev);
@@ -120,11 +120,8 @@ static int caam_jr_shutdown(struct device *dev)
 
 	/* Free rings */
 	inpbusaddr = rd_reg64(&jrp->rregs->inpring_base);
-	outbusaddr = rd_reg64(&jrp->rregs->outring_base);
 	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
 			  jrp->inpring, inpbusaddr);
-	dma_free_coherent(dev, sizeof(struct jr_outentry) * JOBR_DEPTH,
-			  jrp->outring, outbusaddr);
 
 	return ret;
 }
@@ -459,15 +456,16 @@ static int caam_jr_init(struct device *dev)
 	if (!jrp->inpring)
 		goto out_free_irq;
 
-	jrp->outring = dma_alloc_coherent(dev, sizeof(*jrp->outring) *
-					  JOBR_DEPTH, &outbusaddr, GFP_KERNEL);
+	jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
+					   JOBR_DEPTH, &outbusaddr,
+					   GFP_KERNEL);
 	if (!jrp->outring)
 		goto out_free_inpring;
 
 	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
 				    GFP_KERNEL);
 	if (!jrp->entinfo)
-		goto out_free_outring;
+		return -ENOMEM;
 
 	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
 
@@ -495,9 +493,6 @@ static int caam_jr_init(struct device *dev)
 
 	return 0;
 
-out_free_outring:
-	dma_free_coherent(dev, sizeof(struct jr_outentry) * JOBR_DEPTH,
-			  jrp->outring, outbusaddr);
 out_free_inpring:
 	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
 			  jrp->inpring, inpbusaddr);
-- 
2.21.0

Re: [PATCH v4 05/16] crypto: caam - use devres to allocate 'outring'

[Horia Geanta]

On 7/3/2019 11:14 AM, Andrey Smirnov wrote:
> Use devres to allocate 'outring' and drop corresponding call to
> dma_free_coherent() as well as extra references to 'struct
> jr_outentry' (needed in following commits). No functional change
> inteded.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Spencer <christopher.spencer@sea.co.uk>
> Cc: Cory Tusar <cory.tusar@zii.aero>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Horia Geantă <horia.geanta@nxp.com>
> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
> Cc: Leonard Crestez <leonard.crestez@nxp.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/crypto/caam/jr.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> index fc7deb445aa8..1eaa91dcc146 100644
> --- a/drivers/crypto/caam/jr.c
> +++ b/drivers/crypto/caam/jr.c
> @@ -108,7 +108,7 @@ static int caam_reset_hw_jr(struct device *dev)
>  static int caam_jr_shutdown(struct device *dev)
>  {
>  	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
> -	dma_addr_t inpbusaddr, outbusaddr;
> +	dma_addr_t inpbusaddr;
>  	int ret;
>  
>  	ret = caam_reset_hw_jr(dev);
> @@ -120,11 +120,8 @@ static int caam_jr_shutdown(struct device *dev)
>  
>  	/* Free rings */
>  	inpbusaddr = rd_reg64(&jrp->rregs->inpring_base);
> -	outbusaddr = rd_reg64(&jrp->rregs->outring_base);
>  	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
>  			  jrp->inpring, inpbusaddr);
> -	dma_free_coherent(dev, sizeof(struct jr_outentry) * JOBR_DEPTH,
> -			  jrp->outring, outbusaddr);
>  
>  	return ret;
>  }
> @@ -459,15 +456,16 @@ static int caam_jr_init(struct device *dev)
>  	if (!jrp->inpring)
>  		goto out_free_irq;
>  
> -	jrp->outring = dma_alloc_coherent(dev, sizeof(*jrp->outring) *
> -					  JOBR_DEPTH, &outbusaddr, GFP_KERNEL);
> +	jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
> +					   JOBR_DEPTH, &outbusaddr,
> +					   GFP_KERNEL);
>  	if (!jrp->outring)
>  		goto out_free_inpring;
>  
>  	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
>  				    GFP_KERNEL);
>  	if (!jrp->entinfo)
> -		goto out_free_outring;
> +		return -ENOMEM;
>  
This is going to leak resources, so should instead be:
		goto out_free_inpring;

I suggest merging patches 4, 5, 6.
They have the same goal, are rather small and make changes in the same places,
and would avoid issues like this one.

>  	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
>  
> @@ -495,9 +493,6 @@ static int caam_jr_init(struct device *dev)
>  
>  	return 0;
>  
> -out_free_outring:
> -	dma_free_coherent(dev, sizeof(struct jr_outentry) * JOBR_DEPTH,
> -			  jrp->outring, outbusaddr);
>  out_free_inpring:
>  	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
>  			  jrp->inpring, inpbusaddr);
> 

[PATCH v4 06/16] crypto: caam - use devres to allocate 'inpring'

[Andrey Smirnov]

Use devres to allocate 'inpring' and drop corresponding
dma_free_coherent() call as well explicit references to size of
'inpring' elemet (needet in following commits). No functional change
intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/jr.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 1eaa91dcc146..813e9135babd 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -108,7 +108,6 @@ static int caam_reset_hw_jr(struct device *dev)
 static int caam_jr_shutdown(struct device *dev)
 {
 	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
-	dma_addr_t inpbusaddr;
 	int ret;
 
 	ret = caam_reset_hw_jr(dev);
@@ -118,11 +117,6 @@ static int caam_jr_shutdown(struct device *dev)
 	/* Release interrupt */
 	free_irq(jrp->irq, dev);
 
-	/* Free rings */
-	inpbusaddr = rd_reg64(&jrp->rregs->inpring_base);
-	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
-			  jrp->inpring, inpbusaddr);
-
 	return ret;
 }
 
@@ -451,8 +445,9 @@ static int caam_jr_init(struct device *dev)
 		goto out_free_irq;
 
 	error = -ENOMEM;
-	jrp->inpring = dma_alloc_coherent(dev, sizeof(*jrp->inpring) *
-					  JOBR_DEPTH, &inpbusaddr, GFP_KERNEL);
+	jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
+					   JOBR_DEPTH, &inpbusaddr,
+					   GFP_KERNEL);
 	if (!jrp->inpring)
 		goto out_free_irq;
 
@@ -460,7 +455,7 @@ static int caam_jr_init(struct device *dev)
 					   JOBR_DEPTH, &outbusaddr,
 					   GFP_KERNEL);
 	if (!jrp->outring)
-		goto out_free_inpring;
+		return -ENOMEM;
 
 	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
 				    GFP_KERNEL);
@@ -493,10 +488,6 @@ static int caam_jr_init(struct device *dev)
 
 	return 0;
 
-out_free_inpring:
-	dma_free_coherent(dev, sizeof(dma_addr_t) * JOBR_DEPTH,
-			  jrp->inpring, inpbusaddr);
-	dev_err(dev, "can't allocate job rings for %d\n", jrp->ridx);
 out_free_irq:
 	free_irq(jrp->irq, dev);
 	return error;
-- 
2.21.0

[PATCH v4 07/16] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64

[Andrey Smirnov]

In order to be able to unify 64 and 32 bit implementations of
wr_reg64, let's convert it to use helpers from
<linux/io-64-nonatomic-hi-lo.h> first. Here are the steps of the
transformation:

1. Inline wr_reg32 helpers:

	if (!caam_imx && caam_little_end) {
		if (caam_little_end) {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		} else {
			iowrite32be(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32be(data, (u32 __iomem *)(reg));
		}
	} else {
		if (caam_little_end) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite32be(data >> 32, (u32 __iomem *)(reg));
			iowrite32be(data, (u32 __iomem *)(reg) + 1);
		}
	}

2. Transfrom the conditionals such that the check for
'caam_little_end' is at the top level:

	if (caam_little_end) {
		if (!caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		} else {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		}
	} else {
		iowrite32be(data >> 32, (u32 __iomem *)(reg));
		iowrite32be(data, (u32 __iomem *)(reg) + 1);
	}

3. Invert the check for !caam_imx:

	if (caam_little_end) {
		if (caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		}
	} else {
		iowrite32be(data >> 32, (u32 __iomem *)(reg));
		iowrite32be(data, (u32 __iomem *)(reg) + 1);
	}

4. Make use of iowrite64* helpers from <linux/io-64-nonatomic-hi-lo.h>

	if (caam_little_end) {
		if (caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite64(data, reg);
		}
	} else {
		iowrite64be(data, reg);
	}

No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 8591914d5c51..6e8352ac0d92 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 #include <linux/bitops.h>
 #include <linux/io.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
 
 /*
  * Architecture-specific register access methods
@@ -157,12 +158,15 @@ static inline u64 rd_reg64(void __iomem *reg)
 #else /* CONFIG_64BIT */
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
-	if (!caam_imx && caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
+	if (caam_little_end) {
+		if (caam_imx) {
+			iowrite32(data >> 32, (u32 __iomem *)(reg));
+			iowrite32(data, (u32 __iomem *)(reg) + 1);
+		} else {
+			iowrite64(data, reg);
+		}
 	} else {
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
+		iowrite64be(data, reg);
 	}
 }
 
-- 
2.21.0

[PATCH v4 08/16] crypto: caam - use ioread64*_hi_lo in rd_reg64

[Andrey Smirnov]

Following the same transformation logic as outlined in previous commit
converting wr_reg64, convert rd_reg64 to use helpers from
<linux/io-64-nonatomic-hi-lo.h> first. No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 6e8352ac0d92..afdc0d1aa338 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -172,12 +172,20 @@ static inline void wr_reg64(void __iomem *reg, u64 data)
 
 static inline u64 rd_reg64(void __iomem *reg)
 {
-	if (!caam_imx && caam_little_end)
-		return ((u64)rd_reg32((u32 __iomem *)(reg) + 1) << 32 |
-			(u64)rd_reg32((u32 __iomem *)(reg)));
+	if (caam_little_end) {
+		if (caam_imx) {
+			u32 low, high;
 
-	return ((u64)rd_reg32((u32 __iomem *)(reg)) << 32 |
-		(u64)rd_reg32((u32 __iomem *)(reg) + 1));
+			high = ioread32(reg);
+			low  = ioread32(reg + sizeof(u32));
+
+			return low + ((u64)high << 32);
+		} else {
+			return ioread64(reg);
+		}
+	} else {
+		return ioread64be(reg);
+	}
 }
 #endif /* CONFIG_64BIT  */
 
-- 
2.21.0

[PATCH v4 09/16] crypto: caam - drop 64-bit only wr/rd_reg64()

[Andrey Smirnov]

Since 32-bit of both wr_reg64 and rd_reg64 now use 64-bit IO helpers,
these functions should no longer be necessary. No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index afdc0d1aa338..fb494d14f262 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -138,24 +138,6 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
  *    base + 0x0000 : least-significant 32 bits
  *    base + 0x0004 : most-significant 32 bits
  */
-#ifdef CONFIG_64BIT
-static inline void wr_reg64(void __iomem *reg, u64 data)
-{
-	if (caam_little_end)
-		iowrite64(data, reg);
-	else
-		iowrite64be(data, reg);
-}
-
-static inline u64 rd_reg64(void __iomem *reg)
-{
-	if (caam_little_end)
-		return ioread64(reg);
-	else
-		return ioread64be(reg);
-}
-
-#else /* CONFIG_64BIT */
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
 	if (caam_little_end) {
@@ -187,7 +169,6 @@ static inline u64 rd_reg64(void __iomem *reg)
 		return ioread64be(reg);
 	}
 }
-#endif /* CONFIG_64BIT  */
 
 static inline u64 cpu_to_caam_dma64(dma_addr_t value)
 {
-- 
2.21.0

[PATCH v4 10/16] crypto: caam - make CAAM_PTR_SZ dynamic

[Andrey Smirnov]

In order to be able to configure CAAM pointer size at run-time, which
needed to support i.MX8MQ, which is 64-bit SoC with 32-bit pointer
size, convert CAAM_PTR_SZ to refer to a global variable of the same
name ("caam_ptr_sz") and adjust the rest of the code accordingly. No
functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/caamalg.c     |  2 +-
 drivers/crypto/caam/caamhash.c    |  2 +-
 drivers/crypto/caam/caamrng.c     |  2 +-
 drivers/crypto/caam/ctrl.c        |  2 ++
 drivers/crypto/caam/desc_constr.h | 10 ++++++++--
 drivers/crypto/caam/error.c       |  3 +++
 6 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 4b03c967009b..7f13ccc1e603 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -74,7 +74,7 @@
 
 #define CHACHAPOLY_DESC_JOB_IO_LEN	(AEAD_DESC_JOB_IO_LEN + CAAM_CMD_SZ * 6)
 
-#define DESC_MAX_USED_BYTES		(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN)
+#define DESC_MAX_USED_BYTES		(CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN_MIN)
 #define DESC_MAX_USED_LEN		(DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
 
 struct caam_alg_entry {
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index a87526f62737..4bfec53ccb8e 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -538,7 +538,7 @@ struct ahash_edesc {
 	dma_addr_t sec4_sg_dma;
 	int src_nents;
 	int sec4_sg_bytes;
-	u32 hw_desc[DESC_JOB_IO_LEN / sizeof(u32)] ____cacheline_aligned;
+	u32 hw_desc[DESC_JOB_IO_LEN_MAX / sizeof(u32)] ____cacheline_aligned;
 	struct sec4_sg_entry sec4_sg[0];
 };
 
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index 561bcb535184..511f0b44e258 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -53,7 +53,7 @@
 					 L1_CACHE_BYTES)
 
 /* length of descriptors */
-#define DESC_JOB_O_LEN			(CAAM_CMD_SZ * 2 + CAAM_PTR_SZ * 2)
+#define DESC_JOB_O_LEN			(CAAM_CMD_SZ * 2 + CAAM_PTR_SZ_MAX * 2)
 #define DESC_RNG_LEN			(3 * CAAM_CMD_SZ)
 
 /* Buffer, its dma address and lock */
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 908d3ecf6d1c..42692a2bc2f3 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -605,6 +605,8 @@ static int caam_probe(struct platform_device *pdev)
 	}
 	caam_imx = (bool)imx_soc_match;
 
+	caam_ptr_sz = sizeof(dma_addr_t);
+
 	/* Get configuration properties from device tree */
 	/* First, get register page */
 	ctrl = of_iomap(nprop, 0);
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 5988a26a2441..3a83a3332ba9 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -14,9 +14,14 @@
 
 #define IMMEDIATE (1 << 23)
 #define CAAM_CMD_SZ sizeof(u32)
-#define CAAM_PTR_SZ sizeof(dma_addr_t)
+#define CAAM_PTR_SZ caam_ptr_sz
+#define CAAM_PTR_SZ_MAX sizeof(dma_addr_t)
+#define CAAM_PTR_SZ_MIN sizeof(u32)
 #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE)
-#define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3)
+#define __DESC_JOB_IO_LEN(n) (CAAM_CMD_SZ * 5 + (n) * 3)
+#define DESC_JOB_IO_LEN __DESC_JOB_IO_LEN(CAAM_PTR_SZ)
+#define DESC_JOB_IO_LEN_MAX __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MAX)
+#define DESC_JOB_IO_LEN_MIN __DESC_JOB_IO_LEN(CAAM_PTR_SZ_MIN)
 
 #ifdef DEBUG
 #define PRINT_POS do { printk(KERN_DEBUG "%02d: %s\n", desc_len(desc),\
@@ -37,6 +42,7 @@
 			       (LDOFF_ENABLE_AUTO_NFIFO << LDST_OFFSET_SHIFT))
 
 extern bool caam_little_end;
+extern size_t caam_ptr_sz;
 
 /*
  * HW fetches 4 S/G table entries at a time, irrespective of how many entries
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 4f0d45865aa2..885cd364a01d 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -56,6 +56,9 @@ EXPORT_SYMBOL(caam_little_end);
 bool caam_imx;
 EXPORT_SYMBOL(caam_imx);
 
+size_t caam_ptr_sz;
+EXPORT_SYMBOL(caam_ptr_sz);
+
 static const struct {
 	u8 value;
 	const char *error_text;
-- 
2.21.0

[PATCH v4 11/16] crypto: caam - move cpu_to_caam_dma() selection to runtime

[Andrey Smirnov]

Instead of selecting the implementation of
cpu_to_caam_dma()/caam_dma_to_cpu() at build time using the
preprocessor, convert the code to do that at run-time using IS_ENABLED
macro. This is needed to add support for i.MX8MQ. No functional change
intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index fb494d14f262..511e28ba740a 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -188,13 +188,21 @@ static inline u64 caam_dma64_to_cpu(u64 value)
 	return caam64_to_cpu(value);
 }
 
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-#define cpu_to_caam_dma(value) cpu_to_caam_dma64(value)
-#define caam_dma_to_cpu(value) caam_dma64_to_cpu(value)
-#else
-#define cpu_to_caam_dma(value) cpu_to_caam32(value)
-#define caam_dma_to_cpu(value) caam32_to_cpu(value)
-#endif /* CONFIG_ARCH_DMA_ADDR_T_64BIT */
+static inline u64 cpu_to_caam_dma(u64 value)
+{
+	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
+		return cpu_to_caam_dma64(value);
+	else
+		return cpu_to_caam32(value);
+}
+
+static inline u64 caam_dma_to_cpu(u64 value)
+{
+	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
+		return caam_dma64_to_cpu(value);
+	else
+		return caam32_to_cpu(value);
+}
 
 /*
  * jr_outentry
-- 
2.21.0

[PATCH v4 12/16] crypto: caam - drop explicit usage of struct jr_outentry

[Andrey Smirnov]

Using struct jr_outentry to specify the layout of JobR output ring is
not appropriate for all 64-bit SoC, since some of them, like i.MX8MQ,
use 32-bit pointers there which doesn't match 64-bit
dma_addr_t. Convert existing code to use explicit helper functions to
access any of the JobR output ring elements, so that the support for
i.MX8MQ can be added later. No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/intern.h |  2 +-
 drivers/crypto/caam/jr.c     | 10 +++++----
 drivers/crypto/caam/regs.h   | 40 ++++++++++++++++++++++++++++++++----
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 1f01703f510a..081805c0f88b 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -58,7 +58,7 @@ struct caam_drv_private_jr {
 	dma_addr_t *inpring;	/* Base of input ring, alloc DMA-safe */
 	int out_ring_read_index;	/* Output index "tail" */
 	int tail;			/* entinfo (s/w ring) tail index */
-	struct jr_outentry *outring;	/* Base of output ring, DMA-safe */
+	void *outring;			/* Base of output ring, DMA-safe */
 };
 
 /*
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 813e9135babd..88777cc8adcd 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -214,7 +214,7 @@ static void caam_jr_dequeue(unsigned long devarg)
 		for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
 			sw_idx = (tail + i) & (JOBR_DEPTH - 1);
 
-			if (jrp->outring[hw_idx].desc ==
+			if (jr_outentry_desc(jrp->outring, hw_idx) ==
 			    caam_dma_to_cpu(jrp->entinfo[sw_idx].desc_addr_dma))
 				break; /* found */
 		}
@@ -223,7 +223,8 @@ static void caam_jr_dequeue(unsigned long devarg)
 
 		/* Unmap just-run descriptor so we can post-process */
 		dma_unmap_single(dev,
-				 caam_dma_to_cpu(jrp->outring[hw_idx].desc),
+				 caam_dma_to_cpu(jr_outentry_desc(jrp->outring,
+								  hw_idx)),
 				 jrp->entinfo[sw_idx].desc_size,
 				 DMA_TO_DEVICE);
 
@@ -234,7 +235,8 @@ static void caam_jr_dequeue(unsigned long devarg)
 		usercall = jrp->entinfo[sw_idx].callbk;
 		userarg = jrp->entinfo[sw_idx].cbkarg;
 		userdesc = jrp->entinfo[sw_idx].desc_addr_virt;
-		userstatus = caam32_to_cpu(jrp->outring[hw_idx].jrstatus);
+		userstatus = caam32_to_cpu(jr_outentry_jrstatus(jrp->outring,
+								hw_idx));
 
 		/*
 		 * Make sure all information from the job has been obtained
@@ -451,7 +453,7 @@ static int caam_jr_init(struct device *dev)
 	if (!jrp->inpring)
 		goto out_free_irq;
 
-	jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
+	jrp->outring = dmam_alloc_coherent(dev, SIZEOF_JR_OUTENTRY *
 					   JOBR_DEPTH, &outbusaddr,
 					   GFP_KERNEL);
 	if (!jrp->outring)
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 511e28ba740a..0cc4a48dfc30 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -71,6 +71,7 @@
 
 extern bool caam_little_end;
 extern bool caam_imx;
+extern size_t caam_ptr_sz;
 
 #define caam_to_cpu(len)						\
 static inline u##len caam##len ## _to_cpu(u##len val)			\
@@ -208,10 +209,41 @@ static inline u64 caam_dma_to_cpu(u64 value)
  * jr_outentry
  * Represents each entry in a JobR output ring
  */
-struct jr_outentry {
-	dma_addr_t desc;/* Pointer to completed descriptor */
-	u32 jrstatus;	/* Status for completed descriptor */
-} __packed;
+
+static inline void jr_outentry_get(void *outring, int hw_idx, dma_addr_t *desc,
+				   u32 *jrstatus)
+{
+	struct {
+		dma_addr_t desc;/* Pointer to completed descriptor */
+		u32 jrstatus;	/* Status for completed descriptor */
+	} __packed *outentry = outring;
+
+	*desc = outentry[hw_idx].desc;
+	*jrstatus = outentry[hw_idx].jrstatus;
+}
+
+#define SIZEOF_JR_OUTENTRY	(caam_ptr_sz + sizeof(u32))
+
+static inline dma_addr_t jr_outentry_desc(void *outring, int hw_idx)
+{
+	dma_addr_t desc;
+	u32 unused;
+
+	jr_outentry_get(outring, hw_idx, &desc, &unused);
+
+	return desc;
+}
+
+static inline u32 jr_outentry_jrstatus(void *outring, int hw_idx)
+{
+	dma_addr_t unused;
+	u32 jrstatus;
+
+	jr_outentry_get(outring, hw_idx, &unused, &jrstatus);
+
+	return jrstatus;
+}
+
 
 /* Version registers (Era 10+)	e80-eff */
 struct version_regs {
-- 
2.21.0

[PATCH v4 13/16] crypto: caam - don't hardcode inpentry size

[Andrey Smirnov]

Using dma_addr_t for elements of JobR input ring is not appropriate on
all 64-bit SoCs, some of which, like i.MX8MQ, use only 32-bit wide
pointers there. Convert all of the code to use explicit helper
function that can be later extended to support i.MX8MQ. No functional
change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/intern.h | 3 ++-
 drivers/crypto/caam/jr.c     | 4 ++--
 drivers/crypto/caam/regs.h   | 9 +++++++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 081805c0f88b..c00c7c84ec84 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -55,7 +55,8 @@ struct caam_drv_private_jr {
 	spinlock_t inplock ____cacheline_aligned; /* Input ring index lock */
 	u32 inpring_avail;	/* Number of free entries in input ring */
 	int head;			/* entinfo (s/w ring) head index */
-	dma_addr_t *inpring;	/* Base of input ring, alloc DMA-safe */
+	void *inpring;			/* Base of input ring, alloc
+					 * DMA-safe */
 	int out_ring_read_index;	/* Output index "tail" */
 	int tail;			/* entinfo (s/w ring) tail index */
 	void *outring;			/* Base of output ring, DMA-safe */
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 88777cc8adcd..fd20eee8169a 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -391,7 +391,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
 	head_entry->cbkarg = areq;
 	head_entry->desc_addr_dma = desc_dma;
 
-	jrp->inpring[head] = cpu_to_caam_dma(desc_dma);
+	jr_inpentry_set(jrp->inpring, head, cpu_to_caam_dma(desc_dma));
 
 	/*
 	 * Guarantee that the descriptor's DMA address has been written to
@@ -447,7 +447,7 @@ static int caam_jr_init(struct device *dev)
 		goto out_free_irq;
 
 	error = -ENOMEM;
-	jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
+	jrp->inpring = dmam_alloc_coherent(dev, SIZEOF_JR_INPENTRY *
 					   JOBR_DEPTH, &inpbusaddr,
 					   GFP_KERNEL);
 	if (!jrp->inpring)
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 0cc4a48dfc30..ec49f5ba9689 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -244,6 +244,15 @@ static inline u32 jr_outentry_jrstatus(void *outring, int hw_idx)
 	return jrstatus;
 }
 
+static inline void jr_inpentry_set(void *inpring, int hw_idx, dma_addr_t val)
+{
+	dma_addr_t *inpentry = inpring;
+
+	inpentry[hw_idx] = val;
+}
+
+#define SIZEOF_JR_INPENTRY	caam_ptr_sz
+
 
 /* Version registers (Era 10+)	e80-eff */
 struct version_regs {
-- 
2.21.0

[PATCH v4 14/16] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs

[Andrey Smirnov]

i.MX8 SoC still use 32-bit addresses in its CAAM implmentation, so
change all of the code to be able to handle that.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/caampkc.c     |  8 +++----
 drivers/crypto/caam/ctrl.c        |  6 +++--
 drivers/crypto/caam/desc_constr.h | 10 ++++++--
 drivers/crypto/caam/intern.h      |  2 +-
 drivers/crypto/caam/pdb.h         | 16 +++++++++----
 drivers/crypto/caam/pkc_desc.c    |  8 +++----
 drivers/crypto/caam/regs.h        | 39 +++++++++++++++++++++++--------
 7 files changed, 62 insertions(+), 27 deletions(-)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 19b02c1973fc..8badf783791c 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -17,13 +17,13 @@
 #include "sg_sw_sec4.h"
 #include "caampkc.h"
 
-#define DESC_RSA_PUB_LEN	(2 * CAAM_CMD_SZ + sizeof(struct rsa_pub_pdb))
+#define DESC_RSA_PUB_LEN	(2 * CAAM_CMD_SZ + SIZEOF_RSA_PUB_PDB)
 #define DESC_RSA_PRIV_F1_LEN	(2 * CAAM_CMD_SZ + \
-				 sizeof(struct rsa_priv_f1_pdb))
+				 SIZEOF_RSA_PRIV_F1_PDB)
 #define DESC_RSA_PRIV_F2_LEN	(2 * CAAM_CMD_SZ + \
-				 sizeof(struct rsa_priv_f2_pdb))
+				 SIZEOF_RSA_PRIV_F2_PDB)
 #define DESC_RSA_PRIV_F3_LEN	(2 * CAAM_CMD_SZ + \
-				 sizeof(struct rsa_priv_f3_pdb))
+				 SIZEOF_RSA_PRIV_F3_PDB)
 #define CAAM_RSA_MAX_INPUT_SIZE	512 /* for a 4096-bit modulus */
 
 /* buffer filled with zeros, used for padding */
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 42692a2bc2f3..adb560950e59 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -602,11 +602,13 @@ static int caam_probe(struct platform_device *pdev)
 		ret = init_clocks(dev, ctrlpriv, imx_soc_match->data);
 		if (ret)
 			return ret;
+
+		caam_ptr_sz = sizeof(u32);
+	} else {
+		caam_ptr_sz = sizeof(dma_addr_t);
 	}
 	caam_imx = (bool)imx_soc_match;
 
-	caam_ptr_sz = sizeof(dma_addr_t);
-
 	/* Get configuration properties from device tree */
 	/* First, get register page */
 	ctrl = of_iomap(nprop, 0);
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 3a83a3332ba9..5602b8f192de 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -109,9 +109,15 @@ static inline void init_job_desc_pdb(u32 * const desc, u32 options,
 
 static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
 {
-	dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
+	if (caam_ptr_sz == sizeof(dma_addr_t)) {
+		dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
 
-	*offset = cpu_to_caam_dma(ptr);
+		*offset = cpu_to_caam_dma(ptr);
+	} else {
+		u32 *offset = (u32 *)desc_end(desc);
+
+		*offset = cpu_to_caam_dma(ptr);
+	}
 
 	(*desc) = cpu_to_caam32(caam32_to_cpu(*desc) +
 				CAAM_PTR_SZ / CAAM_CMD_SZ);
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index c00c7c84ec84..731b06becd9c 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -219,7 +219,7 @@ static inline u64 caam_get_dma_mask(struct device *dev)
 {
 	struct device_node *nprop = dev->of_node;
 
-	if (sizeof(dma_addr_t) != sizeof(u64))
+	if (caam_ptr_sz != sizeof(u64))
 		return DMA_BIT_MASK(32);
 
 	if (caam_dpaa2)
diff --git a/drivers/crypto/caam/pdb.h b/drivers/crypto/caam/pdb.h
index 810f0bef0652..68c1fd5dee5d 100644
--- a/drivers/crypto/caam/pdb.h
+++ b/drivers/crypto/caam/pdb.h
@@ -512,7 +512,9 @@ struct rsa_pub_pdb {
 	dma_addr_t	n_dma;
 	dma_addr_t	e_dma;
 	u32		f_len;
-} __packed;
+};
+
+#define SIZEOF_RSA_PUB_PDB	(2 * sizeof(u32) + 4 * caam_ptr_sz)
 
 /**
  * RSA Decrypt PDB - Private Key Form #1
@@ -528,7 +530,9 @@ struct rsa_priv_f1_pdb {
 	dma_addr_t	f_dma;
 	dma_addr_t	n_dma;
 	dma_addr_t	d_dma;
-} __packed;
+};
+
+#define SIZEOF_RSA_PRIV_F1_PDB	(sizeof(u32) + 4 * caam_ptr_sz)
 
 /**
  * RSA Decrypt PDB - Private Key Form #2
@@ -554,7 +558,9 @@ struct rsa_priv_f2_pdb {
 	dma_addr_t	tmp1_dma;
 	dma_addr_t	tmp2_dma;
 	u32		p_q_len;
-} __packed;
+};
+
+#define SIZEOF_RSA_PRIV_F2_PDB	(2 * sizeof(u32) + 7 * caam_ptr_sz)
 
 /**
  * RSA Decrypt PDB - Private Key Form #3
@@ -586,6 +592,8 @@ struct rsa_priv_f3_pdb {
 	dma_addr_t	tmp1_dma;
 	dma_addr_t	tmp2_dma;
 	u32		p_q_len;
-} __packed;
+};
+
+#define SIZEOF_RSA_PRIV_F3_PDB	(2 * sizeof(u32) + 9 * caam_ptr_sz)
 
 #endif
diff --git a/drivers/crypto/caam/pkc_desc.c b/drivers/crypto/caam/pkc_desc.c
index 2a8d87ea94bf..0d5ee762e036 100644
--- a/drivers/crypto/caam/pkc_desc.c
+++ b/drivers/crypto/caam/pkc_desc.c
@@ -13,7 +13,7 @@
 /* Descriptor for RSA Public operation */
 void init_rsa_pub_desc(u32 *desc, struct rsa_pub_pdb *pdb)
 {
-	init_job_desc_pdb(desc, 0, sizeof(*pdb));
+	init_job_desc_pdb(desc, 0, SIZEOF_RSA_PUB_PDB);
 	append_cmd(desc, pdb->sgf);
 	append_ptr(desc, pdb->f_dma);
 	append_ptr(desc, pdb->g_dma);
@@ -26,7 +26,7 @@ void init_rsa_pub_desc(u32 *desc, struct rsa_pub_pdb *pdb)
 /* Descriptor for RSA Private operation - Private Key Form #1 */
 void init_rsa_priv_f1_desc(u32 *desc, struct rsa_priv_f1_pdb *pdb)
 {
-	init_job_desc_pdb(desc, 0, sizeof(*pdb));
+	init_job_desc_pdb(desc, 0, SIZEOF_RSA_PRIV_F1_PDB);
 	append_cmd(desc, pdb->sgf);
 	append_ptr(desc, pdb->g_dma);
 	append_ptr(desc, pdb->f_dma);
@@ -39,7 +39,7 @@ void init_rsa_priv_f1_desc(u32 *desc, struct rsa_priv_f1_pdb *pdb)
 /* Descriptor for RSA Private operation - Private Key Form #2 */
 void init_rsa_priv_f2_desc(u32 *desc, struct rsa_priv_f2_pdb *pdb)
 {
-	init_job_desc_pdb(desc, 0, sizeof(*pdb));
+	init_job_desc_pdb(desc, 0, SIZEOF_RSA_PRIV_F2_PDB);
 	append_cmd(desc, pdb->sgf);
 	append_ptr(desc, pdb->g_dma);
 	append_ptr(desc, pdb->f_dma);
@@ -56,7 +56,7 @@ void init_rsa_priv_f2_desc(u32 *desc, struct rsa_priv_f2_pdb *pdb)
 /* Descriptor for RSA Private operation - Private Key Form #3 */
 void init_rsa_priv_f3_desc(u32 *desc, struct rsa_priv_f3_pdb *pdb)
 {
-	init_job_desc_pdb(desc, 0, sizeof(*pdb));
+	init_job_desc_pdb(desc, 0, SIZEOF_RSA_PRIV_F3_PDB);
 	append_cmd(desc, pdb->sgf);
 	append_ptr(desc, pdb->g_dma);
 	append_ptr(desc, pdb->f_dma);
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index ec49f5ba9689..3c3ad474d08f 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -191,7 +191,8 @@ static inline u64 caam_dma64_to_cpu(u64 value)
 
 static inline u64 cpu_to_caam_dma(u64 value)
 {
-	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
+	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
+	    !caam_imx)
 		return cpu_to_caam_dma64(value);
 	else
 		return cpu_to_caam32(value);
@@ -199,7 +200,8 @@ static inline u64 cpu_to_caam_dma(u64 value)
 
 static inline u64 caam_dma_to_cpu(u64 value)
 {
-	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT))
+	if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
+	    !caam_imx)
 		return caam_dma64_to_cpu(value);
 	else
 		return caam32_to_cpu(value);
@@ -213,13 +215,24 @@ static inline u64 caam_dma_to_cpu(u64 value)
 static inline void jr_outentry_get(void *outring, int hw_idx, dma_addr_t *desc,
 				   u32 *jrstatus)
 {
-	struct {
-		dma_addr_t desc;/* Pointer to completed descriptor */
-		u32 jrstatus;	/* Status for completed descriptor */
-	} __packed *outentry = outring;
 
-	*desc = outentry[hw_idx].desc;
-	*jrstatus = outentry[hw_idx].jrstatus;
+	if (caam_imx) {
+		struct {
+			u32 desc;
+			u32 jrstatus;
+		} __packed *outentry = outring;
+
+		*desc = outentry[hw_idx].desc;
+		*jrstatus = outentry[hw_idx].jrstatus;
+	} else {
+		struct {
+			dma_addr_t desc;/* Pointer to completed descriptor */
+			u32 jrstatus;	/* Status for completed descriptor */
+		} __packed *outentry = outring;
+
+		*desc = outentry[hw_idx].desc;
+		*jrstatus = outentry[hw_idx].jrstatus;
+	}
 }
 
 #define SIZEOF_JR_OUTENTRY	(caam_ptr_sz + sizeof(u32))
@@ -246,9 +259,15 @@ static inline u32 jr_outentry_jrstatus(void *outring, int hw_idx)
 
 static inline void jr_inpentry_set(void *inpring, int hw_idx, dma_addr_t val)
 {
-	dma_addr_t *inpentry = inpring;
+	if (caam_imx) {
+		u32 *inpentry = inpring;
 
-	inpentry[hw_idx] = val;
+		inpentry[hw_idx] = val;
+	} else {
+		dma_addr_t *inpentry = inpring;
+
+		inpentry[hw_idx] = val;
+	}
 }
 
 #define SIZEOF_JR_INPENTRY	caam_ptr_sz
-- 
2.21.0

[PATCH v4 15/16] crypto: caam - always select job ring via RSR on i.MX8MQ

[Andrey Smirnov]

Per feedback from NXP tech support the way to use register based
service interface on i.MX8MQ is to follow the same set of steps
outlined for the case when virtualization is enabled, regardless if it
is. Current version of SRM for i.MX8MQ speaks of DECO DID_MS and DECO
DID_LS registers, but apparently those are not implemented, so the
case when SCFGR[VIRT_EN]=0 should be handles the same as the case when
SCFGR[VIRT_EN]=1

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index adb560950e59..a354c64ca39d 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -97,7 +97,12 @@ static inline int run_descriptor_deco0(struct device *ctrldev, u32 *desc,
 	int i;
 
 
-	if (ctrlpriv->virt_en == 1) {
+	if (ctrlpriv->virt_en == 1 ||
+	    /*
+	     * Apparently on i.MX8MQ it doesn't matter if virt_en == 1
+	     * and the following steps should be performed regardless
+	     */
+	    of_machine_is_compatible("fsl,imx8mq")) {
 		clrsetbits_32(&ctrl->deco_rsr, 0, DECORSR_JR0);
 
 		while (!(rd_reg32(&ctrl->deco_rsr) & DECORSR_VALID) &&
-- 
2.21.0

[PATCH v4 16/16] crypto: caam - add clock entry for i.MX8MQ

[Andrey Smirnov]

Add clock entry needed to support i.MX8MQ.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index a354c64ca39d..916b63f4f93b 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -527,6 +527,7 @@ static const struct soc_device_attribute caam_imx_soc_table[] = {
 	{ .soc_id = "i.MX6UL", .data = &caam_imx6ul_data },
 	{ .soc_id = "i.MX6*",  .data = &caam_imx6_data },
 	{ .soc_id = "i.MX7*",  .data = &caam_imx7_data },
+	{ .soc_id = "i.MX8MQ", .data = &caam_imx7_data },
 	{ .family = "Freescale i.MX" },
 };
 
-- 
2.21.0