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

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

[Kim Phillips]

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

Ben Warren:
      Add support for multiple I2C buses
      Multi-bus I2C implementation of MPC834x
      Additional MPC8349 support for multibus i2c

Dave Liu:
      mpc83xx: Changed to unified mpx83xx names and added common 83xx changes
      mpc83xx: Add 8360 specifics to 83xx immap
      mpc83xx: add the QUICC Engine (QE) immap file
      mpc83xx: Add MPC8360EMDS basic board support
      mpc83xx: add QE ethernet support
      mpc83xx: add the README.mpc8360emds
      mpc83xx: Fix the incorrect dcbz operation

Kim Phillips:
      mpc83xx: change ft code to modify local-mac-address property
      mpc83xx: add OF_FLAT_TREE bits to 83xx boards
      mpc83xx: Lindent and clean up cpu/mpc83xx/speed.c

Nick Spence:
      Added RGMII support to the TSECs and Marvell 881111 Phy
      NAND Flash verify across block boundaries

Tanya Jiang:
      mpc83xx: Removed unused file resetvec.S for mpc83xx cpu
      mpc83xx: Fix missing build for mpc8349emds pci.c
      mpc83xx: Unified TQM834x variable names with 83xx and consolidated macros

Timur Tabi:
      mpc83xx: Add support for variable flash memory sizes on 83xx systems
      mpc83xx: fix TQM build by defining a CFG_FLASH_SIZE for it
      mpc83xx: Add support for Errata DDR6 on MPC 834x systems
      mpc83xx: Add support for the MPC8349E-mITX
      mpc83xx: Fix PCI, USB, bootargs for MPC8349E-mITX
      mpc83xx: Fix dual I2C support for the MPC8349ITX, MPC8349EMDS, TQM834x, and MPC8360EMDS
      mpc83xx: Replace CFG_IMMRBAR with CFG_IMMR
      mpc83xx: Update 83xx to use fsl_i2c.c

 CREDITS                         |    5
 MAINTAINERS                     |    4
 MAKEALL                         |    2
 Makefile                        |   36 +
 README                          |   61 +
 board/mpc8349emds/Makefile      |    2
 board/mpc8349emds/mpc8349emds.c |   40 +
 board/mpc8349emds/pci.c         |   53 +
 board/mpc8349itx/Makefile       |   48 +
 board/mpc8349itx/config.mk      |   33 +
 board/mpc8349itx/mpc8349itx.c   |  477 +++++++++
 board/mpc8349itx/pci.c          |  357 +++++++
 board/mpc8349itx/u-boot.lds     |  120 ++
 board/mpc8360emds/Makefile      |   50 +
 board/mpc8360emds/config.mk     |   28 +
 board/mpc8360emds/mpc8360emds.c |  657 ++++++++++++
 board/mpc8360emds/pci.c         |  313 ++++++
 board/mpc8360emds/u-boot.lds    |  123 ++
 board/tqm834x/pci.c             |   18
 board/tqm834x/tqm834x.c         |    4
 common/cmd_i2c.c                |  159 +++
 cpu/mpc83xx/Makefile            |    4
 cpu/mpc83xx/cpu.c               |  155 ++-
 cpu/mpc83xx/cpu_init.c          |   71 +
 cpu/mpc83xx/i2c.c               |  253 -----
 cpu/mpc83xx/interrupts.c        |    2
 cpu/mpc83xx/qe_io.c             |   85 ++
 cpu/mpc83xx/resetvec.S          |    6
 cpu/mpc83xx/spd_sdram.c         |  557 ++++++----
 cpu/mpc83xx/speed.c             |  312 +++---
 cpu/mpc83xx/start.S             |   57 +
 doc/README.mpc8360emds          |  126 ++
 drivers/fsl_i2c.c               |  113 +-
 drivers/nand/nand_base.c        |    1
 drivers/qe/Makefile             |   43 +
 drivers/qe/qe.c                 |  254 +++++
 drivers/qe/qe.h                 |  237 ++++
 drivers/qe/uccf.c               |  404 +++++++
 drivers/qe/uccf.h               |  130 ++
 drivers/qe/uec.c                | 1266 +++++++++++++++++++++++
 drivers/qe/uec.h                |  716 +++++++++++++
 drivers/qe/uec_phy.c            |  604 +++++++++++
 drivers/qe/uec_phy.h            |  256 +++++
 drivers/tsec.c                  |   12
 drivers/tsec.h                  |    2
 include/asm-ppc/e300.h          |    2
 include/asm-ppc/global_data.h   |   16
 include/asm-ppc/i2c.h           |  103 --
 include/asm-ppc/immap_83xx.h    | 2130 ++++++++++++++++++++++++++++-----------
 include/asm-ppc/immap_qe.h      |  550 ++++++++++
 include/common.h                |    5
 include/configs/MPC8349EMDS.h   |   70 +
 include/configs/MPC8349ITX.h    |  804 +++++++++++++++
 include/configs/MPC8360EMDS.h   |  635 ++++++++++++
 include/configs/TQM834x.h       |   28 -
 include/i2c.h                   |   45 +
 include/ioports.h               |   11
 include/mpc83xx.h               |  138 ++-
 lib_ppc/board.c                 |    2
 net/eth.c                       |    7
 60 files changed, 11286 insertions(+), 1516 deletions(-)
 create mode 100644 board/mpc8349itx/Makefile
 create mode 100644 board/mpc8349itx/config.mk
 create mode 100644 board/mpc8349itx/mpc8349itx.c
 create mode 100644 board/mpc8349itx/pci.c
 create mode 100644 board/mpc8349itx/u-boot.lds
 create mode 100644 board/mpc8360emds/Makefile
 create mode 100644 board/mpc8360emds/config.mk
 create mode 100644 board/mpc8360emds/mpc8360emds.c
 create mode 100644 board/mpc8360emds/pci.c
 create mode 100644 board/mpc8360emds/u-boot.lds
 delete mode 100644 cpu/mpc83xx/i2c.c
 create mode 100644 cpu/mpc83xx/qe_io.c
 delete mode 100644 cpu/mpc83xx/resetvec.S
 create mode 100644 doc/README.mpc8360emds
 create mode 100644 drivers/qe/Makefile
 create mode 100644 drivers/qe/qe.c
 create mode 100644 drivers/qe/qe.h
 create mode 100644 drivers/qe/uccf.c
 create mode 100644 drivers/qe/uccf.h
 create mode 100644 drivers/qe/uec.c
 create mode 100644 drivers/qe/uec.h
 create mode 100644 drivers/qe/uec_phy.c
 create mode 100644 drivers/qe/uec_phy.h
 delete mode 100644 include/asm-ppc/i2c.h
 create mode 100644 include/asm-ppc/immap_qe.h
 create mode 100644 include/configs/MPC8349ITX.h
 create mode 100644 include/configs/MPC8360EMDS.h

