Re: [U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

2014-02-26 Thread Tang Yuantian-B29983


On 2014/2/25 星期二 3:11, Scott Wood wrote:

Why what? Why we need it?

It is a help function and used by ASM code in which
we can't determine whether it is a warm reset boot.

Why don't you just open code it?

I can't check the warmboot status in ASM code.
In order to get the warmboot status in ASM code, I wrote this function.

Why can't you check it in asm code?  See lib/asm-offsets.c.

Found it. Still learn how to use it.

Thanks,
Yuantian


+   if (gd-flags  GD_FLG_WARM_BOOT) {
+   src = (u64 *)in_be32(scfg-sparecr[2]);
+   dst = (u64 *)(0);
+   for (i = 0; i  128/sizeof(u64); i++) {
+   *dst = *src;
+   dst++;
+   src++;
+   }
+   }

(u64 *)(0) is a NULL pointer.  Dereferencing NULL pointers is undefined
and GCC has been getting increasingly free with making surprising
optimizations based on that (such as assuming any code path that it knows
can reach a NULL dereference is dead code and can be removed).


Then how we operate 0 address if not dereferencing NULL pointer?


With an I/O accessor (or other inline asm), a TLB mapping, or using a
different memory location.

we found the zero address has benefit.
I don't know how to achieve this in inline asm or TLB mapping, could you
be more specific or write a example for me?

Inline asm would be something like:

asm(stw %1, 0(%0); stw %2, 4(%0) : =r (dst) :
r ((u32)(src  32)), r ((u32)src));

-Scott




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


Re: [U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

2014-02-25 Thread Tang Yuantian-B29983


On 2014/2/25 星期二 3:11, Scott Wood wrote:

On Mon, 2014-02-24 at 14:44 +0800, Tang Yuantian-B29983 wrote:

On 2014/2/18 星期二 3:18, Scott Wood wrote:

On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:

-Original Message-
From: Wood Scott-B07421
To: Tang Yuantian-B29983
Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot@lists.denx.de; Kushwaha
Prabhakar-B32579; Jin Zhengxiong-R64188
Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support

On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:

Use an I/O accessor.

In_be64?? No such function.

Why do you need in_be64() rather than two in_be32()s?


Avoid ECC error. Although, according to my test, in_be32() works too.

Why would you get an ECC error?

-Scott

DDR operation is always in 64bits. if writing a 32bits to memory, you
need to
read a 32bits first, and combine it to form a 64bits. when the new
64bits is written
to memory, ECC occurs.
I was required to do so by hardware team.

U-Boot on PPC is a 32-bit binary (even on 64-bit hardware), so the
compiler is already turning that into two 32-bit writes.

-Scott

I quote: (from Welker James A.-RA8497 )
3) You only need 8-byte (or multiple of 8-byte) transactions when 
initializing the memory.  Typically, we would simply use 2, 64-byte 
writes to DRAM. This is because a sub-doubleword transactions will 
result in a read-modify-write, which would encounter an ECC error (and 
then mask the write data).  After the first 128-bytes of memory have 
been re-initialized, you can issue any transactions to those locations 
again.


I think the transactions between CPU and DDRC are always in 64 bits 
physically because DDRC has 64 bits data bus.

But I am sure about this.
If you are sure about this, I will change as your suggestion.

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


Re: [U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

2014-02-25 Thread Tang Yuantian-B29983


On 2014/2/25 星期二 3:11, Scott Wood wrote:

On Mon, 2014-02-24 at 15:47 +0800, Tang Yuantian-B29983 wrote:

On 2014/2/15 星期六 6:21, Scott Wood wrote:

On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:

Thanks for your review. Please see the reply inline.

Thanks,
Yuantian


-Original Message-
From: Wood Scott-B07421
Sent: 2014年2月13日 星期四 8:44
To: Tang Yuantian-B29983
Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
t...@theia.denx.de; u-boot@lists.denx.de; Kushwaha Prabhakar-B32579; Jin
Zhengxiong-R64188
Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support

On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:

From: Tang Yuantian yuantian.t...@freescale.com

When system wakes up from warm reset, control is passed to the primary
core that starts executing uboot. After re-initialized some IP blocks,
like DDRC, kernel will take responsibility to continue to restore
environment it leaves before.

Signed-off-by: Tang Yuantian yuantian.t...@freescale.com

Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
this isn't necessary for deep sleep on mpc8536, for example.


Currently, it is used by t1040. But we want it to be more general so that
It can be used by later new chips.

But the mechanism is not the same for all mpc85xx derivatives.  You'll
need a more specific name.

OK, will name it as t104x


+#ifdef CONFIG_DEEP_SLEEP

CONFIG symbols need to be documented in README...


Where should I add this README?

Under 85xx CPU Options in the top-level README.

Thanks.


+   /* disable the console if boot from warm reset */
+   if (in_be32(gur-scrtsr[0])  (1  3))
+   gd-flags |=
+   GD_FLG_SILENT | GD_FLG_WARM_BOOT |

GD_FLG_DISABLE_CONSOLE; #endif

...and it should probably be a more specific CONFIG_SYS symbol that
indicates the presence of this guts bit.

where should I put this CONFIG_SYS_'s definition?

Under 85xx CPU Options in the top-level README. :-)

I don't find other gut bit that defined in this README. Do I need to?
Just we are clear that you want me to add a CONFIG_SYS_'s definition for 
(1  3) bit in GUTS, right?



diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
extern void ft_srio_setup(void *blob);
+#ifdef CONFIG_DEEP_SLEEP
+extern ulong __bss_end;
+#endif

Don't ifdef declarations.


   #ifdef CONFIG_MP
   #include mp.h
@@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
u32 bootpg = determine_mp_bootpg(NULL);
u32 id = get_my_id();
const char *enable_method;
+#ifdef CONFIG_DEEP_SLEEP
+   ulong len;
+#endif

off = fdt_node_offset_by_prop_value(blob, -1, device_type, cpu,

4);

while (off != -FDT_ERR_NOTFOUND) {
@@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
device_type, cpu, 4);
}

