Hi
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, December 28, 2021 2:02 PM To: Gaurav Jain gaurav.jain@nxp.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; dl- uboot-imx uboot-imx@nxp.com; Shengzhou Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com; Pramod Kumar pramod.kumar_1@nxp.com; Andy Tang andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@gmail.com Subject: [EXT] Re: [PATCH v7 01/15] crypto/fsl: Add support for CAAM Job ring driver model
Caution: EXT Email
Hi Gaurav,
On Tue, 7 Dec 2021 at 00:42, Gaurav Jain gaurav.jain@nxp.com wrote:
added device tree support for job ring driver. sec is initialized based on job ring information processed from device tree.
Signed-off-by: Gaurav Jain gaurav.jain@nxp.com Reviewed-by: Ye Li ye.li@nxp.com
drivers/crypto/fsl/jr.c | 323 ++++++++++++++++++++++++++-------------- drivers/crypto/fsl/jr.h | 19 +++ 2 files changed, 231 insertions(+), 111 deletions(-)
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index 22b649219e..441550f348 100644 --- a/drivers/crypto/fsl/jr.c +++ b/drivers/crypto/fsl/jr.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ /*
- Copyright 2008-2014 Freescale Semiconductor, Inc.
- Copyright 2018 NXP
*/
- Copyright 2018, 2021 NXP
- Based on CAAM driver in drivers/crypto/caam in Linux
@@ -11,7 +11,6 @@ #include <linux/kernel.h> #include <log.h> #include <malloc.h> -#include "fsl_sec.h" #include "jr.h" #include "jobdesc.h" #include "desc_constr.h" @@ -21,8 +20,11 @@ #include <asm/cache.h> #include <asm/fsl_pamu.h> #endif +#include <dm.h> #include <dm/lists.h> #include <linux/delay.h> +#include <dm/root.h> +#include <dm/device-internal.h>
These should go up one line.
Ok. Will do in next version.
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.de nx.de%2Fwiki%2FU- Boot%2FCodingStyle&data=04%7C01%7Cgaurav.jain%40nxp.com%7C021c 004f07e44d796a7408d9c9dc9e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7 C0%7C0%7C637762771646492648%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a mp;sdata=HOzWAz8vqKuavYOZopqZf3iLD5dbenfY2UEF%2BO0h1W4%3D&r eserved=0
Re the uclass, this is using MISC. What operations are actually supported? I am still not sure why this doesn't deserve its own uclass.
Ioctl operation(caam_jr_ioctl) provided by MISC Uclass is sufficient for this driver. I do not see a need for introducing a new Uclass to support this driver.
[..]
diff --git a/drivers/crypto/fsl/jr.h b/drivers/crypto/fsl/jr.h index 1047aa772c..c5e5ed5c26 100644 --- a/drivers/crypto/fsl/jr.h +++ b/drivers/crypto/fsl/jr.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /*
- Copyright 2008-2014 Freescale Semiconductor, Inc.
*/
- Copyright 2021 NXP
@@ -8,7 +9,9 @@ #define __JR_H
#include <linux/compiler.h> +#include "fsl_sec.h" #include "type.h" +#include <misc.h>
#define JR_SIZE 4 /* Timeout currently defined as 10 sec */ @@ -35,12 +38,21 @@ #define JRSLIODN_SHIFT 0 #define JRSLIODN_MASK 0x00000fff
+#define JRDID_MS_PRIM_DID BIT(0) +#define JRDID_MS_PRIM_TZ BIT(4) +#define JRDID_MS_TZ_OWN BIT(15)
#define JQ_DEQ_ERR -1 #define JQ_DEQ_TO_ERR -2 #define JQ_ENQ_ERR -3
Good idea to put brackets around #defines that are a -ve number.
Will do in next version.
#define RNG4_MAX_HANDLES 2
+enum {
/* Run caam jobring descriptor(in buf) */CAAM_JR_RUN_DESC,+};
struct op_ring { caam_dma_addr_t desc; uint32_t status; @@ -102,6 +114,13 @@ struct result { uint32_t status; };
+struct caam_regs {
ccsr_sec_t *sec;struct jr_regs *regs;u8 jrid;struct jobring jr[CONFIG_SYS_FSL_MAX_NUM_OF_SEC];+};
Don't forget to comment the structs.
Will add in next version.
void caam_jr_strstatus(u32 status); int run_descriptor_jr(uint32_t *desc);
and exported functions (normally drivers shouldn't have these).
Yes I agree. Since run_descriptor_jr() is already in use. I am planning a separate patch to move other user of run_descriptor_jr() to caam_jr_ioctl().
Regards Gaurav Jain
-- 2.17.1
Regards, Simon
Regards, Simon