[U-Boot-Users] Please pull u-boot-83xx.git
Stefan Roese
sr at denx.de
Sun Nov 26 14:46:06 CET 2006
Hi Kim,
On Saturday 04 November 2006 03:11, Kim Phillips wrote:
> Please pull from 'master' branch of:
>
> http://opensource.freescale.com/pub/scm/u-boot-83xx.git
>
> to receive the following updates (essentially MPC8349mITX and MPC8360EMDS
> support):
I did a quick check. The overall impression is quite good. Thanks a lot
for your contribution (and you colleagues too of course).
Here a few notes:
- Please don't add changelog comments in the files (like in
cpu/mpc83xx/i2c.c or spd_sdram.c). Instead please remove all the
file internal changelog comments, since we decided to only use
git for this history.
- I won't comment on the "support for multiple I2C buses" in this mail,
since it's done by Ben Warren. I'll will send a separate mail for this.
- Please add the missing entries to the MAINTAINERS files (MPC8349EMDS &
MPC8360EMDS).
Another idea (this would also affect the other Freescale board ports):
What do you think of moving all your Freescale board ports into a
Freescale board directory (as done for AMCC or esd for example).
So we would get something like:
board/freescale/mpc8349emds
board/freescale/mpc8349itx
board/freescale/mpc8360emds
...
board/freescale/common (if needed)
This way we would not "pollute" the main board directory too much.
Especially when thinking of additional Freescale board, which
hopefully will come...
We could start with the mpc83xx boards and contine soon with the
85xx and 86xx eval boards (and so on...).
Here some further remarks to your patches:
> --------------------------- cpu/mpc83xx/spd_sdram.c
> --------------------------- index 48624fe..153848d 100644
> @@ -1,4 +1,6 @@
> /*
> + * (C) Copyright 2006 Freescale Semiconductor, Inc.
> + *
> * (C) Copyright 2006
> * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> *
> @@ -28,6 +30,10 @@
> *
> * 20050101: Eran Liberty (liberty at freescale.com)
> * Initial file creating (porting from 85XX & 8260)
> + * 20060601: Dave Liu (daveliu at freescale.com)
> + * DDR ECC support
> + * unify variable names for 83xx
> + * code cleanup
Please remove the change history.
> */
>
> #include <common.h>
> @@ -39,7 +45,7 @@
>
> #ifdef CONFIG_SPD_EEPROM
>
> -#if defined(CONFIG_DDR_ECC)
> +#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRC)
> extern void dma_init(void);
> extern uint dma_check(void);
> extern int dma_xfer(void *dest, uint count, void *src);
> @@ -52,15 +58,18 @@ extern int dma_xfer(void *dest, uint cou
> /*
> * Convert picoseconds into clock cycles (rounding up if needed).
> */
> +extern ulong get_ddr_clk(ulong dummy);
>
> int
> picos_to_clk(int picos)
> {
> + unsigned int ddr_bus_clk;
> int clks;
>
> - clks = picos / (2000000000 / (get_bus_freq(0) / 1000));
> - if (picos % (2000000000 / (get_bus_freq(0) / 1000)) != 0) {
> - clks++;
> + ddr_bus_clk = get_ddr_clk(0) >> 1;
> + clks = picos / ((1000000000 / ddr_bus_clk) * 1000);
> + if (picos % ((1000000000 / ddr_bus_clk) * 1000) !=0) {
Add a space after the !=.
> + clks++;
> }
Remove the { } here (1 line statements shouldn't have braces).
<snip>
> @@ -246,36 +304,49 @@ long int spd_sdram()
>
> debug("DDR:timing_cfg_1=0x%08x\n", ddr->timing_cfg_1);
> debug("DDR:timing_cfg_2=0x%08x\n", ddr->timing_cfg_2);
> + /* Setup init value, but not enable */
> + ddr->sdram_cfg = 0x42000000;
> +
> + /* Check DIMM data bus width */
> + if (spd.dataw_lsb == 0x20)
> + {
Please move the brace in the line above.
> + burstlen = 0x03; /* 32 bit data bus, burst len is 8 */
> + printf("\n DDR DIMM: data bus width is 32 bit");
> + }
> + else
> + {
This should be "} else {".
> + burstlen = 0x02; /* Others act as 64 bit bus, burst len is 4 */
> + printf("\n DDR DIMM: data bus width is 64 bit");
> + }
>
> - /*
> - * Only DDR I is supported
> - * DDR I and II have different mode-register-set definition
> + /* Is this an ECC DDR chip? */
> + if (spd.config == 0x02) {
> + printf(" with ECC\n");
> + }
No brace here.
<snip>
> @@ -352,38 +405,48 @@ long int spd_sdram()
> * sdram_cfg[0] = 1 (ddr sdram logic enable)
> * sdram_cfg[1] = 1 (self-refresh-enable)
> * sdram_cfg[6:7] = 2 (SDRAM type = DDR SDRAM)
> + * sdram_cfg[12] = 0 (32_BE =0 , 64 bit bus mode)
> + * sdram_cfg[13] = 0 (8_BE =0, 4-beat bursts)
> */
> - tmp = 0xc2000000;
> + sdram_cfg = 0xC2000000;
>
> -#if defined (CONFIG_DDR_32BIT)
> - /* in 32-Bit mode burst len is 8 beats */
> - tmp |= (SDRAM_CFG_32_BE | SDRAM_CFG_8_BE);
> -#endif
> - /*
> - * sdram_cfg[3] = RD_EN - registered DIMM enable
> - * A value of 0x26 indicates micron registered DIMMS (micron.com)
> - */
> - if (spd.mod_attr == 0x26) {
> - tmp |= 0x10000000;
> + /* sdram_cfg[3] = RD_EN - registered DIMM enable */
> + if (spd.mod_attr & 0x02) {
> + sdram_cfg |= 0x10000000;
> }
Again, no brace.
>
> + /* The DIMM is 32bit width */
> + if (spd.dataw_lsb == 0x20) {
> + sdram_cfg |= 0x000C0000;
> + }
Again.
> ----------------------------- cpu/mpc83xx/speed.c
> ----------------------------- index ad6b3f6..412713f 100644
> @@ -337,6 +337,12 @@ int get_clocks (void)
> return 0;
> }
>
> +ulong get_ddr_clk(ulong dummy)
> +{
> + return gd->ddr_clk;
> +}
Hmmm. Is this function really needed?
> Author: Timur Tabi <timur at freescale.com> 2006-10-26 01:45:23
> Committer: Kim Phillips <kim.phillips at freescale.com> 2006-11-04 02:42:19
> Parent: 31068b7c4abeefcb2c8fd4fbeccc8ec6c6d0475a (mpc83xx: Add support for variable flash memory sizes on 83xx systems)
> Child: bed85caf872714ebf53013967a695c9d63acfc68 (mpc83xx: Add support for Errata DDR6 on MPC 834x systems)
> Branches: master, 83xx
> Follows: U-Boot-1_1_6
> Precedes:
>
> mpc83xx: fix TQM build by defining a CFG_FLASH_SIZE for it
>
> -------------------------- include/configs/TQM834x.h --------------------------
> index b1f033d..f0e4900 100644
> @@ -95,6 +95,7 @@
> #define CFG_FLASH_CFI_DRIVER /* use the CFI driver */
> #undef CFG_FLASH_CHECKSUM
> #define CFG_FLASH_BASE 0x80000000 /* start of FLASH */
> +#define CFG_FLASH_SIZE 8 /* FLASH size in MB */
Do you only support sizes bigger or equal to 1 MByte? What if a board
only has 512kBytes?
I also do get some warning upon compiling for MPC8349ITX:
mpc8349itx.c: In function 'misc_init_r':
mpc8349itx.c:398: warning: pointer targets in passing argument 4 of 'i2c_read' differ in signedness
mpc8349itx.c:443: warning: pointer targets in passing argument 4 of 'i2c_write' differ in signedness
And for MPC8360EMDS:
uccf.c: In function 'ucc_set_clk_src':
uccf.c:121: warning: 'reg_num' is used uninitialized in this function
uccf.c:105: warning: 'shift' may be used uninitialized in this function
uccf.c:103: warning: 'p_cmxucr' may be used uninitialized in this function
Could you please clean this up too?
Thanks a lot for your patience. ;-)
Best regards,
Stefan
More information about the U-Boot
mailing list