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

[Naveen Krishna Ch <naveenkrishna.ch@gmail.com>]

HI Kang,

On 10 February 2010 16:04, Minkyu Kang <promsoft@gmail.com> wrote:

> Dear Naveen Krishna Ch,
>
> On 10 February 2010 15:16, Naveen Krishna Ch <naveenkrishna.ch@gmail.com>
> wrote:
> > Hi Kang,
> >
> > On 10 February 2010 11:18, Minkyu Kang <promsoft@gmail.com> wrote:
> >>
> >> Dear Naveen Krishna Ch,
> >>
> >> On 9 February 2010 18:34, Naveen Krishna Ch <ch.naveen@samsung.com>
> wrote:
> >> > From: Naveen Krishna CH <ch.naveen@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.
> >
> > As the SMC of S5PC100 is supporting several memories SRAM, SROM, NAND,
> NOR,
> > DDR. I kept it as mem.h
>
> So what?
> This patch add about sromc. (not another memories)
> If need,  separate each controller.
>
I have renamed it according to the TRM (smc.h) as it is SMC subsystem.

>
> >>
> >> > @@ -0,0 +1,55 @@
> >> > +/*
> >> > + * (C) Copyright 2010 Samsung Electronics
> >> > + * Naveen Krishna Ch <ch.naveen@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?
> >
> > For Net support i use only  SROM_DATA16_WIDTH
> > But for NAND support we may need SROM_BYTE_ADDR_MODE
> > These Macros can serve for generic pupose.
> >>
> >> > +
> >> > +#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
> >
> > I will change it.
> >>
> >> and () is unnecessary.
> >
> > Should remove it
> >>
> >> and.. what mean X is?
> >
> > The SROM has 6 banks, I used "X" to indicate that
>
> then, it must be different each banks.
> but your code set same value at all banks (Almost hard code)
> Please modify to can set values each banks.
>
For all the SROM bank configuration registers the values remains same.
As all the banks uses HCLK D0 as the base clock.
So, Same value is applicable to all SROM Banks.

>
> And one more question, what mean SMCBC is?
>
Sorry i changed it in V2 patch.

>
> >>
> >> > +
> >> > +#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)
> >
> > I named it according to TRM. there it mentioned smc_bc.
> > I wil change it if u insist on srom_bc or so.
>
> My TRM mentioned srom_bc. (what is your TRM version?)
>
I have a TRM version 1.02 and it says SMC_BC/BW

>
> >>
> >> > +
> >> > +#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
> >> >
> >>
> > Let me know your opinion
> >>
> >> 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 :)
> >
>
> Thanks
> Minkyu Kang
> --
> from. prom.
> www.promsoft.net
>



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