Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support
@ 2019-07-17 15:24 Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 01/14] crypto: caam - move DMA mask selection into a function Andrey Smirnov
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 [v5]:

  - Hunk replacing sizeof(*jrp->inpring) to SIZEOF_JR_INPENTRY in
    "crypto: caam - don't hardcode inpentry size", lost in [v5], is
    back

  - Collected Tested-by from Iuliana

Changes since [v4]:

  - Fixed missing sentinel element in "crypto: caam - simplfy clock
    initialization"
    
  - Squashed all of the devers related patches into a single one and
    converted IRQ allocation to use devres while at it

  - Added "crypto: caam - request JR IRQ as the last step" as
    discussed

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

[v5] lore.kernel.org/r/20190715201942.17309-1-andrew.smirnov@gmail.com
[v4] lore.kernel.org/r/20190703081327.17505-1-andrew.smirnov@gmail.com
[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 (14):
  crypto: caam - move DMA mask selection into a function
  crypto: caam - simplfy clock initialization
  crypto: caam - convert caam_jr_init() to use devres
  crypto: caam - request JR IRQ as the last step
  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        | 225 ++++++++++++++----------------
 drivers/crypto/caam/desc_constr.h |  20 ++-
 drivers/crypto/caam/error.c       |   3 +
 drivers/crypto/caam/intern.h      |  32 ++++-
 drivers/crypto/caam/jr.c          |  95 ++++---------
 drivers/crypto/caam/pdb.h         |  16 ++-
 drivers/crypto/caam/pkc_desc.c    |   8 +-
 drivers/crypto/caam/regs.h        | 139 ++++++++++++------
 12 files changed, 306 insertions(+), 246 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 01/14] crypto: caam - move DMA mask selection into a function
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 02/14] crypto: caam - simplfy clock initialization Andrey Smirnov
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Horia Geantă,
	Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 02/14] crypto: caam - simplfy clock initialization
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 01/14] crypto: caam - move DMA mask selection into a function Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-23 14:17   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres Andrey Smirnov
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Leonard Crestez, Iuliana Prodan, Chris Spencer,
	Cory Tusar, Chris Healy, Lucas Stach, Horia Geantă,
	Aymen Sghaier, linux-kernel

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>
Tested-by: Iuliana Prodan <iuliana.prodan@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   | 204 +++++++++++++++++------------------
 drivers/crypto/caam/intern.h |   7 +-
 2 files changed, 99 insertions(+), 112 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e674d8770cdb..592ce4a05db8 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,103 @@ 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" },
+	{ /* sentinel */ }
+};
+
+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 +593,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 +899,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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 01/14] crypto: caam - move DMA mask selection into a function Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 02/14] crypto: caam - simplfy clock initialization Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-23 15:48   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step Andrey Smirnov
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Cory Tusar, Chris Healy, Lucas Stach,
	Horia Geantă,
	Aymen Sghaier, linux-kernel

Use deveres to allocate all of the resources in caam_jr_init() (DMA
coherent and regular memory, IRQs) drop calls to corresponding
deallocation routines. No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
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/jr.c | 48 ++++++++++++----------------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 4b25b2fa3d02..ea02f7774f7c 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -108,25 +108,12 @@ 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;
 	int ret;
 
 	ret = caam_reset_hw_jr(dev);
 
 	tasklet_kill(&jrp->irqtask);
 
-	/* Release interrupt */
-	free_irq(jrp->irq, 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);
-	kfree(jrp->entinfo);
-
 	return ret;
 }
 
@@ -444,8 +431,8 @@ static int caam_jr_init(struct device *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);
+	error = devm_request_irq(dev, 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);
@@ -454,22 +441,25 @@ static int caam_jr_init(struct device *dev)
 
 	error = caam_reset_hw_jr(dev);
 	if (error)
-		goto out_free_irq;
+		goto out_kill_deq;
 
 	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;
+		goto out_kill_deq;
 
-	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;
+		goto out_kill_deq;
 
-	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;
+		goto out_kill_deq;
 
 	for (i = 0; i < JOBR_DEPTH; i++)
 		jrp->entinfo[i].desc_addr_dma = !0;
@@ -494,16 +484,6 @@ static int caam_jr_init(struct device *dev)
 		      (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
 
 	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);
-	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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (2 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-23 16:02   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64 Andrey Smirnov
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

In order to avoid any risk of JR IRQ request being handled while some
of the resources used for that are not yet allocated move the code
requesting said IRQ to the endo of caam_jr_init(). 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 | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index ea02f7774f7c..98e0a504322f 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -428,38 +428,27 @@ 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 = devm_request_irq(dev, 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;
-	}
-
 	error = caam_reset_hw_jr(dev);
 	if (error)
-		goto out_kill_deq;
+		return error;
 
 	error = -ENOMEM;
 	jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
 					   JOBR_DEPTH, &inpbusaddr,
 					   GFP_KERNEL);
 	if (!jrp->inpring)
-		goto out_kill_deq;
+		return -ENOMEM;
 
 	jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
 					   JOBR_DEPTH, &outbusaddr,
 					   GFP_KERNEL);
 	if (!jrp->outring)
-		goto out_kill_deq;
+		return -ENOMEM;
 
 	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
 				    GFP_KERNEL);
 	if (!jrp->entinfo)
-		goto out_kill_deq;
+		return -ENOMEM;
 
 	for (i = 0; i < JOBR_DEPTH; i++)
 		jrp->entinfo[i].desc_addr_dma = !0;
@@ -483,10 +472,19 @@ static int caam_jr_init(struct device *dev)
 		      (JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) |
 		      (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
 
+	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
+
+	/* Connect job ring interrupt handler. */
+	error = devm_request_irq(dev, 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);
+		tasklet_kill(&jrp->irqtask);
+		return error;
+	}
+
 	return 0;
-out_kill_deq:
-	tasklet_kill(&jrp->irqtask);
-	return error;
 }
 
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (3 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-29 15:29   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64 Andrey Smirnov
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (4 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64 Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-29 15:32   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64() Andrey Smirnov
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64()
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (5 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64 Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-29 15:32   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic Andrey Smirnov
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (6 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64() Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-23  9:57   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 09/14] crypto: caam - move cpu_to_caam_dma() selection to runtime Andrey Smirnov
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 43f18253e5b6..4dda2f50a724 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 e4ac5d591ad6..955cb4d7c910 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 592ce4a05db8..e5eaaf1efe45 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -606,6 +606,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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 09/14] crypto: caam - move cpu_to_caam_dma() selection to runtime
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (7 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 10/14] crypto: caam - drop explicit usage of struct jr_outentry Andrey Smirnov
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 10/14] crypto: caam - drop explicit usage of struct jr_outentry
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (8 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 09/14] crypto: caam - move cpu_to_caam_dma() selection to runtime Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 11/14] crypto: caam - don't hardcode inpentry size Andrey Smirnov
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 98e0a504322f..138f71adb7e6 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -211,7 +211,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 */
 		}
@@ -220,7 +220,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);
 
@@ -231,7 +232,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
@@ -439,7 +441,7 @@ static int caam_jr_init(struct device *dev)
 	if (!jrp->inpring)
 		return -ENOMEM;
 
-	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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 11/14] crypto: caam - don't hardcode inpentry size
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (9 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 10/14] crypto: caam - drop explicit usage of struct jr_outentry Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs Andrey Smirnov
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 138f71adb7e6..4d7a302d0b9b 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -388,7 +388,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
@@ -435,7 +435,7 @@ static int caam_jr_init(struct device *dev)
 		return error;
 
 	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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (10 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 11/14] crypto: caam - don't hardcode inpentry size Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-08-05  8:23   ` Horia Geanta
  2019-07-17 15:24 ` [PATCH v6 13/14] crypto: caam - always select job ring via RSR on i.MX8MQ Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 14/14] crypto: caam - add clock entry for i.MX8MQ Andrey Smirnov
  13 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 80574106af29..0e95ad555156 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 e5eaaf1efe45..b309535f3157 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -603,11 +603,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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 13/14] crypto: caam - always select job ring via RSR on i.MX8MQ
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (11 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  2019-07-17 15:24 ` [PATCH v6 14/14] crypto: caam - add clock entry for i.MX8MQ Andrey Smirnov
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 b309535f3157..ad6ff4040bab 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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v6 14/14] crypto: caam - add clock entry for i.MX8MQ
  2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
                   ` (12 preceding siblings ...)
  2019-07-17 15:24 ` [PATCH v6 13/14] crypto: caam - always select job ring via RSR on i.MX8MQ Andrey Smirnov
