MPC5200 and Lite5200b low power modes

Suspend and resume for FEC on MPC52xx.

Note that resume is a bit different for lite5200b low-power mode.

Signed-off-by Domen Puncer <[email protected]>

Trivial suspend and resume OF OHCI.
On MPC52xx turn off and on power to ports.

Signed-off-by Domen Puncer <[email protected]>

Index: grant.git/drivers/usb/host/ohci-ppc-of.c
===================================================================

drivers/usb/host/ohci-ppc-of.c +4 -0

@@ -214,6 +214,32 @@ MODULE_DEVICE_TABLE(of, ohci_hcd_ppc_of_
 #endif
 
 
+#ifdef CONFIG_PM
+static u32 descr_a;
+static int ohci_hcd_ppc_soc_drv_suspend(struct of_device *op,
+               pm_message_t state)

Hi:

I'm trying to bring up Linux on Xilinx ML403 reference board.

I took 2.6.19.2 kernel from DENX. EDK project for ML403 I took from official Xilinx site.

I could bring up u-boot on the board, everything is working fine (see ml403.h file attached). Then I compiled uImage for kernel and tried to download it - I don't see any kernel's outputs:

TFTP from server 192.168.0.141; our IP address is 192.168.0.203 Filename 'LM200/rel/1.0.1d-403/uImage'.
Load address: 0x1000000
Loading: T
#################################################################

#################################################################

         #######################################
done
Bytes transferred = 861735 (d2627 hex)
## Booting image at 01000000 ...
   Image Name:   Linux-2.6.19.2
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:    861671 Bytes = 841.5 kB

Load Address: 00000000
Entry Point: 00000000
Verifying Checksum ... OK
Uncompressing Kernel Image ... OK
## Current stack ends at 0x03E676F0 => set upper limit to 0x00800000 ## cmdline at 0x007FFF00 ... 0x007FFFF8

memstart    = 0x00000000
memsize     = 0x04000000
flashstart  = 0x28000000
flashsize   = 0x00800000
flashoffset = 0x00000000
sramstart   = 0x00000000
sramsize    = 0x00000000
bootflags   = 0x0000003C
procfreq    =    300 MHz
plb_busfreq =    100 MHz
ethaddr     = 00:01:02:CB:CB:71
IP addr     = 192.168.0.203
baudrate    = 115200 bps

No initrd
## Transferring control to Linux (at address 00000000) ...

I have read a lot of posts and reviewed the source code and I think these kernel trees I'm using are properly patched for using u-boot; I only included asm/ppcboot.h into xilinx_403.h:

/* U-boot expects the bd_t to look like the one in ppcboot.h, not like this. */
#ifdef NOT_USING_UBOOT
typedef struct board_info {
unsigned int bi_memsize; /* DRAM installed, in
bytes */
unsigned char bi_enetaddr[6]; /* Local Ethernet MAC
address */
unsigned int bi_intfreq; /* Processor speed, in
Hz */
unsigned int bi_busfreq; /* PLB Bus speed, in Hz
*/
unsigned int bi_pci_busfreq; /* PCI Bus speed, in Hz
*/
} bd_t;

/* Some 4xx parts use a different timebase frequency from the internal clock.
*/
#define bi_tbfreq bi_intfreq

#else
#include <asm/ppcboot.h>
#endif /* UBOOT */

Anyway I tried to compile zImage.elf and download it directly to RAM using XMD. Here I had partial success: first time after FPGA image downloading I try it, I see the following (it doesn't happen if I try to repeat downloading again without reloading FPGA image first):

====

loaded at:     00400000 004DA13C
board data at: 004D8124 004D813C
relocated to:  00404090 004040A8
zimage at:     00404E1D 004D7401
avail ram:     004DB000 04000000

Linux/PPC load: console=ttyS0,115200
Uncompressing Linux...done.
Now booting the kernel

[    0.000000] Linux version 2.6.19.2 ([email protected]) (gcc
version 4.0.0 (DENX ELDK 4.1 4.0.0)) #6 Thu Mar 15 05:46:09 PST 2007
[    0.000000] Xilinx ML403 Reference System (Virtex-4 FX)
[    0.000000] Zone PFN ranges:
[    0.000000]   DMA             0 ->    16384
[    0.000000]   Normal      16384 ->    16384
[    0.000000] early_node_map[1] active PFN ranges
[    0.000000]     0:        0 ->    16384
[    0.000000] Built 1 zonelists.  Total pages: 16256
[    0.000000] Kernel command line: console=ttyS0,115200
[    0.000000] Xilinx INTC #0 at 0xD1000FC0 mapped to 0xFDFFEFC0
[    0.000000] PID hash table entries: 256 (order: 8, 1024 bytes)
[    0.000188] Console: colour dummy device 80x25
[    0.000701] Dentry cache hash table entries: 8192 (order: 3, 32768
bytes)
[    0.001489] Inode-cache hash table entries: 4096 (order: 2, 16384
bytes)
[    0.015326] Memory: 63104k available (1324k kernel code, 436k data,
92k init, 0k highmem)
[    0.108476] Mount-cache hash table entries: 512
[    0.114051] NET: Registered protocol family 16
[    0.128395] NET: Registered protocol family 2
[    0.168472] IP route cache hash table entries: 512 (order: -1, 2048
bytes)
[    0.169340] TCP established hash table entries: 2048 (order: 1, 8192
bytes)
[    0.169539] TCP bind hash table entries: 1024 (order: 0, 4096 bytes)
[    0.169648] TCP: Hash tables configured (established 2048 bind 1024)
[    0.169679] TCP reno registered
[    0.173555] io scheduler noop registered
[    0.173600] io scheduler anticipatory registered (default)
[    0.173630] io scheduler deadline registered
[    0.173756] io scheduler cfq registered
[    0.221629] Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ
sharing disabled
[    0.227023] serial8250.0: ttyS0 at MMIO 0xa0001003 (irq = 9) is a
16450
[    0.398738] RAMDISK driver initialized: 16 RAM disks of 65536K size
1024 blocksize
[    0.407941] tun: Universal TUN/TAP device driver, 1.6
[    0.412993] tun: (C) 1999-2004 Max Krasnyansky <[email protected]>
[    0.420583] mice: PS/2 mouse device common for all mice
[    0.426040] TCP cubic registered
[    0.429347] NET: Registered protocol family 1
[    0.433727] NET: Registered protocol family 17
[    0.439482] VFS: Cannot open root device "<NULL>" or
unknown-block(0,0)
[    0.446095] Please append a correct "root=" boot option
[    0.451282] Kernel panic - not syncing: VFS: Unable to mount root fs
on unknown-block(0,0)
[    0.459492]  <0>Rebooting in 180 seconds..

I don't care about kernel panic for now - it's to be expected since I don't provide any filesystem.

So, kernel itself is workable, it looks like uImage doesn't pick up correct board info parameters from u-boot. Where u-boot write them and where Linux looks for them? What is the simplest way to sync between u-boot and kernel?

Thanks,

Leonid.

------------------------------------------------------------------------

_______________________________________________
Linuxppc-embedded mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded

drivers/usb/host/ohci-ppc-of.c MODULE_DEVICE_TABLE(of, ohci_hcd_ppc_of_ · line 224

Quoted 5 lines · Domen Puncer

drivers/usb/host/ohci-ppc-of.c +3 -0

+     struct usb_hcd *hcd = dev_get_drvdata(&op->dev);
+     if (machine_is_compatible("generic-mpc5200")) {
+             struct ohci_hcd *ohci = hcd_to_ohci(hcd);

Not good, what if you have a Ohci PCI card on a 5200 board ...

You somehow need to check the compatible list of the of_node associated with the particular instance you're putting to sleep.

Implement deep-sleep on MPC52xx.
SDRAM is put into self-refresh with help of SRAM code (alternatives would be code in FLASH, I-cache). Interrupt code must also not be in SDRAM, so put it in I-cache.
MPC52xx core is static, so contents will remain intact even with clocks turned off.

Index: grant.git/arch/powerpc/platforms/52xx/mpc52xx_pm.c
===================================================================

arch/powerpc/platforms/52xx/mpc52xx_pm.c new file +5 -0

+/* you will want to change this, to match your board gpios, rtc or whatever */
+static void mpc52xx_set_wakeup_mode(void)
+{
+       struct mpc52xx_gpio_wkup __iomem *gpiow;
+       struct mpc52xx_intr __iomem *intr;

arch/powerpc/platforms/52xx/mpc52xx_pm.c new file +4 -0

+static struct pm_ops mpc52xx_pm_ops = {
+       .valid          = mpc52xx_pm_valid,
+       .prepare        = mpc52xx_pm_prepare,
+       .enter          = mpc52xx_pm_enter,

Hi!

How about something like the following.
Changes:
- lots of code moved from asm to C
- add compatible "mpc5200" to lite5200x soc (already is this way on efika). And change Efika's soc device_type to "soc".
- add wakeup supported with RTC (1 to 24*60-1 minutes)
- each board now configures it's wakeup mode and possibly board suspend and resume functions it needs to call (USB on lite)

This code survived > 70k suspend/resume cycles :-)

Comments?

arch/powerpc/platforms/52xx/mpc52xx_pm.c new file +0 -0

arch/powerpc/boot/dts/lite5200.dts +1 -0

@@ -49,6 +49,7 @@
 
        soc5200@f0000000 {
                model = "fsl,mpc5200";
+               compatible = "mpc5200";
                revision = ""                   // from bootloader
                #interrupt-cells = <3>;
                device_type = "soc";

Index: grant.git/arch/powerpc/boot/dts/lite5200b.dts
===================================================================

Implement deep-sleep on MPC52xx.
SDRAM is put into self-refresh with help of SRAM code (alternatives would be code in FLASH, I-cache). Interrupt code must also not be in SDRAM, so put it in I-cache.
MPC52xx core is static, so contents will remain intact even with clocks turned off.

Signed-off-by Domen Puncer <[email protected]>

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

---
 board/icecube/icecube.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

Index: u-boot.git/board/icecube/icecube.c
===================================================================

board/icecube/icecube.c +50 -0

@@ -42,6 +42,54 @@
 #include "mt48lc16m16a2-75.h"
 # endif
 #endif
+
+#ifdef CONFIG_LITE5200B
+/* u-boot part of low-power mode implementation */
+#define SAVED_ADDR (*(void **)0x00000000)
+#define PSC2_4 0x02
+
+void lite5200b_wakeup(void)
+{
+       unsigned char wakeup_pin;
+       void (*linux_wakeup)(void);
+
+       /* check PSC2_4, if it's down "QT" is signaling we have a wakeup
+        * from low power mode */
+       *(vu_char *)MPC5XXX_WU_GPIO_ENABLE = PSC2_4;
+       __asm__ volatile ("sync");
+
+       wakeup_pin = *(vu_char *)MPC5XXX_WU_GPIO_DATA_I;
+       if (wakeup_pin & PSC2_4)
+               return;
+
+       /* acknowledge to "QT"
+        * by holding pin at 1 for 10 uS */
+       *(vu_char *)MPC5XXX_WU_GPIO_DIR = PSC2_4;
+       __asm__ volatile ("sync");
+       *(vu_char *)MPC5XXX_WU_GPIO_DATA_O = PSC2_4;
+       __asm__ volatile ("sync");
+       udelay(10);
+
+       /* put ram out of self-refresh */
+       *(vu_long *)MPC5XXX_SDRAM_CTRL |= 0x80000000;   // mode_en
+       __asm__ volatile ("sync");
+       *(vu_long *)MPC5XXX_SDRAM_CTRL |= 0x50000000;   // cke ref_en
+       __asm__ volatile ("sync");
+       *(vu_long *)MPC5XXX_SDRAM_CTRL &= ~0x80000000;  // !mode_en
+       __asm__ volatile ("sync");
+       udelay(10); /* wait a bit */
+
+       /* jump back to linux kernel code */
+       linux_wakeup = SAVED_ADDR;
+       printf("\n\nLooks like we just woke, transferring control to 0x%08lx\n",
+                       linux_wakeup);
+       linux_wakeup();
+}
+#else
+#define lite5200b_wakeup()
+#endif
+
+
 #ifndef CFG_RAMBOOT
 static void sdram_start (int hi_addr)
 {

@@ -208,6 +256,8 @@ long int initdram (int board_type)
                __asm__ volatile ("sync");
        }
 
+       lite5200b_wakeup();
+
        return dramsize + dramsize2;
 }

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

This looks pretty straight forward.

My only comment is that psc2_4 is probably used as GPIO instead of power control by some users (The lite5200 is an eval board after all). Maybe wrap the code with #ifdef CONFIG_LITE5200B_PM (instead of CONFIG_LITE5200B) so that it can be easily compiled out.

Also, '//' style comments should be changed to '/* */'

Otherwise;

Acked-by Grant Likely <[email protected]>

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

---

Quoted 12 lines · Grant Likely

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

This looks pretty straight forward.

My only comment is that psc2_4 is probably used as GPIO instead of power control by some users (The lite5200 is an eval board after all). Maybe wrap the code with #ifdef CONFIG_LITE5200B_PM (instead of CONFIG_LITE5200B) so that it can be easily compiled out.

Quoted 5 lines

Also, '//' style comments should be changed to '/* */'

Otherwise;

Acked-by Grant Likely <[email protected]>

OK. This one should be better:

 Makefile                |    5 ++++
 board/icecube/icecube.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Index: u-boot.git/board/icecube/icecube.c
===================================================================

board/icecube/icecube.c +49 -0

@@ -42,6 +42,53 @@
 #include "mt48lc16m16a2-75.h"
 # endif
 #endif
+
+#ifdef CONFIG_LITE5200B_PM
+/* u-boot part of low-power mode implementation */
+#define SAVED_ADDR (*(void **)0x00000000)
+#define PSC2_4 0x02
+
+void lite5200b_wakeup(void)
+{
+       unsigned char wakeup_pin;
+       void (*linux_wakeup)(void);
+
+       /* check PSC2_4, if it's down "QT" is signaling we have a wakeup
+        * from low power mode */
+       *(vu_char *)MPC5XXX_WU_GPIO_ENABLE = PSC2_4;
+       __asm__ volatile ("sync");
+
+       wakeup_pin = *(vu_char *)MPC5XXX_WU_GPIO_DATA_I;
+       if (wakeup_pin & PSC2_4)
+               return;
+
+       /* acknowledge to "QT"
+        * by holding pin at 1 for 10 uS */
+       *(vu_char *)MPC5XXX_WU_GPIO_DIR = PSC2_4;
+       __asm__ volatile ("sync");
+       *(vu_char *)MPC5XXX_WU_GPIO_DATA_O = PSC2_4;
+       __asm__ volatile ("sync");
+       udelay(10);
+
+       /* put ram out of self-refresh */
+       *(vu_long *)MPC5XXX_SDRAM_CTRL |= 0x80000000;   /* mode_en */
+       __asm__ volatile ("sync");
+       *(vu_long *)MPC5XXX_SDRAM_CTRL |= 0x50000000;   /* cke ref_en */
+       __asm__ volatile ("sync");
+       *(vu_long *)MPC5XXX_SDRAM_CTRL &= ~0x80000000;  /* !mode_en */
+       __asm__ volatile ("sync");
+       udelay(10); /* wait a bit */
+
+       /* jump back to linux kernel code */
+       linux_wakeup = SAVED_ADDR;
+       printf("\n\nLooks like we just woke, transferring control to 0x%08lx\n",
+                       linux_wakeup);
+       linux_wakeup();
+}
+#else
+#define lite5200b_wakeup()
+#endif
+
 #ifndef CFG_RAMBOOT
 static void sdram_start (int hi_addr)
 {

@@ -208,6 +255,8 @@ long int initdram (int board_type)
                __asm__ volatile ("sync");
        }
 
+       lite5200b_wakeup();
+
        return dramsize + dramsize2;
 }
 

Index: u-boot.git/Makefile
===================================================================

Makefile +5 -0

@@ -430,6 +430,7 @@ inka4x0_config:     unconfig
        @$(MKCONFIG) inka4x0 ppc mpc5xxx inka4x0
 
 lite5200b_config       \
+lite5200b_PM_config    \
 lite5200b_LOWBOOT_config:      unconfig
        @mkdir -p $(obj)include
        @mkdir -p $(obj)board/icecube

@@ -438,6 +439,10 @@ lite5200b_LOWBOOT_config:  unconfig
        @ echo "... DDR memory revision"
        @ echo "#define CONFIG_MPC5200"         >>$(obj)include/config.h
        @ echo "#define CONFIG_LITE5200B"       >>$(obj)include/config.h
+       @[ -z "$(findstring _PM_,$@)" ] || \
+               { echo "#define CONFIG_LITE5200B_PM"    >>$(obj)include/config.h ; \
+                 echo "... with power management (low-power mode) support" ; \
+               }
        @[ -z "$(findstring LOWBOOT_,$@)" ] || \
                { echo "TEXT_BASE = 0xFF000000" >$(obj)board/icecube/config.tmp ; \
                  echo "... with LOWBOOT configuration" ; \

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Looks good; almost there. Only one thing missing... you need to add your "Signed-off-by" line. :-) See Documentation/SubmittingPatches in the Linux source tree. (You can just reply to this message, and whoever merges the patch will add the line to the commit message)

Acked-by Grant Likely <[email protected]>

Stefan; since there is no 5200 custodian, can you please pick up this patch once Domen sends a Signed-of-by replay?

I thought Signed-off-by's are Linux specific.

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Looks good; almost there. Only one thing missing... you need to add your "Signed-off-by" line. :-) See Documentation/SubmittingPatches in the Linux source tree. (You can just reply to this message, and whoever merges the patch will add the line to the commit message)

Acked-by Grant Likely <[email protected]>

Stefan; since there is no 5200 custodian, can you please pick up this patch once Domen sends a Signed-of-by replay?

Quoted 10 lines · Grant Likely

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Looks good; almost there. Only one thing missing... you need to add your "Signed-off-by" line. :-) See Documentation/SubmittingPatches in the Linux source tree. (You can just reply to this message, and whoever merges the patch will add the line to the commit message)

Quoted 4 lines

Acked-by Grant Likely <[email protected]>

Stefan; since there is no 5200 custodian, can you please pick up this patch once Domen sends a Signed-of-by replay?

Done.

Quoted 11 lines · Stefan Roese

Quoted 10 lines · Grant Likely

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Looks good; almost there. Only one thing missing... you need to add your "Signed-off-by" line. :-) See Documentation/SubmittingPatches in the Linux source tree. (You can just reply to this message, and whoever merges the patch will add the line to the commit message)

Quoted 6 lines

Quoted 4 lines

Acked-by Grant Likely <[email protected]>

Stefan; since there is no 5200 custodian, can you please pick up this patch once Domen sends a Signed-of-by replay?

Done.

Thanks!

While at it, can you also take care of (Grant already agreed with the change):

G2 core reference manual says decrementer and time base are decreasing/increasing once every 4 bus clock cycles. Lets fix it, so time in Linux won't run twice as fast

Signed-off-by Domen Puncer <[email protected]>

---
 include/configs/IceCube.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: u-boot.git/include/configs/IceCube.h
===================================================================

include/configs/IceCube.h +1 -1

@@ -182,7 +182,7 @@
 
 #define OF_CPU                 "PowerPC,5200 at 0"
 #define OF_SOC                 "soc5200 at f0000000"
-#define OF_TBCLK               (bd->bi_busfreq / 8)
+#define OF_TBCLK               (bd->bi_busfreq / 4)
 #define OF_STDOUT_PATH         "/soc5200 at f0000000/serial at 2000"
 
 /*

While at it, can you also take care of (Grant already agreed with the change):

since we don't have a 52xx repository, I commited it into the u-boot-testing repository:

git://www.denx.de/git/u-boot-testing.git

Quoted 10 lines · Grant Likely

Quoted 3 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Looks good; almost there. Only one thing missing... you need to add your "Signed-off-by" line. :-) See Documentation/SubmittingPatches in the Linux source tree. (You can just reply to this message, and whoever merges the patch will add the line to the commit message)

Quoted 4 lines

Acked-by Grant Likely <[email protected]>

Stefan; since there is no 5200 custodian, can you please pick up this patch once Domen sends a Signed-of-by replay?

Done.

- regarding the U-Boot piece I'd only like to see some explanation (as a comment or short readme file) of the overall concept and scenario as it is not intuitive at all and rather difficult to understand all the ramifications and constraints from looking at the code only "

So Domen, could you please either send a reworked patch with this documentation (preferable some file in doc/README.xxx) or just send a new patch with only this documentation?

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Quoted 4 lines · Domen Puncer

U-Boot part of Lite5200b low power mode support. Puts SDRAM out of self-refresh and transfers control to address saved at physical 0x0.

Hi Domen,

As I understand while waking up from the low-power mode the machine is effectively powering on, similarly to the cold reset, so U-Boot runs from the beginning as usual, but after the SDRAM controller has been initialised we detect the wakeup and teleport to the saved DRAM address.

Since in case of a wakeup from the low-power mode we skip everything in U-Boot that is happening past initdram(), please clarify the following:

- are you sure there are no steps beyond init_func_ram()/board_init_f() that should be performed while waking up? For example:

- are all timers settings properly re-stored?

- wouldn't the host/PCI bridge need to be re-initialised and re-configured as part of the wakeup process? Did you happen to test some PCI devices and would they survive after wakeup from the the low-power mode? (A similar question would apply to the USB controller)

Also, a more general question: isn't time base update required after the wakeup, specially if it's been a long sleep?

Low-power mode implementation for Lite5200b.
Some I/O registers are also saved here.

A patch to U-Boot that wakes up SDRAM, and transfers control to address saved at physical 0x0 is needed.

Signed-off-by Domen Puncer <[email protected]>

---
 arch/powerpc/platforms/52xx/Makefile         |    3
 arch/powerpc/platforms/52xx/lite5200_pm.c    |  125 ++++++++
 arch/powerpc/platforms/52xx/lite5200_sleep.S |  419 +++++++++++++++++++++++++++
 3 files changed, 547 insertions(+)

Index: grant.git/arch/powerpc/platforms/52xx/lite5200_pm.c
===================================================================

arch/powerpc/platforms/52xx/lite5200_pm.c new file +0 -0

@@ -0,0 +1,125 @@

arch/powerpc/platforms/52xx/lite5200_pm.c new file +19 -0

+
+       memcpy(saved_sram, sdma.sram, sdma.sram_size);
+}
+
+static void lite5200_restore_regs(void)
+{
+       memcpy(sdma.sram, saved_sram, sdma.sram_size);
+
+       _memcpy_toio(gpw, &sgpw, sizeof(*gpw));
+       _memcpy_toio(gps, &sgps, sizeof(*gps));
+       _memcpy_toio(xlb, &sxlb, sizeof(*xlb));
+       _memcpy_toio(cdm, &scdm, sizeof(*cdm));
+       _memcpy_toio(pic, &spic, sizeof(*pic));
+       _memcpy_toio(bes, &sbes, sizeof(*bes));
+}
+
+static int lite5200_pm_enter(suspend_state_t state)
+{
+       /* deep sleep? let mpc52xx code handle that */

Index: grant.git/arch/powerpc/platforms/52xx/lite5200_sleep.S
===================================================================

arch/powerpc/platforms/52xx/lite5200_sleep.S new file +16 -0

@@ -0,0 +1,419 @@
+#include <asm/reg.h>
+#include <asm/ppc_asm.h>
+#include <asm/processor.h>
+#include <asm/cache.h>
+
+
+#define SDRAM_MODE     0x100
+#define SDRAM_CTRL     0x104
+#define SC_MODE_EN     (1<<31)
+#define SC_CKE         (1<<30)
+#define SC_REF_EN      (1<<28)
+#define SC_SOFT_PRE    (1<<1)
+
+#define GPIOW_GPIOE    0xc00
+#define GPIOW_ODE      0xc04
+#define GPIOW_DDR      0xc08

Quoted 5 lines · Domen Puncer

Low-power mode implementation for Lite5200b.
Some I/O registers are also saved here.

A patch to U-Boot that wakes up SDRAM, and transfers control to address saved at physical 0x0 is needed.

I don't see any structural problems with this code, but I have a few comments below. I'm also concerned about the blind register save/restore by memcpy_to/fromio. I haven't looked at the chip documentation, but it looks scary. Is it safe to restore those registers in that manner?

arch/powerpc/platforms/52xx/lite5200_pm.c line 36

Quoted 2 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +2 -0

+       /* map registers */
+       mbar = ioremap_nocache(0xf0000000, 0x8000);

Magic numbers? Really? This should be retrieved from the device tree. There is always the possibility of mbar getting moved.

arch/powerpc/platforms/52xx/lite5200_pm.c line 96

Quoted 3 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +3 -0

+       gpw = mbar + 0xc00;
+       bes = mbar + 0x1200;
+       xlb = mbar + 0x1f00;

Again, magic numbers

arch/powerpc/platforms/52xx/lite5200_pm.c line 67

Quoted 4 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +4 -0

+       _memcpy_fromio(&scdm, cdm, sizeof(*cdm));
+       _memcpy_fromio(&sxlb, xlb, sizeof(*xlb));
+       _memcpy_fromio(&sgps, gps, sizeof(*gps));
+       _memcpy_fromio(&sgpw, gpw, sizeof(*gpw));

Hmmm. I have not dug into this deeply, but blind save/restore to blocks of registers scares me.

arch/powerpc/platforms/52xx/lite5200_sleep.S line 26

Quoted 3 lines

arch/powerpc/platforms/52xx/lite5200_sleep.S new file +3 -0

+// about 2000 cpu cycles for one sdram cycle here
+// just increase, to be on the safe side?
+#define TCK    5000

Please avoid c++ comments

Quoted 1 line

+#define DONT_DEBUG 1

Convention is to #define DEBUG to enable debugging (as opposed to #defining something to disable it)

Quoted 1 line · Grant Likely

On 3/15/07, Domen Puncer <[email protected]> wrote:

...

Quoted 6 lines

arch/powerpc/platforms/52xx/lite5200_pm.c line 36

Quoted 2 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +2 -0

+       /* map registers */
+       mbar = ioremap_nocache(0xf0000000, 0x8000);

Magic numbers? Really? This should be retrieved from the device tree. There is always the possibility of mbar getting moved.

...

Quoted 4 lines

Quoted 3 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +3 -0

+       gpw = mbar + 0xc00;
+       bes = mbar + 0x1200;
+       xlb = mbar + 0x1f00;

Quoted 1 line

Again, magic numbers

Well... the code is only applicable for Lite5200b/mpc5200 and numbers are from specs.
And it's shorter than mpc52xx_find_and_map() lines. I guess I could rewrite it.

Quoted 16 lines · Domen Puncer

Quoted 1 line · Grant Likely

On 3/15/07, Domen Puncer <[email protected]> wrote:

...

Quoted 6 lines

arch/powerpc/platforms/52xx/lite5200_pm.c line 36

Quoted 2 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +2 -0

+       /* map registers */
+       mbar = ioremap_nocache(0xf0000000, 0x8000);

Magic numbers? Really? This should be retrieved from the device tree. There is always the possibility of mbar getting moved.

...

Quoted 5 lines

arch/powerpc/platforms/52xx/lite5200_pm.c line 96

Quoted 3 lines

arch/powerpc/platforms/52xx/lite5200_pm.c new file +3 -0

+       gpw = mbar + 0xc00;
+       bes = mbar + 0x1200;
+       xlb = mbar + 0x1f00;

Again, magic numbers

Quoted 4 lines

Well... the code is only applicable for Lite5200b/mpc5200 and numbers are from specs.
And it's shorter than mpc52xx_find_and_map() lines. I guess I could rewrite it.

In asm code I need access to SDRAM controller registers (MBAR+0x100). Do I add an entry to dts, or do you have other suggestions?

Quoted 8 lines · Domen Puncer

Quoted 4 lines · Domen Puncer

Well... the code is only applicable for Lite5200b/mpc5200 and numbers are from specs.
And it's shorter than mpc52xx_find_and_map() lines. I guess I could rewrite it.

In asm code I need access to SDRAM controller registers (MBAR+0x100). Do I add an entry to dts, or do you have other suggestions?

Low-power mode implementation for Lite5200b. (Most of devices on board, including the CPU, are powered down)

Some I/O registers are also saved here.

A patch to U-Boot that wakes up SDRAM, and transfers control to address saved at physical 0x0 is needed, and is already merged in git://www.denx.de/git/u-boot-ppc4xx.git tree.

Signed-off-by Domen Puncer <[email protected]>