[PATCH v2 0/4] crypto: caam - Add i.MX8MQ support

[PATCH v2 0/4] 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 [v1]

  - Series reworked to continuse 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

[v1] https://patchwork.kernel.org/cover/10825625/

Andrey Smirnov (2):
  crypto: caam - move DMA mask selection into a function
  crypto: caam - always select job ring via RSR on i.MX8MQ

Chris Spencer (2):
  crypto: caam - do not initialise clocks on the i.MX8
  crypto: caam - correct DMA address size for the i.MX8

 drivers/crypto/caam/ctrl.c        | 164 +++++++++++++++++-------------
 drivers/crypto/caam/desc_constr.h |  24 ++---
 drivers/crypto/caam/intern.h      |  24 ++++-
 drivers/crypto/caam/jr.c          |  15 +--
 drivers/crypto/caam/pdb.h         |  49 ++++-----
 drivers/crypto/caam/regs.h        |  21 +++-
 6 files changed, 170 insertions(+), 127 deletions(-)

-- 
2.21.0

[PATCH v2 1/4] crypto: caam - do not initialise clocks on the i.MX8

[Andrey Smirnov]

From: Chris Spencer <christopher.spencer@sea.co.uk>

There are no clocks that the CAAM driver needs to initialise on the
i.MX8.

Signed-off-by: Aymen Sghaier <aymen.sghaier@nxp.com>
Signed-off-by: Chris Spencer <christopher.spencer@sea.co.uk>
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: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/ctrl.c | 146 +++++++++++++++++++++----------------
 1 file changed, 85 insertions(+), 61 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index fec39c35c877..39334e71a14f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -309,6 +309,19 @@ static int deinstantiate_rng(struct device *ctrldev, int state_handle_mask)
 	return ret;
 }
 
+static void disable_clocking(struct caam_drv_private *ctrlpriv)
+{
+	if (of_machine_is_compatible("fsl,imx8mq"))
+		return;
+
+	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);
+}
+
 static int caam_remove(struct platform_device *pdev)
 {
 	struct device *ctrldev;
@@ -343,12 +356,8 @@ static int caam_remove(struct platform_device *pdev)
 	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);
+	disable_clocking(ctrlpriv);
+
 	return 0;
 }
 
@@ -497,65 +506,38 @@ static const struct of_device_id caam_match[] = {
 };
 MODULE_DEVICE_TABLE(of, caam_match);
 
-/* Probe routine for CAAM top (controller) level */
-static int caam_probe(struct platform_device *pdev)
+static int init_clocking(struct device *dev, struct caam_drv_private *ctrlpriv)
 {
-	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"},
-		{},
-	};
-	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
-	u32 scfgr, comp_params;
-	u8 rng_vid;
-	int pg_size;
-	int BLOCK_OFFSET = 0;
-
-	ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL);
-	if (!ctrlpriv)
-		return -ENOMEM;
-
-	dev = &pdev->dev;
-	dev_set_drvdata(dev, ctrlpriv);
-	nprop = pdev->dev.of_node;
+	int ret;
 
-	caam_imx = (bool)soc_device_match(imx_soc);
+	if (of_machine_is_compatible("fsl,imx8mq"))
+		return 0;
 
 	/* Enable clocking */