For the resulting (cumulative) diff, please reference:

http://opensource.freescale.com/pub/mpc83xx-origin-master.diff

Thanks,

Kim

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

[Joakim Tjernlund]

Any progress on this? I have a 83xx CPU and would love to see this in u-boot.

 Jocke

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Kim Phillips
> Sent: den 4 november 2006 03:12
> To: Wolfgang Denk
> Cc: u-boot-users at lists.sourceforge.net
> Subject: [U-Boot-Users] Please pull u-boot-83xx.git
> 
> 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):
> 
> Ben Warren:
>       Add support for multiple I2C buses
>       Multi-bus I2C implementation of MPC834x
>       Additional MPC8349 support for multibus i2c
> 
> Dave Liu:
>       mpc83xx: Changed to unified mpx83xx names and added 
> common 83xx changes
>       mpc83xx: Add 8360 specifics to 83xx immap
>       mpc83xx: add the QUICC Engine (QE) immap file
>       mpc83xx: Add MPC8360EMDS basic board support
>       mpc83xx: add QE ethernet support
>       mpc83xx: add the README.mpc8360emds
>       mpc83xx: Fix the incorrect dcbz operation
> 
> Kim Phillips:
>       mpc83xx: change ft code to modify local-mac-address property
>       mpc83xx: add OF_FLAT_TREE bits to 83xx boards
>       mpc83xx: Lindent and clean up cpu/mpc83xx/speed.c
> 
> Nick Spence:
>       Added RGMII support to the TSECs and Marvell 881111 Phy
>       NAND Flash verify across block boundaries
> 
> Tanya Jiang:
>       mpc83xx: Removed unused file resetvec.S for mpc83xx cpu
>       mpc83xx: Fix missing build for mpc8349emds pci.c
>       mpc83xx: Unified TQM834x variable names with 83xx and 
> consolidated macros
> 
> Timur Tabi:
>       mpc83xx: Add support for variable flash memory sizes on 
> 83xx systems
>       mpc83xx: fix TQM build by defining a CFG_FLASH_SIZE for it
>       mpc83xx: Add support for Errata DDR6 on MPC 834x systems
>       mpc83xx: Add support for the MPC8349E-mITX
>       mpc83xx: Fix PCI, USB, bootargs for MPC8349E-mITX
>       mpc83xx: Fix dual I2C support for the MPC8349ITX, 
> MPC8349EMDS, TQM834x, and MPC8360EMDS
>       mpc83xx: Replace CFG_IMMRBAR with CFG_IMMR
>       mpc83xx: Update 83xx to use fsl_i2c.c

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

