Reset questions

Andreas,

The reasons behind this is documented in the silicon errata of the dm368 as Advisory 1.2.1

regards,

miguel

Ok, I see. Are you planning to implement the part that actually saves the RSTYPE, too?

Andreas,

Which PSP version are you using?

            "Are you planning to implement the part that actually saves the RSTYPE, too?"

Which specific part is this?

regards,

miguel

The advisory I have (sprz316.pdf) talks about code that handles watchdog resets "In the event that a Watchdog Reset is ever executed...". The code seems to use two addresses in internal RAM to store information about the type of boot:

#define TMPBUF *(unsigned int *)(0x17ff8)
#define TMPSTATUS *(unsigned int *)(0x17ff0)

Now this part of the code is completely missing in the UBL I have (from DVSDK 3_10_00_16 http://software-dl.ti.com/dsps/dsps_public_sw/sdo_sb/targetcontent/dvsdk/DVSDK_3_10/latest/exports/flash-utils-davinci.tar.gz).

I understand the code this way:

1. If power-on reset (RSTYPE & 0x3)
=> remove mark in TMPBUF and do VPSS reset

2. If MAX_RESET (RSTYPE & 0x4) and magic in TMPBUF
=> This is a watchdog reset!
=> remove mark in TMPBUF and do VPSS reset

3. mark TMPBUF with magic and boot

Ok, all this assumes that nobody touches the internal RAM. But the UBL actually uses the internal RAM as stack, so the magic would be overwritten. The u-boot does not seem to use the internal RAM. I'm not sure if linux uses it.

So I wanted to ask if this part of the advisory will also be implemented in the UBL.

Before stumbling over this reset handling stuff I just wanted to find a way to tell what caused the last reset: power-on, user request or watchdog. But it seems that without software support in UBL, u-boot and linux that is not possible.

The fix will be included in the next release, this is in the works.

Your understanding is correct.

You have a good point about the Flags being in internal memory, we will look for a better place to put it in.

regards,

miguel

I'm wondering why the fix uses this magic in TMPBUF to signal that there was a successful boot. That way the magic has to survive both u-boot and linux.

Why not just use the magic within the UBL to signal if this is the first or second pass through UBL?

if(*TMPBUF != MAGIC) {

  *TMPBUF = MAGIC;

  do_reset();

  while(1) ;

}

*TMPBUF = 0;

// rest of UBL ...

Regards,

 Andreas.

Andreas,

You are right that TMPBUF and TMPSTATUS memory locations have to live through the uboot and kernel. Currently uBoot does not use IRAM and kernel inherently does not use IRAM. But ofcourse users can change the driver code or write their own application to use IRAM.

We do use IRAM for video codecs but take care that the TMPSTATUS and TMPBUF do not get overwritten.

The whole logic has been put because we need to do a VPSS_Sync_Reset every time we power on the device. This has to be followed by WDT reset to reset the device again and restart upfresh.

Now the WDT reset can happen by soft reboot (using linux "reboot" call). We need to detect whether this WDT reset has come from the soft reboot or from the use case mentioned above. The logic of TMPSTATUS and TMPBUF has been put to differentiate the WDT resets in normal POR reset or soft reboot.

I hope this answers both your questions.

Regards,

Anshuman

PS: Please mark this post as verified, if you think it answered your question. Thanks.

Still I would like to suggest to change the logic in a way that the magic is only used temporarily.

In UBL: use magic to mark that this will be the second reset
In u-boot/linux: use magic to mark a soft reboot (and detect that in UBL)

I've prepared two patches that implement this logic, one for UBL one for u-boot. The linux reset method has to be changed in the same way. What do you think?

UBL:

diff --git a/DM36x/Common/src/device.c b/DM36x/Common/src/device.c
index 3214413..913ad2a 100755
--- a/DM36x/Common/src/device.c
+++ b/DM36x/Common/src/device.c
@@ -40,7 +40,24 @@ extern __FAR__ VUint32* DDRStart;
 /************************************************************
 * Local Function Declarations                               *
 ************************************************************/
-
+static char get_reset_type();
+
+/* Use IRAM 0x17ff0 as marker for signalling second reboot within UBL  */
+#define STATUS_0_ADR      0x17ff0
+#define STATUS_0          ((unsigned int *)STATUS_0_ADR)
+#define MAGIC_STATUS_0    0x4d535430
+
+/* Use IRAM 0x17ff8 as marker for signaling soft-reboot from u-boot/linux
+ * Also reserve some bits for saving the "real" reset reason. */
+#define STATUS_1_ADR      0x17ff8
+#define STATUS_1          ((unsigned int *)STATUS_1_ADR)
+#define MASK_MAGIC        0xffffff00
+#define MAGIC_STATUS_1    (0x4d533100 & MASK_MAGIC)
+#define FLG_1_POR         (1 << 0)
+#define FLG_1_XWRST       (1 << 1)
+#define FLG_1_MRST        (1 << 2)
+#define FLG_1_SRST        (1 << 3)
+#define FLG_1_SOFT_RESET  (1 << 4)
 
 /************************************************************
 * Local Variable Definitions                                *
@@ -55,16 +72,9 @@ extern __FAR__ VUint32* DDRStart;
 
 const String devString = "DM36x";
 
-#define TMPBUF          (unsigned int *)(0x0017ff8)
-#define TMPSTATUS       (unsigned int *)(0x0017ff0)
 #define GPINT_GPEN        (unsigned int *)(0x01C21C08)        // WDT special function
 #define GPTDAT_GPDIR    (unsigned int *)(0x01C21C0c)        // WDT special function
 
-#define FLAG_PORRST     (0x00000001)
-#define FLAG_WDTRST     (0x00000002)
-#define FLAG_FLGON         (0x00000004)
-#define FLAG_FLGOFF     (0x00000010)
-
 /************************************************************
 * Global Function Definitions                               *
 ************************************************************/
@@ -87,7 +97,6 @@ Uint32 DEVICE_init()
 #ifndef SKIP_LOW_LEVEL_INIT
   SYSTEM->PERI_CLKCTRL |= 1;  //disable output 24Mhze
   POR_RESET();
-  WDT_RESET();
 
   // System PSC setup - enable all
   DEVICE_PSCInit();
@@ -148,7 +157,7 @@ Uint32 DEVICE_init()
 
   // System PLL setup
   if (status == E_PASS) status |= DEVICE_PLL1Init(PLL1_Mult);
-  WDT_FLAG_ON();
+
   // DDR PLL setup
   if (status == E_PASS) status |= DEVICE_PLL2Init();
 
@@ -175,6 +184,16 @@ Uint32 DEVICE_init()
   // I2C0 Setup
   if (status == E_PASS) status |= DEVICE_I2C0Init();
 
+  char r[] = "i";
+  r[0] = get_reset_type();
+  DEBUG_printString("Reset by: ");
+  DEBUG_printString(r);
+  DEBUG_printString("\r\n");
+ 
+  /* Clear STATUS_1 after reading the reset type, so in case of a wgd reset
+   * STATUS_1 does not contain magic. */
+  *STATUS_1 = 0;
+ 
   return status;
 }
 
@@ -224,35 +243,52 @@ Uint8 RTCIF_getreg(Uint16 regnum, Uint8 *regval)
 
 void POR_RESET()
 {
-    Uint8 Val;
-    Uint32 i;
-    if ((PLL1->RSTYPE)&3) {
-        VPSS_SYNC_RESET();  // VPSS sync reset
-        *TMPBUF = 0;
-          *TMPSTATUS |= FLAG_PORRST;
-        *GPINT_GPEN = 0x00020000;
-        *GPTDAT_GPDIR = 0x00020002;
-        while(1);
-      }
-    waitRtc();
-    RTCIF_setreg(0x10,0);
-    RTCIF_getreg(0x10,&Val);
-}
-
-void WDT_RESET()
-{
-    volatile unsigned int s;
-
-    if((*TMPBUF == 0x591b3ed7)){
-        *TMPBUF = 0;
-        *TMPSTATUS |= FLAG_PORRST;
-        *TMPSTATUS |= FLAG_FLGOFF; 
-        for (s=0;s<0x100;s++) {}
-        VPSS_SYNC_RESET();
-        *GPINT_GPEN = 0x00020000;                                   // WDT
-        *GPTDAT_GPDIR = 0x00020002;                                 // execute >
+    /* Workaround for silicon errata advisory 1.2.1
+     * Note: RSTYPE will always be 0x4 afterwards, so we save the "real" reset
+     * reason in some flags in a fixed internal memory location.
+     *
+     * The method works like this:
+     *
+     * 1. If STATUS_0 does not contain magic, this must be the first time
+     *    we pass this function.
+     *
+     * 1a. Try to figure out reset reason and save it
+     *     into STATUS_1 (together with a magic).
+     * 1b. Reset VPSS and force a wdg reset
+     *
+     * 2. STATUS_0 contains magic, so this is the second pass.
+     *
+     * 2a. Clear STATUS_0 so we're armed for the next reset.
+     *
+     * After this STATUS_1 contains a magic and some bits that encode the real
+     * reason for the last reset.
+     */
+    if(*STATUS_0 != MAGIC_STATUS_0) {
+        unsigned char flgs = 0;
+
+        // First reset, save magic
+        *STATUS_0 = MAGIC_STATUS_0;
+
+        // Save real reset type in flags
+        flgs = ((PLL1->RSTYPE) & 0xff);
+
+        // STATUS_1 already has magic => deliberate soft reset (no wdg)
+        if((*STATUS_1 & MASK_MAGIC) == MAGIC_STATUS_1) {
+            flgs |= FLG_1_SOFT_RESET;
+        }
+
+        // Save flags together with magic in second status
+        *STATUS_1 = MAGIC_STATUS_1 | flgs;
+
+        // Now do the resetting
+        VPSS_SYNC_RESET();  // VPSS sync reset
+        *GPINT_GPEN = 0x00020000;
+        *GPTDAT_GPDIR = 0x00020002;
         while(1);
-    }
+    }
+
+    // arm for next reset
+    *STATUS_0 = 0;
 }
 
 void VPSS_SYNC_RESET()