@ 2019-07-17 15:24 ` Andrey Smirnov
  13 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-07-17 15:24 UTC (permalink / raw)
  To: linux-crypto
  Cc: Andrey Smirnov, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Horia Geantă,
	Aymen Sghaier, Leonard Crestez, linux-kernel

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 ad6ff4040bab..6f3b4405dcba 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" },
 	{ /* sentinel */ }
 };
-- 
2.21.0


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic
  2019-07-17 15:24 ` [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic Andrey Smirnov
@ 2019-07-23  9:57   ` Horia Geanta
  2019-08-12 17:56     ` Andrey Smirnov
  0 siblings, 1 reply; 29+ messages in thread
From: Horia Geanta @ 2019-07-23  9:57 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> 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.
> 
I am seeing compilation errors like:

In file included from drivers/crypto/caam/ctrl.c:25:0:
drivers/crypto/caam/qi.h:87:6: error: variably modified 'sh_desc' at file scope
  u32 sh_desc[MAX_SDLEN];
      ^

Adding comments for this commit, since it looks like the fixes
should be included here (related to DESC_JOB_IO_LEN vs. DESC_JOB_IO_LEN_MAX).

Please make sure caam/qi and and caam/qi2 drivers are at least compile-tested.

By caam/qi I am referring to:
CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=y/m

and caam/qi2:
CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM=y/m

Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 02/14] crypto: caam - simplfy clock initialization
  2019-07-17 15:24 ` [PATCH v6 02/14] crypto: caam - simplfy clock initialization Andrey Smirnov
@ 2019-07-23 14:17   ` Horia Geanta
  2019-08-12 17:56     ` Andrey Smirnov
  0 siblings, 1 reply; 29+ messages in thread
From: Horia Geanta @ 2019-07-23 14:17 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Leonard Crestez, Iuliana Prodan, Chris Spencer, Cory Tusar,
	Chris Healy, Lucas Stach, Aymen Sghaier, linux-kernel

On 7/17/2019 6:25 PM, 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.
> 
Thanks.
Two nitpicks below.

> +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;
Or directly:
	return devm_add_action_or_reset(dev, disable_clocks, ctrlpriv);

> +	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;
[...]
> +		ret = init_clocks(dev, ctrlpriv, imx_soc_match->data);
ctrlpriv can be retrieved using dev_get_drvdata(dev), thus there's no need
to pass it as parameter.

Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres
  2019-07-17 15:24 ` [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres Andrey Smirnov
@ 2019-07-23 15:48   ` Horia Geanta
  0 siblings, 0 replies; 29+ messages in thread
From: Horia Geanta @ 2019-07-23 15:48 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Cory Tusar, Chris Healy, Lucas Stach, Aymen Sghaier, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> Use deveres to allocate all of the resources in caam_jr_init() (DMA
> coherent and regular memory, IRQs) drop calls to corresponding
> deallocation routines. No functional change intended.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Reviewed-by: Horia Geantã <horia.geanta@nxp.com>

Thanks,
Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step
  2019-07-17 15:24 ` [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step Andrey Smirnov
@ 2019-07-23 16:02   ` Horia Geanta
  2019-08-12 17:59     ` Andrey Smirnov
  0 siblings, 1 reply; 29+ messages in thread
From: Horia Geanta @ 2019-07-23 16:02 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> In order to avoid any risk of JR IRQ request being handled while some
> of the resources used for that are not yet allocated move the code
> requesting said IRQ to the endo of caam_jr_init(). No functional
                             ^ typo
> change intended.
> 
What qualifies as a "functional change"?
I've seen this comment in several commits.

>  	error = caam_reset_hw_jr(dev);
>  	if (error)
> -		goto out_kill_deq;
> +		return error;
>  
>  	error = -ENOMEM;
>  	jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
>  					   JOBR_DEPTH, &inpbusaddr,
>  					   GFP_KERNEL);
>  	if (!jrp->inpring)
> -		goto out_kill_deq;
> +		return -ENOMEM;
Above there's "error = -ENOMEM;", so why not "return err;" here and
in all the other cases below?

>  
>  	jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
>  					   JOBR_DEPTH, &outbusaddr,
>  					   GFP_KERNEL);
>  	if (!jrp->outring)
> -		goto out_kill_deq;
> +		return -ENOMEM;
>  
>  	jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
>  				    GFP_KERNEL);
>  	if (!jrp->entinfo)
> -		goto out_kill_deq;
> +		return -ENOMEM;
>  
>  	for (i = 0; i < JOBR_DEPTH; i++)
>  		jrp->entinfo[i].desc_addr_dma = !0;
> @@ -483,10 +472,19 @@ static int caam_jr_init(struct device *dev)
>  		      (JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) |
>  		      (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
>  
> +	tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
> +
> +	/* Connect job ring interrupt handler. */
> +	error = devm_request_irq(dev, 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);
> +		tasklet_kill(&jrp->irqtask);
> +		return error;
"return error;" should be moved out the if block.

> +	}
> +
>  	return 0;
> -out_kill_deq:
> -	tasklet_kill(&jrp->irqtask);
> -	return error;
>  }

Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64
  2019-07-17 15:24 ` [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64 Andrey Smirnov
@ 2019-07-29 15:29   ` Horia Geanta
  0 siblings, 0 replies; 29+ messages in thread