-	clk = caam_drv_identify_clk(&pdev->dev, "ipg");
+	clk = caam_drv_identify_clk(dev, "ipg");
 	if (IS_ERR(clk)) {
 		ret = PTR_ERR(clk);
-		dev_err(&pdev->dev,
-			"can't identify CAAM ipg clk: %d\n", ret);
+		dev_err(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")) {
-		clk = caam_drv_identify_clk(&pdev->dev, "mem");
+		clk = caam_drv_identify_clk(dev, "mem");
 		if (IS_ERR(clk)) {
 			ret = PTR_ERR(clk);
-			dev_err(&pdev->dev,
-				"can't identify CAAM mem clk: %d\n", ret);
+			dev_err(dev, "can't identify CAAM mem clk: %d\n", ret);
 			return ret;
 		}
 		ctrlpriv->caam_mem = clk;
 	}
 
-	clk = caam_drv_identify_clk(&pdev->dev, "aclk");
+	clk = caam_drv_identify_clk(dev, "aclk");
 	if (IS_ERR(clk)) {
 		ret = PTR_ERR(clk);
-		dev_err(&pdev->dev,
-			"can't identify CAAM aclk clk: %d\n", ret);
+		dev_err(dev, "can't identify CAAM aclk clk: %d\n", ret);
 		return ret;
 	}
 	ctrlpriv->caam_aclk = clk;
@@ -563,11 +545,11 @@ static int caam_probe(struct platform_device *pdev)
 	if (!of_machine_is_compatible("fsl,imx6ul") &&
 	    !of_machine_is_compatible("fsl,imx7d") &&
 	    !of_machine_is_compatible("fsl,imx7s")) {
-		clk = caam_drv_identify_clk(&pdev->dev, "emi_slow");
+		clk = caam_drv_identify_clk(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);
+			dev_err(dev, "can't identify CAAM emi_slow clk: %d\n",
+				ret);
 			return ret;
 		}
 		ctrlpriv->caam_emi_slow = clk;
@@ -575,14 +557,15 @@ 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);
+		dev_err(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",
+			dev_err(dev,
+				"can't enable CAAM secure mem clock: %d\n",
 				ret);
 			goto disable_caam_ipg;
 		}
@@ -590,26 +573,74 @@ static int caam_probe(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(ctrlpriv->caam_aclk);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "can't enable CAAM aclk clock: %d\n", ret);
+		dev_err(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",
+			dev_err(dev, "can't enable CAAM emi slow clock: %d\n",
 				ret);
 			goto disable_caam_aclk;
 		}
 	}
 
+	return 0;
+
+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;
+}
+
+/* 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"},
+		{},
+	};
+	struct device *dev;
+	struct device_node *nprop, *np;
+	struct caam_ctrl __iomem *ctrl;
+	struct caam_drv_private *ctrlpriv;
+#ifdef CONFIG_DEBUG_FS
+	struct caam_perfmon *perfmon;
+#endif
+	u32 scfgr, comp_params;
+	u8 rng_vid;
+	int pg_size;
+	int BLOCK_OFFSET = 0;
+
+	ctrlpriv = devm_kzalloc(&pdev->dev, sizeof(*ctrlpriv), GFP_KERNEL);
+	if (!ctrlpriv)
+		return -ENOMEM;
+
+	dev = &pdev->dev;
+	dev_set_drvdata(dev, ctrlpriv);
+	nprop = pdev->dev.of_node;
+
+	caam_imx = (bool)soc_device_match(imx_soc);
+
+	ret = init_clocking(dev, ctrlpriv);
+	if (ret)
+		return ret;
+
 	/* 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;
+		goto disable_clocks;
 	}
 
 	caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
@@ -900,16 +931,9 @@ static int caam_probe(struct platform_device *pdev)
 
 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);
+disable_clocks:
+	disable_clocking(ctrlpriv);
+
 	return ret;
 }
 
-- 
2.21.0

[PATCH v2 2/4] 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>
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: 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 39334e71a14f..9750fcfd37fc 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -717,16 +717,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 3392615dc91b..b0e32a91cdea 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)
 
@@ -127,4 +129,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 1de2562d0982..c2b99139a10a 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -502,20 +502,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 v2 3/4] crypto: caam - correct DMA address size for the i.MX8

[Andrey Smirnov]

From: Chris Spencer <christopher.spencer@sea.co.uk>

The i.MX8 is arm64, but its CAAM DMA address size is 32-bits.

Signed-off-by: Aymen Sghaier <aymen.sghaier@nxp.com>
Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
Signed-off-by: Chris Spencer <christopher.spencer@sea.co.uk>
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: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/desc_constr.h | 24 +++++++--------
 drivers/crypto/caam/intern.h      |  6 ++--
 drivers/crypto/caam/pdb.h         | 49 ++++++++++++++++---------------
 drivers/crypto/caam/regs.h        | 21 +++++++++++--
 4 files changed, 58 insertions(+), 42 deletions(-)

diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 2980b8ef1fb1..ab1a1c786c18 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -13,7 +13,7 @@
 
 #define IMMEDIATE (1 << 23)
 #define CAAM_CMD_SZ sizeof(u32)
-#define CAAM_PTR_SZ sizeof(dma_addr_t)
+#define CAAM_PTR_SZ sizeof(caam_dma_addr_t)
 #define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE)
 #define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3)
 
@@ -90,9 +90,9 @@ static inline void init_job_desc_pdb(u32 * const desc, u32 options,
 	init_job_desc(desc, (((pdb_len + 1) << HDR_START_IDX_SHIFT)) | options);
 }
 
-static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
+static inline void append_ptr(u32 * const desc, caam_dma_addr_t ptr)
 {
-	dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
+	caam_dma_addr_t *offset = (caam_dma_addr_t *)desc_end(desc);
 
 	*offset = cpu_to_caam_dma(ptr);
 
@@ -100,7 +100,7 @@ static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
 				CAAM_PTR_SZ / CAAM_CMD_SZ);
 }
 
-static inline void init_job_desc_shared(u32 * const desc, dma_addr_t ptr,
+static inline void init_job_desc_shared(u32 * const desc, caam_dma_addr_t ptr,
 					int len, u32 options)
 {
 	PRINT_POS;
@@ -155,7 +155,7 @@ static inline u32 *write_cmd(u32 * const desc, u32 command)
 	return desc + 1;
 }
 
-static inline void append_cmd_ptr(u32 * const desc, dma_addr_t ptr, int len,
+static inline void append_cmd_ptr(u32 * const desc, caam_dma_addr_t ptr, int len,
 				  u32 command)
 {
 	append_cmd(desc, command | len);
@@ -163,7 +163,7 @@ static inline void append_cmd_ptr(u32 * const desc, dma_addr_t ptr, int len,
 }
 
 /* Write length after pointer, rather than inside command */