@@ -271,13 +307,6 @@ void VPSS_SYNC_RESET()
     while(!((PSC->MDSTAT[47] &  0x0000001F) == 0x1));          
 }
 
-void WDT_FLAG_ON()
-{
-    SYSTEM->VPSS_CLKCTL &= 0xffffff7f;      // VPSS_CLKMD 1:2
-    *TMPBUF = 0x591b3ed7;
-    *TMPSTATUS |= FLAG_FLGON; 
-}
-
 void DEVICE_LPSCTransition(Uint8 module, Uint8 domain, Uint8 state)
 {
   // Wait for any outstanding transition to complete
@@ -725,6 +754,29 @@ Uint32 DEVICE_TIMER0Status(void)
 * Local Function Definitions                                *
 ************************************************************/
 
+char get_reset_type() {
+
+    /* Get status value */
+    unsigned int status = *STATUS_1;
+
+    if((status & MASK_MAGIC) == MAGIC_STATUS_1) {
+        if(status & FLG_1_MRST) {
+            // MRST can mean WDG or software reset
+            if(status & FLG_1_SOFT_RESET) {
+                return 'o'; // Soft reset
+            } else {
+                return 'w'; // must be watchdog
+            }
+        } else if(status & FLG_1_POR) {
+            return 'p'; // Power on
+        } else if(status & FLG_1_XWRST) {
+            return 'x'; // External warm
+        } else if(status & FLG_1_SRST) {
+            return 's'; // System Reset
+        }
+    }
+    return 'U'; // unknown (really)
+}
 
 /***********************************************************
 * End file                                                 *

u-boot:

diff --git a/arch/arm/cpu/arm926ejs/davinci/reset.S b/arch/arm/cpu/arm926ejs/davinci/reset.S
index ba0a7c3..8f8417b 100644
--- a/arch/arm/cpu/arm926ejs/davinci/reset.S
+++ b/arch/arm/cpu/arm926ejs/davinci/reset.S
@@ -23,6 +23,14 @@
 
 .globl reset_cpu
 reset_cpu:
+
+    /* Save in status field in internal RAM that we are
+     * doing a software reset */
+    ldr     r0, STATUS_1
+    ldr     r1, STATUS_1_MAGIC
+    str     r1, [r0]
+
+    /* Use WDG for reset */
     ldr    r0, WDT_TGCR
     mov    r1, $0x08
     str    r1, [r0]
@@ -50,8 +58,10 @@ reset_cpu:
     str    r1, [r0]
     ldr    r1, WDTCR_VAL2
     str    r1, [r0]
+
     /* Write an invalid value to the WDKEY field to trigger
      * an immediate watchdog reset */
+    ldr    r0, WDT_WDTCR
     mov     r1, $0x4000
     str     r1, [r0]
     nop
@@ -79,3 +89,8 @@ WDTCR_VAL1:
     .word    0xa5c64000
 WDTCR_VAL2:
     .word    0xda7e4000
+
+STATUS_1:
+    .word   0x17ff8
+STATUS_1_MAGIC:
+    .word   0x4d533100

I haven't tried the actual patch in a running system but it seems correct to me. Did you submit it to the "git" kernel and DVSDK team?

I use this patch in my own system now. As I'm not aware of an "official" UBL GIT repo, there's no place I could post it. Before the UBL team agrees to change the UBL the proposed way, there's no point in submitting something to u-boot/linux.

Regards,

Andreas.

Marco,

Yes, you are correct.  We are working to fix the error you pointed out in the errata.

Current Errata states:

#define TMPBUF *(unsigned int *)(0x17ff8)

#define TMPSTATUS *(unsigned int *)(0x17ff0)

Whereas the right definition is:

#define TMPBUF (unsigned int *)(0x17ff8)

#define TMPSTATUS (unsigned int *)(0x17ff0)

Sorry for any confusion this may have caused.