On Thu, Dec 24, 2009 at 04:33:49AM +0100, Marcel Ziswiler wrote: > +/* XXX U-BOOT XXX */ > +#define __U_BOOT__ > + > +#ifndef __U_BOOT__
I don't think we need this. In fact, it could make it harder to spot conflicts when merging new versions of the file from linux, since patch could find the context it's looking for in the middle of an #ifndef __U_BOOT__ section when the change really needs to be applied to the other side of the #else. Plus, it makes it harder to read the code. Let's share what we reasonably can, and get rid of the rest. > +#undef DEBUG Don't undef it, just don't define it. Maybe someone wants to turn debugging on system-wide. > +#ifdef DEBUG > +#define DEBUGF(fmt, args...) printf(fmt, ##args) > +#else /* DEBUG */ > +#define DEBUGF(x...) > +#endif /* DEBUG */ We've already got MTDDEBUG. > +#define CONFIG_MTD_NAND_PXA3xx_BUILTIN This is marked deprecated in Linux; perhaps we should take the preferred approach of platform-specified data here as well? > +#define CONFIG_MTD_NAND_PXA3xx_POLLING I don't see this upstream. > +#ifdef CONFIG_NAND_SPL > +/** > + * memset - Fill a region of memory with the given value > + * @s: Pointer to the start of the area. > + * @c: The byte to fill the area with > + * @count: The size of the area. > + * > + * Do not use memset() to access IO space, use memset_io() instead. > + */ > +void *memset(void *s, int c, size_t count) > +{ > + char *xs = (char *) s; > + > + while (count--) > + *xs++ = c; > + > + return s; > +} > + > +/** > + * memcpy - Copy one area of memory to another > + * @dest: Where to copy to > + * @src: Where to copy from > + * @count: The size of the area. > + * > + * You should not use this function to access IO space, use memcpy_toio() > + * or memcpy_fromio() instead. > + */ > +void *memcpy(void *dest, const void *src, size_t count) > +{ > + char *tmp = (char *) dest, *s = (char *) src; > + > + while (count--) > + *tmp++ = *s++; > + > + return dest; > +} > +#endif /* CONFIG_NAND_SPL */ > + > +void msleep(int count) > +{ > + int i; > + > + for (i = 0; i < count; i++) > + udelay(1000); > +} Can we arrange to pull in the relevant functions from the rest of u-boot rather than duplicate them? Or try to avoid the need in the NAND loader -- for example, msleep is only used in handling NAND_CMD_RESET, which you shouldn't need. In fact, I'm not sure how this driver can be used at all with NAND_SPL. The normal nand_spl/nand_boot.c uses cmd_ctrl. More complicated controllers that can't use cmd_ctrl generally need their own nand_spl driver (such as nand_spl/nand_boot_fsl_elbc.c). If you have a particularly large NAND boot region, you might be able to create a nand_spl driver that uses this driver, but typically space is too tight. > +/* macros for registers read/write */ > +#ifdef __U_BOOT__ > +#define nand_writel(info, off, val) \ > + ((*(volatile u32 *) ((info)->mmio_base + (off))) = (val)) > + > +static void nand_writesl(volatile u32 *dest, u8 *src, int size) > +{ > + int i; > + u32 *source = (u32 *)src; > + > + for (i = 0; i < size; i++) > + *dest = *(source++); > +} > + > +#define nand_readl(info, off) \ > + (*(volatile u32 *)((info)->mmio_base + (off))) > + > +static void nand_readsl(volatile u32 *source, u8 *dst, int size) > +{ > + u32 *dest = (u32 *)dst; > + int i; > + > + for (i = 0; i < size; i++) > + *(dest++) = *source; > +} > +#else /* __U_BOOT__ */ > +#define nand_writel(info, off, val) \ > + __raw_writel((val), (info)->mmio_base + (off)) > + > +#define nand_readl(info, off) \ > + __raw_readl((info)->mmio_base + (off)) > +#endif /* __U_BOOT__ */ You can (and should) use __raw_writel/__raw_readl in U-Boot as well. There is also a prototype for__raw_writesl/__raw_readsl, but the implementation seems to be missing -- perhaps you could add it generically, rather than just in one driver. -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot