[U-Boot] [PATCH 4/6] S5PC100: onenand driver for SMDKC100 support

[Scott Wood]

On Thu, Jun 25, 2009 at 05:10:37PM +0900, HeungJun Kim wrote:
> This patch includes the onenand driver for SMDKC100 Board.
> 
> Signed-off-by: HeungJun, Kim <riverful.kim at samsung.com>
> 
> ---
> 
>  drivers/mtd/onenand/Makefile      |    8 +-
>  drivers/mtd/onenand/s5p_onenand.c | 2034 +++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/onenand_regs.h  |    5 +
>  include/linux/mtd/s5p_onenand.h   |  425 ++++++++

Please try to refactor the existing onenand code to support controller
variations, rather than duplicating parts of it.

Or if you can find absolutely no common ground (but I hope you can), at
least change the names so they don't conflict.  Are you planning on
supporting this in Linux?  What is it going to look like there?

Kyungmin, any thoughts?  Why do we have a specific controller
implementation in something generically named "onenand_base.c"?  Or is
this different because of a different type of onenand chip?

I don't understand OneNAND very well (and google doesn't turn up much but
marketing), which makes including it under the same custodianship as NAND
somewhat awkward. :-(

> diff --git a/drivers/mtd/onenand/Makefile b/drivers/mtd/onenand/Makefile
> index 1d35a57..aad1362 100644
> --- a/drivers/mtd/onenand/Makefile
> +++ b/drivers/mtd/onenand/Makefile
> @@ -25,7 +25,13 @@ include $(TOPDIR)/config.mk
> 
>  LIB	:= $(obj)libonenand.a
> 
> -COBJS-$(CONFIG_CMD_ONENAND)	:= onenand_uboot.o onenand_base.o onenand_bbt.o
> +COBJS-$(CONFIG_CMD_ONENAND)	:= onenand_uboot.o
> +
> +ifdef CONFIG_S5PC1XX
> +COBJS-$(CONFIG_CMD_ONENAND)	+= s5p_onenand.o
> +else
> +COBJS-$(CONFIG_CMD_ONENAND)	+= onenand_base.o onenand_bbt.o
> +endif

Why no bbt?

> +/**
> + * onenand_read_burst
> + *
> + * 16 Burst read: performance is improved up to 40%.
> + */
> +static void onenand_read_burst(void *dest, const void *src, size_t len)
> +{
> +	int count;
> +
> +	if (len % 16 != 0)
> +		return;
> +
> +	count = len / 16;
> +	
> +	__asm__ __volatile__(
> +		"	stmdb	r13!, {r0-r3,r9-r12}\n"
> +		"	mov	r2, %0\n"
> +		"1:\n"
> +		"	ldmia	r1, {r9-r12}\n"
> +		"	stmia	r0!, {r9-r12}\n"
> +		"	subs	r2, r2, #0x1\n"
> +		"	bne	1b\n"
> +		"	ldmia	r13!, {r0-r3,r9-r12}\n"::"r" (count));
> +}

What is this doing that we couldn't generically make memcpy do?

-Scott