+#ifdef CONFIG_DEEP_SLEEP
+   /*
+* reserve the memory space for warm reset.
+* This space will be re-used next time when boot from deep sleep.
+* The space includes bd_t, gd_t, stack and uboot image.
+* Reserve 1K for stack.
+*/
+   len = sizeof(bd_t) + sizeof(gd_t) + (1  10);
+   /* round up to 4K */
+   len = (len + (4096 - 1))  ~(4096 - 1);
+
+   off = fdt_add_mem_rsv(blob, gd-relocaddr - len,
+   len + ((ulong)__bss_end - gd-relocaddr));
+   if (off  0)
+   printf(Failed to reserve memory for warm reset: %s\n,
+   fdt_strerror(off));
+
+#endif

Why do you need to reserve memory for this?  We didn't need to for deep
sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
MPC8313ERDB we transfer control to the kernel before relocating, so U-
Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
L1 cache, and the u-boot image should be in flash (or locked CPC if this
is not a NOR flash boot).


In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
are re-initialized after relocating.
So, we must jump to kernel after relocating.

The DDR controller is initialized before relocating -- and of course the
CPU is powered off on MPC8313ERDB deep sleep as well.

LIODNs are a new concern for deep sleep, but that doesn't mean we should
go through a bunch of unrelated code to get there.  Refactor the LIODN
code to be usable before relocation, and not be tied to fdt fixups.

There are other IP blocks that need to be re-initialized, like SerDes,
SMP, PCIe and
a lot of Errata. I don't want to refactor one by one.
Besides, coding in this way will not change the current execute flow.

Changing the execution flow is better than adding a bunch of special
cases to the current execution flow