From: Horia Geanta @ 2019-07-29 15:29 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel, dl-linux-imx

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> 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>
Reviewed-by: Horia Geantã <horia.geanta@nxp.com>

Just to clarify one thing.

For a previous patch I mentioned:
> To be consistent with CAAM engine HW spec: in case of 64-bit registers,
> irrespective of device endianness, the lower address should be read from
> / written to first, followed by the upper address.
https://lore.kernel.org/linux-crypto/VI1PR0401MB259145C2DFDB5E4084EA5DFC98D20@VI1PR0401MB2591.eurprd04.prod.outlook.com/

I've checked again and actually there is no limitation wrt. the order in which
the two 32-bit parts of 64-bit registers are read from / written to,
except for performance counters (only available on DN parts, not on i.MX).
However, performance counters do not user {rd,wr}_reg64 and should be fixed
separately.

In conclusion, it's ok to use either hi_lo or lo_hi semantics (which is _data_
semantics btw).
It makes more sense for this patch to include io-64-nonatomic-hi-lo.h since
that's what regs.h currently uses.

Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64
  2019-07-17 15:24 ` [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64 Andrey Smirnov
@ 2019-07-29 15:32   ` Horia Geanta
  0 siblings, 0 replies; 29+ messages in thread
From: Horia Geanta @ 2019-07-29 15:32 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> 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>
Reviewed-by: Horia Geantã <horia.geanta@nxp.com>

Thanks,
Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64()
  2019-07-17 15:24 ` [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64() Andrey Smirnov
@ 2019-07-29 15:32   ` Horia Geanta
  0 siblings, 0 replies; 29+ messages in thread
From: Horia Geanta @ 2019-07-29 15:32 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> 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>
Reviewed-by: Horia Geantã <horia.geanta@nxp.com>