[Stefan Roese]

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@freescale.com>  2006-10-26 01:45:23
> Committer: Kim Phillips <kim.phillips@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

[U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)

[Stefan Roese]

Hi Ben,

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):
>
> Ben Warren:
>       Add support for multiple I2C buses
>       Multi-bus I2C implementation of MPC834x
>       Additional MPC8349 support for multibus i2c

I'm commenting on the I2C code you submitted, which is now included in the
git tree of Kim Phillips. Sorry for the late review, but I have some more
or less cosmetic comments:

> ------------------------------- common/cmd_i2c.c
> ------------------------------- index c543bb5..62378af 100644
> @@ -101,8 +101,31 @@ static uchar	i2c_mm_last_chip;
>  static uint	i2c_mm_last_addr;
>  static uint	i2c_mm_last_alen;
>
> +/* If only one I2C bus is present, the list of devices to ignore when
> + * the probe command is issued is represented by a 1D array of addresses.
> + * When multiple buses are present, the list is an array of bus-address
> + * pairs.  The following macros take care of this */
> +
>  #if defined(CFG_I2C_NOPROBES)
> +#if defined(CONFIG_I2C_MULTI_BUS)
> +static struct
> +{
> +	uchar	bus;
> +	uchar	addr;
> +} i2c_no_probes[] = CFG_I2C_NOPROBES;
> +#define GET_BUS_NUM	i2c_get_bus_num()
> +#define COMPARE_BUS(b,i)	(i2c_no_probes[(i)].bus == (b))
> +#define COMPARE_ADDR(a,i)	(i2c_no_probes[(i)].addr == (a))
> +#define NO_PROBE_ADDR(i)	i2c_no_probes[(i)].addr
> +#else		/* single bus */
>  static uchar i2c_no_probes[] = CFG_I2C_NOPROBES;
> +#define GET_BUS_NUM	0
> +#define COMPARE_BUS(b,i)	((b) == 0)	/* Make compiler happy */
> +#define COMPARE_ADDR(a,i)	(i2c_no_probes[(i)] == (a))
> +#define NO_PROBE_ADDR(i)	i2c_no_probes[(i)]
> +#endif	/* CONFIG_MULTI_BUS */
> +
> +#define NUM_ELEMENTS_NOPROBE
> (sizeof(i2c_no_probes)/sizeof(i2c_no_probes[0])) #endif
>
>  static int
> @@ -538,14 +561,17 @@ int do_i2c_probe (cmd_tbl_t *cmdtp, int
>  	int j;
>  #if defined(CFG_I2C_NOPROBES)
>  	int k, skip;
> -#endif
> +	uchar bus = GET_BUS_NUM;
> +#endif	/* NOPROBES */
>
>  	puts ("Valid chip addresses:");
>  	for(j = 0; j < 128; j++) {
>  #if defined(CFG_I2C_NOPROBES)
>  		skip = 0;
> -		for (k = 0; k < sizeof(i2c_no_probes); k++){
> -			if (j == i2c_no_probes[k]){
> +		for(k=0; k < NUM_ELEMENTS_NOPROBE; k++)
> +		{

Please move the "{" brace into the "for" loop line. And please also
insert a space between "for" and the "(" parenthesis. So the line above
should look like this:

		for (k=0; k < NUM_ELEMENTS_NOPROBE; k++) {

It seems that you use this "brace style" throughout the complete patch.
This is not according to the U-Boot coding styles (and Linux by the way).
So could you please rework your patch and merge it with Kim again? Or
perhaps Kim can rework the patch according to the U-Boot/Linux coding
style (Lindent?).

Thanks.

> +			if(COMPARE_BUS(bus, k) && COMPARE_ADDR(j, k))
> +			{
>  				skip = 1;
>  				break;
>  			}
> @@ -561,8 +587,11 @@ int do_i2c_probe (cmd_tbl_t *cmdtp, int
>
>  #if defined(CFG_I2C_NOPROBES)
>  	puts ("Excluded chip addresses:");
> -	for( k = 0; k < sizeof(i2c_no_probes); k++ )
> -		printf(" %02X", i2c_no_probes[k] );
> +	for(k=0; k < NUM_ELEMENTS_NOPROBE; k++)
> +	{
> +		if(COMPARE_BUS(bus,k))

Again, please insert a space after the "if".

> +			printf(" %02X", NO_PROBE_ADDR(k));
> +	}
>  	putc ('\n');
>  #endif
>
> @@ -873,6 +902,104 @@ int do_sdram  ( cmd_tbl_t *cmdtp, int fl
>  }
>  #endif	/* CFG_CMD_SDRAM */
>
> +#if defined(CONFIG_I2C_CMD_TREE)
> +#if defined(CONFIG_I2C_MULTI_BUS)
> +int do_i2c_bus_num(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
> +{
> +	int bus_idx, ret=0;
> +
> +	if (argc == 1)  /* querying current setting */
> +	{
> +		printf("Current bus is %d\n", i2c_get_bus_num());
> +	}
> +	else
> +	{

That should be

	} else {

> +		bus_idx = simple_strtoul(argv[1], NULL, 10);
> +		printf("Setting bus to %d\n", bus_idx);
> +		ret = i2c_set_bus_num(bus_idx);
> +		if(ret)
> +		{
> +			printf("Failure changing bus number (%d)\n", ret);
> +		}

Single line statements don't have braces.

And so on...

Please "talk" with Kim on how to clean up this patch.

Thanks.

Best regards,
Stefan

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

[Wolfgang Denk]

In message <200611261446.24607.sr@denx.de> you wrote:
> 
> >     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 object against such changes which have not been discussed with
the maintainer of the board, and most probably have not been tested
either.

TQ has a pretty long list of configuration options for their modules,
and the change above will probably break quite a few, or most.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Life is a garment we continuously alter, but which  never  seems  to
fit."                                                  - David McCord

[U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)

[Ben Warren]

Hi Stefan,

--- Stefan Roese <sr@denx.de> wrote:

> Hi Ben,
> 
> 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):
> >
> > Ben Warren:
> >       Add support for multiple I2C buses
> >       Multi-bus I2C implementation of MPC834x
> >       Additional MPC8349 support for multibus i2c
> 
> I'm commenting on the I2C code you submitted, which
> is now included in the
> git tree of Kim Phillips. Sorry for the late review,
> but I have some more
> or less cosmetic comments:

Sorry about the formatting.  I've been influenced by
many coding 'standards' over the years and haven't
quite adapted to the kernel way yet...  Old habits die
hard. 

To keep things moving, I'll fix this up and forward to
Kim on Monday morning.  I'm sure he has better things
to do than fix my sloppy styling.

Speaking of keeping things moving - I submitted a SPI
driver for the MPC834x several months back.  I'm sure
it has the same formatting problems.  Will it be
faster for me to resubmit?

regards,
Ben

[U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)

[Stefan Roese]

Hi Ben,

On Monday 27 November 2006 03:45, Ben Warren wrote:
> Sorry about the formatting.  I've been influenced by
> many coding 'standards' over the years and haven't
> quite adapted to the kernel way yet...  Old habits die
> hard.

;-)

> To keep things moving, I'll fix this up and forward to
> Kim on Monday morning.  I'm sure he has better things
> to do than fix my sloppy styling.

Good idea. If I remember correctly, Kim is not available until mid of this 
week.

> Speaking of keeping things moving - I submitted a SPI
> driver for the MPC834x several months back.  I'm sure
> it has the same formatting problems.  Will it be
> faster for me to resubmit?

Yes. You would save us at least one round of code review & resubmit. It's also 
easier to have a patch based on the current git tree.

Best regards,
Stefan

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

[Timur Tabi]

Wolfgang Denk wrote:
> In message <200611261446.24607.sr@denx.de> you wrote:
>>>     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 object against such changes which have not been discussed with
> the maintainer of the board, and most probably have not been tested
> either.
> 
> TQ has a pretty long list of configuration options for their modules,
> and the change above will probably break quite a few, or most.

The above change is necessary to make the board compatible with the flash-size 
code in mpc83xx/start.S:

	/* Store 0x80000012 + log2(CFG_FLASH_SIZE) into LBLAWAR1 */
	lis r4, (0x80000012)@h
	ori r4, r4, (0x80000012)@l
	li r5, CFG_FLASH_SIZE
1:	srawi. r5, r5, 1	/* r5 = r5 >> 1 */
	addi r4, r4, 1
	bne 1b

Previously, this code just assumed that flash was 8MB.  However, the 8349ITX has 
16MB of flash.  Rather than have #ifdefs in start.S, this code was changed to 
calculate the correct value for LBLAWAR1 based on CFG_FLASH_SIZE.

I believe that this code is the most elegant solution to the problem, however it 
does require that each board header file define a value for CFG_FLASH_SIZE.  The 
alternative would be to add these lines above the new code:

#ifndef CFG_FLASH_SIZE
#define CFG_FLASH_SIZE 8
#endif

If you prefer that we use this approach, then we can make that change.  However, 
I think our approach makes more sense.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Timur Tabi]

Stefan Roese wrote:
> 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).

We're happy to make it available.  Once you pull this code into your tree, 
there's a lot more coming.

Kim Phillips is out today, so I'm going to address your concerns in his stead. 
I can make the changes, but they won't be posted until after Kim reviews them.

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

Okay.

> - Please add the missing entries to the MAINTAINERS files (MPC8349EMDS &
>   MPC8360EMDS).

Okay.

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

Yes, we can do that.  I agree that the current code base is a little messy 
because of so many board-specific files.  We already have plans to address that, 
but again, we'll start work once the current code has been pulled.

> We could start with the mpc83xx boards and contine soon with the
> 85xx and 86xx eval boards (and so on...).

That's how I was planning on doing it.

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

I'll check.

> 
>> Author: Timur Tabi <timur@freescale.com>  2006-10-26 01:45:23
>> Committer: Kim Phillips <kim.phillips@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?

As I mentioned in my other post, this code is for the LBLAWAR1 code in start.S. 
  That code assumes that CFG_FLASH_SIZE is at least 1.  I'm not aware of any 
83xx board that has less than that much flash, but you're right - it is 
possible.  If you like, I can provide an enhanced version of this code to 
support flash sizes less than 1MB, but I would prefer to do that after you pull 
our current code.

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

I've never seen these, but I'll look into it.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)

[Timur Tabi]

Stefan Roese wrote:

> I'm commenting on the I2C code you submitted, which is now included in the
> git tree of Kim Phillips. Sorry for the late review, but I have some more
> or less cosmetic comments:

I will make the requested changes and put them into our 83xx tree.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Wolfgang Denk]

Dear Timur,

in message <456B1543.2040006@freescale.com> you wrote:
>
> > TQ has a pretty long list of configuration options for their modules,
> > and the change above will probably break quite a few, or most.
> 
> The above change is necessary to make the board compatible with the flash-size 
> code in mpc83xx/start.S:

Maybe we can find a better way to address this issue.

> Previously, this code just assumed that flash was 8MB.  However, the 8349ITX has 
> 16MB of flash.  Rather than have #ifdefs in start.S, this code was changed to 
> calculate the correct value for LBLAWAR1 based on CFG_FLASH_SIZE.

The problem is that I don't want to see any  such  hard-coded  sizes.
What  I want is documented in the (very old!) "System Initialization"
section in the README: we may have to configure for a  maximum  size,
but  except  from that, U-Boot should auto-adjust to different seizes
of RAM and flash memory.

> I believe that this code is the most elegant solution to the problem, however it 
> does require that each board header file define a value for CFG_FLASH_SIZE.  The 
> alternative would be to add these lines above the new code:

What exactly is the meaning of CFG_FLASH_SIZE in your code?

Is  this  a  _maximum_  size  which  is  supported  by   this   board
configuration, or the exact size that must be presen on the board? If
the  board  comes  with  a different flash size, what will the U-Boot
start messages report, and what size will  be  actually  mapped  once
U-Boot enters interactive mode?

> #ifndef CFG_FLASH_SIZE
> #define CFG_FLASH_SIZE 8
> #endif
> 
> If you prefer that we use this approach, then we can make that change.  However, 

No, this is even worse, I think.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Be kind to unkind people - they need it the most.

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

[Timur Tabi]

Wolfgang Denk wrote:

> The problem is that I don't want to see any  such  hard-coded  sizes.
> What  I want is documented in the (very old!) "System Initialization"
> section in the README: we may have to configure for a  maximum  size,
> but  except  from that, U-Boot should auto-adjust to different seizes
> of RAM and flash memory.

If I knew how to auto-adjust, I would have coded it that way.  To my knowledge, 
there is no generic way to query to amount of flash on the system when the code 
in start.S is being run.  The flash configuration code (e.g. flash_init()) is 
executed much later in the boot process.

> What exactly is the meaning of CFG_FLASH_SIZE in your code?

It is the size of flash, in megabytes.

> Is  this  a  _maximum_  size  which  is  supported  by   this   board
> configuration, or the exact size that must be presen on the board? 

The maximum size.  The value is used to program LBLAWAR1, so it determines the 
size of the memory window.  I'm not expert on these things, but I presume that 
the window determines what memory can be accessed.  That is, if you have 16MB of 
flash, but you program the window to be 8MB, then you will not be able to access 
the upper half of flash.

This issue becomes interesting when it's the other way around - 8MB of flash, 
and the window is 16MB.  In most cases, I believe this is mostly harmless.

So one suggested to me a while back that I should just program LBLAWAR1 to 16MB 
for everyone.  But then what do I do if a new system comes out with 32MB of 
flash?  Or more?  LBLAWAR1 supports up to 2GB of flash.

So I decided to make it board configurable.  If your board has multiple 
variations, such as some boards with 8MB and some with 16MB, and it's okay to 
program LBLAWAR1 to 16MB for all these boards, then by all means, set 
CFG_FLASH_SIZE to 16 and you're finished.

 > If
> the  board  comes  with  a different flash size, what will the U-Boot
> start messages report, and what size will  be  actually  mapped  once
> U-Boot enters interactive mode?

U-Boot will report whatever flash_init() says, which is typically calculated by 
querying the hardware.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Wolfgang Denk]

In message <456B57F9.9070405@freescale.com> you wrote:
>
> > The problem is that I don't want to see any  such  hard-coded  sizes.
> > What  I want is documented in the (very old!) "System Initialization"
> > section in the README: we may have to configure for a  maximum  size,
> > but  except  from that, U-Boot should auto-adjust to different seizes
> > of RAM and flash memory.
> 
> If I knew how to auto-adjust, I would have coded it that way.  To my knowledge, 
> there is no generic way to query to amount of flash on the system when the code 
> in start.S is being run.  The flash configuration code (e.g. flash_init()) is 
> executed much later in the boot process.

Right. that's why  we  start  with  preliminary  mappings  which  get
adjusted  later,  when the memory sizes are known. Please see section
"System Initialization" in the README.

> So one suggested to me a while back that I should just program LBLAWAR1 to 16MB 
> for everyone.  But then what do I do if a new system comes out with 32MB of 
> flash?  Or more?  LBLAWAR1 supports up to 2GB of flash.

I think it is a good idea to have the maximum size configurable.

>  > If
> > the  board  comes  with  a different flash size, what will the U-Boot
> > start messages report, and what size will  be  actually  mapped  once
> > U-Boot enters interactive mode?
> 
> U-Boot will report whatever flash_init() says, which is typically calculated by 
> querying the hardware.

Will the mapping be adjusted as well?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The first 90% of a project takes 90% of the time, the last 10%  takes
the other 90% of the time.

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

[Timur Tabi]

Wolfgang Denk wrote:

>> So one suggested to me a while back that I should just program LBLAWAR1 to 16MB 
>> for everyone.  But then what do I do if a new system comes out with 32MB of 
>> flash?  Or more?  LBLAWAR1 supports up to 2GB of flash.
> 
> I think it is a good idea to have the maximum size configurable.

That's why I wrote that code - to make it board configurable.

>> U-Boot will report whatever flash_init() says, which is typically calculated by 
>> querying the hardware.
> 
> Will the mapping be adjusted as well?

Not on any 83xx board.

Maybe it does in misc_init_r() of tqm85xx.c, but I'm not sure.  It programs 
OR0/BR0 and maybe OR1/BR1, but not LBLAWAR1, but I don't know what affect that 
has.  But since that's an 85xx board, this code doesn't apply.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Wolfgang Denk]

In message <456B5D21.2010601@freescale.com> you wrote:
> 
> >> U-Boot will report whatever flash_init() says, which is typically calculated by 
> >> querying the hardware.
> > 
> > Will the mapping be adjusted as well?
> 
> Not on any 83xx board.

Can you please add this?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We learn from history that we learn nothing from history."
- George Bernard Shaw

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

[Timur Tabi]

Wolfgang Denk wrote:
> In message <456B5D21.2010601@freescale.com> you wrote:
>>>> U-Boot will report whatever flash_init() says, which is typically calculated by 
>>>> querying the hardware.
>>> Will the mapping be adjusted as well?
>> Not on any 83xx board.
> 
> Can you please add this?

It would take me some time to figure out, and technically it's a new feature for 
83xx.  I already have a big to-do list of changes that are waiting for your 
pull.  Would you be willing to let me add your request to this to-do list, 
rather than holding up the pull of our current code?



-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[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

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

[Wolfgang Denk]

In message <456B5EB5.9030408@freescale.com> you wrote:
>
> It would take me some time to figure out, and technically it's a new feature for 
> 83xx.  I already have a big to-do list of changes that are waiting for your 
> pull.  Would you be willing to let me add your request to this to-do list, 
> rather than holding up the pull of our current code?

Yes, of course. Sorry, I did not intend to delay things. I'm happy if
it gets added at all :-)

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A modem is a baudy house.

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

[Wolfgang Denk]

In message <456B6A78.1080705@freescale.com> you wrote:
> 
> > 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.

Feel free to use ELDK 4.0 (or any other GCC-4.x based tool chain) to
catch such things. These new GCC versions are terribly finnicky ;-)

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Felson's Law:
	To steal ideas from one person is plagiarism; to steal from
	many is research.

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

[Dan Malek]

On Nov 27, 2006, at 4:48 PM, Timur Tabi wrote:

> Not on any 83xx board.

Why not?  It shouldn't be any different from other
PowerPC cores that do it.  I've done it on other
83xx boards, and in fact is quite easy if you
use the low boot vector.

All of my 83xx boards had to go back to clients,
but I'll try to find another one to test out a patch
and send it.

> Maybe it does in misc_init_r() of tqm85xx.c, but I'm not sure.  It  
> programs
> OR0/BR0 and maybe OR1/BR1, but not LBLAWAR1, but I don't know what  
> affect that
> has.

Considering your employer, you should know :-)