Re: [U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

2014-02-23 Thread Tang Yuantian-B29983


On 2014/2/18 星期二 3:18, Scott Wood wrote:

On Sun, 2014-02-16 at 21:35 -0600, Tang Yuantian-B29983 wrote:

-Original Message-
From: Wood Scott-B07421
To: Tang Yuantian-B29983
Cc: Sun York-R58495; Li Yang-Leo-R58472; u-boot@lists.denx.de; Kushwaha
Prabhakar-B32579; Jin Zhengxiong-R64188
Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support

On Thu, 2014-02-13 at 02:12 -0600, Tang Yuantian-B29983 wrote:

Use an I/O accessor.

In_be64?? No such function.

Why do you need in_be64() rather than two in_be32()s?


Avoid ECC error. Although, according to my test, in_be32() works too.

Why would you get an ECC error?

-Scott
DDR operation is always in 64bits. if writing a 32bits to memory, you 
need to
read a 32bits first, and combine it to form a 64bits. when the new 
64bits is written

to memory, ECC occurs.
I was required to do so by hardware team.

Regards,
Yuantian





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


Re: [U-Boot] [U-Boot, 2/3] mpc85xx: Add deep sleep framework support

2014-02-23 Thread Tang Yuantian-B29983


On 2014/2/15 星期六 6:21, Scott Wood wrote:

On Thu, 2014-02-13 at 01:05 -0600, Tang Yuantian-B29983 wrote:

Thanks for your review. Please see the reply inline.

Thanks,
Yuantian


-Original Message-
From: Wood Scott-B07421
Sent: 2014年2月13日 星期四 8:44
To: Tang Yuantian-B29983
Cc: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472;
t...@theia.denx.de; u-boot@lists.denx.de; Kushwaha Prabhakar-B32579; Jin
Zhengxiong-R64188
Subject: Re: [U-Boot,2/3] mpc85xx: Add deep sleep framework support

On Sun, Jan 26, 2014 at 02:00:40PM +0800, tang yuantian wrote:

From: Tang Yuantian yuantian.t...@freescale.com

When system wakes up from warm reset, control is passed to the primary
core that starts executing uboot. After re-initialized some IP blocks,
like DDRC, kernel will take responsibility to continue to restore
environment it leaves before.

Signed-off-by: Tang Yuantian yuantian.t...@freescale.com

Is this for some specific mpc85xx chip (e.g. T1040)?  I'm pretty sure
this isn't necessary for deep sleep on mpc8536, for example.


Currently, it is used by t1040. But we want it to be more general so that
It can be used by later new chips.

But the mechanism is not the same for all mpc85xx derivatives.  You'll
need a more specific name.

OK, will name it as t104x


+#ifdef CONFIG_DEEP_SLEEP

CONFIG symbols need to be documented in README...


Where should I add this README?

Under 85xx CPU Options in the top-level README.

Thanks.


+   /* disable the console if boot from warm reset */
+   if (in_be32(gur-scrtsr[0])  (1  3))
+   gd-flags |=
+   GD_FLG_SILENT | GD_FLG_WARM_BOOT |

GD_FLG_DISABLE_CONSOLE; #endif

...and it should probably be a more specific CONFIG_SYS symbol that
indicates the presence of this guts bit.

where should I put this CONFIG_SYS_'s definition?


diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c
b/arch/powerpc/cpu/mpc85xx/fdt.c index 33bc900..b3b9250 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -24,6 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;  extern void
ft_qe_setup(void *blob);  extern void ft_fixup_num_cores(void *blob);
extern void ft_srio_setup(void *blob);
+#ifdef CONFIG_DEEP_SLEEP
+extern ulong __bss_end;
+#endif

Don't ifdef declarations.


  #ifdef CONFIG_MP
  #include mp.h
@@ -35,6 +38,9 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
u32 bootpg = determine_mp_bootpg(NULL);
u32 id = get_my_id();
const char *enable_method;
+#ifdef CONFIG_DEEP_SLEEP
+   ulong len;
+#endif

off = fdt_node_offset_by_prop_value(blob, -1, device_type, cpu,

4);

while (off != -FDT_ERR_NOTFOUND) {
@@ -77,6 +83,25 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
device_type, cpu, 4);
}

+#ifdef CONFIG_DEEP_SLEEP
+   /*
+* reserve the memory space for warm reset.
+* This space will be re-used next time when boot from deep sleep.
+* The space includes bd_t, gd_t, stack and uboot image.
+* Reserve 1K for stack.
+*/
+   len = sizeof(bd_t) + sizeof(gd_t) + (1  10);
+   /* round up to 4K */
+   len = (len + (4096 - 1))  ~(4096 - 1);
+
+   off = fdt_add_mem_rsv(blob, gd-relocaddr - len,
+   len + ((ulong)__bss_end - gd-relocaddr));
+   if (off  0)
+   printf(Failed to reserve memory for warm reset: %s\n,
+   fdt_strerror(off));
+
+#endif

Why do you need to reserve memory for this?  We didn't need to for deep
sleep on MPC8313ERDB, which also goes through U-Boot on wake.  On
MPC8313ERDB we transfer control to the kernel before relocating, so U-
Boot never touches DDR.  bd_t, gd_t, and the stack should be in locked
L1 cache, and the u-boot image should be in flash (or locked CPC if this
is not a NOR flash boot).


In deep sleep many IP blocks are powered off like DDRC, LIODN, cpu. These IPs
are re-initialized after relocating.
So, we must jump to kernel after relocating.

The DDR controller is initialized before relocating -- and of course the
CPU is powered off on MPC8313ERDB deep sleep as well.

LIODNs are a new concern for deep sleep, but that doesn't mean we should
go through a bunch of unrelated code to get there.  Refactor the LIODN
code to be usable before relocation, and not be tied to fdt fixups.
There are other IP blocks that need to be re-initialized, like SerDes, 
SMP, PCIe and

a lot of Errata. I don't want to refactor one by one.
Besides, coding in this way will not change the current execute flow.


