glitch-free clock not glitch-free

[bruelltuete]

I took inspiration from clocks_init() for switching clocks for entering and leaving power-save sleeping.

clocks_init() first switches clk_ref and clk_sys to a save input, specifically rosc.
But in the order of clk_sys to clk_ref, and afterwards clk_ref to rosc.
It then switches clk_ref to xosc and thus clk_sys indirectly as well.

I have observed that switching clk_ref and indirectly clk_sys in such a way will introduce glitches into clk_sys.
To avoid you'd have to switch clk_sys to anything else first.

(c.f. https://forums.raspberrypi.com/viewtopic.php?t=347154)

[P33M]

Please provide a minimal C SDK testcase example that demonstrates the lockup.

[bruelltuete]

I'll come up with an example...

But just to be clear: I have no way of observing clk_sys directly, I can only see the aftermath of the glitching: xip cache and memory corruption.
Switching clk_sys away from clk_ref first makes all the problems go away.

The pseudo code looks like:

while (true)
{
   do_stuff();

   // switch clk_ref & sys to rosc
   // go to sleep

   // wake up from rtc alarm
   // switch clk_ref & sys to xosc                      <-- after a while bad stuff happens here
   // switch clk_sys to pll
}

[bruelltuete]

This crashes after a few seconds:

#include "hardware/clocks.h"
#include "hardware/structs/xip_ctrl.h"

static void __no_inline_not_in_flash_func(xipflush)()
{
    xip_ctrl_hw->flush = XIP_FLUSH_BITS;
    volatile int a = xip_ctrl_hw->flush;
}

int main()
{

    while (true)
    {
        // flush xip cache to make sure that there's something observable relying on clk_sys.
        // if it glitches then fetching instruction bytes from flash will go wrong pretty quickly.
        // if there are no glitches then nothing bad happens.
        xipflush();

        clocks_init();
    }
}

[bruelltuete]

maybe related: should xosc_init() at https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/hardware_clocks/clocks.c#L138 move below the while loops?

[P33M]

xosc_init should be benign if called on an already-enabled xosc.

One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked.

[bruelltuete]

One thing to note is that if you have clksrc -> clk_ref -> clk_sys, and you switch clk_ref between its two glitchless sources, then there is a period of time where the whole chip is not clocked.

I think that pause should be ok... as long as there's no pheripheral i/o depending on that (sys-)clock.

edit: and in the case of xip, I was expecting the pause to not matter because there is an explicit clock signal to the flash.

[bruelltuete]

any thoughts? was this reproducible on your end?

[P33M]

Unfortunately not. Works fine on a Pico (B1 step) that I have here, in either relwithdebuginfo or debug builds.

[bruelltuete]

dang...
I only have B2s here.

[alastairpatrick]

I can reproduce this on a B1 board every time. Example GDB session, starting with it paused on entry to main():

> disassemble __xipflush_veneer
Dump of assembler code for function __xipflush_veneer:
   0x10003cf8 <+0>:	push	{r0}
   0x10003cfa <+2>:	ldr	r0, [pc, #8]	; (0x10003d04 <__xipflush_veneer+12>)
   0x10003cfc <+4>:	mov	r12, r0
   0x10003cfe <+6>:	pop	{r0}
   0x10003d00 <+8>:	bx	r12
   0x10003d02 <+10>:	nop
   0x10003d04 <+12>:	lsls	r1, r0, #3
   0x10003d06 <+14>:	movs	r0, #0
End of assembler dump.
> continue
Continuing.

Note that the disassembly is of a function in XIP cache and the disassembly is correct. Now wait for it to crash, which takes less than one second:

Thread 
1 stopped.
main () at D:\src\rprand\main.c:18
18	        xipflush();
> disassemble __xipflush_veneer
Dump of assembler code for function __xipflush_veneer:
   0x10003cf8 <+0>:	push	{r0}
   0x10003cfa <+2>:	ldr	r0, [pc, #8]	; (0x10003d04 <__xipflush_veneer+12>)
   0x10003cfc <+4>:	mov	r12, r0
   0x10003cfe <+6>:	pop	{r0}
   0x10003d00 <+8>:	movs	r0, r0
   0x10003d02 <+10>:	movs	r0, r0
   0x10003d04 <+12>:	movs	r0, r0
   0x10003d06 <+14>:	movs	r0, r0
End of assembler dump.

Note that the disassembly of the function has changed and is incorrect, suggesting that XIP read bogus data from flash.

[P33M]

Please post the full project directory (including cmakelists.txt) as a zip.

[alastairpatrick]

Project attached.
repro.zip