>   But since that's an 85xx board, this code doesn't apply.

Oh come on.  :-) The beauty of these processor variants
is their incredible similarity.  The further beauty of the
83xx is we get the nice bus controller with the
traditional PowerPC core.

Take the time to understand both the processor
technology and the features of U-Boot.  Using
the CFI driver with compliant flash (which is
nearly everything these days), the only flash
configuration variable should be the maximum
size of the local bus window you want to allocate
to flash.

Thanks.

	-- Dan

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

[Timur Tabi]

Dan Malek wrote:
> 
> On Nov 27, 2006, at 4:48 PM, Timur Tabi wrote:
> 
>> Not on any 83xx board.
> 
> Why not? 

What I mean is - the code doesn't currently do it.  I wasn't implying 
that it should or shouldn't, just that it doesn't.

Wolfgang asked me to add this support for 83xx, and I will.  Just not today.

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

[Kim Phillips]

On Sun, 26 Nov 2006 14:46:06 +0100 (MET)
Stefan Roese <sr@denx.de> wrote:

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

thanks for your thorough review.

> 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.
> 
all change log commentary has been removed.

> - 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.
> 
Timur addressed these issues.

> - Please add the missing entries to the MAINTAINERS files (MPC8349EMDS &
>   MPC8360EMDS).
>
done.
 