+#ifndef CONFIG_DEEP_SLEEP
/* Reserve spin table page */
if (spin_tbl_addr  memory_limit) {
off = fdt_add_mem_rsv(blob,
@@ -108,6 +134,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
printf(Failed to reserve memory for spin table: %s\n,
fdt_strerror(off));
}
+#endif

Explain.


Spin_tbl_addr has been reserved already.

Where, and why

Re: [U-Boot] [PATCH] mpc85xx: Fix the offset of register address error

2013-10-08 Thread Tang Yuantian-B29983
 
  Hi York,
  I double checked the offset address of GPIO, I found that the offset
  addresses of GPIO on the boards you mentioned above are all changed to
  0x0, not 0xc00 according to the newest RM.
  I do found that the offset address is 0xc00 in some old RMs.
  You can find the newest RM here:
  For MPC8572:
  http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/
  200815/108253488/223469393/223503931/226628079/226445024/MPC8572ERM_Re
  v3_DRAFT1.pdf?nodeid=226438746vernum=-2
  For p1023:
  http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/
  200815/108253488/223469393/223506436/223521755/223743960/P1023RM_Mark-
  up.pdf?nodeid=229647544vernum=-2
  for 1020:
  http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/
  200815/108253488/223469393/223506436/223522385/224515312/P1020RM_Rev6_
  Mark-up.pdf?nodeid=228476444vernum=-2
 
  If the offset addresses on these boards were 0xc00, the driver is
  still wrong, because in that case The GPIO address should be:
 CONFIG_SYS_IMMR + 0xc00, not CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00.
  (CONFIG_SYS_MPC85xx_GPIO_ADDR == CONFIG_SYS_IMMR +
 CONFIG_SYS_MPC85xx_GPIO_OFFSET).
 
  So, please apply this patch, I need the GPIO driver to operate GPIO.
 
 
 
 
 Please update the commit message to list all SoCs you have confirmed the
 offset. And don't say no reason to add 0xc00. The reason was clear when
 the code was written. Reference manuals said so.
 
 York

OK, I will resend this patch once the offset is confirmed by the RM owner.

Thanks,
Yuantian

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


Re: [U-Boot] [PATCH] mpc85xx: Fix the offset of register address error

2013-10-07 Thread Tang Yuantian-B29983
  diff --git a/arch/powerpc/include/asm/mpc85xx_gpio.h
  b/arch/powerpc/include/asm/mpc85xx_gpio.h
  index 3d11884..87bb4a0 100644
  --- a/arch/powerpc/include/asm/mpc85xx_gpio.h
  +++ b/arch/powerpc/include/asm/mpc85xx_gpio.h
  @@ -20,7 +20,7 @@
   static inline void mpc85xx_gpio_set(unsigned int mask,
  unsigned int dir, unsigned int val)  {
  -   ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00);
  +   ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR);
 
  /* First mask off the unwanted parts of dir and val */
  dir = mask;
  @@ -56,7 +56,7 @@ static inline void mpc85xx_gpio_set_high(unsigned
  int gpios)
 
   static inline unsigned int mpc85xx_gpio_get(unsigned int mask)  {
  -   ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00);
  +   ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR);
 
  /* Read the requested values */
  return in_be32(gpio-gpdat)  mask;
 
 
 Yuantian,
 
 Please go through the base address again. I think some SoCs do use 0xc00
 offset from 0xF000, for eample P1020, P1023, MPC8572. I only spot checked
 several.
 

Hi York,
I double checked the offset address of GPIO, I found that the offset addresses 
of 
GPIO on the boards you mentioned above are all changed to 0x0, not 0xc00 
according
to the newest RM.
I do found that the offset address is 0xc00 in some old RMs.
You can find the newest RM here: 
For MPC8572: 
http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/200815/108253488/223469393/223503931/226628079/226445024/MPC8572ERM_Rev3_DRAFT1.pdf?nodeid=226438746vernum=-2
For p1023:
http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/200815/108253488/223469393/223506436/223521755/223743960/P1023RM_Mark-up.pdf?nodeid=229647544vernum=-2
for 1020:
http://compass.freescale.net/livelink/livelink/fetch/2001/3448/223475/200815/108253488/223469393/223506436/223522385/224515312/P1020RM_Rev6_Mark-up.pdf?nodeid=228476444vernum=-2

If the offset addresses on these boards were 0xc00, the driver is still wrong, 
because in that case
The GPIO address should be: CONFIG_SYS_IMMR + 0xc00, not 
CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00.
(CONFIG_SYS_MPC85xx_GPIO_ADDR == CONFIG_SYS_IMMR + 
CONFIG_SYS_MPC85xx_GPIO_OFFSET).

So, please apply this patch, I need the GPIO driver to operate GPIO.

Regards,
Yuantian

 York


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