[U-Boot] [PATCH 1/3] S5PC100: Memory SubSystem Header file, register description(SROMC).

[Minkyu Kang]

Dear Naveen Krishna Ch,

On 9 February 2010 18:34, Naveen Krishna Ch <ch.naveen at samsung.com> wrote:
> From: Naveen Krishna CH <ch.naveen at samsung.com>
>
> Memory subsystem of S5PC100 handles SROM, SRAM, OneDRAM, OneNand,
> NAND Flash, DDRs.
> mem.h is a common place for the register description of Memory subsystem
> of S5PC100.
> Note: Only SROM related registers are descibed now.
>
> Signed-off-by: Naveen Krishna Ch <ch.naveen <at> samsung.com>
> ---
>  include/asm-arm/arch-s5pc1xx/mem.h |   55 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 55 insertions(+), 0 deletions(-)
>  create mode 100644 include/asm-arm/arch-s5pc1xx/mem.h
>
> diff --git a/include/asm-arm/arch-s5pc1xx/mem.h b/include/asm-arm/arch-s5pc1xx/mem.h
> new file mode 100644
> index 0000000..66272ff
> --- /dev/null
> +++ b/include/asm-arm/arch-s5pc1xx/mem.h

I think srom.h is better than mem.h.

> @@ -0,0 +1,55 @@
> +/*
> + * (C) Copyright 2010 Samsung Electronics
> + * Naveen Krishna Ch <ch.naveen at samsung.com>
> + *
> + * 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
> + *
> + * Note: This file contains the register description for Memory subsystem
> + *      (SROM, NAND Flash, OneNand, DDR, OneDRAM) on S5PC1XX.
> + *
> + *      Only SROMC is defined as of now
> + */
> +
> +#ifndef __ASM_ARCH_MEM_H_
> +#define __ASM_ARCH_MEM_H_
> +
> +#define SROM_DATA16_WIDTH(x)    (1<<((x*4)+0))
> +#define SROM_BYTE_ADDR_MODE(x)  (1<<((x*4)+1))  /* 0-> Half-word base address*/
> +                                               /* 1-> Byte base address*/
> +#define SROM_WAIT_ENABLE(x)     (1<<((x*4)+2))
> +#define SROM_BYTE_ENABLE(x)     (1<<((x*4)+3))

Do you use all of these defines?

> +
> +#define SMCBC_X_Tacs    (0x0)   /* 0clk         address set-up */
> +#define SMCBC_X_Tcos    (0x4)   /* 4clk         chip selection set-up */
> +#define SMCBC_X_Tacc    (0xe)   /* 14clk        access cycle */
> +#define SMCBC_X_Tcoh    (0x1)   /* 1clk         chip selection hold */
> +#define SMCBC_X_Tah     (0x4)   /* 4clk         address holding time */
> +#define SMCBC_X_Tacp    (0x6)   /* 6clk         page mode access cycle */
> +#define SMCBC_X_PMC     (0x0)   /* normal(1data)page mode configuration */

Please don't use lowercase at define
and () is unnecessary.
and.. what mean X is?

> +
> +#define SMC_BC_X_CON    ((SMCBC_X_Tacs<<28)|(SMCBC_X_Tcos<<24)| \
> +                        (SMCBC_X_Tacc<<16)|(SMCBC_X_Tcoh<<12)| \
> +                        (SMCBC_X_Tah<<8)|(SMCBC_X_Tacp<<4)|    \
> +                        (SMCBC_X_PMC))
> +
> +#ifndef __ASSEMBLY__
> +struct s5pc1xx_sromc {
> +       unsigned int    smc_bw;
> +       unsigned int    smc_bc[6];
> +};
> +#endif /* __ASSEMBLY__ */

smc_bw and smc_bc are already belong to sromc structure.
is "smc_" really need?
Please modify it to bw and bc or srom_bw and srom_bc (according to TRM)

> +
> +#endif /* __ASM_ARCH_MEM_H_ */
> --
> 1.6.6
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Thanks,
Minkyu Kang.
-- 
from. prom.
www.promsoft.net