[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 (I2C rework)

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Stefan Roese
> Sent: 27 November 2006 07:22
> To: u-boot-users at lists.sourceforge.net
> Cc: Kim Phillips; Ben Warren; Wolfgang Denk
> Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
> 
> 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

I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts
with
soft I2C.

 Jocke

[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 (I2C rework)

[Timur Tabi]

Joakim Tjernlund wrote:

> I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts
> with
> soft I2C.

Can you be more specific?  These two macros are defined in a variety of ways in 
U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Ben Warren]

On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
> Joakim Tjernlund wrote:
> 
> > I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts
> > with
> > soft I2C.
> 
> Can you be more specific?  These two macros are defined in a variety of ways in 
> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> 
While it's not used on any Freescale evaluation boards, it could
certainly be implemented on boards with Freescale CPUs.  I'm not sure
why you'd bit-bang I2C if you have nice hardware controllers, but there
may be situations where this makes sense.  On the other hand, I don't
know if SOFT_I2C and HARD_I2C can co-exist.

Either way, I don't think we should preclude the use of SOFT I2C on
Freescale CPUs.

regards,
Ben

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

[Jerry Van Baren]

Ben Warren wrote:
> On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
>> Joakim Tjernlund wrote:
>>
>>> I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h conflicts
>>> with
>>> soft I2C.
>> Can you be more specific?  These two macros are defined in a variety of ways in 
>> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
>>
> While it's not used on any Freescale evaluation boards, it could
> certainly be implemented on boards with Freescale CPUs.  I'm not sure
> why you'd bit-bang I2C if you have nice hardware controllers, but there
> may be situations where this makes sense.  On the other hand, I don't
> know if SOFT_I2C and HARD_I2C can co-exist.
> 
> Either way, I don't think we should preclude the use of SOFT I2C on
> Freescale CPUs.
> 
> regards,
> Ben

It is used on some boards with Mot/Freescale CPUs, the WindRiver (EST) 
SBC8260 for example (although configurable hard vs. soft IIRC).  The 
hardware i2c can be more effort than it is worth compared to bit-banging 
it.  Perhaps the new QUICCs are easier to use, perhaps y'all are just 
smarter than we were back then.  :-)

gvb

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

[Pantelis Antoniou]

On 28 ??? 2006, at 8:17 ??, Ben Warren wrote:

> On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
>> Joakim Tjernlund wrote:
>>
>>> I think that the I2C_READ and I2C_WRITE #defines in fsl_i2c.h  
>>> conflicts
>>> with
>>> soft I2C.
>>
>> Can you be more specific?  These two macros are defined in a  
>> variety of ways in
>> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
>>
> While it's not used on any Freescale evaluation boards, it could
> certainly be implemented on boards with Freescale CPUs.  I'm not sure
> why you'd bit-bang I2C if you have nice hardware controllers, but  
> there
> may be situations where this makes sense.  On the other hand, I don't
> know if SOFT_I2C and HARD_I2C can co-exist.
>
> Either way, I don't think we should preclude the use of SOFT I2C on
> Freescale CPUs.
>
> regards,
> Ben
>

In the same note, definitely, definitely there are designs with multiple
I2C buses. Just because something does not exist on an evaluation board,
it doesn't mean it's not used in the real world.

So please make sure that both HW I2C & SW I2C can coexist.

-- Pantelis

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

[Joakim Tjernlund]

> -----Original Message-----
> From: Ben Warren [mailto:bwarren at qstreams.com] 
> Sent: den 28 november 2006 19:18
> To: Timur Tabi
> Cc: Joakim Tjernlund; Stefan Roese; 
> u-boot-users at lists.sourceforge.net; Kim Phillips; Wolfgang Denk
> Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
> 
> On Tue, 2006-11-28 at 12:04 -0600, Timur Tabi wrote:
> > Joakim Tjernlund wrote:
> > 
> > > I think that the I2C_READ and I2C_WRITE #defines in 
> fsl_i2c.h conflicts
> > > with
> > > soft I2C.
> > 
> > Can you be more specific?  These two macros are defined in 
> a variety of ways in 
> > U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> > 
> While it's not used on any Freescale evaluation boards, it could
> certainly be implemented on boards with Freescale CPUs.  I'm not sure
> why you'd bit-bang I2C if you have nice hardware controllers, 
> but there
> may be situations where this makes sense.  On the other hand, I don't
> know if SOFT_I2C and HARD_I2C can co-exist.

