Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Kim Phillips
On Fri, 16 Apr 2010 20:06:02 +0300
Michael Zaidman michael.zaid...@gmail.com wrote:

 On Fri, Apr 16, 2010 at 1:36 AM, Kim Phillips
 kim.phill...@freescale.com wrote:
  before, MPC8349ITX boots u-boot in 4.3sec:
 [snip]
  after, MPC8349ITX boots u-boot in 3.0sec:
 
 Thanks for the good news! Memory POST test on my board with icach
 disabled was lasting for hours until I enabled icach locally before
 and disabled it after time consuming tests.
 
 BTW is there any reason that you enable icach in board specific rather
 than in common code?

not really, was just following the existing HID0-setting paradigm -
I presume the paradigm came about to allow different cpu/board
combinations to enable/disable different HID bits.  Does that not sound
valid to you?

Kim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Scott Wood
Kim Phillips wrote:
 On Fri, 16 Apr 2010 20:06:02 +0300
 Michael Zaidman michael.zaid...@gmail.com wrote:
 
 On Fri, Apr 16, 2010 at 1:36 AM, Kim Phillips
 kim.phill...@freescale.com wrote:
 before, MPC8349ITX boots u-boot in 4.3sec:
 [snip]
 after, MPC8349ITX boots u-boot in 3.0sec:
 Thanks for the good news! Memory POST test on my board with icach
 disabled was lasting for hours until I enabled icach locally before
 and disabled it after time consuming tests.

 BTW is there any reason that you enable icach in board specific rather
 than in common code?
 
 not really, was just following the existing HID0-setting paradigm -
 I presume the paradigm came about to allow different cpu/board
 combinations to enable/disable different HID bits.  Does that not sound
 valid to you?

Shouldn't you be using icache_enable(), or at least using HID0_INIT to 
do invalidation and lock clearing?

-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Timur Tabi
On Thu, Apr 15, 2010 at 5:36 PM, Kim Phillips
kim.phill...@freescale.com wrote:

  #define CONFIG_SYS_HID0_FINAL  (HID0_ENABLE_MACHINE_CHECK | \
 -                        HID0_ENABLE_DYNAMIC_POWER_MANAGMENT)
 +                                HID0_ENABLE_INSTRUCTION_CACHE | \
 +                                HID0_ENABLE_DYNAMIC_POWER_MANAGMENT)

What was so special about the 8349ITX that icache() didn't work?
You're changing the HDI0 values for all of the 83xx boards, but AFAIK,
only the ITX had this problem.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Scott Wood
Timur Tabi wrote:
 On Thu, Apr 15, 2010 at 5:36 PM, Kim Phillips
 kim.phill...@freescale.com wrote:
 
  #define CONFIG_SYS_HID0_FINAL  (HID0_ENABLE_MACHINE_CHECK | \
 -HID0_ENABLE_DYNAMIC_POWER_MANAGMENT)
 +HID0_ENABLE_INSTRUCTION_CACHE | \
 +HID0_ENABLE_DYNAMIC_POWER_MANAGMENT)
 
 What was so special about the 8349ITX that icache() didn't work?
 You're changing the HDI0 values for all of the 83xx boards, but AFAIK,
 only the ITX had this problem.

All 83xx boards had the performance problem of not enabling icache until 
after relocation.

-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Timur Tabi
On Mon, Apr 19, 2010 at 3:41 PM, Scott Wood scottw...@freescale.com wrote:

 What was so special about the 8349ITX that icache() didn't work?
 You're changing the HDI0 values for all of the 83xx boards, but AFAIK,
 only the ITX had this problem.

 All 83xx boards had the performance problem of not enabling icache until
 after relocation.

That's not what I was asking.  The ITX was different from other 83xx
boards.  It doesn't define CONFIG_MPC83xx (and with this patch, it
still won't define it), so a lot of things weren't happening,
including this:

#if defined(CONFIG_SYS_DELAYED_ICACHE) || defined(CONFIG_MPC83xx)
icache_enable ();   /* it's time to enable the instruction cache */
#endif

Kim references the ITX in this patch, but I don't see how the problem
I just described is fixed.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Kim Phillips
On Mon, 19 Apr 2010 16:05:11 -0500
Timur Tabi timur.t...@gmail.com wrote:

 On Mon, Apr 19, 2010 at 3:41 PM, Scott Wood scottw...@freescale.com wrote:
 
  What was so special about the 8349ITX that icache() didn't work?
  You're changing the HDI0 values for all of the 83xx boards, but AFAIK,
  only the ITX had this problem.
 
  All 83xx boards had the performance problem of not enabling icache until
  after relocation.
 
 That's not what I was asking.  The ITX was different from other 83xx
 boards.  It doesn't define CONFIG_MPC83xx (and with this patch, it