> 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...).
> 
understood.

> Here some further remarks to your patches:
> 
these issues have been addressed by Timur.

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

> 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
> 
Timur fixed this.

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

> 
> Thanks a lot for your patience. ;-)
> 

please pull from 'master' branch of:

  http://opensource.freescale.com/pub/scm/u-boot-83xx.git

to receive the following updates, in addition to those from the last request:

Joakim Tjernlund:
      Fix I2C master address initialization.
      Make fsl-i2c not conflict with SOFT I2C

Kim Phillips:
      Eliminate gcc 4 'used uninitialized' warnings in drivers/qe/uccf.c
      Assign maintainers for mpc8349emds and mpc8360emds

Timur Tabi:
      mpc83xx: Miscellaneous code style fixes

Thanks,

Kim

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

[Wolfgang Denk]

Dear Kim,

in message <20061129011808.38a2928e.kim.phillips@freescale.com> you wrote:
...
> please pull from 'master' branch of:
> 
>   http://opensource.freescale.com/pub/scm/u-boot-83xx.git
> 
> to receive the following updates, in addition to those from the last request:
> 
> Joakim Tjernlund:
>       Fix I2C master address initialization.
>       Make fsl-i2c not conflict with SOFT I2C
> 
> Kim Phillips:
>       Eliminate gcc 4 'used uninitialized' warnings in drivers/qe/uccf.c
>       Assign maintainers for mpc8349emds and mpc8360emds
> 
> Timur Tabi:
>       mpc83xx: Miscellaneous code style fixes

