From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vipin Kumar Date: Fri, 7 Dec 2012 14:41:03 +0530 Subject: [U-Boot] [PATCH resend] misc/crypto: Add support for C3 In-Reply-To: <50C087E1.8020300@denx.de> References: <50C087E1.8020300@denx.de> Message-ID: <50C1B2A7.9040909@st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 Kumar >> Reviewed-by: Shiraz Hashim >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> + >> +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 >> + >> +#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 > > . >