Thanks,
Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  2019-07-17 15:24 ` [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs Andrey Smirnov
@ 2019-08-05  8:23   ` Horia Geanta
  2019-08-12 19:27     ` Andrey Smirnov
  0 siblings, 1 reply; 29+ messages in thread
From: Horia Geanta @ 2019-08-05  8:23 UTC (permalink / raw)
  To: Andrey Smirnov, linux-crypto
  Cc: Chris Spencer, Cory Tusar, Chris Healy, Lucas Stach,
	Aymen Sghaier, Leonard Crestez, linux-kernel

On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> i.MX8 SoC still use 32-bit addresses in its CAAM implmentation, so
i.MX8 SoC or i.MX8 mScale?
Looking at the documentation, some i.MX8 parts (for e.g. QM and QXP)
allow for 36-bit addresses.

> change all of the code to be able to handle that.
> 
Shouldn't this case (32-bit CAAM and CONFIG_ARCH_DMA_ADDR_T_64BIT=y) work
for any ARMv8 SoC, i.e. how is this i.MX-specific?

> @@ -603,11 +603,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_ptr_sz should be deduced by reading MCFGR[PS] bit, i.e. decoupled
from dma_addr_t.

There is another configuration that should be considered
(even though highly unlikely):
caam_ptr_sz=1  - > 32-bit addresses for CAAM
CONFIG_ARCH_DMA_ADDR_T_64BIT=n - 32-bit dma_addr_t
so the logic has to be carefully evaluated.

> @@ -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)
Related to my previous comment (i.MX-specific vs. SoC-generic),
this should probably change to smth. like: caam_ptr_sz == sizeof(u64)

>  		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)
Same here.

>  		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) {
Same here: if (caam_ptr_sz == sizeof(u32))

> +		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) {
And here: if (caam_ptr_sz == sizeof(u32))

> +		u32 *inpentry = inpring;
>  
> -	inpentry[hw_idx] = val;
> +		inpentry[hw_idx] = val;
> +	} else {
> +		dma_addr_t *inpentry = inpring;
> +
> +		inpentry[hw_idx] = val;
> +	}
>  }

Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 02/14] crypto: caam - simplfy clock initialization
  2019-07-23 14:17   ` Horia Geanta
@ 2019-08-12 17:56     ` Andrey Smirnov
  0 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-08-12 17:56 UTC (permalink / raw)
  To: Horia Geanta
  Cc: linux-crypto, Leonard Crestez, Iuliana Prodan, Chris Spencer,
	Cory Tusar, Chris Healy, Lucas Stach, Aymen Sghaier,
	linux-kernel

On Tue, Jul 23, 2019 at 7:17 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 7/17/2019 6:25 PM, 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.
> >
> Thanks.
> Two nitpicks below.
>
> > +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;
> Or directly:
>         return devm_add_action_or_reset(dev, disable_clocks, ctrlpriv);
>
> > +     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;
> [...]
> > +             ret = init_clocks(dev, ctrlpriv, imx_soc_match->data);
> ctrlpriv can be retrieved using dev_get_drvdata(dev), thus there's no need
> to pass it as parameter.
>

Will fix in v7.

Thanks,
Andrey Smirnov

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic
  2019-07-23  9:57   ` Horia Geanta
@ 2019-08-12 17:56     ` Andrey Smirnov
  0 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-08-12 17:56 UTC (permalink / raw)
  To: Horia Geanta
  Cc: linux-crypto, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Aymen Sghaier, Leonard Crestez, linux-kernel

On Tue, Jul 23, 2019 at 2:57 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> > 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.
> >
> I am seeing compilation errors like:
>
> In file included from drivers/crypto/caam/ctrl.c:25:0:
> drivers/crypto/caam/qi.h:87:6: error: variably modified 'sh_desc' at file scope
>   u32 sh_desc[MAX_SDLEN];
>       ^
>
> Adding comments for this commit, since it looks like the fixes
> should be included here (related to DESC_JOB_IO_LEN vs. DESC_JOB_IO_LEN_MAX).
>
> Please make sure caam/qi and and caam/qi2 drivers are at least compile-tested.
>
> By caam/qi I am referring to:
> CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI=y/m
>
> and caam/qi2:
> CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM=y/m
>

Sorry about that, should be fixed in v7.

Thanks,
Andrey Smirnov

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step
  2019-07-23 16:02   ` Horia Geanta