OK. There were still a few coding style issues, but I cleaned most of
them up.

We can close this thread now :-)

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"How is this place run - is it an anarchy?"
"No, I wouldn't say so; it is not that well organised..."

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

[Kim Phillips]

On Thu, 30 Nov 2006 18:07:14 +0100
Wolfgang Denk <wd@denx.de> wrote:

> Dear Kim,
> 
<snip>

> OK. There were still a few coding style issues, but I cleaned most of
> them up.
> 
I appreciate that.

> We can close this thread now :-)
> 
and that.

Dankesch?n!

Kim

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

[Timur Tabi]

Wolfgang Denk wrote:

> OK. There were still a few coding style issues, but I cleaned most of
> them up.
> 
> We can close this thread now :-)

Yea!  Thanks a bunch for pulling our tree.  Some of that code represents months 
of work on my part.

As I mentioned in a previous email, I have a bunch of other changes for U-Boot 
in my queue.  Some are major, most are minor.  Do you want to receive these 
patches via our 83xx tree also?  We'd probably create a separate branch for you 
to use.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] ERROR: Cannot determine a common read delay

[k b]

Hi,
I'm using a board similar to ecotea
here's the initial prompt.
So initailly it had 256 mb ddr sdram. now i want to use 512 ddr sdram.
i put that part in and now i get this error.

