From: Igor Grinberg <grinberg@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/2] OMAP3: use a single board file for IGEP devices
Date: Thu, 27 Dec 2012 10:07:41 +0200 [thread overview]
Message-ID: <50DC01CD.3010208@compulab.co.il> (raw)
In-Reply-To: <1356492241-14015-1-git-send-email-javier.martinez@collabora.co.uk>
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
--
Regards,
Igor.
next prev parent reply other threads:[~2012-12-27 8:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-26 3:24 [U-Boot] [PATCH 1/2] OMAP3: use a single board file for IGEP devices Javier Martinez Canillas
2012-12-26 3:24 ` [U-Boot] [PATCH v4 2/2] OMAP3: igep00x0: add boot status GPIO LED Javier Martinez Canillas
2012-12-27 13:16 ` Igor Grinberg
2012-12-27 13:26 ` Javier Martinez Canillas
2012-12-27 8:07 ` Igor Grinberg [this message]
2012-12-27 10:39 ` [U-Boot] [PATCH 1/2] OMAP3: use a single board file for IGEP devices Javier Martinez Canillas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50DC01CD.3010208@compulab.co.il \
--to=grinberg@compulab.co.il \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.