why doesn't it, btw?

 still won't define it), so a lot of things weren't happening,
 including this:
 #if defined(CONFIG_SYS_DELAYED_ICACHE) || defined(CONFIG_MPC83xx)
   icache_enable ();   /* it's time to enable the instruction cache */
 #endif
 
 Kim references the ITX in this patch, but I don't see how the problem
 I just described is fixed.

patch submission snafu; I'll send another version that includes the ITX.

Kim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Kim Phillips
On Mon, 19 Apr 2010 15:19:24 -0500
Scott Wood scottw...@freescale.com wrote:

 Kim Phillips wrote:
  On Fri, 16 Apr 2010 20:06:02 +0300
  Michael Zaidman michael.zaid...@gmail.com wrote:
  
  On Fri, Apr 16, 2010 at 1:36 AM, Kim Phillips
  kim.phill...@freescale.com wrote:
  before, MPC8349ITX boots u-boot in 4.3sec:
  [snip]
  after, MPC8349ITX boots u-boot in 3.0sec:
  Thanks for the good news! Memory POST test on my board with icach
  disabled was lasting for hours until I enabled icach locally before
  and disabled it after time consuming tests.
 
  BTW is there any reason that you enable icach in board specific rather
  than in common code?
  
  not really, was just following the existing HID0-setting paradigm -
  I presume the paradigm came about to allow different cpu/board
  combinations to enable/disable different HID bits.  Does that not sound
  valid to you?
 
 Shouldn't you be using icache_enable(), or at least using HID0_INIT to 
 do invalidation and lock clearing?

the invalidation should occur whether or not the cache enable bit is
set in HID0_INIT, and there is no locking being done prior to this
point in the code.  But I see your point; we should be using a more
formal approach.  I'll see what I can do - it's just that this patch
preserved the existing code size, which could be important for e.g.,
future nand bootstrap development.

Kim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Scott Wood
Kim Phillips wrote:
 On Mon, 19 Apr 2010 15:19:24 -0500
 Scott Wood scottw...@freescale.com wrote:
 Shouldn't you be using icache_enable(), or at least using HID0_INIT to 
 do invalidation and lock clearing?
 
 the invalidation should occur whether or not the cache enable bit is
 set in HID0_INIT,

OK, was thinking of some other caches that don't automatically clear out 
the random junk on power-on -- but e300 manual says it does.

 But I see your point; we should be using a more
 formal approach.  I'll see what I can do - it's just that this patch
 preserved the existing code size, which could be important for e.g.,
 future nand bootstrap development.

Right.  It looks like we could shrink the NAND SPL some more by 
#ifndefing the cache functions in start.S.

-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-19 Thread Timur Tabi
On Mon, Apr 19, 2010 at 4:37 PM, Kim Phillips
kim.phill...@freescale.com wrote:

 That's not what I was asking.  The ITX was different from other 83xx
 boards.  It doesn't define CONFIG_MPC83xx (and with this patch, it

 why doesn't it, btw?

It's been a while, but I believe the reason the ITX does not defined
CONFIG_MPC83xx is specifically so that this code:

#if defined(CONFIG_SYS_DELAYED_ICACHE) || defined(CONFIG_MPC83xx)
   icache_enable ();   /* it's time to enable the instruction cache */
#endif

is *not* executed.  That is, on the ITX, calling icache_enable() at
that point causes U-Boot to hang or crash.

Therefore, I don't think any 83xx cache-related patch is complete
until MPC8349ITX.h includes CONFIG_MPC83xx.

-- 
Timur Tabi
Linux kernel developer at Freescale
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] mpc83xx: turn on icache in core initialization to improve u-boot boot time

2010-04-16 Thread Michael Zaidman
On Fri, Apr 16, 2010 at 1:36 AM, Kim Phillips
kim.phill...@freescale.com wrote:
 before, MPC8349ITX boots u-boot in 4.3sec:
[snip]
 after, MPC8349ITX boots u-boot in 3.0sec:

Thanks for the good news! Memory POST test on my board with icach
disabled was lasting for hours until I enabled icach locally before
and disabled it after time consuming tests.

BTW is there any reason that you enable icach in board specific rather
than in common code?

-michael
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot