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.martinez@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