From: Igor Grinberg <grinberg@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/1] ARMV7: Add support For Logic OMAP35x/DM37x modules
Date: Thu, 15 Dec 2011 10:47:05 +0200 [thread overview]
Message-ID: <4EE9B409.6040906@compulab.co.il> (raw)
In-Reply-To: <1323902879-571-1-git-send-email-peter.barada@logicpd.com>
Hi Peter,
In addition to Tom's comments, several comments below:
On 12/15/11 00:47, Peter Barada wrote:
> From: Peter Barada <peterb@logicpd.com>
>
> This patch adds basic support for OMAP35x/DM37x SOM LV/Torpedo
> reference boards. It assumes U-boot is loaded to SDRAM with the
> help of another small bootloader (x-load) running from SRAM.
>
> Signed-off-by: Peter Barada <peter.barada@logicpd.com>
> ---
> board/logicpd/omap3som/Makefile | 48 +++
> board/logicpd/omap3som/config.mk | 33 ++
> board/logicpd/omap3som/omap3logic.c | 566 +++++++++++++++++++++++++++++++++++
> board/logicpd/omap3som/omap3logic.h | 35 +++
> boards.cfg | 1 +
> include/configs/omap3_logic.h | 356 ++++++++++++++++++++++
> 6 files changed, 1039 insertions(+), 0 deletions(-)
> create mode 100644 board/logicpd/omap3som/Makefile
> create mode 100644 board/logicpd/omap3som/config.mk
> create mode 100644 board/logicpd/omap3som/omap3logic.c
> create mode 100644 board/logicpd/omap3som/omap3logic.h
> create mode 100644 include/configs/omap3_logic.h
>
> diff --git a/board/logicpd/omap3som/Makefile b/board/logicpd/omap3som/Makefile
> new file mode 100644
> index 0000000..ef0409f
> --- /dev/null
> +++ b/board/logicpd/omap3som/Makefile
> @@ -0,0 +1,48 @@
> +#
> +# (C) Copyright 2000, 2001, 2002
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB = $(obj)lib$(BOARD).o
> +
> +COBJS-y := omap3logic.o
> +
> +COBJS := $(sort $(COBJS-y))
> +SRCS := $(COBJS:.o=.c)
> +OBJS := $(addprefix $(obj),$(COBJS))
> +
> +$(LIB): $(obj).depend $(OBJS)
> + $(call cmd_link_o_target, $(OBJS))
> +
> +clean:
> + rm -f $(OBJS)
> +
> +distclean: clean
> + rm -f $(LIB) core *.bak $(obj).depend
clean and distclean are obsolete in this directory level, please remove.
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> diff --git a/board/logicpd/omap3som/config.mk b/board/logicpd/omap3som/config.mk
> new file mode 100644
> index 0000000..897b252
> --- /dev/null
> +++ b/board/logicpd/omap3som/config.mk
> @@ -0,0 +1,33 @@
> +#
> +# (C) Copyright 2006 - 2008
> +# Texas Instruments, <www.ti.com>
> +#
> +# EVM uses OMAP3 (ARM-CortexA8) cpu
> +# see http://www.ti.com/ for more information on Texas Instruments
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +# Physical Address:
> +# 8000'0000 (bank0)
> +# A000/0000 (bank1)
> +# Linux-Kernel is expected to be at 8000'8000, entry 8000'8000
> +# (mem base + reserved)
> +
> +# For use with external or internal boots.
> +CONFIG_SYS_TEXT_BASE = 0x80400000
As Tom already said, this should be in the board config file
and this file should be removed completely.
> diff --git a/board/logicpd/omap3som/omap3logic.c b/board/logicpd/omap3som/omap3logic.c
> new file mode 100644
> index 0000000..5c6e896
> --- /dev/null
> +++ b/board/logicpd/omap3som/omap3logic.c
> @@ -0,0 +1,566 @@
> +/*
> + * (C) Copyright 2011
> + * Logic Product Development <www.logicpd.com>
> + *
> + * Author :
> + * Peter Barada <peter.barada@logicpd.com>
> + *
> + * Derived from Beagle Board and 3430 SDP code by
> + * Richard Woodruff <r-woodruff2@ti.com>
> + * Syed Mohammed Khasim <khasim@ti.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
I vote for removing the postal address,
because it is subject to change and you will not follow it,
but it is not a blocker.
[...]
> +/*
> + * Routine: logic_identify
> + * Description: Detect if we are running on a Logic or Torpedo.
> + * This can be done by GPIO_189. If its low after driving it high,
> + * then its a SOM LV, else Torpedo.
> + */
> +unsigned int logic_identify(void)
> +{
> + unsigned int val = 0;
> + u32 cpu_family = get_cpu_family();
You only use this once, so IMO can be inlined.
> + int i;
> +
> + MUX_LOGIC_HSUSB0_DATA5_GPIO_MUX();
> +
> + if (!gpio_request(189, "")) {
This does not look good... can it be:
if (gpio_request(...) == 0)
and also please provide a label with a description
instead of an empty one.
> +
> + gpio_direction_output(189, 0);
> + gpio_set_value(189, 1);
> +
> + /* Let it soak for a bit */
> + for (i = 0; i < 0x100; ++i)
> + asm("nop");
> +
> + gpio_direction_input(189);
> + val = gpio_get_value(189);
> + gpio_free(189);
> +
> + printf("Board:");
> + if (cpu_family == CPU_OMAP36XX) {
> + printf(" DM37xx");
> + if (val) {
> + printf(" Torpedo\n");
> + val = MACH_TYPE_DM3730_TORPEDO;
> + } else {
> + printf(" SOM LV\n");
> + val = MACH_TYPE_DM3730_SOM_LV;
> + }
> + } else {
> + printf(" OMAP35xx");
> + if (val) {
> + printf(" Torpedo\n");
> + val = MACH_TYPE_OMAP3_TORPEDO;
> + } else {
> + printf(" SOM LV\n");
> + val = MACH_TYPE_OMAP3530_LV_SOM;
> + }
> + }
> + }
> +
> + MUX_LOGIC_HSUSB0_DATA5_DATA5();
> +
> + return val;
> +}
The whole function looks like checkboard(), isn't it?
I think it should be then.
[...]
> +/*
> + * Routine: board_init
> + * Description: Early hardware init.
> + */
> +int board_init(void)
> +{
> + gpmc_init(); /* in SRAM or SDRAM, finish GPMC */
> +
> + /* board id for Linux */
> + gd->bd->bi_arch_number = logic_identify();
This also can be moved to checkboard().
> +
> + /* boot param addr */
> + gd->bd->bi_boot_params = (OMAP34XX_SDRC_CS0 + 0x100);
> +
> +
> + return 0;
> +}
[...]
> +/*
> + * Routine: misc_init_r
> + * Description: Init ethernet (done here so udelay works)
> + */
> +int misc_init_r(void)
> +{
> +#if defined(CONFIG_CMD_NET)
> + setup_net_chip();
> +#endif
Can this be done in board_eth_init()?
So the net init code will be close to each other?
> +
> + dieid_num_r();
> +
> + return 0;
> +}
> +
> +
> +
Three empty lines?
There should be only one line between functions.
> +int board_eth_init(bd_t *bis)
> +{
> + int rc = 0;
> +#ifdef CONFIG_SMC911X
> + rc = smc911x_initialize(0, CONFIG_SMC911X_BASE);
> +#endif
> + return rc;
> +}
board_eth_init() has a weak implementation, so
I think it would be much nicer:
#ifdef CONFIG_SMC911X
int board_eth_init(bd_t *bis)
{
setup_net_chip();
return smc911x_initialize(0, CONFIG_SMC911X_BASE);
}
#endif
> +
> +
> +
Again three empty lines?
> +/*
> + * 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
> + */
> +
> +/*
> + * 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)
> +{
> + /*SDRC*/
Alignment...
[...]
> + /*GPMC*/
same here, comments must be aligned to code.
[...]
> + /*DSS*/
ditto
[...]
> + /*CAMERA*/
ditto
[...]
> + /*Audio Interface */
ditto
> + MUX_VAL(CP(MCBSP2_FSX), (IEN | PTD | EN | M7)); /*safe mode */
> + MUX_VAL(CP(MCBSP2_CLKX), (IEN | PTD | EN | M7)); /*safe mode */
> + MUX_VAL(CP(MCBSP2_DR), (IEN | PTD | EN | M7)); /*safe mode */
> + MUX_VAL(CP(MCBSP2_DX), (IEN | PTD | EN | M7)); /*safe mode */
> +
> + /*Expansion card */
ditto
> + MUX_VAL(CP(MMC1_CLK), (IDIS | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_CMD), (IEN | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_DAT0), (IEN | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_DAT1), (IEN | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_DAT2), (IEN | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_DAT3), (IEN | PTU | EN | M0));
> + MUX_VAL(CP(MMC1_DAT4), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC1_DAT5), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC1_DAT6), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC1_DAT7), (IEN | PTD | EN | M7)); /*safe mode*/
> + /*Wireless LAN */
ditto
> + MUX_VAL(CP(MMC2_CLK), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_CMD), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT0), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT1), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT2), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT3), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT4), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT5), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT6), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MMC2_DAT7), (IEN | PTD | EN | M7)); /*safe mode*/
> + /*Bluetooth*/
ditto
> + MUX_VAL(CP(MCBSP3_DX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP3_DR), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP3_CLKX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP3_FSX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(UART2_CTS), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(UART2_RTS), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(UART2_TX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(UART2_RX), (IEN | PTD | EN | M7)); /*safe mode*/
> + /*Modem Interface */
ditto
> + MUX_VAL(CP(UART1_TX), (IDIS | PTD | DIS | M0));
> + MUX_VAL(CP(UART1_RTS), (IDIS | PTD | DIS | M0));
> + MUX_VAL(CP(UART1_CTS), (IEN | PTU | DIS | M0));
> + MUX_VAL(CP(UART1_RX), (IEN | PTD | DIS | M0));
> + MUX_VAL(CP(MCBSP4_CLKX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP4_DR), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP4_DX), (IDIS | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP4_FSX), (IDIS | PTD | EN | M7)); /*safe mode*/
> +
> + MUX_VAL(CP(MCBSP1_CLKR), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP1_FSR), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP1_DX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP1_DR), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP_CLKS), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP1_FSX), (IEN | PTD | EN | M7)); /*safe mode*/
> + MUX_VAL(CP(MCBSP1_CLKX), (IEN | PTD | EN | M7)); /*safe mode*/
> + /*Serial Interface*/
ditto
[...]
> + /*Control and debug */
ditto
[...]
> + /*Die to Die */
ditto
[...]
> +}
I think this function is *much better* than the mux config done .h file.
Good job!
--
Regards,
Igor.
next prev parent reply other threads:[~2011-12-15 8:47 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-14 22:47 [U-Boot] [PATCH 1/1] ARMV7: Add support For Logic OMAP35x/DM37x modules Peter Barada
2011-12-15 0:15 ` Tom Rini
2011-12-15 4:59 ` Peter Barada
2011-12-15 8:47 ` Igor Grinberg [this message]
2011-12-15 17:15 ` [U-Boot] [PATCH v2] " Peter Barada
2011-12-15 18:30 ` Tom Rini
2011-12-16 6:09 ` Peter Barada
2011-12-16 16:28 ` Tom Rini
2011-12-16 20:31 ` [U-Boot] [PATCH] " Peter Barada
2011-12-16 20:33 ` Peter Barada
2011-12-16 22:43 ` Tom Rini
2011-12-17 20:44 ` Wolfgang Denk
2011-12-18 9:16 ` Igor Grinberg
2011-12-18 17:25 ` [U-Boot] [PATCH v4] " Peter Barada
2011-12-19 7:37 ` Igor Grinberg
2011-12-19 15:44 ` Peter Barada
2011-12-20 5:54 ` [U-Boot] [PATCH v5] " Peter Barada
2011-12-20 7:18 ` Igor Grinberg
2012-01-03 16:48 ` Tom Rini
2011-12-18 9:00 ` [U-Boot] [PATCH v2] " Igor Grinberg
2011-12-18 12:33 ` Wolfgang Denk
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=4EE9B409.6040906@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.