> -----Original Message----- > From: Lei Wen [mailto:adrian.w...@gmail.com] > Sent: Wednesday, January 05, 2011 8:59 PM > To: Prafulla Wadaskar > Cc: Lei Wen; u-boot@lists.denx.de; Yu Tang; Ashish Karkare; Prabhanjan > Sarnaik > Subject: Re: [U-BOOT] [PATCH 2/6] mv: seperate kirkwood and mmp from > common setting > ...snip... > >> + * NAND configuration > >> + */ > >> +#ifdef CONFIG_CMD_NAND > >> +#define CONFIG_NAND_KIRKWOOD > >> +#define CONFIG_SYS_MAX_NAND_DEVICE 1 > >> +#define NAND_MAX_CHIPS 1 > >> +#define CONFIG_SYS_NAND_BASE 0xD8000000 /* > MV_DEFADR_NANDF */ > >> +#define NAND_ALLOW_ERASE_ALL 1 > >> +#define CONFIG_SYS_64BIT_VSPRINTF /* needed for nand_util.c */ > >> +#endif > >> + > > > > I think split below makes more sense- overall objective is to avoid > code duplication here. > > > > For mv-common.h > > +/* > > + * NAND configuration > > + */ > > +#ifdef CONFIG_CMD_NAND > > +#define CONFIG_SYS_MAX_NAND_DEVICE 1 > > +#define NAND_MAX_CHIPS 1 > > +#define CONFIG_SYS_64BIT_VSPRINTF /* needed for nand_util.c */ > > +#endif > > > > For arch-kirkwood/config.h > > +/* > > + * NAND configuration > > + */ > > +#ifdef CONFIG_CMD_NAND > > +#define CONFIG_NAND_KIRKWOOD > > +#define CONFIG_SYS_NAND_BASE 0xD8000000 /* > MV_DEFADR_NANDF */ > > +#define NAND_ALLOW_ERASE_ALL 1 > > +#endif > > > >> +/* > >> + * SPI Flash configuration > >> + */ > >> +#ifdef CONFIG_CMD_SF > >> +#define CONFIG_SPI_FLASH 1 > >> +#define CONFIG_HARD_SPI 1 > >> +#define CONFIG_KIRKWOOD_SPI 1 > >> +#define CONFIG_SPI_FLASH_MACRONIX 1 > >> +#define CONFIG_ENV_SPI_BUS 0 > >> +#define CONFIG_ENV_SPI_CS 0 > >> +#define CONFIG_ENV_SPI_MAX_HZ 50000000 /*50Mhz > */ > >> +#endif > >> + > > > > Same applies for rest of the definitions. > > mv-common.h should represent common definitions for > Kirkwood,armada100, and future SoCs based boards so that > > > Yes, agree... > Since current armada100 havn't added nand and spi support, how about > move those nand and spi definition to kirkwood and > then after armada100 add the nand, we reconsider to move back the common > back?
On the other hand, keep your patch small by not moving this driver specific stuff to arch-kirkwood/config.h. As new common driver will get supported on armada100, we will push them to config.h, this way even future patches will also be smaller. > > Especially for those code below, I really believe this should not be > put into the common code... > #ifndef CONFIG_ARMADA100 /* will be removed latter */-#define > CONFIG_CMD_EXT2 > #define CONFIG_CMD_JFFS2 > #define CONFIG_CMD_FAT > #define CONFIG_CMD_UBI > #define CONFIG_CMD_UBIFS > #define CONFIG_RBTREE > #define CONFIG_MTD_DEVICE /* needed for mtdparts commands > */ > #define CONFIG_MTD_PARTITIONS > #define CONFIG_CMD_MTDPARTS > #define CONFIG_LZO > #endif Well, we can.. only replace #ifndef CONFIG_ARMADA100 with #ifdef CONFIG_SYS_MVFS and define this macro in arch-kirkwood/config.h, this way in future after enabling ide, nand, spi driver support just defining the same macro in armada100/config.h will enable the file system support for armada100 platforms. Regards.. Prafulla . . _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
