[U-Boot-Users] Please pull u-boot-83xx.git

[Timur Tabi]

Stefan Roese wrote:

> - 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 will remove the changelog comments from all the files in cpu/mpc83xx.

> - 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.

I see the brace is already on the correct line.  I think some of the style 
errors you're seeing have already been fixed in a later commit that's also part 
of our 83xx tree.

>> +	if (spd.config == 0x02) {
>> +		printf(" with ECC\n");
>> +	}
> 
> No brace here.

I will go through all of spd_sdram.c and remove the braces from all one-line 
if-else statements and make sure the remaining braces are on the right lines.

>> +ulong get_ddr_clk(ulong dummy)
>> +{
>> +	return gd->ddr_clk;
>> +}
> 
> Hmmm. Is this function really needed?

I will delete it.

> 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

I don't get these warnings, but I think I fixed the problem.  You will need to 
tell me if they still show up.

> 
> 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

I don't get these warnings either, but I don't see how you could.  Perhaps 
you're not using the top of our tree?  All of these variables are initialized by 
the call to ucc_get-cmxucr_reg().


-- 
Timur Tabi
Linux Kernel Developer @ Freescale