[U-Boot] [PATCH] imx51:Add support basic boot code of freescale imx51 bbg board
Fred Fan
fanyefeng at gmail.com
Tue Dec 1 17:44:14 CET 2009
Dear wolfgang,
I almost fixed all of your comments.
By some of them I can not fix them now.
1) Copyright block. It defined freescale other team. I can not change them
directly.
2) iomux module can not use C structure to access registers. Because
register has
encoded in to pin definition.
When I tested the new code on target board, I will send out to review
again.
Best Regards
Fred
2009/9/24 Wolfgang Denk <wd at denx.de>
> Dear Fred Fan,
>
> In message <12537172421605-git-send-email-fanyefeng at gmail.com> you wrote:
> > This patch just supports boot into u-boot from mmc or spi-nor flash.
> > It just implements console, iomux and clock. There are no ethernet,
> > nor-flash, mmc or other peripheral drivers.
> >
> > Signed-off-by: Fred.Fan <fanyefeng at gmail.com>
> > Signed-off-by: Fred.Fan <r01011 at freescale.com>
>
> Please use a shorter subject (it gets tructaed), something like:
>
> mx51_babbage: add support for Freescale iMX51 board
> [Fred] ok
> > diff --git a/MAKEALL b/MAKEALL
> > index edebaea..852baf1 100755
> > --- a/MAKEALL
> > +++ b/MAKEALL
> > @@ -581,6 +581,7 @@ LIST_ARM_CORTEX_A8=" \
> > omap3_pandora \
> > omap3_zoom1 \
> > omap3_zoom2 \
> > + mx51_babbage \
>
> Please keep lists sorted.
> [Fred] ok.
> > diff --git a/board/freescale/mx51_babbage/board-mx51_babbage.h
> b/board/freescale/mx51_babbage/board-mx51_babbage.h
> > new file mode 100644
> > index 0000000..31e3875
> > --- /dev/null
> > +++ b/board/freescale/mx51_babbage/board-mx51_babbage.h
>
> Please drop the "board-" part of the file name.
> [Fred] ok.
> > +/*
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
>
> Please use correct licnese terms. Specify which version of the license
> applies, i. e. use something like:
>
> This code is licensed under the GPL, version 2 (or any later version).
> [Fred] Thanks. But this copyright block is defined by freescale.
>
It is used at the latest freescale's source or header file in linux
kernel.
I will tell our team about this comment.
Could I keep the block until it is changed by freescale?
> > +/*!
> > + * @defgroup BRDCFG_MX51_BABBAGE Board Configuration Options
> > + * @ingroup MSL_MX51_BABBAGE
> > + */
>
> Please get rid of this "!", "@defgroup", "@ingroup" etc. stuff.
> [Fred] ok
> > +/*!
> > + * @file mx51_babbage/board-mx51_babbage.h
>
> Drop this line. It carries no information.
> [Fred] ok
> > + * @brief This file contains all the board level configuration options.
> > + *
>
> Drop the blank line.
> [Fred] ok.
> > + * It currently hold the options defined for MX51 babbage Platform.
> > + *
> > + * @ingroup BRDCFG_MX51_BABBAGE
>
> Drop this line, Please apply similar to the rest of the code.
> [Fred] ok.
> > +/* CPLD offsets */
> > +#define PBC_LED_CTRL (0x20000)
> > +#define PBC_SB_STAT (0x20008)
> > +#define PBC_ID_AAAA (0x20040)
> > +#define PBC_ID_5555 (0x20048)
> > +#define PBC_VERSION (0x20050)
> > +#define PBC_ID_CAFE (0x20058)
> > +#define PBC_INT_STAT (0x20010)
> > +#define PBC_INT_MASK (0x20038)
> > +#define PBC_INT_REST (0x20020)
> > +#define PBC_SW_RESET (0x20060)
>
> Don't use base address + offsets, but declare proper C structures to
> describe the register layout. Then use I/O accessors to access
> registers.
> [Fred] ok
>
> > diff --git a/board/freescale/mx51_babbage/config.mkb/board/freescale/mx51_babbage/
> config.mk
> > new file mode 100644
> > index 0000000..d8b0f10
> > --- /dev/null
> > +++ b/board/freescale/mx51_babbage/config.mk
> > @@ -0,0 +1,2 @@
> > +LDSCRIPT = board/$(VENDOR)/$(BOARD)/u-boot.lds
> > +TEXT_BASE = 0x97800000
>
> Even though the file is short, please add a (C) and license header.
> [Fred] ok
> > diff --git a/board/freescale/mx51_babbage/flash_header.S
> b/board/freescale/mx51_babbage/flash_header.S
> > new file mode 100644
> > index 0000000..bbac2c1
> > --- /dev/null
> > +++ b/board/freescale/mx51_babbage/flash_header.S
> ...
> > +/*!
> > + * @file mx51_babbage/flash_head.S
> > + * @brief Thie file contains the information as the image header for
> boot.
>
> S/Thie/This/ ?
> [Fred] ok
> Please explain what "image header" means. U-Boot has an image geader,
> too, but you probably refer to something different.
>
> > + * The boot rom code will parse this structure , load image and initial
> > + * hardware as the description of hardware setting.
> > + */
>
> I read: "The ROM code will ... load ... initial hardware" ?
> [Fred] MX family soc has internal boot mode. At this mode, the rom code in
> soc will
>
read the setting array to initial hardware.
Something seems to be missing, or do you mean s/initial/initialize/ ?
>
> Please clean up.
> [Fred] Changed
>
> > +#include <config.h>
> > +#include <asm/arch/mx51.h>
> > +#include "board-mx51_babbage.h"
> > +
> > +#ifdef CONFIG_FLASH_HEADER
> > +#ifndef CONFIG_FLASH_HEADER_OFFSET
>
> It seems this is a iMX51 specific thing, right? Then please make this
> clear in the name oif the file and the macro names. "FLASH_HEADER" or
> "flash_head" is way too generic.
> [Fred] It is coming from freescale spec. It is the header of boot image for
> freescale internal boot mode.
>
> > diff --git a/board/freescale/mx51_babbage/lowlevel_init.S
> b/board/freescale/mx51_babbage/lowlevel_init.S
> > new file mode 100644
> > index 0000000..bf536b1
> > --- /dev/null
> > +++ b/board/freescale/mx51_babbage/lowlevel_init.S
>
> How much of this is really board specific code, or how much of it
> should be moved to the CPU directory instead?
> [Fred] Although freescale's boards use same configuration about M4IF, AIPS
> or others.
>
we can not sure other boards will use same configuration use freescale
boards.
>
> > +int dram_init(void)
> > +{
> > + gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> > + gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>
> Please use get_ram_size() for autosizing and (simple) RAM test.
> [Fred] ok
> > +}
> > +
> > +static void setup_uart(void)
> > +{
> > + unsigned int pad = PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE |
> > + PAD_CTL_PUE_PULL | PAD_CTL_DRV_HIGH;
> > +
> > + mxc_request_iomux(MX51_PIN_UART1_RXD, IOMUX_CONFIG_ALT0);
> > + mxc_iomux_set_pad(MX51_PIN_UART1_RXD, pad | PAD_CTL_SRE_FAST);
> > + mxc_request_iomux(MX51_PIN_UART1_TXD, IOMUX_CONFIG_ALT0);
> > + mxc_iomux_set_pad(MX51_PIN_UART1_TXD, pad | PAD_CTL_SRE_FAST);
> > + mxc_request_iomux(MX51_PIN_UART1_RTS, IOMUX_CONFIG_ALT0);
> > + mxc_iomux_set_pad(MX51_PIN_UART1_RTS, pad);
> > + mxc_request_iomux(MX51_PIN_UART1_CTS, IOMUX_CONFIG_ALT0);
> > + mxc_iomux_set_pad(MX51_PIN_UART1_CTS, pad);
>
> Hmm... the comment in the code for mxc_request_iomux() reads:
>
> * Request ownership for an IO pin. This function has to be the first one
> * being called before that pin is used. The caller has to check the
> * return value to make sure it returns 0.
>
> I do not see any such checks in your code?
>
> Either they are missing here, or the whole mxc_request_iomux() is not
> needed at all.
> [Fred] OK. I changed these functions to non return function.
>
Because boot load is not dynamic active or modules, there are no modules
with muxed pins.
> > +static void setup_expio(void)
> > +{
> ...
> > + /* WAL=0, WBED=1, WWSC=50, WADVA=2, WADVN=6, WEA=0,
> > + * WEN=0, WCSA=0, WCSN=0
> > + */
>
> Incorrect multiline comment. Please fix globally.
> [Fred] oK
> > + /* Reset interrupt status reg */
> > + writew(0x1F, mx51_io_base_addr + PBC_INT_REST);
> > + writew(0x00, mx51_io_base_addr + PBC_INT_REST);
> > + writew(0xFFFF, mx51_io_base_addr + PBC_INT_MASK);
>
> As mentioned above: do not use base address plus offset, but use a
> proper C structure to define the register map.
> [Fred] ok.
>
> > +#ifdef BOARD_LATE_INIT
> > +int board_late_init(void)
> > +{
> > + return 0;
> > +}
> > +#endif
>
> If the function is not needed, then don't implement it. Don;t add dead
> code.
> [Fred] ok.
> > +int checkboard(void)
> > +{
> > + printf("Board: MX51 BABBAGE ");
> > +
> > + if (system_rev & CHIP_REV_2_5)
> > + printf("2.5 [");
> > + else if (system_rev & CHIP_REV_2_0)
> > + printf("2.0 [");
> > + else if (system_rev & CHIP_REV_1_1)
> > + printf("1.1 [");
> > + else
> > + printf("1.0 [");
>
> Please use puts() for constant strings that don;t need any formatting.
> [Fred] ok.
>
> > diff --git a/board/freescale/mx51_babbage/u-boot.lds
> b/board/freescale/mx51_babbage/u-boot.lds
> > new file mode 100644
> > index 0000000..f608a6d
> > --- /dev/null
> > +++ b/board/freescale/mx51_babbage/u-boot.lds
>
> Does this board really need a specific linker script? Can we not use a
> generic one for all (or most of the) i.MX51 boards?
> [Fred] Need link flash header at the top of image.
>
Later tefano Babic will fix it.
> > + . = ALIGN(4);
> > + .text :
> > + {
> > + /* WARNING - the following is hand-optimized to fit within */
> > + /* the sector layout of our flash chips! XXX FIXME XXX */
>
> Why is this needed / useful? It doesn't seem to match the environment
> definitions in your board config file.
> [Fred] i.MX51 babbage no such issue.
> ...
> > diff --git a/cpu/arm_cortexa8/mx51/clock.c
> b/cpu/arm_cortexa8/mx51/clock.c
> > new file mode 100644
> > index 0000000..3abcdc8
> > --- /dev/null
> > +++ b/cpu/arm_cortexa8/mx51/clock.c
> ...
> > +static u32 __decode_pll(enum pll_clocks pll, u32 infreq)
> ...
> > +u32 __get_mcu_main_clk(void)
> ...
> > +static u32 __get_periph_clk(void)
>
> Is there any special reason for the "__" pat in all these names? You
> are aware that this has a special meaning in standard conforming C
> code, aren't you?
>
>
> Please also add comments what all these functions are doing.
>
> Hey, and add comments what the code is doing. It's not exactly obvious
> to me.
> [Fred] OK.
>
> > + u32 reg;
> > +
> > + reg = __raw_readl(MXC_CCM_CBCDR);
> > + if (reg & MXC_CCM_CBCDR_PERIPH_CLK_SEL) {
> > + reg = __raw_readl(MXC_CCM_CBCMR);
> > + switch ((reg & MXC_CCM_CBCMR_PERIPH_CLK_SEL_MASK) >>
> > + MXC_CCM_CBCMR_PERIPH_CLK_SEL_OFFSET) {
> > + case 0:
> > + return __decode_pll(PLL1_CLK,
> CONFIG_MX51_HCLK_FREQ);
> > + case 1:
> > + return __decode_pll(PLL3_CLK,
> CONFIG_MX51_HCLK_FREQ);
> > + default:
> > + return 0;
> > + }
> > + }
> > + return __decode_pll(PLL2_CLK, CONFIG_MX51_HCLK_FREQ);
>
> In such cases it's better to turn around the logig, i. e. write:
>
> if ((reg & MXC_CCM_CBCDR_PERIPH_CLK_SEL) == 0)
> return __decode_pll(PLL2_CLK, CONFIG_MX51_HCLK_FREQ);
>
> reg = __raw_readl(MXC_CCM_CBCMR);
> switch ((reg & MXC_CCM_CBCMR_PERIPH_CLK_SEL_MASK) >>
> MXC_CCM_CBCMR_PERIPH_CLK_SEL_OFFSET) {
> case 0:
> return __decode_pll(PLL1_CLK, CONFIG_MX51_HCLK_FREQ);
> case 1:
> return __decode_pll(PLL3_CLK, CONFIG_MX51_HCLK_FREQ);
> default:
> return 0;
> }
> /* NOTREACHED */
>
> You see? Less indentation = easier to read.
> [Fred] Thanks very mush.
> ...
> > +void mxc_show_clocks(void)
> > +{
> > + u32 freq;
> > +
> > + freq = __decode_pll(PLL1_CLK, CONFIG_MX51_HCLK_FREQ);
> > + printf("mx51 pll1: %dMHz\n", freq / 1000000);
> > + freq = __decode_pll(PLL2_CLK, CONFIG_MX51_HCLK_FREQ);
> > + printf("mx51 pll2: %dMHz\n", freq / 1000000);
> > + freq = __decode_pll(PLL3_CLK, CONFIG_MX51_HCLK_FREQ);
> > + printf("mx51 pll3: %dMHz\n", freq / 1000000);
> > + printf("ipg clock : %dHz\n", mxc_get_clock(MXC_IPG_CLK));
> > + printf("ipg per clock : %dHz\n", mxc_get_clock(MXC_IPG_PERCLK));
> > + printf("uart clock : %dHz\n", mxc_get_clock(MXC_UART_CLK));
> > + printf("cspi clock : %dHz\n", mxc_get_clock(MXC_CSPI_CLK));
>
> Please don't print too much during regular boot. Either make this
> debug() code, or provide a separate command so the user can print this
> if he is interested in this information. Standard boot output shall
> be terse, as it affects boot ime.
> [Fred] OK. I just keep some core clock.
>
> > diff --git a/cpu/arm_cortexa8/mx51/crm_regs.h
> b/cpu/arm_cortexa8/mx51/crm_regs.h
> > new file mode 100644
> > index 0000000..6436bed
> > --- /dev/null
> > +++ b/cpu/arm_cortexa8/mx51/crm_regs.h
>
> Should this file go to include/... ?
> [Fred] ok.
> > +/*
> > + * Copyright 2009 Freescale Semiconductor, Inc. All Rights Reserved.
> > + */
> > +
> > +/*
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
>
> See license header comments above.
> [Fred] Could I keep our block until freescale agree to change them?
>
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html> + */
> > +#ifndef __ARCH_ARM_MACH_MX51_CRM_REGS_H__
> > +#define __ARCH_ARM_MACH_MX51_CRM_REGS_H__
> > +
> > +#define MXC_CCM_BASE CCM_BASE_ADDR
> > +#define MXC_DPLL1_BASE PLL1_BASE_ADDR
> > +#define MXC_DPLL2_BASE PLL2_BASE_ADDR
> > +#define MXC_DPLL3_BASE PLL3_BASE_ADDR
> > +
> > +/* PLL Register Offsets */
> > +#define MXC_PLL_DP_CTL 0x00
> > +#define MXC_PLL_DP_CONFIG 0x04
> > +#define MXC_PLL_DP_OP 0x08
> > +#define MXC_PLL_DP_MFD 0x0C
> > +#define MXC_PLL_DP_MFN 0x10
> > +#define MXC_PLL_DP_MFNMINUS 0x14
> > +#define MXC_PLL_DP_MFNPLUS 0x18
> > +#define MXC_PLL_DP_HFS_OP 0x1C
> > +#define MXC_PLL_DP_HFS_MFD 0x20
> > +#define MXC_PLL_DP_HFS_MFN 0x24
> > +#define MXC_PLL_DP_MFN_TOGC 0x28
> > +#define MXC_PLL_DP_DESTAT 0x2c
>
> See comments above - use C structs instead of offset definitions.
>
> > diff --git a/cpu/arm_cortexa8/mx51/iomux.c
> b/cpu/arm_cortexa8/mx51/iomux.c
> > new file mode 100644
> > index 0000000..80082aa
> > --- /dev/null
> > +++ b/cpu/arm_cortexa8/mx51/iomux.c
> ...
> > +static inline u32 _get_mux_reg(iomux_pin_name_t pin)
> > +{
> > + u32 mux_reg = PIN_TO_IOMUX_MUX(pin);
> > +
> > + if (is_soc_rev(CHIP_REV_2_0) < 0) {
> > + if ((pin == MX51_PIN_NANDF_RB5) ||
> > + (pin == MX51_PIN_NANDF_RB6) ||
> > + (pin == MX51_PIN_NANDF_RB7))
> > + ; /* Do nothing */
> > + else if (mux_reg >= 0x2FC)
> > + mux_reg += 8;
> > + else if (mux_reg >= 0x130)
> > + mux_reg += 0xC;
> > + }
> > + mux_reg += IOMUXSW_MUX_CTL;
> > + return mux_reg;
> > +}
> > +
> > +static inline u32 _get_pad_reg(iomux_pin_name_t pin)
> > +{
>
>
>
> Please add comments what all this stuff is supposed to do.
> [Fred] OK.
> > +
> > + if (is_soc_rev(CHIP_REV_2_0) < 0) {
> > + if ((pin == MX51_PIN_NANDF_RB5) ||
> > + (pin == MX51_PIN_NANDF_RB6) ||
> > + (pin == MX51_PIN_NANDF_RB7))
> > + ; /* Do nothing */
> > + else if (pad_reg == 0x4D0 - PAD_I_START)
> > + pad_reg += 0x4C;
> > + else if (pad_reg == 0x860 - PAD_I_START)
> > + pad_reg += 0x9C;
> > + else if (pad_reg >= 0x804 - PAD_I_START)
> > + pad_reg += 0xB0;
> > + else if (pad_reg >= 0x7FC - PAD_I_START)
> > + pad_reg += 0xB4;
> > + else if (pad_reg >= 0x4E4 - PAD_I_START)
> > + pad_reg += 0xCC;
> > + else
> > + pad_reg += 8;
>
> You may also want to explain suchblack magic like the code here.
>
> And please add parens around the (0x??? - PAD_I_START) parts.
> [Fred] OK.
> iomux also use base + offset to access register. Because freescale
>
encode the register information into the pin definition. It is not easy to
change to structure access.
> +/*!
> > + * This function is used to configure a pin through the IOMUX module.
> > + * FIXED ME: for backward compatible. Will be static function!
>
> I don't understand what this means. Please explain.
> [Fred] It is copied from kernel source code. At the old version in kernel,
>
there are different implementation.
> > + * @param pin a pin number as defined in \b
> #iomux_pin_name_t
> > + * @param cfg an output function as defined in \b
> #iomux_pin_cfg_t
> > + *
> > + * @return 0 if successful; Non-zero otherwise
> > + */
> > +static int iomux_config_mux(iomux_pin_name_t pin, iomux_pin_cfg_t cfg)
>
> ...
>
> > +#if defined(CONFIG_DISPLAY_CPUINFO)
> > +int print_cpuinfo(void)
> > +{
> > + printf("CPU: Freescale i.MX51 family %d.%dV at %d MHz\n",
> > + (get_board_rev() & 0xFF) >> 4,
> > + (get_board_rev() & 0xF),
> > + __get_mcu_main_clk() / 1000000);
> > + mxc_show_clocks();
>
> Please see above - please be terse.
> [Fred] We want to show the chip version: TO1 version or TO2, etc.
>
So customer get the hardware information about soc level.
> > diff --git a/cpu/arm_cortexa8/mx51/timer.c
> b/cpu/arm_cortexa8/mx51/timer.c
> > new file mode 100644
> > index 0000000..5201768
> > --- /dev/null
> > +++ b/cpu/arm_cortexa8/mx51/timer.c
> ...
> > +/* General purpose timers registers */
> > +#define GPTCR GPT1_BASE_ADDR /* Control register */
> > +#define GPTPR (GPT1_BASE_ADDR + 0x4) /* Prescaler register */
> > +#define GPTSR (GPT1_BASE_ADDR + 0x8) /* Status register */
> > +#define GPTCNT (GPT1_BASE_ADDR + 0x24) /* Counter register */
> > +
> > +/* General purpose timers bitfields */
> > +#define GPTCR_SWR (1<<15) /* Software reset */
> > +#define GPTCR_FRR (1<<9) /* Freerun / restart */
> > +#define GPTCR_CLKSOURCE_32 (4<<6) /* Clock source */
> > +#define GPTCR_TEN (1) /* Timer enable */
>
> Move to header file. Use C struct instead of base addr + offsets.
> [Fred] ok.
> ...
> > +/*!
> > + * defines for SPBA modules
> > + */
> > +#define SPBA_SDHC1 0x04
> > +#define SPBA_SDHC2 0x08
> > +#define SPBA_UART3 0x0C
> > +#define SPBA_CSPI1 0x10
> > +#define SPBA_SSI2 0x14
> > +#define SPBA_SDHC3 0x20
> > +#define SPBA_SDHC4 0x24
> > +#define SPBA_SPDIF 0x28
> > +#define SPBA_ATA 0x30
> > +#define SPBA_SLIM 0x34
> > +#define SPBA_HSI2C 0x38
> > +#define SPBA_CTRL 0x3C
>
> Use C struct ?
> [Fred] ok.
> > +/*
> > + * Interrupt numbers
> > + */
> > +#define MXC_INT_BASE 0
> > +#define MXC_INT_RESV0 0
> > +#define MXC_INT_MMC_SDHC1 1
> > +#define MXC_INT_MMC_SDHC2 2
> > +#define MXC_INT_MMC_SDHC3 3
> > +#define MXC_INT_MMC_SDHC4 4
> > +#define MXC_INT_RESV5 5
> > +#define MXC_INT_SDMA 6
> > +#define MXC_INT_IOMUX 7
> > +#define MXC_INT_NFC 8
> > +#define MXC_INT_VPU 9
> > +#define MXC_INT_IPU_ERR 10
> > +#define MXC_INT_IPU_SYN 11
> > +#define MXC_INT_GPU 12
> > +#define MXC_INT_RESV13 13
> > +#define MXC_INT_USB_H1 14
> > +#define MXC_INT_EMI 15
> > +#define MXC_INT_USB_H2 16
> > +#define MXC_INT_USB_H3 17
> > +#define MXC_INT_USB_OTG 18
> > +#define MXC_INT_SAHARA_H0 19
> > +#define MXC_INT_SAHARA_H1 20
> > +#define MXC_INT_SCC_SMN 21
> > +#define MXC_INT_SCC_STZ 22
> > +#define MXC_INT_SCC_SCM 23
> > +#define MXC_INT_SRTC_NTZ 24
> > +#define MXC_INT_SRTC_TZ 25
> > +#define MXC_INT_RTIC 26
> > +#define MXC_INT_CSU 27
> > +#define MXC_INT_SLIM_B 28
> > +#define MXC_INT_SSI1 29
> > +#define MXC_INT_SSI2 30
> > +#define MXC_INT_UART1 31
> > +#define MXC_INT_UART2 32
> > +#define MXC_INT_UART3 33
> > +#define MXC_INT_RESV34 34
> > +#define MXC_INT_RESV35 35
> > +#define MXC_INT_CSPI1 36
> > +#define MXC_INT_CSPI2 37
> > +#define MXC_INT_CSPI 38
> > +#define MXC_INT_GPT 39
> > +#define MXC_INT_EPIT1 40
> > +#define MXC_INT_EPIT2 41
> > +#define MXC_INT_GPIO1_INT7 42
> > +#define MXC_INT_GPIO1_INT6 43
> > +#define MXC_INT_GPIO1_INT5 44
> > +#define MXC_INT_GPIO1_INT4 45
> > +#define MXC_INT_GPIO1_INT3 46
> > +#define MXC_INT_GPIO1_INT2 47
> > +#define MXC_INT_GPIO1_INT1 48
> > +#define MXC_INT_GPIO1_INT0 49
> > +#define MXC_INT_GPIO1_LOW 50
> > +#define MXC_INT_GPIO1_HIGH 51
> > +#define MXC_INT_GPIO2_LOW 52
> > +#define MXC_INT_GPIO2_HIGH 53
> > +#define MXC_INT_GPIO3_LOW 54
> > +#define MXC_INT_GPIO3_HIGH 55
> > +#define MXC_INT_GPIO4_LOW 56
> > +#define MXC_INT_GPIO4_HIGH 57
> > +#define MXC_INT_WDOG1 58
> > +#define MXC_INT_WDOG2 59
> > +#define MXC_INT_KPP 60
> > +#define MXC_INT_PWM1 61
> > +#define MXC_INT_I2C1 62
> > +#define MXC_INT_I2C2 63
> > +#define MXC_INT_HS_I2C 64
> > +#define MXC_INT_RESV65 65
> > +#define MXC_INT_RESV66 66
> > +#define MXC_INT_SIM_IPB 67
> > +#define MXC_INT_SIM_DAT 68
> > +#define MXC_INT_IIM 69
> > +#define MXC_INT_ATA 70
> > +#define MXC_INT_CCM1 71
> > +#define MXC_INT_CCM2 72
> > +#define MXC_INT_GPC1 73
> > +#define MXC_INT_GPC2 74
> > +#define MXC_INT_SRC 75
> > +#define MXC_INT_NM 76
> > +#define MXC_INT_PMU 77
> > +#define MXC_INT_CTI_IRQ 78
> > +#define MXC_INT_CTI1_TG0 79
> > +#define MXC_INT_CTI1_TG1 80
> > +#define MXC_INT_MCG_ERR 81
> > +#define MXC_INT_MCG_TMR 82
> > +#define MXC_INT_MCG_FUNC 83
> > +#define MXC_INT_RESV84 84
> > +#define MXC_INT_RESV85 85
> > +#define MXC_INT_RESV86 86
> > +#define MXC_INT_FEC 87
> > +#define MXC_INT_OWIRE 88
> > +#define MXC_INT_CTI1_TG2 89
> > +#define MXC_INT_SJC 90
> > +#define MXC_INT_SPDIF 91
> > +#define MXC_INT_TVE 92
> > +#define MXC_INT_FIRI 93
> > +#define MXC_INT_PWM2 94
> > +#define MXC_INT_SLIM_EXP 95
> > +#define MXC_INT_SSI3 96
> > +#define MXC_INT_RESV97 97
> > +#define MXC_INT_CTI1_TG3 98
> > +#define MXC_INT_SMC_RX 99
> > +#define MXC_INT_VPU_IDLE 100
> > +#define MXC_INT_RESV101 101
> > +#define MXC_INT_GPU_IDLE 102
> > +
> > +#define MXC_MAX_INT_LINES 128
>
> Use C struct ?
>
> And so on and on ...
> [Fred] It is not register definitions. It is irq number. But uboot is not
> enable them.
>
I will remove them.
And there are some definitions will be used in assemble code.
Must keep them.
> diff --git a/include/configs/mx51_babbage.h
> b/include/configs/mx51_babbage.h
> > new file mode 100644
> > index 0000000..a24b39b
> > --- /dev/null
> > +++ b/include/configs/mx51_babbage.h
> ...
> > +#define BOARD_LATE_INIT
>
> Why do you #define this, when you don't need it at all?
> [Fred] I removed. But it will be added in the furture for power supply
> operation.
>
> > +#define CONFIG_SYS_MEMTEST_START 0 /* memtest works on */
> > +#define CONFIG_SYS_MEMTEST_END 0x10000
>
> That's not exactly a thorough testing, then. And is low memory really
> unused? Please check!
> [Fred] yes. It is not used. We just the bottom of ram.
>
> > +#undef CONFIG_SYS_CLKS_IN_HZ /* everything, incl board
> info, in Hz */
>
> Don't undef what doesn't exist.
> [Fred] OK. I will fix them.
> > +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
> > +
> > +#define CONFIG_SYS_HZ CONFIG_MX51_CLK32/* use 32kHz clock as
> source */
>
> CONFIG_SYS_HZ must be 1000.
> [Fred] done.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Voodoo Programming: Things programmers do that they know shouldn't
> work but they try anyway, and which sometimes actually work, such as
> recompiling everything. - Karl Lehenbauer
>
More information about the U-Boot
mailing list