I use it in u-boot to send an I2C reset sequence* 
which the hardware controller can't do(or so Freescale tells me).
Futhermore, one can not use the I2C pins as GPIO on
these parts, so I had to connect two pins from PORT D** to
the I2C bus to do the bitbanging. If someone knows if this
can be done differently, I am all ears.

so I removed I2C_READ/I2C_WRITE from asm-ppc/i2c.h
and renamed I2C_READ to I2C_READ_BIT(same for write)
locally in cpu/mpc83xx/i2c.c(now renamed to fsl-i2c.c)

* The reset sequence in soft i2c isn't complete. One has
  to send a start in each clock cycle as well.

** Something isn't working properly with PORTD in ODR mode.
   I have change direction from output to input for SDA in
   I2C_TRISTATE. It should be enough to set SDA high since
   its open drain.

 Jocke
> 
> Either way, I don't think we should preclude the use of SOFT I2C on
> Freescale CPUs.

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

[Timur Tabi]

Pantelis Antoniou wrote:

> In the same note, definitely, definitely there are designs with multiple
> I2C buses. Just because something does not exist on an evaluation board,
> it doesn't mean it's not used in the real world.

The new fsl_i2c.c supports multiple HW I2C buses.  Well, it supports only 2 
buses, but I have plans to fix that.

> So please make sure that both HW I2C & SW I2C can coexist.

I'm going to need some help with that.  All I'm hearing so far are comments that 
the two should work together.  I don't see how any of the I2C changes I made 
affect soft I2C.  On top of that, I've never used soft I2C, so I don't even know 
how it's supposed to work.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Wolfgang Denk]

In message <1164737878.31193.38.camel@saruman.qstreams.net> you wrote:
>
> > Can you be more specific?  These two macros are defined in a variety of ways in 
> > U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> > 
> While it's not used on any Freescale evaluation boards, it could
> certainly be implemented on boards with Freescale CPUs.  I'm not sure
> why you'd bit-bang I2C if you have nice hardware controllers, but there

...because  the  bitbanging  code  is  much  smaller  and  easier  to
implement and debug than the code that uses the HW controller?

> may be situations where this makes sense.  On the other hand, I don't
> know if SOFT_I2C and HARD_I2C can co-exist.

No, theu=y are exclusive. But it should be possible to select any  of
these interfaces.

> Either way, I don't think we should preclude the use of SOFT I2C on
> Freescale CPUs.

Agreed.

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
"Wagner's music is better than it sounds."               - Mark Twain

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

[Wolfgang Denk]

In message <456C7A18.1090309@freescale.com> you wrote:
> 
> Can you be more specific?  These two macros are defined in a variety of ways in 
> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).

But it should be possible to use it. Please don't use incompatible
definitions.

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
When a man sits with a pretty girl for  an  hour,  it  seems  like  a
minute.  But let him sit on a hot stove for a minute -- and it's lon-
ger than any hour. That's relativity.              -- Albert Einstein

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

[Timur Tabi]

Wolfgang Denk wrote:
> In message <456C7A18.1090309@freescale.com> you wrote:
>> Can you be more specific?  These two macros are defined in a variety of ways in 
>> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> 
> But it should be possible to use it. Please don't use incompatible
> definitions.

So you're saying that in MPC8349ITX.h, I should be able to delete this line:

#define CONFIG_HARC_I2C

and replace it with this line:

#define CONFIG_SOFT_I2C

and everything should still work????

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Tolunay Orkun]

Wolfgang Denk wrote:
> In message <1164737878.31193.38.camel@saruman.qstreams.net> you wrote:
>>> Can you be more specific?  These two macros are defined in a variety of ways in 
>>> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
>>>
>> While it's not used on any Freescale evaluation boards, it could
>> certainly be implemented on boards with Freescale CPUs.  I'm not sure
>> why you'd bit-bang I2C if you have nice hardware controllers, but there
> 
> ...because  the  bitbanging  code  is  much  smaller  and  easier  to
> implement and debug than the code that uses the HW controller?
> 
>> may be situations where this makes sense.  On the other hand, I don't
>> know if SOFT_I2C and HARD_I2C can co-exist.
> 
> No, theu=y are exclusive. But it should be possible to select any  of
> these interfaces.

How do we handle the case that there is one hard I2C interface and 
another soft I2C interface (bus) via a pair of GPIO port pins? I have a 
PPC405EP based custom board that has such a case and I was looking 
forward to enabling multiple I2C bus support in U-Boot via both SOFT_I2C 
as well as HARD_I2C defined.

Tolunay

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

[Joakim Tjernlund]

On Tue, 2006-11-28 at 15:08 -0600, Timur Tabi wrote:
> Wolfgang Denk wrote:
> > In message <456C7A18.1090309@freescale.com> you wrote:
> >> Can you be more specific?  These two macros are defined in a variety of ways in 
> >> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> > 
> > But it should be possible to use it. Please don't use incompatible
> > definitions.
> 
> So you're saying that in MPC8349ITX.h, I should be able to delete this line:
> 
> #define CONFIG_HARC_I2C
> 
> and replace it with this line:
> 
> #define CONFIG_SOFT_I2C
> 
> and everything should still work????

No, see attached patch(s)

Not tested in your tree as I don't use that one (yet)

 Jocke
-------------- next part --------------

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

[Ben Warren]

On Tue, 2006-11-28 at 15:08 -0600, Timur Tabi wrote:
> Wolfgang Denk wrote:
> > In message <456C7A18.1090309@freescale.com> you wrote:
> >> Can you be more specific?  These two macros are defined in a variety of ways in 
> >> U-Boot.  Soft I2C is not used on any Freescale parts (AFAIK).
> > 
> > But it should be possible to use it. Please don't use incompatible
> > definitions.
> 
> So you're saying that in MPC8349ITX.h, I should be able to delete this line:
> 
> #define CONFIG_HARC_I2C
> 
> and replace it with this line:
> 
> #define CONFIG_SOFT_I2C
> 
> and everything should still work????
> 
No.  To implement Soft I2C (bit-banging) you need to provide information
about the I/O that you're using, among other things.

regards,
Ben

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

[Timur Tabi]

Joakim Tjernlund wrote:

> No, see attached patch(s)

Ah, I see.

> Not tested in your tree as I don't use that one (yet)

Git didn't like your patches, for some reason, so I had to apply them by hand, 
but everything seems to be okay.  I will apply them to our tree for Wolfgang's 
convenience.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Timur Tabi
> Sent: den 28 november 2006 23:16
> To: joakim.tjernlund at transmode.se
> Cc: u-boot-users at lists.sourceforge.net; Stefan Roese; Ben 
> Warren; Wolfgang Denk; Kim Phillips
> Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
> 
> Joakim Tjernlund wrote:
> 
> > No, see attached patch(s)
> 
> Ah, I see.

:)

> 
> > Not tested in your tree as I don't use that one (yet)
> 
> Git didn't like your patches, for some reason, so I had to 
> apply them by hand, 
> but everything seems to be okay.  I will apply them to our 
> tree for Wolfgang's 
> convenience.

eeh? I just did them aginst your tree with git-format-patch.
Are you using an old git perhaps? I think something changed
with git's diff format.

 Jocke

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

[Timur Tabi]

Joakim Tjernlund wrote:

> eeh? I just did them aginst your tree with git-format-patch.
> Are you using an old git perhaps? I think something changed
> with git's diff format.

I do have one patch in my sandbox that's not in the tree, but even so, I can't 
explain why git-apply and git-am complained.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

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

[Joakim Tjernlund]

On Tue, 2006-11-28 at 16:16 -0600, Timur Tabi wrote:
> Joakim Tjernlund wrote:
> 
> > No, see attached patch(s)
> 
> Ah, I see.
> 
> > Not tested in your tree as I don't use that one (yet)
> 
> Git didn't like your patches, for some reason, so I had to apply them by hand, 
> but everything seems to be okay.  I will apply them to our tree for Wolfgang's 
> convenience.
> 