the error is generated in program_tr1() in cpu/ppc4xxx/spd_sdram.c
here are some questions
1) what exactly is this program tr doing. i guess cp
2) Can someone gimme any pointers as to what's that problem here.
3) why isn't program_tr0 used at all ?

Any inputs are appreciated,

U-boot-1.1.5

AMCC PowerPC 440 UNKNOWN (PVR=51b21894)
Board: Dolby DSP100 AMCC 440GX
        VCO: 800 MHz
        CPU: 800 MHz
        PLB: 100 MHz
        OPB: 50 MHz
        EPB: 50 MHz
I2C:   ready
CPLD: Interface check...OK
DRAM:  Starting memory test ....
ERROR: Cannot determine a common read delay.
### ERROR ### Please RESET the board ###

_________________________________________________________________
Get free, personalized commercial-free online radio with MSN Radio powered 
by Pandora http://radio.msn.com/?icid=T002MSN03A07001

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

[Wolfgang Denk]

Dear Timur,

in message <456F1F57.5090407@freescale.com> you wrote:
> 
> Yea!  Thanks a bunch for pulling our tree.  Some of that code represents months 
> of work on my part.

Thanks for all your efforts.

> As I mentioned in a previous email, I have a bunch of other changes for U-Boot 
> in my queue.  Some are major, most are minor.  Do you want to receive these 
> patches via our 83xx tree also?  We'd probably create a separate branch for you 
> to use.

