[U-Boot] [PATCH resend] misc/crypto: Add support for C3

[Stefan Roese <sr@denx.de>]

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