Hi Javier, On 12/26/12 05:24, Javier Martinez Canillas wrote: > Even when the IGEPv2 board and the IGEP Computer-on-Module > are different from a form factor point of view, they are > very similar in the fact that share many components and how > they are wired. > > So, it is possible (and better) to have a single board file > for both devices and just use the CONFIG_MACH_TYPE to make > a differentiation between both boards when needed.
I'm wondering... isn't there a way to distinguish between the boards in runtime? Because if there is, you could even use the same binary. This is what we do on cm-t3530/cm-t3730 and is very maintainable, avoids code duplication, and avoids multiple ifdeffery. > > This change avoids duplication by removing 298 lines of code > and makes future maintenance easier. > > Signed-off-by: Javier Martinez Canillas <javier.marti...@collabora.co.uk> > --- > board/isee/igep0020/Makefile | 43 --------- > board/isee/igep0020/igep0020.c | 177 -------------------------------------- > board/isee/igep0020/igep0020.h | 151 -------------------------------- > board/isee/igep0030/Makefile | 43 --------- > board/isee/igep0030/igep0030.c | 118 ------------------------- > board/isee/igep0030/igep0030.h | 151 -------------------------------- > board/isee/igep00x0/Makefile | 43 +++++++++ > board/isee/igep00x0/igep00x0.c | 186 > ++++++++++++++++++++++++++++++++++++++++ > board/isee/igep00x0/igep00x0.h | 165 +++++++++++++++++++++++++++++++++++ > boards.cfg | 8 +- > 10 files changed, 398 insertions(+), 687 deletions(-) > delete mode 100644 board/isee/igep0020/Makefile > delete mode 100644 board/isee/igep0020/igep0020.c > delete mode 100644 board/isee/igep0020/igep0020.h > delete mode 100644 board/isee/igep0030/Makefile > delete mode 100644 board/isee/igep0030/igep0030.c > delete mode 100644 board/isee/igep0030/igep0030.h > create mode 100644 board/isee/igep00x0/Makefile > create mode 100644 board/isee/igep00x0/igep00x0.c > create mode 100644 board/isee/igep00x0/igep00x0.h [...] > diff --git a/board/isee/igep00x0/igep00x0.c b/board/isee/igep00x0/igep00x0.c > new file mode 100644 > index 0000000..c8b2fbf > --- /dev/null > +++ b/board/isee/igep00x0/igep00x0.c [...] > +#include <common.h> > +#include <twl4030.h> > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) > +#include <netdev.h> > +#include <asm/gpio.h> > +#include <asm/arch/omap_gpmc.h> > +#endif Is there a problem including three above files for igep0030? If not, then it will be better to remove the if. > +#include <asm/io.h> > +#include <asm/arch/mem.h> > +#include <asm/arch/mmc_host_def.h> > +#include <asm/arch/mux.h> > +#include <asm/arch/sys_proto.h> > +#include <asm/mach-types.h> > +#include "igep00x0.h" > + > +DECLARE_GLOBAL_DATA_PTR; > + > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) && defined(CONFIG_CMD_NET) Does the igep0030 uses CONFIG_CMD_NET? If it does not, then you can just disable CONFIG_CMD_NET for igep0030 and here (and other net related places) just use #ifdef CONFIG_CMD_NET > +/* GPMC definitions for LAN9221 chips */ > +static const u32 gpmc_lan_config[] = { > + NET_LAN9221_GPMC_CONFIG1, > + NET_LAN9221_GPMC_CONFIG2, > + NET_LAN9221_GPMC_CONFIG3, > + NET_LAN9221_GPMC_CONFIG4, > + NET_LAN9221_GPMC_CONFIG5, > + NET_LAN9221_GPMC_CONFIG6, > +}; > +#endif [...] > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) && defined(CONFIG_CMD_NET) > +/* > + * Routine: setup_net_chip > + * Description: Setting up the configuration GPMC registers specific to the > + * Ethernet hardware. > + */ > +static void setup_net_chip(void) > +{ > + struct ctrl *ctrl_base = (struct ctrl *)OMAP34XX_CTRL_BASE; > + > + enable_gpmc_cs_config(gpmc_lan_config, &gpmc_cfg->cs[5], 0x2C000000, > + GPMC_SIZE_16M); > + > + /* Enable off mode for NWE in PADCONF_GPMC_NWE register */ > + writew(readw(&ctrl_base->gpmc_nwe) | 0x0E00, &ctrl_base->gpmc_nwe); > + /* Enable off mode for NOE in PADCONF_GPMC_NADV_ALE register */ > + writew(readw(&ctrl_base->gpmc_noe) | 0x0E00, &ctrl_base->gpmc_noe); > + /* Enable off mode for ALE in PADCONF_GPMC_NADV_ALE register */ > + writew(readw(&ctrl_base->gpmc_nadv_ale) | 0x0E00, > + &ctrl_base->gpmc_nadv_ale); > + > + /* Make GPIO 64 as output pin and send a magic pulse through it */ > + if (!gpio_request(64, "")) { > + gpio_direction_output(64, 0); > + gpio_set_value(64, 1); > + udelay(1); > + gpio_set_value(64, 0); > + udelay(1); > + gpio_set_value(64, 1); > + } > +} having here: #else static inline void setup_net_chip(void) {} or equivalent will remove the need to place the call inside the ifdef below. > +#endif [...] > +/* > + * Routine: misc_init_r > + * Description: Configure board specific parts > + */ > +int misc_init_r(void) > +{ > + twl4030_power_init(); > + > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) && defined(CONFIG_CMD_NET) > + MUX_IGEP0020(); > + setup_net_chip(); > +#endif > + > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0030) > + MUX_IGEP0030(); > +#endif Why do you need to setup the board specific muxes in misc_init_r()? Why not do this in set_muxconf_regs() along with the common muxes. > + > + dieid_num_r(); > + > + return 0; > +} > + > +/* > + * Routine: set_muxconf_regs > + * Description: Setting up the configuration Mux registers specific to the > + * hardware. Many pins need to be moved from protect to primary > + * mode. > + */ > +void set_muxconf_regs(void) > +{ > + MUX_DEFAULT(); > +} [...] > diff --git a/board/isee/igep00x0/igep00x0.h b/board/isee/igep00x0/igep00x0.h > new file mode 100644 > index 0000000..83822d2 > --- /dev/null > +++ b/board/isee/igep00x0/igep00x0.h [...] > +const omap3_sysinfo sysinfo = { > + DDR_STACKED, > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) > + "OMAP3 IGEP v2 board", > +#endif > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0030) > + "OMAP3 IGEP COM Module", > +#endif > +#if defined(CONFIG_ENV_IS_IN_ONENAND) > + "ONENAND", > +#else > + "NAND", > +#endif > +}; > + > +#if (CONFIG_MACH_TYPE == MACH_TYPE_IGEP0020) && defined(CONFIG_CMD_NET) > +static void setup_net_chip(void); > +#endif Why do you need this declaration? > + > +/* > + * IEN - Input Enable > + * IDIS - Input Disable > + * PTD - Pull type Down > + * PTU - Pull type Up > + * DIS - Pull type selection is inactive > + * EN - Pull type selection is active > + * M0 - Mode 0 > + * The commented string gives the final mux configuration for that pin > + */ > +#define MUX_DEFAULT()\ > + MUX_VAL(CP(SDRC_D0), (IEN | PTD | DIS | M0)) /* SDRC_D0 */\ [...] > + MUX_VAL(CP(SDRC_CKE1), (IDIS | PTU | EN | M0)) /* SDRC_CKE1 */ > +#endif > + > +#define MUX_IGEP0020() \ > + MUX_VAL(CP(GPMC_WAIT2), (IEN | PTU | DIS | M4)) /* > GPIO_64-ETH_NRST */\ > + > +#define MUX_IGEP0030() \ > + MUX_VAL(CP(UART1_TX), (IDIS | PTD | DIS | M0)) /* UART1_TX */\ > + MUX_VAL(CP(UART1_RX), (IEN | PTD | DIS | M0)) /* UART1_RX */\ > + > diff --git a/boards.cfg b/boards.cfg > index 91504c0..35fbc85 100644 > --- a/boards.cfg > +++ b/boards.cfg > @@ -253,10 +253,10 @@ cm_t35 arm armv7 > cm_t35 - > omap3_overo arm armv7 overo - > omap3 > omap3_pandora arm armv7 pandora - > omap3 > dig297 arm armv7 dig297 > comelit omap3 > -igep0020 arm armv7 igep0020 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0020,BOOT_ONENAND > -igep0020_nand arm armv7 igep0020 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0020,BOOT_NAND > -igep0030 arm armv7 igep0030 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0030,BOOT_ONENAND > -igep0030_nand arm armv7 igep0030 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0030,BOOT_NAND > +igep0020 arm armv7 igep00x0 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0020,BOOT_ONENAND > +igep0020_nand arm armv7 igep00x0 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0020,BOOT_NAND > +igep0030 arm armv7 igep00x0 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0030,BOOT_ONENAND > +igep0030_nand arm armv7 igep00x0 > isee omap3 > igep00x0:MACH_TYPE=MACH_TYPE_IGEP0030,BOOT_NAND > am3517_evm arm armv7 am3517evm > logicpd omap3 > mt_ventoux arm armv7 mt_ventoux > teejet omap3 > omap3_zoom1 arm armv7 zoom1 > logicpd omap3 -- Regards, Igor. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot