On 28/06/2012 22:36, Philippe Reynes wrote:
> Signed-off-by: Philippe Reynes <trem...@yahoo.fr>
> ---

Hi Philippe,

>  include/configs/apf27.h |  916 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 916 insertions(+), 0 deletions(-)
>  create mode 100644 include/configs/apf27.h
> 
> diff --git a/include/configs/apf27.h b/include/configs/apf27.h
> new file mode 100644

Really this patch must be merged with patch 2/5: "Add support for the
armadeus APF27". They belong together, and it maintains the tree bisectable.

> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +#define CONFIG_VERSION_VARIABLE
> +#define CONFIG_ENV_VERSION   "3.1"
> +#define CONFIG_IDENT_STRING  " apf27 patch 3.6"
> +#define CONFIG_BOARD_NAME    apf27
> +
> +/*
> + * SoC configurations
> + */
> +#define CONFIG_ARM926EJS             /* this is an ARM926EJS CPU */
> +#define CONFIG_MX27                  /* in a Freescale i.MX27 Chip */
> +#define CONFIG_MACH_TYPE     1698    /* APF27 */
> +
> +/*
> + * Enable the call to miscellaneous platform dependent initialization.
> + */
> +#define CONFIG_SYS_NO_FLASH  /* to be define before <config_cmd_default.h> */
> +#define CONFIG_SYS_DCACHE_OFF
> +#define CONFIG_MISC_INIT_R
> +
> +/*
> + * Board display option
> + */
> +#define CONFIG_DISPLAY_BOARDINFO
> +#define CONFIG_DISPLAY_CPUINFO
> +
> +/*
> + * SPL
> + */
> +#define CONFIG_NAND_U_BOOT
> +/* Copy SPL+U-Boot here           */
> +#define CONFIG_SYS_NAND_U_BOOT_DST   CONFIG_SYS_LOAD_ADDR
> +/* Size is the partion size  */
> +#define CONFIG_SYS_NAND_U_BOOT_SIZE  CONFIG_SYS_MONITOR_LEN

See my comment about the SPL patch.

> +
> +#define PHYS_SDRAM_1                 0xA0000000
> +#define PHYS_SDRAM_2                 0xB0000000
> +#define CONFIG_SYS_SDRAM_BASE                PHYS_SDRAM_1
> +#define CONFIG_SYS_MALLOC_LEN                (CONFIG_ENV_SIZE + (512<<10))
> +#define CONFIG_SYS_GBL_DATA_SIZE     256             /* Initial data */

We use a macro for CONFIG_SYS_GBL_DATA_SIZE. Is it not suitable for your
needs ?

> +#define CONFIG_SYS_TEXT_BASE 0xA1000000
> +
> +/*
> + * FLASH organization
> + */
> +/*#define    CONFIG_SYS_MONITOR_BASE 0xAFF00000*/

Do not let dead code

> +#define      CONFIG_FIRMWARE_OFFSET          0x00200000
> +#define      CONFIG_KERNEL_OFFSET            0x00300000
> +#define      CONFIG_ROOTFS_OFFSET            0x00800000
> +
> +#define CONFIG_MTDMAP                        "mxc_nand.0"
> +#define MTDIDS_DEFAULT                       "nand0=" CONFIG_MTDMAP
> +#define MTDPARTS_DEFAULT     "mtdparts=" CONFIG_MTDMAP \
> +                             ":1M(u-boot)ro," \
> +                             "512K(env)," \
> +                             "512K(env2)," \
> +                             "512K(firmware)," \
> +                             "512K(dtb)," \
> +                             "5M(kernel)," \
> +                             "-(rootfs)"
> +
> +/*
> + * U-Boot general configurations
> + */
> +#define CONFIG_SYS_LONGHELP
> +#define CONFIG_SYS_PROMPT            "BIOS> "        /* prompt string */
> +#define CONFIG_SYS_CBSIZE            2048            /* console I/O buffer */
> +#define CONFIG_SYS_PBSIZE            \
> +                             (CONFIG_SYS_CBSIZE+sizeof(CONFIG_SYS_PROMPT)+16)
> +                                             /* Print buffer size */
> +#define CONFIG_SYS_MAXARGS           16              /* max command args */
> +#define CONFIG_SYS_BARGSIZE          CONFIG_SYS_CBSIZE
> +                                             /* Boot argument buffer size */
> +#define CONFIG_AUTO_COMPLETE
> +#define CONFIG_CMDLINE_EDITING
> +#define CONFIG_SYS_HUSH_PARSER                       /* enable the "hush" 
> shell */
> +#define CONFIG_SYS_PROMPT_HUSH_PS2   "> "    /* secondary prompt string */
> +
> +/*
> + * Boot Linux
> + */
> +#define CONFIG_CMDLINE_TAG           /* send commandline to Kernel   */
> +#define CONFIG_SETUP_MEMORY_TAGS     /* send memory definition to kernel */
> +#define CONFIG_INITRD_TAG            /* send initrd params   */
> +
> +/* #define CONFIG_REVISION_TAG */

Drop also this line - check in the whole file for these occurrencies


> +#if 0
> +#define CONFIG_SYS_FPGA_IS_PROTO /* to be defined with apf27 board prototype 
> */
> +#endif

drop also this dead code

> +#ifndef CONFIG_MX27_CLK26
> +#if (CONFIG_MX27_CLK32 == 32000)
> +#define CONFIG_SYS_MPCTL0_VAL                0x00211803      /* 398.998 MHz 
> */
> +#define CONFIG_SYS_MPCTL1_VAL                0
> +#define CONFIG_MPLL_FREQ     399
> +#else /* CONFIG_MX27_CLK32 == 32768*/
> +#define CONFIG_SYS_MPCTL0_VAL                0x01EF15D5      /* 399.000 MHz 
> */
> +#define CONFIG_SYS_MPCTL1_VAL                0
> +#define CONFIG_MPLL_FREQ     399
> +#endif /* CONFIG_MX27_CLK32 */
> +#else /* CONFIG_MX27_CLK26 in use*/
> +#define CONFIG_SYS_MPCTL0_VAL                0x00331C23      /* 399.000 MHz 
> */
> +#define CONFIG_SYS_MPCTL1_VAL                0
> +#define CONFIG_MPLL_FREQ     399
> +#endif /* CONFIG_MX27_CLK26 */

Do you have several hardware version of the same board or which is the
reason for that ?

> +/* ARM bus frequency (have to be a CONFIG_MPLL_FREQ ratio) */
> +#define CONFIG_ARM_FREQ              399     /* up to 400 MHz */
> +
> +/* external bus frequency (have to be a CONFIG_SYS_CLK_FREQ ratio) */
> +#define CONFIG_HCLK_FREQ     133     /* (CONFIG_SYS_CLK_FREQ/2) */
> +
> +#define CONFIG_PERIF1_FREQ   16      /* 16.625 MHz UART, GPT, PWM*/
> +#define CONFIG_PERIF2_FREQ   33      /* 33.25 MHz CSPI and SDHC */
> +#define CONFIG_PERIF3_FREQ   33      /* 33.25 MHz LCD*/
> +#define CONFIG_PERIF4_FREQ   33      /* 33.25 MHz CSI*/
> +#define CONFIG_SSI1_FREQ     66      /* 66.50 MHz SSI1*/
> +#define CONFIG_SSI2_FREQ     66      /* 66.50 MHz SSI2*/
> +#define CONFIG_MSHC_FREQ     66      /* 66.50 MHz MSHC*/
> +#define CONFIG_H264_FREQ     66      /* 66.50 MHz H264*/
> +#define CONFIG_CLK0_DIV      3  /* Divide CLK0 by 4 */
> +#define CONFIG_CLK0_EN       1  /* CLK0 enabled */

Really there is another way to get the peripheral clocks for the i.MX.
Each SOC implements or should implement mxc_get_clock(), passing as
parameter the peripheral you need to know the clock.

> +/*
> + * SDRAM
> + */
> +#if (CONFIG_SYS_SDRAM_MBYTE_SYZE == 64) /* micron MT46H16M32LF -6 */
> +/* micron 64MB */
> +#define PHYS_SDRAM_1_SIZE                    0x04000000 /* 64 MB */
> +#define PHYS_SDRAM_2_SIZE                    0x04000000 /* 64 MB */
> +#define CONFIG_SYS_SDRAM_NUM_COL             9  /* 8, 9, 10 or 11
> +                                                   column address bits */
> +#define CONFIG_SYS_SDRAM_NUM_ROW             13 /* 11, 12 or 13
> +                                                   row address bits */
> +#define CONFIG_SYS_SDRAM_REFRESH             3  /* 0=OFF 1=2048
> +                                                   2=4096 3=8192 refresh */
> +#define CONFIG_SYS_SDRAM_EXIT_PWD            25 /* ns exit power
> +                                                   down delay */
> +#define CONFIG_SYS_SDRAM_W2R_DELAY           1  /* write to read
> +                                                   cycle delay > 0 */
> +#define CONFIG_SYS_SDRAM_ROW_PRECHARGE_DELAY 18 /* ns */
> +#define CONFIG_SYS_SDRAM_TMRD_DELAY          2  /* Load mode register
> +                                                   cycle delay 1..4 */
> +#define CONFIG_SYS_SDRAM_TWR_DELAY           1  /* LPDDR: 0=2ck 1=3ck;
> +                                                   SDRAM: 0=1ck 1=2ck*/
> +#define CONFIG_SYS_SDRAM_RAS_DELAY           42 /* ns ACTIVE-to-PRECHARGE
> +                                                   delay */
> +#define CONFIG_SYS_SDRAM_RRD_DELAY           12 /* ns ACTIVE-to-ACTIVE
> +                                                   delay */
> +#define CONFIG_SYS_SDRAM_RCD_DELAY           18 /* ns Row to Column delay */
> +#define CONFIG_SYS_SDRAM_RC_DELAY            70 /* ns Row cycle delay (tRFC
> +                                                   refresh to command) */
> +#define CONFIG_SYS_SDRAM_CLOCK_CYCLE_CL_1    0 /* ns clock cycle time
> +                                                  estimated fo CL=1
> +                                                  0=force 3 for lpddr */
> +#define CONFIG_SYS_SDRAM_PARTIAL_ARRAY_SR    0  /* 0=full 1=half 2=quater
> +                                                   3=Eighth 4=Sixteenth */
> +#define CONFIG_SYS_SDRAM_DRIVE_STRENGH               0  /* 0=Full-strength 
> 1=half
> +                                                   2=quater 3=Eighth */
> +#define CONFIG_SYS_SDRAM_BURST_LENGTH                3  /* 2^N BYTES 
> (N=0..3) */
> +#define CONFIG_SYS_SDRAM_SINGLE_ACCESS               0  /* 1= single access;
> +                                                   0 = Burst mode */
> +#endif

Why do these config belong to the config file ? You introduce several
new CONFIG_SYS, and they must be documented. Anyway, you do not need it,
because I assume all these values are used only in the board directory,
not generally for other subsystem of u-boot.


> +
> +/* FPGA specific settings */
> +/* CLKO */
> +#define CONFIG_SYS_CCSR_VAL 0x00000305
> +/* drive strength CLKO set to 2*/
> +#define CONFIG_SYS_DSCR10_VAL 0x00020000
> +/* drive strength A1..A12 set to 2*/
> +#define CONFIG_SYS_DSCR3_VAL 0x02AAAAA8
> +/* drive strength ctrl*/
> +#define CONFIG_SYS_DSCR7_VAL 0x00020880
> +/* drive strength data*/
> +#define CONFIG_SYS_DSCR2_VAL 0xAAAAAAAA

Maybe you should move also FPGA related values to the fpga file - and as
I said, a CONFIG_SYS_ define has meaning for the whole code across
architectures, and this is not the case. The same comment for the GPIOs
and so on.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to