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

[Naveen Krishna Ch]

HI Kang,
Thanks for your comments,
On 12 February 2010 13:48, Minkyu Kang <promsoft at gmail.com> wrote:

> Dear Naveen Krishna Ch,
>
> On 10 February 2010 21:42, Naveen Krishna Ch <ch.naveen at samsung.com>
> wrote:
> > From: Naveen Krishna CH <chnaveen at chnaveen.localdomain>
> >
> > 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>
> > ---
> > Changes since V1:
> >
> > 1. The header file is renamed to smc.h from mem.h
> > 2. The Macros are renamed according to TRM.
> > Comments from Minkyu kang are fixed.
> >
> > Note: Some of the defined macros are not used now. They are added to
> facilitiate
> > simple cofiguration of SMC registers.
> >
> >  include/asm-arm/arch-s5pc1xx/smc.h |   54
> ++++++++++++++++++++++++++++++++++++
> >  1 files changed, 54 insertions(+), 0 deletions(-)
> >  create mode 100644 include/asm-arm/arch-s5pc1xx/smc.h
> >
> > diff --git a/include/asm-arm/arch-s5pc1xx/smc.h
> b/include/asm-arm/arch-s5pc1xx/smc.h
> > new file mode 100644
> > index 0000000..2a1ee07
> > --- /dev/null
> > +++ b/include/asm-arm/arch-s5pc1xx/smc.h
> > @@ -0,0 +1,54 @@
> > +/*
> > + * (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_SMC_H_
> > +#define __ASM_ARCH_SMC_H_
> > +
> > +#define SMC_DATA16_WIDTH(x)    (1<<((x*4)+0))
> > +#define SMC_BYTE_ADDR_MODE(x)  (1<<((x*4)+1))  /* 0-> Half-word base
> address*/
> > +                                               /* 1-> Byte base
> address*/
> > +#define SMC_WAIT_ENABLE(x)     (1<<((x*4)+2))
> > +#define SMC_BYTE_ENABLE(x)     (1<<((x*4)+3))
> > +
> > +#define SMC_BC_TACS    0x0   /* 0clk         address set-up */
> > +#define SMC_BC_TCOS    0x4   /* 4clk         chip selection set-up */
> > +#define SMC_BC_TACC    0xe   /* 14clk        access cycle */
> > +#define SMC_BC_TCOH    0x1   /* 1clk         chip selection hold */
> > +#define SMC_BC_TAH     0x4   /* 4clk         address holding time */
> > +#define SMC_BC_TACP    0x6   /* 6clk         page mode access cycle */
> > +#define SMC_BC_PMC     0x0   /* normal(1data)page mode configuration */
> > +
> > +#define SMC_BC_CON    ((SMC_BC_TACS<<28)|(SMC_BC_TCOS<<24)| \
> > +                      (SMC_BC_TACC<<16)|(SMC_BC_TCOH<<12)| \
> > +                      (SMC_BC_TAH<<8)|(SMC_BC_TACP<<4)|(SMC_BC_PMC))
>
> This code is meaningless.
> Even though all banks are same value, this macro must be support can
> set any value.
>
> How about this.
>
> #define SMC_BC_TACS(x)  (x << 28)
> #define SMC_BC_TCOS(x)  (x << 24)
> .
> .
>
> smc_bc_conf = SMC_BC_TACS(0x0) | SMC_BC_TCOS(0x4) | ...... ;
> s5pc1xx_config_sromc(CONFIG_ENV_SROM_BANK, smc_bw_conf, smc_bc_conf);
>
> give your opinion
>
Yeah this seems better. I wil make changes in the next revision then.

>
> > +
> > +#ifndef __ASSEMBLY__
> > +struct s5pc1xx_smc {
> > +       unsigned int    bw;
> > +       unsigned int    bc[6];
> > +};
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#endif /* __ASM_ARCH_SMC_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
> _______________________________________________
>  U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>



-- 
Shine bright,
(: Naveen Krishna Ch :)