While I am at it, I would also like to see this in u-boot
We use I2C as HRCW since we wan't to haw our flash reset connetced to
HRESET, otherwise you might be unable to boot if the flash is in non
read array mode when the board resets.
We also need to have the version info in the begining of the flash so
we can identify what version of u-boot we have installed. 
-------------- next part --------------

[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 (I2C rework)

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Tue, 2006-11-28 at 16:16 -0600, Timur Tabi wrote:
>> Joakim Tjernlund wrote:
>>
>>> No, see attached patch(s)
>> Ah, I see.
>>
>>> Not tested in your tree as I don't use that one (yet)
>> Git didn't like your patches, for some reason, so I had to apply them by hand, 
>> but everything seems to be okay.  I will apply them to our tree for Wolfgang's 
>> convenience.
>>
> 
> While I am at it, I would also like to see this in u-boot
> We use I2C as HRCW since we wan't to haw our flash reset connetced to
> HRESET, otherwise you might be unable to boot if the flash is in non
> read array mode when the board resets.
> We also need to have the version info in the begining of the flash so
> we can identify what version of u-boot we have installed. 
> 
> ------------------------------------------------------------------------
> 
> From 89b60f21af0d04959d93ccb70fd781c8aba9e66c Mon Sep 17 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Tue, 28 Nov 2006 23:42:31 +0100
> Subject: [PATCH] Make HRCW and version info in data segment configurable.
> 
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  cpu/mpc83xx/start.S |   28 +++++++++++++++++-----------
>  1 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/cpu/mpc83xx/start.S b/cpu/mpc83xx/start.S
> index 0f27bb6..44bca26 100644
> --- a/cpu/mpc83xx/start.S
> +++ b/cpu/mpc83xx/start.S
> @@ -77,20 +77,12 @@
>  	END_GOT
>  
>  /*
> - * Version string - must be in data segment because MPC83xx uses the
> - * first 256 bytes for the Hard Reset Configuration Word table (see
> - * below).  Similarly, can't have the U-Boot Magic Number as the first
> - * thing in the image - don't know how this will affect the image tools,
> - * but I guess I'll find out soon.
> + * MPC83xx can use the first 0x40 bytes for the Hard Reset Configuration Word 
> + * table (see below) if so configured.
>   */
> -	.data
> -	.globl	version_string
> -version_string:
> -	.ascii U_BOOT_VERSION
> -	.ascii " (", __DATE__, " - ", __TIME__, ")"
> -	.ascii " ", CONFIG_IDENT_STRING, "\0"
>  
>  	.text
> +#ifndef CFG_HRCW_IN_I2C_EEPROM
>  #define _HRCW_TABLE_ENTRY(w)		\
>  	.fill	8,1,(((w)>>24)&0xff);	\
>  	.fill	8,1,(((w)>>16)&0xff);	\
> @@ -99,7 +91,21 @@ version_string:
>  
>  	_HRCW_TABLE_ENTRY(CFG_HRCW_LOW)
>  	_HRCW_TABLE_ENTRY(CFG_HRCW_HIGH)
> +#endif
>  
> +/*
> + * Version string - May be in data segment if one wants to reserve the
> + * space left to address 0x100 for future expansion of HRCW bytes.
> + */
> +#ifdef CFG_VERSION_STRING_IN_DATA
> +	.data
> +#endif
> +        .long   0x27051956              /* U-Boot Magic Number */
> +	.globl	version_string
> +version_string:
> +	.ascii U_BOOT_VERSION
> +	.ascii " (", __DATE__, " - ", __TIME__, ")"
> +	.ascii " ", CONFIG_IDENT_STRING, "\0"
>  
>  #ifndef CONFIG_DEFAULT_IMMR
>  #error CONFIG_DEFAULT_IMMR must be defined

Hi Timur,

I don't believe you want to do this: you are removing the HRCW entirely 
from flash if you have it in I2C (CFG_HRCW_IN_I2C_EEPROM).  Unless I'm 
missing something (possible), I don't see why you even need a 
configuration option CFG_HRCW_IN_I2C_EEPROM.

* You can chose to ignore the flash-HRCW and use the I2C one simply by 
pulling a config pin up/down on power up (if I understand the manuals 
correctly), so you can store your HRCW in both places and chose which 
you use via a hardware configuration jumper.  This is a Very Good 
Thing[tm].  Suppressing the flash version simply because you chose not 
to use it is not something _I_ want to do on my 8360.

* If you put your HRCW in I2C _and_ in flash, you will be able to boot 
via the flash HRCW if your I2C version gets hosed.  If you don't, you 
will be hosed.

* Having a redundant HRCW in flash is essentially free: it costs you 64 
bytes of otherwise unused memory (well, other than the version string 
which you moved there, but there should be room for both).

My suggestion for putting the u-boot version string in a fixed place in 
flash was to put it _after_ the HRCW (e.g. at offset 0x40).  0x60 (96) 
bytes should be plenty for the version string.  Instead, you've unwisely 
(IMHO) _replaced_ the HRCW.  Browsing some of the other PowerPC CPU 
types, the ones that can appear to put the version string at the very 
start of flash.  This implies putting the version string in flash at 
offset 0x40 is OK (I was concerned about relocation fixup issues with 
pointers to flash vs. RAM after relocation).

On a related topic, the comment at the head of start.S says
/*
  * Version string - must be in data segment because MPC83xx uses the
  * first 256 bytes for the Hard Reset Configuration Word table (see
  * below).  Similarly, can't have the U-Boot Magic Number as the first
  * thing in the image - don't know how this will affect the image tools,
  * but I guess I'll find out soon.
  */
I see the comment was cut'n'pasted from the 8260 start.S.  Actually only 
the first 64 (0x40) bytes of memory are used for the HRCW, _not_ all 256 
bytes.

Looks like I (or somebody) needs to submit a clean up patch for that 
comment for the 8260 and 8360.  Sigh.  I just _had_ to look.  Curiosity 
will be the death of me yet.  ;-)

