2019-06-17 16:04:28

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 0/5] crypto: caam - Add i.MX8MQ support

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 [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

[v2] lore.kernel.org/r/[email protected]
[v1] https://patchwork.kernel.org/cover/10825625/


Andrey Smirnov (4):
crypto: caam - move DMA mask selection into a function
crypto: caam - always select job ring via RSR on i.MX8MQ
crypto: caam - simplfy clock initialization
crypto: caam - add clock entry for i.MX8MQ

Chris Spencer (1):
crypto: caam - correct DMA address size for the i.MX8

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

--
2.21.0


2019-06-17 16:04:31

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 4/5] crypto: caam - simplfy clock initialization

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 <[email protected]>
Cc: Chris Spencer <[email protected]>
Cc: Cory Tusar <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Horia Geantă <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/caam/ctrl.c | 214 +++++++++++++++++------------------
drivers/crypto/caam/intern.h | 5 -
2 files changed, 107 insertions(+), 112 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e6e2457a573e..b9655957d369 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
@@ -347,13 +337,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;
}

@@ -502,20 +485,113 @@ static const struct of_device_id caam_match[] = {
};
MODULE_DEVICE_TABLE(of, caam_match);

+struct clk_bulk_caam {
+ 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 clk_bulk_caam caam_imx6_clk_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 clk_bulk_caam caam_imx7_clk_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 clk_bulk_caam caam_imx6ul_clk_data = {
+ .clks = caam_imx6ul_clks,
+ .num_clks = ARRAY_SIZE(caam_imx6ul_clks),
+};
+
+
+static const struct soc_device_attribute imx_soc[] = {
+ { .soc_id = "i.MX6UL", .data = &caam_imx6ul_clk_data },
+ { .soc_id = "i.MX6*", .data = &caam_imx6_clk_data },
+ { .soc_id = "i.MX7*", .data = &caam_imx7_clk_data },
+ { .family = "Freescale i.MX" },
+};
+
+static void disable_clocks(void *private)
+{
+ struct clk_bulk_caam *context = private;
+
+ clk_bulk_disable_unprepare(context->num_clks,
+ (struct clk_bulk_data *)context->clks);
+}
+
+static int init_clocks(struct device *dev,
+ const struct clk_bulk_caam *data)
+{
+ struct clk_bulk_data *clks;
+ struct clk_bulk_caam *context;
+ int num_clks;
+ int ret;
+
+ num_clks = data->num_clks;
+ clks = devm_kmemdup(dev, data->clks,
+ data->num_clks * sizeof(data->clks[0]),
+ GFP_KERNEL);
+ if (!clks)
+ return -ENOMEM;
+
+ ret = devm_clk_bulk_get(dev, num_clks, clks);
+ if (ret) {
+ dev_err(dev,
+ "Failed to request all necessary clocks\n");
+ return ret;
+ }
+
+ ret = clk_bulk_prepare_enable(num_clks, clks);
+ if (ret) {
+ dev_err(dev,
+ "Failed to prepare/enable all necessary clocks\n");
+ return ret;
+ }
+
+ context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
+ if (!context)
+ return -ENOMEM;
+
+ context->num_clks = num_clks;
+ context->clks = clks;
+
+ ret = devm_add_action_or_reset(dev, disable_clocks, context);
+ 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 *soc_attr;
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
@@ -532,91 +608,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;
+ soc_attr = soc_device_match(imx_soc);
+ if (soc_attr) {
+ if (!soc_attr->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, soc_attr->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)soc_attr;

/* 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) &
@@ -904,16 +914,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 bf115f8ddb93..81f73d32a9f8 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -94,11 +94,6 @@ 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;
-
/*
* debugfs entries for developer view into driver/device
* variables at runtime.
--
2.21.0

2019-06-17 16:04:40

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 5/5] crypto: caam - add clock entry for i.MX8MQ

Add clock entry needed to support i.MX8MQ.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Spencer <[email protected]>
Cc: Cory Tusar <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Horia Geantă <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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 b9655957d369..888eacc7c17d 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -528,6 +528,7 @@ static const struct soc_device_attribute imx_soc[] = {
{ .soc_id = "i.MX6UL", .data = &caam_imx6ul_clk_data },
{ .soc_id = "i.MX6*", .data = &caam_imx6_clk_data },
{ .soc_id = "i.MX7*", .data = &caam_imx7_clk_data },
+ { .soc_id = "i.MX8MQ", .data = &caam_imx7_clk_data },
{ .family = "Freescale i.MX" },
};

--
2.21.0

2019-06-17 16:05:13

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH v3 2/5] crypto: caam - correct DMA address size for the i.MX8

From: Chris Spencer <[email protected]>

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

Signed-off-by: Aymen Sghaier <[email protected]>
Signed-off-by: Franck LENORMAND <[email protected]>
Signed-off-by: Chris Spencer <[email protected]>
Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Spencer <[email protected]>
Cc: Cory Tusar <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Horia Geantă <[email protected]>
Cc: Aymen Sghaier <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/caam/desc_constr.h | 24 +++++++--------
drivers/crypto/caam/intern.h | 6 ++--
drivers/crypto/caam/pdb.h | 49 ++++++++++++++++---------------
drivers/crypto/caam/regs.h | 21 +++++++++++--
4 files changed, 58 insertions(+), 42 deletions(-)

diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index 5988a26a2441..fc8042e63b18 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -14,7 +14,7 @@

#define IMMEDIATE (1 << 23)
#define CAAM_CMD_SZ sizeof(u32)
-#define CAAM_PTR_SZ sizeof(dma_addr_t)
+#define CAAM_PTR_SZ sizeof(caam_dma_addr_t)
#define CAAM_DESC_BYTES_MAX (CAAM_CMD_SZ * MAX_CAAM_DESCSIZE)
#define DESC_JOB_IO_LEN (CAAM_CMD_SZ * 5 + CAAM_PTR_SZ * 3)

@@ -101,9 +101,9 @@ static inline void init_job_desc_pdb(u32 * const desc, u32 options,
init_job_desc(desc, (((pdb_len + 1) << HDR_START_IDX_SHIFT)) | options);
}

-static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
+static inline void append_ptr(u32 * const desc, caam_dma_addr_t ptr)
{
- dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
+ caam_dma_addr_t *offset = (caam_dma_addr_t *)desc_end(desc);

*offset = cpu_to_caam_dma(ptr);

@@ -111,7 +111,7 @@ static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
CAAM_PTR_SZ / CAAM_CMD_SZ);
}

-static inline void init_job_desc_shared(u32 * const desc, dma_addr_t ptr,
+static inline void init_job_desc_shared(u32 * const desc, caam_dma_addr_t ptr,
int len, u32 options)
{
PRINT_POS;
@@ -166,7 +166,7 @@ static inline u32 *write_cmd(u32 * const desc, u32 command)
return desc + 1;
}

-static inline void append_cmd_ptr(u32 * const desc, dma_addr_t ptr, int len,
+static inline void append_cmd_ptr(u32 * const desc, caam_dma_addr_t ptr, int len,
u32 command)
{
append_cmd(desc, command | len);
@@ -174,7 +174,7 @@ static inline void append_cmd_ptr(u32 * const desc, dma_addr_t ptr, int len,
}

/* Write length after pointer, rather than inside command */
-static inline void append_cmd_ptr_extlen(u32 * const desc, dma_addr_t ptr,
+static inline void append_cmd_ptr_extlen(u32 * const desc, caam_dma_addr_t ptr,
unsigned int len, u32 command)
{
append_cmd(desc, command);
@@ -239,7 +239,7 @@ APPEND_CMD_LEN(seq_fifo_load, SEQ_FIFO_LOAD)
APPEND_CMD_LEN(seq_fifo_store, SEQ_FIFO_STORE)

#define APPEND_CMD_PTR(cmd, op) \
-static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd(u32 * const desc, caam_dma_addr_t ptr, \
unsigned int len, u32 options) \
{ \
PRINT_POS; \
@@ -250,7 +250,7 @@ APPEND_CMD_PTR(load, LOAD)
APPEND_CMD_PTR(fifo_load, FIFO_LOAD)
APPEND_CMD_PTR(fifo_store, FIFO_STORE)

-static inline void append_store(u32 * const desc, dma_addr_t ptr,
+static inline void append_store(u32 * const desc, caam_dma_addr_t ptr,
unsigned int len, u32 options)
{
u32 cmd_src;
@@ -269,7 +269,7 @@ static inline void append_store(u32 * const desc, dma_addr_t ptr,

#define APPEND_SEQ_PTR_INTLEN(cmd, op) \
static inline void append_seq_##cmd##_ptr_intlen(u32 * const desc, \
- dma_addr_t ptr, \
+ caam_dma_addr_t ptr, \
unsigned int len, \
u32 options) \
{ \
@@ -293,7 +293,7 @@ APPEND_CMD_PTR_TO_IMM(load, LOAD);
APPEND_CMD_PTR_TO_IMM(fifo_load, FIFO_LOAD);

#define APPEND_CMD_PTR_EXTLEN(cmd, op) \
-static inline void append_##cmd##_extlen(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd##_extlen(u32 * const desc, caam_dma_addr_t ptr, \
unsigned int len, u32 options) \
{ \
PRINT_POS; \
@@ -307,7 +307,7 @@ APPEND_CMD_PTR_EXTLEN(seq_out_ptr, SEQ_OUT_PTR)
* the size of its type
*/
#define APPEND_CMD_PTR_LEN(cmd, op, type) \
-static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
+static inline void append_##cmd(u32 * const desc, caam_dma_addr_t ptr, \
type len, u32 options) \
{ \
PRINT_POS; \
@@ -467,7 +467,7 @@ struct alginfo {
unsigned int keylen;
unsigned int keylen_pad;
union {
- dma_addr_t key_dma;
+ caam_dma_addr_t key_dma;
const void *key_virt;
};
bool key_inline;
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index ec25d260fa40..bf115f8ddb93 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -34,7 +34,7 @@ struct caam_jrentry_info {
void (*callbk)(struct device *dev, u32 *desc, u32 status, void *arg);
void *cbkarg; /* Argument per ring entry */
u32 *desc_addr_virt; /* Stored virt addr for postprocessing */
- dma_addr_t desc_addr_dma; /* Stored bus addr for done matching */
+ caam_dma_addr_t desc_addr_dma; /* Stored bus addr for done matching */
u32 desc_size; /* Stored size for postprocessing, header derived */
};

@@ -55,7 +55,7 @@ struct caam_drv_private_jr {
spinlock_t inplock ____cacheline_aligned; /* Input ring index lock */
u32 inpring_avail; /* Number of free entries in input ring */
int head; /* entinfo (s/w ring) head index */
- dma_addr_t *inpring; /* Base of input ring, alloc DMA-safe */
+ caam_dma_addr_t *inpring; /* Base of input ring, alloc DMA-safe */
int out_ring_read_index; /* Output index "tail" */
int tail; /* entinfo (s/w ring) tail index */
struct jr_outentry *outring; /* Base of output ring, DMA-safe */
@@ -221,7 +221,7 @@ static inline u64 caam_get_dma_mask(struct device *dev)
{
struct device_node *nprop = dev->of_node;

- if (sizeof(dma_addr_t) != sizeof(u64))
+ if (sizeof(caam_dma_addr_t) != sizeof(u64))
return DMA_BIT_MASK(32);

if (caam_dpaa2)
diff --git a/drivers/crypto/caam/pdb.h b/drivers/crypto/caam/pdb.h
index 810f0bef0652..128d16682feb 100644
--- a/drivers/crypto/caam/pdb.h
+++ b/drivers/crypto/caam/pdb.h
@@ -9,6 +9,7 @@
#ifndef CAAM_PDB_H
#define CAAM_PDB_H
#include "compat.h"
+#include "regs.h"

/*
* PDB- IPSec ESP Header Modification Options
@@ -507,10 +508,10 @@ struct dsa_verify_pdb {
*/
struct rsa_pub_pdb {
u32 sgf;
- dma_addr_t f_dma;
- dma_addr_t g_dma;
- dma_addr_t n_dma;
- dma_addr_t e_dma;
+ caam_dma_addr_t f_dma;
+ caam_dma_addr_t g_dma;
+ caam_dma_addr_t n_dma;
+ caam_dma_addr_t e_dma;
u32 f_len;
} __packed;

@@ -524,10 +525,10 @@ struct rsa_pub_pdb {
*/
struct rsa_priv_f1_pdb {
u32 sgf;
- dma_addr_t g_dma;
- dma_addr_t f_dma;
- dma_addr_t n_dma;
- dma_addr_t d_dma;
+ caam_dma_addr_t g_dma;
+ caam_dma_addr_t f_dma;
+ caam_dma_addr_t n_dma;
+ caam_dma_addr_t d_dma;
} __packed;

/**
@@ -546,13 +547,13 @@ struct rsa_priv_f1_pdb {
*/
struct rsa_priv_f2_pdb {
u32 sgf;
- dma_addr_t g_dma;
- dma_addr_t f_dma;
- dma_addr_t d_dma;
- dma_addr_t p_dma;
- dma_addr_t q_dma;
- dma_addr_t tmp1_dma;
- dma_addr_t tmp2_dma;
+ caam_dma_addr_t g_dma;
+ caam_dma_addr_t f_dma;
+ caam_dma_addr_t d_dma;
+ caam_dma_addr_t p_dma;
+ caam_dma_addr_t q_dma;
+ caam_dma_addr_t tmp1_dma;
+ caam_dma_addr_t tmp2_dma;
u32 p_q_len;
} __packed;

@@ -576,15 +577,15 @@ struct rsa_priv_f2_pdb {
*/
struct rsa_priv_f3_pdb {
u32 sgf;
- dma_addr_t g_dma;
- dma_addr_t f_dma;
- dma_addr_t c_dma;
- dma_addr_t p_dma;
- dma_addr_t q_dma;
- dma_addr_t dp_dma;
- dma_addr_t dq_dma;
- dma_addr_t tmp1_dma;
- dma_addr_t tmp2_dma;
+ caam_dma_addr_t g_dma;
+ caam_dma_addr_t f_dma;
+ caam_dma_addr_t c_dma;
+ caam_dma_addr_t p_dma;
+ caam_dma_addr_t q_dma;
+ caam_dma_addr_t dp_dma;
+ caam_dma_addr_t dq_dma;
+ caam_dma_addr_t tmp1_dma;
+ caam_dma_addr_t tmp2_dma;
u32 p_q_len;
} __packed;

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 8591914d5c51..6e7f8d4f3f2b 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -137,7 +137,7 @@ static inline void clrsetbits_32(void __iomem *reg, u32 clear, u32 set)
* base + 0x0000 : least-significant 32 bits
* base + 0x0004 : most-significant 32 bits
*/
-#ifdef CONFIG_64BIT
+#if defined(CONFIG_64BIT) && !defined(CONFIG_ARCH_MXC)
static inline void wr_reg64(void __iomem *reg, u64 data)
{
if (caam_little_end)
@@ -195,7 +195,7 @@ static inline u64 caam_dma64_to_cpu(u64 value)
return caam64_to_cpu(value);
}

-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+#if defined(CONFIG_ARCH_DMA_ADDR_T_64BIT) && !defined(CONFIG_ARCH_MXC)
#define cpu_to_caam_dma(value) cpu_to_caam_dma64(value)
#define caam_dma_to_cpu(value) caam_dma64_to_cpu(value)
#else
@@ -203,12 +203,27 @@ static inline u64 caam_dma64_to_cpu(u64 value)
#define caam_dma_to_cpu(value) caam32_to_cpu(value)
#endif /* CONFIG_ARCH_DMA_ADDR_T_64BIT */

+/*
+ * On i.MX8 boards the arch is arm64 but the CAAM dma address size is
+ * 32 bits on 8MQ and 36 bits on 8QM and 8QXP.
+ * For 8QM and 8QXP there is a configurable field PS called pointer size
+ * in the MCFGR register to switch between 32 and 64 (default 32)
+ * But this register is only accessible by the SECO and is left to its
+ * default value.
+ * Here we set the CAAM dma address size to 32 bits for all i.MX8
+ */
+#if defined(CONFIG_ARM64) && defined(CONFIG_ARCH_MXC)
+#define caam_dma_addr_t u32
+#else
+#define caam_dma_addr_t dma_addr_t
+#endif
+
/*
* jr_outentry
* Represents each entry in a JobR output ring
*/
struct jr_outentry {
- dma_addr_t desc;/* Pointer to completed descriptor */
+ caam_dma_addr_t desc;/* Pointer to completed descriptor */
u32 jrstatus; /* Status for completed descriptor */
} __packed;

--
2.21.0

2019-06-17 19:25:10

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] crypto: caam - correct DMA address size for the i.MX8

On 6/17/2019 7:04 PM, Andrey Smirnov wrote:
> From: Chris Spencer <[email protected]>
>
> The i.MX8 is arm64, but its CAAM DMA address size is 32-bits.

> +/*
> + * On i.MX8 boards the arch is arm64 but the CAAM dma address size is
> + * 32 bits on 8MQ and 36 bits on 8QM and 8QXP.
> + * For 8QM and 8QXP there is a configurable field PS called pointer size
> + * in the MCFGR register to switch between 32 and 64 (default 32)
> + * But this register is only accessible by the SECO and is left to its
> + * default value.
> + * Here we set the CAAM dma address size to 32 bits for all i.MX8
> + */
> +#if defined(CONFIG_ARM64) && defined(CONFIG_ARCH_MXC)
> +#define caam_dma_addr_t u32
> +#else
> +#define caam_dma_addr_t dma_addr_t
> +#endif

Wait, doesn't this break Layerscape? Support for multiple SOC families
can be enabled at the same time and it is something that we actually
want to support.

--
Regards,
Leonard

2019-06-17 19:48:32

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] crypto: caam - simplfy clock initialization

On 6/17/2019 7:04 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.

Subject is misspelled.

> +struct clk_bulk_caam {
> + const struct clk_bulk_data *clks;
> + int num_clks;
> +};

clks could be an array[0] at the end to avoid an additional allocation.

> +static void disable_clocks(void *private)
> +{
> + struct clk_bulk_caam *context = private;
> +
> + clk_bulk_disable_unprepare(context->num_clks,
> + (struct clk_bulk_data *)context->clks);
> +}

Not sure using devm for this is worthwhile. Maybe someday CAAM clks will
be enabled dynamically?

It would be make sense to reference "clk" instead of "clocks".

> +static int init_clocks(struct device *dev,
> + const struct clk_bulk_caam *data)
> +{
> + struct clk_bulk_data *clks;
> + struct clk_bulk_caam *context;
> + int num_clks;
> + int ret;
> +
> + num_clks = data->num_clks;
> + clks = devm_kmemdup(dev, data->clks,
> + data->num_clks * sizeof(data->clks[0]),
> + GFP_KERNEL);
> + if (!clks)
> + return -ENOMEM;
> +
> + ret = devm_clk_bulk_get(dev, num_clks, clks);
> + if (ret) {
> + dev_err(dev,
> + "Failed to request all necessary clocks\n");
> + return ret;
> + }
> +
> + ret = clk_bulk_prepare_enable(num_clks, clks);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable all necessary clocks\n");
> + return ret;
> + }
> +
> + context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> + if (!context)
> + return -ENOMEM;

Aren't clks left enabled if this fails? Can move this allocation higher.

> + context->num_clks = num_clks;
> + context->clks = clks;
> +
> + ret = devm_add_action_or_reset(dev, disable_clocks, context);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}

> 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 *soc_attr;

This "soc_attr" is difficult to understand, maybe rename to something
like "imx_soc_match"?

2019-06-18 00:21:49

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] crypto: caam - simplfy clock initialization

On Mon, Jun 17, 2019 at 12:48 PM Leonard Crestez
<[email protected]> wrote:
>
> On 6/17/2019 7:04 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.
>
> Subject is misspelled.
>

Will fix.

> > +struct clk_bulk_caam {
> > + const struct clk_bulk_data *clks;
> > + int num_clks;
> > +};
>
> clks could be an array[0] at the end to avoid an additional allocation.
>

struct clk_bulk_caam is also used to declare caam_imx*_clk_data
variables, where this approach wouldn't work.

> > +static void disable_clocks(void *private)
> > +{
> > + struct clk_bulk_caam *context = private;
> > +
> > + clk_bulk_disable_unprepare(context->num_clks,
> > + (struct clk_bulk_data *)context->clks);
> > +}
>
> Not sure using devm for this is worthwhile. Maybe someday CAAM clks will
> be enabled dynamically?
>

I don't see a reason why this code can't be changed _if_ and when that
ever happens.

> It would be make sense to reference "clk" instead of "clocks".
>

I am not sure what you reference here. I'd rather we avoid discussing
trivial spelling aspects like this.

> > +static int init_clocks(struct device *dev,
> > + const struct clk_bulk_caam *data)
> > +{
> > + struct clk_bulk_data *clks;
> > + struct clk_bulk_caam *context;
> > + int num_clks;
> > + int ret;
> > +
> > + num_clks = data->num_clks;
> > + clks = devm_kmemdup(dev, data->clks,
> > + data->num_clks * sizeof(data->clks[0]),
> > + GFP_KERNEL);
> > + if (!clks)
> > + return -ENOMEM;
> > +
> > + ret = devm_clk_bulk_get(dev, num_clks, clks);
> > + if (ret) {
> > + dev_err(dev,
> > + "Failed to request all necessary clocks\n");
> > + return ret;
> > + }
> > +
> > + ret = clk_bulk_prepare_enable(num_clks, clks);
> > + if (ret) {
> > + dev_err(dev,
> > + "Failed to prepare/enable all necessary clocks\n");
> > + return ret;
> > + }
> > +
> > + context = devm_kzalloc(dev, sizeof(*context), GFP_KERNEL);
> > + if (!context)
> > + return -ENOMEM;
>
> Aren't clks left enabled if this fails? Can move this allocation higher.
>

Good point, will do.

> > + context->num_clks = num_clks;
> > + context->clks = clks;
> > +
> > + ret = devm_add_action_or_reset(dev, disable_clocks, context);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
>
> > 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 *soc_attr;
>
> This "soc_attr" is difficult to understand, maybe rename to something
> like "imx_soc_match"?

Sure, will do in next version.

Thanks,
Andrey Smirnov