@ 2019-08-12 17:59     ` Andrey Smirnov
  0 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-08-12 17:59 UTC (permalink / raw)
  To: Horia Geanta
  Cc: linux-crypto, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Aymen Sghaier, Leonard Crestez, linux-kernel

On Tue, Jul 23, 2019 at 9:02 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> > In order to avoid any risk of JR IRQ request being handled while some
> > of the resources used for that are not yet allocated move the code
> > requesting said IRQ to the endo of caam_jr_init(). No functional
>                              ^ typo
> > change intended.
> >
> What qualifies as a "functional change"?
> I've seen this comment in several commits.
>

My intent was to mark refactoring only changes as such. Probably not
appropriate for this commit. Will drop in v7.

> >       error = caam_reset_hw_jr(dev);
> >       if (error)
> > -             goto out_kill_deq;
> > +             return error;
> >
> >       error = -ENOMEM;
> >       jrp->inpring = dmam_alloc_coherent(dev, sizeof(*jrp->inpring) *
> >                                          JOBR_DEPTH, &inpbusaddr,
> >                                          GFP_KERNEL);
> >       if (!jrp->inpring)
> > -             goto out_kill_deq;
> > +             return -ENOMEM;
> Above there's "error = -ENOMEM;", so why not "return err;" here and
> in all the other cases below?
>

I was going to remove that "error = -ENOMEM;", but forgot. Will do in v7.

> >
> >       jrp->outring = dmam_alloc_coherent(dev, sizeof(*jrp->outring) *
> >                                          JOBR_DEPTH, &outbusaddr,
> >                                          GFP_KERNEL);
> >       if (!jrp->outring)
> > -             goto out_kill_deq;
> > +             return -ENOMEM;
> >
> >       jrp->entinfo = devm_kcalloc(dev, JOBR_DEPTH, sizeof(*jrp->entinfo),
> >                                   GFP_KERNEL);
> >       if (!jrp->entinfo)
> > -             goto out_kill_deq;
> > +             return -ENOMEM;
> >
> >       for (i = 0; i < JOBR_DEPTH; i++)
> >               jrp->entinfo[i].desc_addr_dma = !0;
> > @@ -483,10 +472,19 @@ static int caam_jr_init(struct device *dev)
> >                     (JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) |
> >                     (JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
> >
> > +     tasklet_init(&jrp->irqtask, caam_jr_dequeue, (unsigned long)dev);
> > +
> > +     /* Connect job ring interrupt handler. */
> > +     error = devm_request_irq(dev, 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);
> > +             tasklet_kill(&jrp->irqtask);
> > +             return error;
> "return error;" should be moved out the if block.
>

Sure, will do in v7.

Thanks,
Andrey Smirnov

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  2019-08-05  8:23   ` Horia Geanta
@ 2019-08-12 19:27     ` Andrey Smirnov
  2019-08-13 13:38       ` Horia Geanta
  0 siblings, 1 reply; 29+ messages in thread
From: Andrey Smirnov @ 2019-08-12 19:27 UTC (permalink / raw)
  To: Horia Geanta
  Cc: linux-crypto, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Aymen Sghaier, Leonard Crestez, linux-kernel

On Mon, Aug 5, 2019 at 1:23 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> > i.MX8 SoC still use 32-bit addresses in its CAAM implmentation, so
> i.MX8 SoC or i.MX8 mScale?
> Looking at the documentation, some i.MX8 parts (for e.g. QM and QXP)
> allow for 36-bit addresses.
>

mScale. Will update the message.

> > change all of the code to be able to handle that.
> >
> Shouldn't this case (32-bit CAAM and CONFIG_ARCH_DMA_ADDR_T_64BIT=y) work
> for any ARMv8 SoC, i.e. how is this i.MX-specific?
>

It's a generic change.

> > @@ -603,11 +603,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_ptr_sz should be deduced by reading MCFGR[PS] bit, i.e. decoupled
> from dma_addr_t.
>

MCFGR[PS] is not mentioned in i.MX8MQ SRM and MCFG_PS in CTPR_MS is
documented as set to "0" (seems to match in real HW as well). Doesn't
seem like a workable solution for i.MX8MQ. Is there something I am
missing?