gvb

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

[Wolfgang Denk]

In message <456CA564.8060509@freescale.com> you wrote:
>
> > But it should be possible to use it. Please don't use incompatible
> > definitions.
> 
> So you're saying that in MPC8349ITX.h, I should be able to delete this line:
> 
> #define CONFIG_HARC_I2C
> 
> and replace it with this line:
> 
> #define CONFIG_SOFT_I2C
> 
> and everything should still work????

Yes - assuming you provide the required definitions for port pin
initialization and access in you rboard config file.

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 penny saved is a penny to squander.
- Ambrose Bierce

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

[Wolfgang Denk]

In message <456CA573.4020103@orkun.us> you wrote:
>
> How do we handle the case that there is one hard I2C interface and 
> another soft I2C interface (bus) via a pair of GPIO port pins? I have a 
> PPC405EP based custom board that has such a case and I was looking 
> forward to enabling multiple I2C bus support in U-Boot via both SOFT_I2C 
> as well as HARD_I2C defined.

Good question. I guess you're the first one who wants to do this,  so
you  will  have  to  sort  out  the  problems. I agree that this is a
reasonable request, it's  just  that  it  has  never  been  attempted
before.

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 came home the other night and tried to open the door  with  my  car
keys...and  the  building started up. So I took it out for a drive. A
cop pulled me over for speeding. He asked me where I  live...  "Right
here".                                                - Steven Wright

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

