[U-Boot] [U-BOOT] [PATCH 2/6] mv: seperate kirkwood and mmp from common setting

[Prafulla Wadaskar]

> -----Original Message-----
> From: Lei Wen [mailto:adrian.wenl at gmail.com]
> Sent: Wednesday, January 05, 2011 8:59 PM
> To: Prafulla Wadaskar
> Cc: Lei Wen; u-boot at 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 . .