If possible, I would like to stick (for now - changes might be in the
pipe) with the model that patches get posted here on the  list  as  a
means  of making them public. For pulling them, I would appreciate if
youc an make them available in a git repo - for me it does not matter
if it's the trunk or any other branch.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I'm what passes for a Unix guru in my office. This is  a  frightening
concept. - Lee Ann Goldstein, in <3k55ba$c43@butch.lmsc.lockheed.com>

[U-Boot-Users] ERROR: Cannot determine a common read delay

[Stefan Roese]

On Thursday 30 November 2006 22:14, k b wrote:
> I'm using a board similar to ecotea
> here's the initial prompt.
> So initailly it had 256 mb ddr sdram. now i want to use 512 ddr sdram.
> i put that part in and now i get this error.

So to sum up, 256MB i working (U-Boot get's to the prompt) and 512MB is not 
working. Right?

> the error is generated in program_tr1() in cpu/ppc4xxx/spd_sdram.c
> here are some questions
> 1) what exactly is this program tr doing. i guess cp

No. It's doing a simple memory test do determine a common TR1 value for all 
installed memory bank's. Please see the 440GX users manual for more details.

> 2) Can someone gimme any pointers as to what's that problem here.

Not easy to say. Could be a hardware problem. Could be still a bug in the 440 
SPD-DDR setup routine. Did you try a module from another vendor?

> 3) why isn't program_tr0 used at all ?

It seems to be used for me. Please look again.

Best regards,
Stefan