Proxmark developers community

Research, development and trades concerning the powerful Proxmark3 device.

Remember; sharing is caring. Bring something back to the community.


"Learn the tools of the trade the hard way." +Fravia

You are not logged in.

#1 2010-10-04 12:07:08

notserpe
Member
From: Montreal
Registered: 2009-09-15
Posts: 14
Website

Invalid OUT bits set for CKGR_PPLR in bootloader?

In the following code from bootrom/bootrom.c (line 48-ish):

    ...
    // PLL output clock frequency in range 150 - 180 MHz needs CKGR_PLL = 10
    // PLL output is MAINCK * multiplier / divisor = 16Mhz * 12 / 2 = 96Mhz
    AT91C_BASE_PMC->PMC_PLLR =
        PMC_PLL_DIVISOR(2) |
        PMC_PLL_COUNT_BEFORE_LOCK(0x50) |
        PMC_PLL_FREQUENCY_RANGE(0) |
        PMC_PLL_MULTIPLIER(12) |
        PMC_PLL_USB_DIVISOR(1);

    // wait for PLL to lock
    while ( !(AT91C_BASE_PMC->PMC_SR & AT91C_PMC_LOCK) )
    ...

where from include/proxmark3.h:

#define PMC_MAIN_OSC_STARTUP_DELAY(x)           ((x)<<8)
#define PMC_PLL_DIVISOR(x)                      (x)
#define PMC_PLL_MULTIPLIER(x)                   (((x)-1)<<16)
#define PMC_PLL_COUNT_BEFORE_LOCK(x)            ((x)<<8)
#define PMC_PLL_FREQUENCY_RANGE(x)              ((x)<<14)
#define PMC_PLL_USB_DIVISOR(x)                  ((x)<<28)

It appears to me from my reading of the AT91SAM7S256 documentation (doc6175.pdf, page 206 and page 564), that
PMC_PLL_COUNT_BEFORE_LOCK(0x50) is pushing a bit into location 14 (of the OUT field) which this code thinks is setting to 0 with PMC_PLL_FREQUENCY_RANGE(0). This consequently gives out the value of (01) which is given as Reserved in some versions of the doc or simply not listed in the others.

Can someone confirm?

Simple patch to fix is included below:

Index: include/proxmark3.h
===================================================================
--- include/proxmark3.h  (revision 459)
+++ include/proxmark3.h  (working copy)
@@ -43,8 +43,8 @@
 #define PMC_MAIN_OSC_STARTUP_DELAY(x)      ((x)<<8)
 #define PMC_PLL_DIVISOR(x)            (x)
 #define PMC_PLL_MULTIPLIER(x)          (((x)-1)<<16)
-#define PMC_PLL_COUNT_BEFORE_LOCK(x)      ((x)<<8)
-#define PMC_PLL_FREQUENCY_RANGE(x)        ((x)<<14)
+#define PMC_PLL_COUNT_BEFORE_LOCK(x)      ((x & 0x3F)<<8)
+#define PMC_PLL_FREQUENCY_RANGE(x)        ((x & 0x2)<<14)
 #define PMC_PLL_USB_DIVISOR(x)          ((x)<<28)
 
 #define UDP_INTERRUPT_ENDPOINT(x)        (1<<(x))
Index: bootrom/bootrom.c
===================================================================
--- bootrom/bootrom.c  (revision 459)
+++ bootrom/bootrom.c  (working copy)
@@ -47,7 +47,7 @@
     // PLL output is MAINCK * multiplier / divisor = 16Mhz * 12 / 2 = 96Mhz
     AT91C_BASE_PMC->PMC_PLLR =
       PMC_PLL_DIVISOR(2) |
-    PMC_PLL_COUNT_BEFORE_LOCK(0x50) |
+    PMC_PLL_COUNT_BEFORE_LOCK(0x10) |
     PMC_PLL_FREQUENCY_RANGE(0) |
     PMC_PLL_MULTIPLIER(12) |
     PMC_PLL_USB_DIVISOR(1);

Offline

#2 2014-07-11 15:32:23

gbhuk
Contributor
Registered: 2012-09-20
Posts: 33

Re: Invalid OUT bits set for CKGR_PPLR in bootloader?

Old post but I think notserpe's point is valid.

The PLLCOUNT field within the 'PMC PLL Clock Generator Register' is 6 bits wide, so the default value of 0x50 in the current bootrom.c source won't fit.  The extra bit spills over into the OUT (frequency range) field, setting bit 0 of this field to 1 - which is invalid.
This is then OR'd with 0 (PMC_PLL_FREQUENCY_RANGE(0)) and so the OUT field remains set to 01 - invalid.

notserpe's proposed fix would work:

+#define PMC_PLL_COUNT_BEFORE_LOCK(x)    ((x & 0x3F)<<8) will limit any data set in the PLLCOUNT field to 6 bits, and
+#define PMC_PLL_FREQUENCY_RANGE(x)        ((x & 0x2)<<14) will ensure that bit 0 of the OUT field is always 0.

The question then is did the original coder expect the PLLCOUNT to be 0x50, 80 clock cycles, or 0x10, 16 clock cycles?

If it was 80 clock cycles then the line in bootrom.c should be PMC_PLL_COUNT_BEFORE_LOCK(0x3F) (max 63 clock cycles).
If it was 16 clock cycles then the line in bootrom.c should be PMC_PLL_COUNT_BEFORE_LOCK(0x10), as notserpe proposes.

Offline

#3 2017-09-25 23:48:55

iceman
Administrator
Registered: 2013-04-25
Posts: 3,964
Website

Re: Invalid OUT bits set for CKGR_PPLR in bootloader?

Revived from the dead.

Found this thread,  pushed a fix.  The suggested fix for the 2bit register is wrong.  You need  (x &  0x3) [0011] not 0x2..

Fix: https://github.com/iceman1001/proxmark3 … e5aa8c7055

The question of clock cycles remain.  I set it as @OP.   PMC_PLL_COUNT_BEFORE_LOCK(0x10)


modhex(hkhehghthbhudcfcdchkigiehgduiehg)

Offline

#4 2017-09-27 04:59:33

marshmellow
Moderator
From: US
Registered: 2013-06-10
Posts: 2,073

Re: Invalid OUT bits set for CKGR_PPLR in bootloader?

I think you missed the point of the 0x2.  His intent was 0x2 to exclude the 1 bit to ensure it couldn't get set causing the proposed invalid value.

Offline

#5 2017-09-27 06:20:59

iceman
Administrator
Registered: 2013-04-25
Posts: 3,964
Website

Re: Invalid OUT bits set for CKGR_PPLR in bootloader?

I most definitly miss the intent if its based on bad programming practices.
If I have a mask for a two bits register, in order to set them,  changing the mask to withhold one bit because a RFU without even comment it...   So, correct bit behavior is there now,  an extra control for not setting a bad value would be needed if it made some difference... but given the 0x50 that has been there since 2010 and nada problem seem to make this a theoretical problem.


modhex(hkhehghthbhudcfcdchkigiehgduiehg)

Offline

Board footer

Powered by FluxBB