* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-04 2:11 [U-Boot-Users] Please pull u-boot-83xx.git Kim Phillips
@ 2006-11-16 23:55 ` Joakim Tjernlund
2006-11-26 13:46 ` Stefan Roese
2006-11-26 13:49 ` [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework) Stefan Roese
2 siblings, 0 replies; 30+ messages in thread
From: Joakim Tjernlund @ 2006-11-16 23:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-04 2:11 [U-Boot-Users] Please pull u-boot-83xx.git Kim Phillips
2006-11-16 23:55 ` Joakim Tjernlund
@ 2006-11-26 13:46 ` Stefan Roese
2006-11-26 20:02 ` Wolfgang Denk
` (3 more replies)
2006-11-26 13:49 ` [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework) Stefan Roese
2 siblings, 4 replies; 30+ messages in thread
From: Stefan Roese @ 2006-11-26 13:46 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-26 13:46 ` Stefan Roese
@ 2006-11-26 20:02 ` Wolfgang Denk
2006-11-27 16:41 ` Timur Tabi
2006-11-27 16:55 ` Timur Tabi
` (2 subsequent siblings)
3 siblings, 1 reply; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-26 20:02 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-26 20:02 ` Wolfgang Denk
@ 2006-11-27 16:41 ` Timur Tabi
2006-11-27 21:10 ` Wolfgang Denk
0 siblings, 1 reply; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 16:41 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 16:41 ` Timur Tabi
@ 2006-11-27 21:10 ` Wolfgang Denk
2006-11-27 21:26 ` Timur Tabi
0 siblings, 1 reply; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-27 21:10 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:10 ` Wolfgang Denk
@ 2006-11-27 21:26 ` Timur Tabi
2006-11-27 21:36 ` Wolfgang Denk
0 siblings, 1 reply; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 21:26 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:26 ` Timur Tabi
@ 2006-11-27 21:36 ` Wolfgang Denk
2006-11-27 21:48 ` Timur Tabi
0 siblings, 1 reply; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-27 21:36 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:36 ` Wolfgang Denk
@ 2006-11-27 21:48 ` Timur Tabi
2006-11-27 21:53 ` Wolfgang Denk
2006-11-28 0:25 ` Dan Malek
0 siblings, 2 replies; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 21:48 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:48 ` Timur Tabi
@ 2006-11-27 21:53 ` Wolfgang Denk
2006-11-27 21:55 ` Timur Tabi
2006-11-28 0:25 ` Dan Malek
1 sibling, 1 reply; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-27 21:53 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:53 ` Wolfgang Denk
@ 2006-11-27 21:55 ` Timur Tabi
2006-11-27 22:56 ` Wolfgang Denk
0 siblings, 1 reply; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 21:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:55 ` Timur Tabi
@ 2006-11-27 22:56 ` Wolfgang Denk
0 siblings, 0 replies; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-27 22:56 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 21:48 ` Timur Tabi
2006-11-27 21:53 ` Wolfgang Denk
@ 2006-11-28 0:25 ` Dan Malek
2006-11-28 15:18 ` Timur Tabi
1 sibling, 1 reply; 30+ messages in thread
From: Dan Malek @ 2006-11-28 0:25 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-26 13:46 ` Stefan Roese
2006-11-26 20:02 ` Wolfgang Denk
@ 2006-11-27 16:55 ` Timur Tabi
2006-11-27 22:45 ` Timur Tabi
2006-11-29 7:18 ` Kim Phillips
3 siblings, 0 replies; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 16:55 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-26 13:46 ` Stefan Roese
2006-11-26 20:02 ` Wolfgang Denk
2006-11-27 16:55 ` Timur Tabi
@ 2006-11-27 22:45 ` Timur Tabi
2006-11-27 22:59 ` Wolfgang Denk
2006-11-29 7:18 ` Kim Phillips
3 siblings, 1 reply; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 22:45 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-27 22:45 ` Timur Tabi
@ 2006-11-27 22:59 ` Wolfgang Denk
0 siblings, 0 replies; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-27 22:59 UTC (permalink / raw)
To: u-boot
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-26 13:46 ` Stefan Roese
` (2 preceding siblings ...)
2006-11-27 22:45 ` Timur Tabi
@ 2006-11-29 7:18 ` Kim Phillips
2006-11-30 17:07 ` Wolfgang Denk
3 siblings, 1 reply; 30+ messages in thread
From: Kim Phillips @ 2006-11-29 7:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-29 7:18 ` Kim Phillips
@ 2006-11-30 17:07 ` Wolfgang Denk
2006-11-30 17:49 ` Kim Phillips
2006-11-30 18:13 ` Timur Tabi
0 siblings, 2 replies; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-30 17:07 UTC (permalink / raw)
To: u-boot
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..."
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-30 17:07 ` Wolfgang Denk
@ 2006-11-30 17:49 ` Kim Phillips
2006-11-30 18:13 ` Timur Tabi
1 sibling, 0 replies; 30+ messages in thread
From: Kim Phillips @ 2006-11-30 17:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-30 17:07 ` Wolfgang Denk
2006-11-30 17:49 ` Kim Phillips
@ 2006-11-30 18:13 ` Timur Tabi
2006-11-30 21:14 ` [U-Boot-Users] ERROR: Cannot determine a common read delay k b
2006-11-30 23:09 ` [U-Boot-Users] Please pull u-boot-83xx.git Wolfgang Denk
1 sibling, 2 replies; 30+ messages in thread
From: Timur Tabi @ 2006-11-30 18:13 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] ERROR: Cannot determine a common read delay
2006-11-30 18:13 ` Timur Tabi
@ 2006-11-30 21:14 ` k b
2006-12-02 12:02 ` Stefan Roese
2006-11-30 23:09 ` [U-Boot-Users] Please pull u-boot-83xx.git Wolfgang Denk
1 sibling, 1 reply; 30+ messages in thread
From: k b @ 2006-11-30 21:14 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] ERROR: Cannot determine a common read delay
2006-11-30 21:14 ` [U-Boot-Users] ERROR: Cannot determine a common read delay k b
@ 2006-12-02 12:02 ` Stefan Roese
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Roese @ 2006-12-02 12:02 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git
2006-11-30 18:13 ` Timur Tabi
2006-11-30 21:14 ` [U-Boot-Users] ERROR: Cannot determine a common read delay k b
@ 2006-11-30 23:09 ` Wolfgang Denk
1 sibling, 0 replies; 30+ messages in thread
From: Wolfgang Denk @ 2006-11-30 23:09 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
2006-11-04 2:11 [U-Boot-Users] Please pull u-boot-83xx.git Kim Phillips
2006-11-16 23:55 ` Joakim Tjernlund
2006-11-26 13:46 ` Stefan Roese
@ 2006-11-26 13:49 ` Stefan Roese
2006-11-27 2:45 ` Ben Warren
2006-11-27 17:28 ` Timur Tabi
2 siblings, 2 replies; 30+ messages in thread
From: Stefan Roese @ 2006-11-26 13:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread* [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
2006-11-26 13:49 ` [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework) Stefan Roese
@ 2006-11-27 2:45 ` Ben Warren
2006-11-27 6:22 ` Stefan Roese
2006-11-27 17:28 ` Timur Tabi
1 sibling, 1 reply; 30+ messages in thread
From: Ben Warren @ 2006-11-27 2:45 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
2006-11-27 2:45 ` Ben Warren
@ 2006-11-27 6:22 ` Stefan Roese
0 siblings, 0 replies; 30+ messages in thread
From: Stefan Roese @ 2006-11-27 6:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework)
2006-11-26 13:49 ` [U-Boot-Users] Please pull u-boot-83xx.git (I2C rework) Stefan Roese
2006-11-27 2:45 ` Ben Warren
@ 2006-11-27 17:28 ` Timur Tabi
1 sibling, 0 replies; 30+ messages in thread
From: Timur Tabi @ 2006-11-27 17:28 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 30+ messages in thread