Dear Kyungmin Park,
In message <9c9fda240907192016i32c7312dh490629f2f2bb3...@mail.gmail.com> you
wrote:
>
> >> /* read a page with ECC */
> >> static inline int onenand_read_page(ulong block, ulong page,
> >> u_char * buf> , int pagesize)
> >> {
> >> +#ifdef CONFIG_S5PC1XX
> >> + unsigned int *p = (unsigned int *) buf;
> >> + int mem_addr, i;
> >> +
> >> + mem_addr = MEM_ADDR(block, page, 0);
> >> +
> >> + pagesize >>= 2;
> >> +
> >> + for (i = 0; i < pagesize; i++)
> >> + *p++ = *(volatile unsigned int *)(CMD_MAP_01> (mem_addr));
> >> +#else /* CONFIG_S5PC1XX */
> >> +
> >> unsigned long *base;
> >
> > I don't like to see such board specific code in global files.
>
> I think it's not board specific code. S3C64XX and S5PC1XX series have
> own OneNAND controller and to access the OneNAND, it should use the
> this controller.
OK, so it is SoC specific code in a common file - that's just
marginally better.
> If you don't like the ifdef. we can separate the function but I'm not
> sure it's really required.
It would be great if we can get rid of the #ifdef.
> > Also, please use I/O accessor functions instead of register accesses.
>
> readl(...)? If yes, I agree it.
Yes.
> >> @@ -114,6 +129,9 @@ int onenand_read_block(unsigned char *buf)
> >>
> >> erasesize = ONENAND_PAGES_PER_BLOCK * pagesize;
> >> nblocks = (CONFIG_SYS_MONITOR_LEN + erasesize - 1) >> eras> e_shift;
> >> +#ifdef CONFIG_S5PC1XX
> >> + nblocks = 1;
> >> +#endif
> >
> > Again: why do we need such board specific code here?
>
> It should be fixed.
Thanks.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Just about every computer on the market today runs Unix, except the
Mac (and nobody cares about it). - Bill Joy 6/21/85
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