-static inline void append_cmd_ptr_extlen(u32 * const desc, dma_addr_t ptr,
+static inline void append_cmd_ptr_extlen(u32 * const desc, caam_dma_addr_t ptr,
 					 unsigned int len, u32 command)
 {
 	append_cmd(desc, command);
@@ -228,7 +228,7 @@ APPEND_CMD_LEN(seq_fifo_load, SEQ_FIFO_LOAD)
 APPEND_CMD_LEN(seq_fifo_store, SEQ_FIFO_STORE)
 
 #define APPEND_CMD_PTR(cmd, op) \
-static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd(u32 * const desc, caam_dma_addr_t ptr, \
 				unsigned int len, u32 options) \
 { \
 	PRINT_POS; \
@@ -239,7 +239,7 @@ APPEND_CMD_PTR(load, LOAD)
 APPEND_CMD_PTR(fifo_load, FIFO_LOAD)
 APPEND_CMD_PTR(fifo_store, FIFO_STORE)
 
-static inline void append_store(u32 * const desc, dma_addr_t ptr,
+static inline void append_store(u32 * const desc, caam_dma_addr_t ptr,
 				unsigned int len, u32 options)
 {
 	u32 cmd_src;
@@ -258,7 +258,7 @@ static inline void append_store(u32 * const desc, dma_addr_t ptr,
 
 #define APPEND_SEQ_PTR_INTLEN(cmd, op) \
 static inline void append_seq_##cmd##_ptr_intlen(u32 * const desc, \
-						 dma_addr_t ptr, \
+						 caam_dma_addr_t ptr, \
 						 unsigned int len, \
 						 u32 options) \
 { \
@@ -282,7 +282,7 @@ APPEND_CMD_PTR_TO_IMM(load, LOAD);
 APPEND_CMD_PTR_TO_IMM(fifo_load, FIFO_LOAD);
 
 #define APPEND_CMD_PTR_EXTLEN(cmd, op) \
-static inline void append_##cmd##_extlen(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd##_extlen(u32 * const desc, caam_dma_addr_t ptr, \
 					 unsigned int len, u32 options) \
 { \
 	PRINT_POS; \
@@ -296,7 +296,7 @@ APPEND_CMD_PTR_EXTLEN(seq_out_ptr, SEQ_OUT_PTR)
  * the size of its type
  */
 #define APPEND_CMD_PTR_LEN(cmd, op, type) \
-static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd(u32 * const desc, caam_dma_addr_t ptr, \
 				type len, u32 options) \
 { \
 	PRINT_POS; \
@@ -456,7 +456,7 @@ struct alginfo {
 	unsigned int keylen;
 	unsigned int keylen_pad;
 	union {
-		dma_addr_t key_dma;
+		caam_dma_addr_t key_dma;
 		const void *key_virt;
 	};
 	bool key_inline;
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index b0e32a91cdea..12d261e0d531 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -34,7 +34,7 @@ struct caam_jrentry_info {
 	void (*callbk)(struct device *dev, u32 *desc, u32 status, void *arg);
 	void *cbkarg;	/* Argument per ring entry */
 	u32 *desc_addr_virt;	/* Stored virt addr for postprocessing */
-	dma_addr_t desc_addr_dma;	/* Stored bus addr for done matching */
+	caam_dma_addr_t desc_addr_dma;	/* Stored bus addr for done matching */
 	u32 desc_size;	/* Stored size for postprocessing, header derived */
 };
 
@@ -55,7 +55,7 @@ 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 */
+	caam_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 */
@@ -133,7 +133,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 (sizeof(caam_dma_addr_t) != 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..128d16682feb 100644
--- a/drivers/crypto/caam/pdb.h
+++ b/drivers/crypto/caam/pdb.h
@@ -9,6 +9,7 @@
 #ifndef CAAM_PDB_H
 #define CAAM_PDB_H
 #include "compat.h"
+#include "regs.h"
 
 /*
  * PDB- IPSec ESP Header Modification Options
@@ -507,10 +508,10 @@ struct dsa_verify_pdb {
  */
 struct rsa_pub_pdb {
 	u32		sgf;
-	dma_addr_t	f_dma;
-	dma_addr_t	g_dma;
-	dma_addr_t	n_dma;
-	dma_addr_t	e_dma;
+	caam_dma_addr_t	f_dma;
+	caam_dma_addr_t	g_dma;
+	caam_dma_addr_t	n_dma;
+	caam_dma_addr_t	e_dma;
 	u32		f_len;
 } __packed;
 
@@ -524,10 +525,10 @@ struct rsa_pub_pdb {
  */
 struct rsa_priv_f1_pdb {
 	u32		sgf;
-	dma_addr_t	g_dma;
-	dma_addr_t	f_dma;
-	dma_addr_t	n_dma;
-	dma_addr_t	d_dma;
+	caam_dma_addr_t	g_dma;
+	caam_dma_addr_t	f_dma;
+	caam_dma_addr_t	n_dma;
+	caam_dma_addr_t	d_dma;
 } __packed;
 
 /**
@@ -546,13 +547,13 @@ struct rsa_priv_f1_pdb {
  */
 struct rsa_priv_f2_pdb {
 	u32		sgf;
-	dma_addr_t	g_dma;
-	dma_addr_t	f_dma;
-	dma_addr_t	d_dma;
-	dma_addr_t	p_dma;
-	dma_addr_t	q_dma;
-	dma_addr_t	tmp1_dma;
-	dma_addr_t	tmp2_dma;
+	caam_dma_addr_t	g_dma;
+	caam_dma_addr_t	f_dma;
+	caam_dma_addr_t	d_dma;
+	caam_dma_addr_t	p_dma;
+	caam_dma_addr_t	q_dma;
+	caam_dma_addr_t	tmp1_dma;
+	caam_dma_addr_t	tmp2_dma;
 	u32		p_q_len;
 } __packed;
 
@@ -576,15 +577,15 @@ struct rsa_priv_f2_pdb {
  */
 struct rsa_priv_f3_pdb {
 	u32		sgf;
-	dma_addr_t	g_dma;
-	dma_addr_t	f_dma;
-	dma_addr_t	c_dma;
-	dma_addr_t	p_dma;
-	dma_addr_t	q_dma;
-	dma_addr_t	dp_dma;
-	dma_addr_t	dq_dma;
-	dma_addr_t	tmp1_dma;
-	dma_addr_t	tmp2_dma;
+	caam_dma_addr_t	g_dma;
+	caam_dma_addr_t	f_dma;
+	caam_dma_addr_t	c_dma;
+	caam_dma_addr_t	p_dma;
+	caam_dma_addr_t	q_dma;
+	caam_dma_addr_t	dp_dma;
+	caam_dma_addr_t	dq_dma;
+	caam_dma_addr_t	tmp1_dma;
+	caam_dma_addr_t	tmp2_dma;
 	u32		p_q_len;
 } __packed;
 
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 8591914d5c51..6e7f8d4f3f2b 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -137,7 +137,7 @@ 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
+#if defined(CONFIG_64BIT) && !defined(CONFIG_ARCH_MXC)
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
 	if (caam_little_end)
@@ -195,7 +195,7 @@ static inline u64 caam_dma64_to_cpu(u64 value)
 	return caam64_to_cpu(value);
 }
 
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) && !defined(CONFIG_ARCH_MXC)
 #define cpu_to_caam_dma(value) cpu_to_caam_dma64(value)
 #define caam_dma_to_cpu(value) caam_dma64_to_cpu(value)
 #else
@@ -203,12 +203,27 @@ static inline u64 caam_dma64_to_cpu(u64 value)
 #define caam_dma_to_cpu(value) caam32_to_cpu(value)
 #endif /* CONFIG_ARCH_DMA_ADDR_T_64BIT */
 
+/*
+ * On i.MX8 boards the arch is arm64 but the CAAM dma address size is
+ * 32 bits on 8MQ and 36 bits on 8QM and 8QXP.
+ * For 8QM and 8QXP there is a configurable field PS called pointer size
+ * in the MCFGR register to switch between 32 and 64 (default 32)
+ * But this register is only accessible by the SECO and is left to its
+ * default value.
+ * Here we set the CAAM dma address size to 32 bits for all i.MX8
+ */
+#if defined(CONFIG_ARM64) && defined(CONFIG_ARCH_MXC)
+#define caam_dma_addr_t u32
+#else
+#define caam_dma_addr_t dma_addr_t
+#endif
+
 /*
  * jr_outentry
  * Represents each entry in a JobR output ring
  */
 struct jr_outentry {
-	dma_addr_t desc;/* Pointer to completed descriptor */
+	caam_dma_addr_t desc;/* Pointer to completed descriptor */
 	u32 jrstatus;	/* Status for completed descriptor */
 } __packed;
 
-- 
2.21.0

[PATCH v2 4/4] 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: 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 9750fcfd37fc..c50fc059a441 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -107,7 +107,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

Re: [PATCH v2 1/4] crypto: caam - do not initialise clocks on the i.MX8

[Horia Geanta]

On 6/7/2019 11:03 PM, Andrey Smirnov wrote:
> From: Chris Spencer <christopher.spencer@sea.co.uk>
> 
> There are no clocks that the CAAM driver needs to initialise on the
> i.MX8.
> 
RM lists 5 clocks for CAAM module (instance.clock): caam.aclk, caam.ipg_clk,
caam.ipg_clk_s, caam_exsc.aclk_exsc, caam_mem.clk
				
Wouldn't it be better to have these clocks in DT, instead of relying that their
root clocks (ccm_ahb_clk_root, ccm_ipg_clk_root) are critical / always on?

Thanks,
Horia

Re: [PATCH v2 1/4] crypto: caam - do not initialise clocks on the i.MX8

[Andrey Smirnov]

On Tue, Jun 11, 2019 at 2:56 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 6/7/2019 11:03 PM, Andrey Smirnov wrote:
> > From: Chris Spencer <christopher.spencer@sea.co.uk>
> >
> > There are no clocks that the CAAM driver needs to initialise on the
> > i.MX8.
> >
> RM lists 5 clocks for CAAM module (instance.clock): caam.aclk, caam.ipg_clk,
> caam.ipg_clk_s, caam_exsc.aclk_exsc, caam_mem.clk
>
> Wouldn't it be better to have these clocks in DT, instead of relying that their
> root clocks (ccm_ahb_clk_root, ccm_ipg_clk_root) are critical / always on?
>

Is it even possible to use the SoC in a meaningful way if
ccm_ahb_clk_root or ccm_ipg_clk_root are disabled? It doesn't seem
that dependency on either is expressed in a consistent manner for
other IP blocks in imx8mq.dtsi. OTOH, it should be trivial to add
<&clk IMX8MQ_CLK_IPG_ROOT> and <&clk IMX8MQ_CLK_AHB> as "ipg" and
"aclk" to CAAM node and it would allow me to drop an extra patch from
the series, so I may as well do it.

Thanks,
Andrey Smirnov

Re: [PATCH v2 1/4] crypto: caam - do not initialise clocks on the i.MX8

[Leonard Crestez]

On 07.06.2019 23:03, Andrey Smirnov wrote:

> There are no clocks that the CAAM driver needs to initialise on the
> i.MX8.

The clk handling inside CAAM is very convoluted and this patch doesn't 
help. All the driver actually does is "enable all required clocks", this 
shouldn't be complicated.

I propose adding a const caam_soc_data struct which has a bool flag 
marking if each clock is required or not, the replace all the 
of_machine_is_compatible() logic with statements of the form:

if (ctrlpriv->soc_data->need_ipg_clk)
     ctrlpriv->caam_ipg = devm_clk_get("ipg");

You could even make all clks optional and claim that if a clk is not 
listed in DT then it's assumed to be always on. However that means that 
on some SOCs if DT is incorrect you can get a hang (due to missing clk) 
instead of a probe error.

> +	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);

Clock APIs have no effect if clk argument is NULL, please just drop 
these if statements.

--
Regards,
Leonard

Re: [PATCH v2 1/4] crypto: caam - do not initialise clocks on the i.MX8

[Andrey Smirnov]

On Wed, Jun 12, 2019 at 6:53 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 07.06.2019 23:03, Andrey Smirnov wrote:
>
> > There are no clocks that the CAAM driver needs to initialise on the
> > i.MX8.
>
> The clk handling inside CAAM is very convoluted and this patch doesn't
> help. All the driver actually does is "enable all required clocks", this
> shouldn't be complicated.
>

As I mentioned before, this patch is now dropped. It is replaced with
a one-liner to avoid requesting "emi_slow" clock on i.mx8mq.

> I propose adding a const caam_soc_data struct which has a bool flag
> marking if each clock is required or not, the replace all the
> of_machine_is_compatible() logic with statements of the form:
>
> if (ctrlpriv->soc_data->need_ipg_clk)
>      ctrlpriv->caam_ipg = devm_clk_get("ipg");
>
> You could even make all clks optional and claim that if a clk is not
> listed in DT then it's assumed to be always on. However that means that
> on some SOCs if DT is incorrect you can get a hang (due to missing clk)
> instead of a probe error.
>

I agree with this approach in principle, but I'd rather focus this
series on adding i.MX8MQ support with least amount changes needed. I
have a separate series doing a bunch of devres related cleanup on this
driver, which I plan to submit once i.MX8MQ is supported and we can
(and I'd rather) address the problem there.

> > +     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);
>
> Clock APIs have no effect if clk argument is NULL, please just drop
> these if statements.

As a part of my refactoring changes I replaced all of that code with
devres, so it should already be addressed there.

Thanks,
Andrey Smirnov