[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