> There is another configuration that should be considered
> (even though highly unlikely):
> caam_ptr_sz=1  - > 32-bit addresses for CAAM
> CONFIG_ARCH_DMA_ADDR_T_64BIT=n - 32-bit dma_addr_t
> so the logic has to be carefully evaluated.
>

I don't understand what you mean here. 32-bit CAAM + 32-bit dma_addr_t
should already be the case for i.MX6, etc. how is what you describe
different?

> > @@ -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)
> Related to my previous comment (i.MX-specific vs. SoC-generic),
> this should probably change to smth. like: caam_ptr_sz == sizeof(u64)
>

Makes sense, will do here and in other places.

Thanks,
Andrey Smirnov

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  2019-08-12 19:27     ` Andrey Smirnov
@ 2019-08-13 13:38       ` Horia Geanta
  2019-08-13 18:50         ` Andrey Smirnov
  0 siblings, 1 reply; 29+ messages in thread
From: Horia Geanta @ 2019-08-13 13:38 UTC (permalink / raw)
  To: Andrey Smirnov
  Cc: linux-crypto, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Aymen Sghaier, Leonard Crestez, linux-kernel

On 8/12/2019 10:27 PM, Andrey Smirnov wrote:
> On Mon, Aug 5, 2019 at 1:23 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>>
>> On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
>>> @@ -603,11 +603,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_ptr_sz should be deduced by reading MCFGR[PS] bit, i.e. decoupled
>> from dma_addr_t.
>>
> 
> MCFGR[PS] is not mentioned in i.MX8MQ SRM and MCFG_PS in CTPR_MS is
> documented as set to "0" (seems to match in real HW as well). Doesn't
> seem like a workable solution for i.MX8MQ. Is there something I am
> missing?
> 
If CTPR_MS[PS]=0, this means CAAM does not allow choosing the "pointer size"
via MCFGR[PS]. Usually in this case the RM does not document MCFGR[PS] bit,
which is identical to MCFGR[PS]=0.

Thus the logic should be smth. like:
	caam_ptr_sz = CTPR_MS[PS] && MCFGR[PS] ? 64 : 32;

>> There is another configuration that should be considered
>> (even though highly unlikely):
>> caam_ptr_sz=1  - > 32-bit addresses for CAAM
>> CONFIG_ARCH_DMA_ADDR_T_64BIT=n - 32-bit dma_addr_t
>> so the logic has to be carefully evaluated.
>>
> 
> I don't understand what you mean here. 32-bit CAAM + 32-bit dma_addr_t
> should already be the case for i.MX6, etc. how is what you describe
> different?
> 
Sorry for not being clear.

caam_ptr_sz=1  - > 32-bit addresses for CAAM
should have been
caam_ptr_sz=*64*  - > 32-bit addresses for CAAM
i.e. CAAM address has "more than" (>) 32 bits (exact number of bits is
SoC / chassis specific) and thus will be represented on 8 bytes.

Thanks,
Horia

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs
  2019-08-13 13:38       ` Horia Geanta