[Joakim Tjernlund]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Jerry Van Baren
> Sent: 29 November 2006 14:21
> To: u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
> 
> Joakim Tjernlund wrote:
> > On Tue, 2006-11-28 at 16:16 -0600, Timur Tabi wrote:
> >> Joakim Tjernlund wrote:
> >>
> >>> No, see attached patch(s)
> >> Ah, I see.
> >>
> >>> Not tested in your tree as I don't use that one (yet)
> >> Git didn't like your patches, for some reason, so I had to 
> apply them by hand, 
> >> but everything seems to be okay.  I will apply them to our 
> tree for Wolfgang's 
> >> convenience.
> >>
> > 
> > While I am at it, I would also like to see this in u-boot
> > We use I2C as HRCW since we wan't to haw our flash reset 
> connetced to
> > HRESET, otherwise you might be unable to boot if the flash is in non
> > read array mode when the board resets.
> > We also need to have the version info in the begining of 
> the flash so
> > we can identify what version of u-boot we have installed. 
> > 
> > 
> --------------------------------------------------------------
> ----------
> > 
> > From 89b60f21af0d04959d93ccb70fd781c8aba9e66c Mon Sep 17 
> 00:00:00 2001
> > From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > Date: Tue, 28 Nov 2006 23:42:31 +0100
> > Subject: [PATCH] Make HRCW and version info in data segment 
> configurable.
> > 
> > 
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  cpu/mpc83xx/start.S |   28 +++++++++++++++++-----------
> >  1 files changed, 17 insertions(+), 11 deletions(-)
> > 
> > diff --git a/cpu/mpc83xx/start.S b/cpu/mpc83xx/start.S
> > index 0f27bb6..44bca26 100644
> > --- a/cpu/mpc83xx/start.S
> > +++ b/cpu/mpc83xx/start.S
> > @@ -77,20 +77,12 @@
> >  	END_GOT
> >  
> >  /*
> > - * Version string - must be in data segment because 
> MPC83xx uses the
> > - * first 256 bytes for the Hard Reset Configuration Word table (see
> > - * below).  Similarly, can't have the U-Boot Magic Number 
> as the first
> > - * thing in the image - don't know how this will affect 
> the image tools,
> > - * but I guess I'll find out soon.
> > + * MPC83xx can use the first 0x40 bytes for the Hard Reset 
> Configuration Word 
> > + * table (see below) if so configured.
> >   */
> > -	.data
> > -	.globl	version_string
> > -version_string:
> > -	.ascii U_BOOT_VERSION
> > -	.ascii " (", __DATE__, " - ", __TIME__, ")"
> > -	.ascii " ", CONFIG_IDENT_STRING, "\0"
> >  
> >  	.text
> > +#ifndef CFG_HRCW_IN_I2C_EEPROM
> >  #define _HRCW_TABLE_ENTRY(w)		\
> >  	.fill	8,1,(((w)>>24)&0xff);	\
> >  	.fill	8,1,(((w)>>16)&0xff);	\
> > @@ -99,7 +91,21 @@ version_string:
> >  
> >  	_HRCW_TABLE_ENTRY(CFG_HRCW_LOW)
> >  	_HRCW_TABLE_ENTRY(CFG_HRCW_HIGH)
> > +#endif
> >  
> > +/*
> > + * Version string - May be in data segment if one wants to 
> reserve the
> > + * space left to address 0x100 for future expansion of HRCW bytes.
> > + */
> > +#ifdef CFG_VERSION_STRING_IN_DATA
> > +	.data
> > +#endif
> > +        .long   0x27051956              /* U-Boot Magic Number */
> > +	.globl	version_string
> > +version_string:
> > +	.ascii U_BOOT_VERSION
> > +	.ascii " (", __DATE__, " - ", __TIME__, ")"
> > +	.ascii " ", CONFIG_IDENT_STRING, "\0"
> >  
> >  #ifndef CONFIG_DEFAULT_IMMR
> >  #error CONFIG_DEFAULT_IMMR must be defined
> 
> Hi Timur,
> 
> I don't believe you want to do this: you are removing the 
> HRCW entirely 
> from flash if you have it in I2C (CFG_HRCW_IN_I2C_EEPROM).  
> Unless I'm 
> missing something (possible), I don't see why you even need a 
> configuration option CFG_HRCW_IN_I2C_EEPROM.

[SNIP good arugments]

The only reason to possibly want this is if you want to keep the format
where the u-boot header is the first thing in the image. I can manage
to have HRCW first in flash, but not having the version string in data
segment.

Summary:
  Remove the CFG_HRCW_IN_I2C_EEPROM if you want, but keep
  CFG_VERSION_STRING_IN_DATA.

 Jocke

[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