[U-Boot] [PATCH resend] misc/crypto: Add support for C3
C3 is a cryptographic controller which is used by the SPL when DDR ECC support is enabled.
Basically, the DDR ECC feature requires the initialization of ECC values before the DDR can actually be used. To accomplish this, the complete on board DDR is initialized with zeroes. This initialization can be done using * CPU * CPU (with Dcache enabled) * C3
The current SPL code uses C3 because the initialization using the CPU is slow and we do not have enough memory in SPL to initialize page tables required to enable MMU and Dcache
Signed-off-by: Vipin Kumar vipin.kumar@st.com Reviewed-by: Shiraz Hashim shiraz.hashim@st.com --- drivers/misc/Makefile | 1 + drivers/misc/c3.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/c3.h | 63 ++++++++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 drivers/misc/c3.c create mode 100644 include/c3.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 9fac190..3ef8177 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB := $(obj)libmisc.o
COBJS-$(CONFIG_ALI152X) += ali512x.o +COBJS-$(CONFIG_C3) += c3.o COBJS-$(CONFIG_DS4510) += ds4510.o COBJS-$(CONFIG_FSL_LAW) += fsl_law.o COBJS-$(CONFIG_GPIO_LED) += gpio_led.o diff --git a/drivers/misc/c3.c b/drivers/misc/c3.c new file mode 100644 index 0000000..5f1eaee --- /dev/null +++ b/drivers/misc/c3.c @@ -0,0 +1,122 @@ +/* + * (C) Copyright 2012 + * ST Micoelectronics Pvt. Ltd. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <c3.h> +#include <errno.h> +#include <asm/io.h> +#include <asm/arch/hardware.h> + +static unsigned long c3_mem_xlate(void *addr) +{ + + if (((ulong)addr < C3_INT_MEM_BASE_ADDR) || \ + ((ulong)addr >= (C3_INT_MEM_BASE_ADDR + C3_INT_MEM_SIZE))) + return (ulong)addr; + + return (unsigned long)addr - C3_INT_MEM_BASE_ADDR + + C3_LOCAL_MEM_ADDR; +} + +int c3_init(void) +{ + if (readl(C3_ID0_SCR) != C3_ID0_DEF_RDY_VAL) + writel(C3_ID0_SCR_RST, C3_ID0_SCR); + + if (readl(C3_MOVE_CHANNEL_ID) == C3_MOVE_CHANNEL_ID_VAL) + return -EINVAL; + + writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR); + writel(C3_LOCAL_MEM_ADDR, C3_HIF_MBAR); + + return 0; +} + +static int c3_run(void *prog_start) +{ + writel(c3_mem_xlate(prog_start), C3_ID0_IP); + + while ((readl(C3_ID0_SCR) & C3_ID0_STATE_MASK) == C3_ID0_STATE_RUN) + ; + + if ((readl(C3_ID0_SCR) & C3_ID0_STATE_MASK) != C3_ID0_STATE_IDLE) { + /* If not back to idle an error occured */ + writel(C3_ID0_SCR_RST, C3_ID0_SCR); + + /* Set internal access to run c3 programs */ + writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR); + + return -EIO; + } + + return 0; +} + +static int c3_move(void *dest, void *src, int cnt, int optype, int opdata) +{ + unsigned long *c3_prog; + int ret = 0; + + /* 3.b Prepare program */ + c3_prog = (unsigned long *)C3_INT_MEM_BASE_ADDR; + + /* 3.b.i. Mov init */ + c3_prog[0] = C3_CMD_MOVE_INIT; + c3_prog[1] = opdata; + + /* 3.b.ii. Mov data */ + c3_prog[2] = C3_CMD_MOVE_DATA + cnt + optype; + c3_prog[3] = c3_mem_xlate(src); + c3_prog[4] = c3_mem_xlate(dest); + + /* 3.b.iii. Stop */ + c3_prog[5] = C3_CMD_STOP; + + /* 4. Execute and wait */ + ret = c3_run(c3_prog); + + return ret; +} + +void *c3_memset(void *s, int c, size_t count) +{ +#define DATA_SIZE (1024*4) + u32 data = C3_INT_MEM_BASE_ADDR + 0x100; + u32 size; + size_t cur = 0; + + writel(0x100, C3_HIF_MAAR); + writel(c, C3_HIF_MADR); + + for (size = 4; size < DATA_SIZE; size <<= 1) + c3_move((void *)(data + size), (void *)data, size, + C3_MOVE_AND, 0xffffffff); + + while (cur < count) { + c3_move(s + cur, (void *)data, DATA_SIZE, + C3_MOVE_AND, 0xffffffff); + cur += DATA_SIZE; + } + + return s; +} diff --git a/include/c3.h b/include/c3.h new file mode 100644 index 0000000..541d702 --- /dev/null +++ b/include/c3.h @@ -0,0 +1,63 @@ +/* + * (C) Copyright 2012 + * ST Micoelectronics Pvt. Ltd. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#ifndef _MISC_C3_ +#define _MISC_C3_ + +#include <asm/arch/hardware.h> + +#define C3_HIF_OFF 0x400 /* master interface registers */ + +#define C3_INT_MEM_BASE_ADDR (CONFIG_SYS_C3_BASE + 0x400) +#define C3_HIF_MBAR (C3_INT_MEM_BASE_ADDR + 0x304) + #define C3_LOCAL_MEM_ADDR 0xF0000000 +#define C3_HIF_MCR (C3_INT_MEM_BASE_ADDR + 0x308) + #define C3_HIF_MCR_ENB_INT_MEM 0x01 +#define C3_HIF_MAAR (C3_INT_MEM_BASE_ADDR + 0x310) +#define C3_HIF_MADR (C3_INT_MEM_BASE_ADDR + 0x314) + +/* ID0 Registers definition */ +#define C3_ID0_SCR (CONFIG_SYS_C3_BASE + 0x1000) + #define C3_ID0_SCR_RST (1 << 16) +#define C3_ID0_IP (CONFIG_SYS_C3_BASE + 0x1000 + 0x10) + #define C3_ID0_DEF_RDY_VAL 0x80002AAA + #define C3_ID0_STATE_MASK 0xC0000000 + #define C3_ID0_STATE_RUN 0xC0000000 + #define C3_ID0_STATE_IDLE 0x80000000 + +/* C3 Register Offsets */ +#define C3_MOVE_CHANNEL_ID (CONFIG_SYS_C3_BASE + 0x2000 + 0x3FC) + #define C3_MOVE_CHANNEL_ID_VAL 0x00000102 + +#define C3_INT_MEM_SIZE 0x4000 +#define C3_MOVE_AND (1 << 21) + +/* Some Commands */ +#define C3_CMD_MOVE_INIT 0x06000000 +#define C3_CMD_MOVE_DATA 0x0A800000 +#define C3_CMD_STOP 0x00000000 + +void *c3_memset(void *s, int c, size_t count); +int c3_init(void); + +#endif
On 12/06/2012 10:15 AM, Vipin Kumar wrote:
C3 is a cryptographic controller which is used by the SPL when DDR ECC support is enabled.
Basically, the DDR ECC feature requires the initialization of ECC values before the DDR can actually be used. To accomplish this, the complete on board DDR is initialized with zeroes. This initialization can be done using
- CPU
- CPU (with Dcache enabled)
- C3
The current SPL code uses C3 because the initialization using the CPU is slow and we do not have enough memory in SPL to initialize page tables required to enable MMU and Dcache
Signed-off-by: Vipin Kumar vipin.kumar@st.com Reviewed-by: Shiraz Hashim shiraz.hashim@st.com
drivers/misc/Makefile | 1 + drivers/misc/c3.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/c3.h | 63 ++++++++++++++++++++++++++
I'm not so sure about the name of this "driver" and its location in drivers/misc. Is "C3" a generic crypto IP name? On which devices/SoC's is it currently implemented? Perhaps the name should be a little less generic, e.g. "spear-c3" or "st-crypto-c3"...?
And if this "driver" only supports this memory fill operation for some ST SoC (SPEAr?), then its perhaps better located in arch/arm/ right now. Not sure.
Still some review comments below.
3 files changed, 186 insertions(+) create mode 100644 drivers/misc/c3.c create mode 100644 include/c3.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 9fac190..3ef8177 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB := $(obj)libmisc.o
COBJS-$(CONFIG_ALI152X) += ali512x.o +COBJS-$(CONFIG_C3) += c3.o COBJS-$(CONFIG_DS4510) += ds4510.o COBJS-$(CONFIG_FSL_LAW) += fsl_law.o COBJS-$(CONFIG_GPIO_LED) += gpio_led.o diff --git a/drivers/misc/c3.c b/drivers/misc/c3.c new file mode 100644 index 0000000..5f1eaee --- /dev/null +++ b/drivers/misc/c3.c @@ -0,0 +1,122 @@ +/*
- (C) Copyright 2012
- ST Micoelectronics Pvt. Ltd.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h> +#include <c3.h> +#include <errno.h> +#include <asm/io.h> +#include <asm/arch/hardware.h>
+static unsigned long c3_mem_xlate(void *addr) +{
Remove empty line.
- if (((ulong)addr < C3_INT_MEM_BASE_ADDR) || \
The "" is not necessary, or?
((ulong)addr >= (C3_INT_MEM_BASE_ADDR + C3_INT_MEM_SIZE)))return (ulong)addr;- return (unsigned long)addr - C3_INT_MEM_BASE_ADDR +
C3_LOCAL_MEM_ADDR;+}
+int c3_init(void) +{
- if (readl(C3_ID0_SCR) != C3_ID0_DEF_RDY_VAL)
writel(C3_ID0_SCR_RST, C3_ID0_SCR);
Please use structs to access the SoC registers, see below (.h).
- if (readl(C3_MOVE_CHANNEL_ID) == C3_MOVE_CHANNEL_ID_VAL)
return -EINVAL;- writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR);
- writel(C3_LOCAL_MEM_ADDR, C3_HIF_MBAR);
- return 0;
+}
+static int c3_run(void *prog_start) +{
- writel(c3_mem_xlate(prog_start), C3_ID0_IP);
- while ((readl(C3_ID0_SCR) & C3_ID0_STATE_MASK) == C3_ID0_STATE_RUN)
;- if ((readl(C3_ID0_SCR) & C3_ID0_STATE_MASK) != C3_ID0_STATE_IDLE) {
/* If not back to idle an error occured */writel(C3_ID0_SCR_RST, C3_ID0_SCR);/* Set internal access to run c3 programs */writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR);return -EIO;- }
- return 0;
+}
+static int c3_move(void *dest, void *src, int cnt, int optype, int opdata) +{
- unsigned long *c3_prog;
- int ret = 0;
- /* 3.b Prepare program */
- c3_prog = (unsigned long *)C3_INT_MEM_BASE_ADDR;
- /* 3.b.i. Mov init */
- c3_prog[0] = C3_CMD_MOVE_INIT;
- c3_prog[1] = opdata;
- /* 3.b.ii. Mov data */
- c3_prog[2] = C3_CMD_MOVE_DATA + cnt + optype;
- c3_prog[3] = c3_mem_xlate(src);
- c3_prog[4] = c3_mem_xlate(dest);
- /* 3.b.iii. Stop */
- c3_prog[5] = C3_CMD_STOP;
- /* 4. Execute and wait */
- ret = c3_run(c3_prog);
- return ret;
+}
+void *c3_memset(void *s, int c, size_t count) +{ +#define DATA_SIZE (1024*4)
Move this define up or into the header please. And space before and after "*".
- u32 data = C3_INT_MEM_BASE_ADDR + 0x100;
- u32 size;
- size_t cur = 0;
- writel(0x100, C3_HIF_MAAR);
- writel(c, C3_HIF_MADR);
- for (size = 4; size < DATA_SIZE; size <<= 1)
c3_move((void *)(data + size), (void *)data, size,C3_MOVE_AND, 0xffffffff);- while (cur < count) {
c3_move(s + cur, (void *)data, DATA_SIZE,C3_MOVE_AND, 0xffffffff);cur += DATA_SIZE;- }
- return s;
+} diff --git a/include/c3.h b/include/c3.h new file mode 100644 index 0000000..541d702 --- /dev/null +++ b/include/c3.h @@ -0,0 +1,63 @@ +/*
- (C) Copyright 2012
- ST Micoelectronics Pvt. Ltd.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#ifndef _MISC_C3_ +#define _MISC_C3_
+#include <asm/arch/hardware.h>
+#define C3_HIF_OFF 0x400 /* master interface registers */
+#define C3_INT_MEM_BASE_ADDR (CONFIG_SYS_C3_BASE + 0x400) +#define C3_HIF_MBAR (C3_INT_MEM_BASE_ADDR + 0x304)
- #define C3_LOCAL_MEM_ADDR 0xF0000000
+#define C3_HIF_MCR (C3_INT_MEM_BASE_ADDR + 0x308)
- #define C3_HIF_MCR_ENB_INT_MEM 0x01
+#define C3_HIF_MAAR (C3_INT_MEM_BASE_ADDR + 0x310) +#define C3_HIF_MADR (C3_INT_MEM_BASE_ADDR + 0x314)
These seem to be registers. Better to define a struct for them and access via struct.
Thanks, Stefan
On 12/6/2012 5:26 PM, Stefan Roese wrote:
On 12/06/2012 10:15 AM, Vipin Kumar wrote:
C3 is a cryptographic controller which is used by the SPL when DDR ECC support is enabled.
Basically, the DDR ECC feature requires the initialization of ECC values before the DDR can actually be used. To accomplish this, the complete on board DDR is initialized with zeroes. This initialization can be done using
- CPU
- CPU (with Dcache enabled)
- C3
The current SPL code uses C3 because the initialization using the CPU is slow and we do not have enough memory in SPL to initialize page tables required to enable MMU and Dcache
Signed-off-by: Vipin Kumarvipin.kumar@st.com Reviewed-by: Shiraz Hashimshiraz.hashim@st.com
drivers/misc/Makefile | 1 + drivers/misc/c3.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/c3.h | 63 ++++++++++++++++++++++++++
I'm not so sure about the name of this "driver" and its location in drivers/misc. Is "C3" a generic crypto IP name? On which devices/SoC's is it currently implemented? Perhaps the name should be a little less
It is an ST peripheral and is used in spear SoCs and could be used in other ST SoCs
generic, e.g. "spear-c3" or "st-crypto-c3"...?
hmm, ok. I can rename it to st-crypto-c3
And if this "driver" only supports this memory fill operation for some ST SoC (SPEAr?), then its perhaps better located in arch/arm/ right now. Not sure.
You mean arch/arm/cpu/armv7/spear13xx/ ? Is the drivers/misc a special place. Why not here ?
Still some review comments below.
3 files changed, 186 insertions(+) create mode 100644 drivers/misc/c3.c create mode 100644 include/c3.h
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 9fac190..3ef8177 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk LIB := $(obj)libmisc.o
COBJS-$(CONFIG_ALI152X) += ali512x.o +COBJS-$(CONFIG_C3) += c3.o COBJS-$(CONFIG_DS4510) += ds4510.o COBJS-$(CONFIG_FSL_LAW) += fsl_law.o COBJS-$(CONFIG_GPIO_LED) += gpio_led.o diff --git a/drivers/misc/c3.c b/drivers/misc/c3.c new file mode 100644 index 0000000..5f1eaee --- /dev/null +++ b/drivers/misc/c3.c @@ -0,0 +1,122 @@ +/*
- (C) Copyright 2012
- ST Micoelectronics Pvt. Ltd.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include<common.h> +#include<c3.h> +#include<errno.h> +#include<asm/io.h> +#include<asm/arch/hardware.h>
+static unsigned long c3_mem_xlate(void *addr) +{
Remove empty line.
- if (((ulong)addr< C3_INT_MEM_BASE_ADDR) || \
The "" is not necessary, or?
((ulong)addr>= (C3_INT_MEM_BASE_ADDR + C3_INT_MEM_SIZE)))return (ulong)addr;- return (unsigned long)addr - C3_INT_MEM_BASE_ADDR +
C3_LOCAL_MEM_ADDR;+}
+int c3_init(void) +{
- if (readl(C3_ID0_SCR) != C3_ID0_DEF_RDY_VAL)
writel(C3_ID0_SCR_RST, C3_ID0_SCR);Please use structs to access the SoC registers, see below (.h).
- if (readl(C3_MOVE_CHANNEL_ID) == C3_MOVE_CHANNEL_ID_VAL)
return -EINVAL;- writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR);
- writel(C3_LOCAL_MEM_ADDR, C3_HIF_MBAR);
- return 0;
+}
+static int c3_run(void *prog_start) +{
- writel(c3_mem_xlate(prog_start), C3_ID0_IP);
- while ((readl(C3_ID0_SCR)& C3_ID0_STATE_MASK) == C3_ID0_STATE_RUN)
;- if ((readl(C3_ID0_SCR)& C3_ID0_STATE_MASK) != C3_ID0_STATE_IDLE) {
/* If not back to idle an error occured */writel(C3_ID0_SCR_RST, C3_ID0_SCR);/* Set internal access to run c3 programs */writel(C3_HIF_MCR_ENB_INT_MEM, C3_HIF_MCR);return -EIO;- }
- return 0;
+}
+static int c3_move(void *dest, void *src, int cnt, int optype, int opdata) +{
- unsigned long *c3_prog;
- int ret = 0;
- /* 3.b Prepare program */
- c3_prog = (unsigned long *)C3_INT_MEM_BASE_ADDR;
- /* 3.b.i. Mov init */
- c3_prog[0] = C3_CMD_MOVE_INIT;
- c3_prog[1] = opdata;
- /* 3.b.ii. Mov data */
- c3_prog[2] = C3_CMD_MOVE_DATA + cnt + optype;
- c3_prog[3] = c3_mem_xlate(src);
- c3_prog[4] = c3_mem_xlate(dest);
- /* 3.b.iii. Stop */
- c3_prog[5] = C3_CMD_STOP;
- /* 4. Execute and wait */
- ret = c3_run(c3_prog);
- return ret;
+}
+void *c3_memset(void *s, int c, size_t count) +{ +#define DATA_SIZE (1024*4)
Move this define up or into the header please. And space before and after "*".
- u32 data = C3_INT_MEM_BASE_ADDR + 0x100;
- u32 size;
- size_t cur = 0;
- writel(0x100, C3_HIF_MAAR);
- writel(c, C3_HIF_MADR);
- for (size = 4; size< DATA_SIZE; size<<= 1)
c3_move((void *)(data + size), (void *)data, size,C3_MOVE_AND, 0xffffffff);- while (cur< count) {
c3_move(s + cur, (void *)data, DATA_SIZE,C3_MOVE_AND, 0xffffffff);cur += DATA_SIZE;- }
- return s;
+} diff --git a/include/c3.h b/include/c3.h new file mode 100644 index 0000000..541d702 --- /dev/null +++ b/include/c3.h @@ -0,0 +1,63 @@ +/*
- (C) Copyright 2012
- ST Micoelectronics Pvt. Ltd.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#ifndef _MISC_C3_ +#define _MISC_C3_
+#include<asm/arch/hardware.h>
+#define C3_HIF_OFF 0x400 /* master interface registers */
+#define C3_INT_MEM_BASE_ADDR (CONFIG_SYS_C3_BASE + 0x400) +#define C3_HIF_MBAR (C3_INT_MEM_BASE_ADDR + 0x304)
- #define C3_LOCAL_MEM_ADDR 0xF0000000
+#define C3_HIF_MCR (C3_INT_MEM_BASE_ADDR + 0x308)
- #define C3_HIF_MCR_ENB_INT_MEM 0x01
+#define C3_HIF_MAAR (C3_INT_MEM_BASE_ADDR + 0x310) +#define C3_HIF_MADR (C3_INT_MEM_BASE_ADDR + 0x314)
These seem to be registers. Better to define a struct for them and access via struct.
All other review comments accepted. I can float a v2 once we finalize the location where we are going to place it
Thanks Vipin
Thanks, Stefan
.
On 12/07/2012 10:11 AM, Vipin Kumar wrote:
I'm not so sure about the name of this "driver" and its location in drivers/misc. Is "C3" a generic crypto IP name? On which devices/SoC's is it currently implemented? Perhaps the name should be a little less
It is an ST peripheral and is used in spear SoCs and could be used in other ST SoCs
generic, e.g. "spear-c3" or "st-crypto-c3"...?
hmm, ok. I can rename it to st-crypto-c3
Okay.
And if this "driver" only supports this memory fill operation for some ST SoC (SPEAr?), then its perhaps better located in arch/arm/ right now. Not sure.
You mean arch/arm/cpu/armv7/spear13xx/ ?
Or perhaps arch/arm/lib?
On powerpc we have a special SoC specific memcpy version as well. You might want to take a look at it:
arch/powerpc/lib/memcpy_mpc5200.c
It doesn't use a special device for transfer though, only has some alignment restrictions.
Is the drivers/misc a special place. Why not here ?
From my understanding the C3 "driver" you implemented with this memset()
is not a "real" *driver*. Thats my main reasoning why its a bit misplaced in "drivers/*".
But I have no strong feelings here. Perhaps others have thought/ideas/comments as well. Let's wait for further input a bit...
Thanks, Stefan
participants (2)
-
Stefan Roese -
Vipin Kumar