@ 2019-08-13 18:50         ` Andrey Smirnov
  0 siblings, 0 replies; 29+ messages in thread
From: Andrey Smirnov @ 2019-08-13 18:50 UTC (permalink / raw)
  To: Horia Geanta
  Cc: linux-crypto, Chris Spencer, Cory Tusar, Chris Healy,
	Lucas Stach, Aymen Sghaier, Leonard Crestez, linux-kernel

On Tue, Aug 13, 2019 at 6:38 AM Horia Geanta <horia.geanta@nxp.com> wrote:
>
> On 8/12/2019 10:27 PM, Andrey Smirnov wrote:
> > On Mon, Aug 5, 2019 at 1:23 AM Horia Geanta <horia.geanta@nxp.com> wrote:
> >>
> >> On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> >>> @@ -603,11 +603,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_ptr_sz should be deduced by reading MCFGR[PS] bit, i.e. decoupled
> >> from dma_addr_t.
> >>
> >
> > MCFGR[PS] is not mentioned in i.MX8MQ SRM and MCFG_PS in CTPR_MS is
> > documented as set to "0" (seems to match in real HW as well). Doesn't
> > seem like a workable solution for i.MX8MQ. Is there something I am
> > missing?
> >
> If CTPR_MS[PS]=0, this means CAAM does not allow choosing the "pointer size"
> via MCFGR[PS]. Usually in this case the RM does not document MCFGR[PS] bit,
> which is identical to MCFGR[PS]=0.
>
> Thus the logic should be smth. like:
>         caam_ptr_sz = CTPR_MS[PS] && MCFGR[PS] ? 64 : 32;
>

Where is PS located in MCFGR? Same as in CTPR_MS, i.e. BIT(17)?

> >> There is another configuration that should be considered
> >> (even though highly unlikely):
> >> caam_ptr_sz=1  - > 32-bit addresses for CAAM
> >> CONFIG_ARCH_DMA_ADDR_T_64BIT=n - 32-bit dma_addr_t
> >> so the logic has to be carefully evaluated.
> >>
> >
> > I don't understand what you mean here. 32-bit CAAM + 32-bit dma_addr_t
> > should already be the case for i.MX6, etc. how is what you describe
> > different?
> >
> Sorry for not being clear.
>
> caam_ptr_sz=1  - > 32-bit addresses for CAAM
> should have been
> caam_ptr_sz=*64*  - > 32-bit addresses for CAAM
> i.e. CAAM address has "more than" (>) 32 bits (exact number of bits is
> SoC / chassis specific) and thus will be represented on 8 bytes.
>

Ah, I see. Can this use-case be addressed in a separate series when
the need for it arises?

Thanks,
Andrey Smirnov

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, back to index

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 15:24 [PATCH v6 00/14] crypto: caam - Add i.MX8MQ support Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 01/14] crypto: caam - move DMA mask selection into a function Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 02/14] crypto: caam - simplfy clock initialization Andrey Smirnov
2019-07-23 14:17   ` Horia Geanta
2019-08-12 17:56     ` Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 03/14] crypto: caam - convert caam_jr_init() to use devres Andrey Smirnov
2019-07-23 15:48   ` Horia Geanta
2019-07-17 15:24 ` [PATCH v6 04/14] crypto: caam - request JR IRQ as the last step Andrey Smirnov
2019-07-23 16:02   ` Horia Geanta
2019-08-12 17:59     ` Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64 Andrey Smirnov
2019-07-29 15:29   ` Horia Geanta
2019-07-17 15:24 ` [PATCH v6 06/14] crypto: caam - use ioread64*_hi_lo in rd_reg64 Andrey Smirnov
2019-07-29 15:32   ` Horia Geanta
2019-07-17 15:24 ` [PATCH v6 07/14] crypto: caam - drop 64-bit only wr/rd_reg64() Andrey Smirnov
2019-07-29 15:32   ` Horia Geanta
2019-07-17 15:24 ` [PATCH v6 08/14] crypto: caam - make CAAM_PTR_SZ dynamic Andrey Smirnov
2019-07-23  9:57   ` Horia Geanta
2019-08-12 17:56     ` Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 09/14] crypto: caam - move cpu_to_caam_dma() selection to runtime Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 10/14] crypto: caam - drop explicit usage of struct jr_outentry Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 11/14] crypto: caam - don't hardcode inpentry size Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 12/14] crypto: caam - force DMA address to 32-bit on 64-bit i.MX SoCs Andrey Smirnov
2019-08-05  8:23   ` Horia Geanta
2019-08-12 19:27     ` Andrey Smirnov
2019-08-13 13:38       ` Horia Geanta
2019-08-13 18:50         ` Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 13/14] crypto: caam - always select job ring via RSR on i.MX8MQ Andrey Smirnov
2019-07-17 15:24 ` [PATCH v6 14/14] crypto: caam - add clock entry for i.MX8MQ Andrey Smirnov

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org linux-crypto@archiver.kernel.org
	public-inbox-index linux-crypto


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/ public-inbox