set_sys_clock_khz() cannot be called multiple times #94

New issue

Closed

opened 2021-02-07 15:39:59 -06:00 by Hermann-SW · 4 comments

Hermann-SW commented

2021-02-07 15:39:59 -06:00

(Migrated from github.com)

Minicom output:

Press CTRL-A Z for help on special keys

tud_cdc_connected()
foobar
ok
40_000,424000,440000
408000,424000,440000

*** PANIC ***

System clock of 424000 kHz cannot be exactly achiev

The ok output shows that set_sys_clock_khz(424000, false) returned true.
But calling set_sys_clock_khz(424000, true) afterwards results in assert(panic).

Code:

#include "pico/stdlib.h"
#include "hardware/vreg.h"
#include <tusb.h>

#define N 1000000

int main() {
    stdio_init_all();
    while (!tud_cdc_connected()) { sleep_ms(100);  }
    printf("tud_cdc_connected()\n");

    gpio_init(3);  gpio_set_dir(3, GPIO_OUT);

    vreg_set_voltage(VREG_VOLTAGE_1_30);
    sleep_ms(1);

    int mi, lo = 408000, hi = 440000;
    do
    {
      set_sys_clock_48mhz(); sleep_ms(200); printf("foobar\n");

      mi = (lo+hi)/2;
      if (set_sys_clock_khz(mi, false))
        printf("ok\n");

      printf("%d,%d,%d\n",lo,mi,hi);
      printf("%d,%d,%d\n",lo,mi,hi);
      fflush(stdout);
      set_sys_clock_khz(mi, true);
    }
    while (hi-lo>1);

    printf("done\n");
}

I need to figure out if a I can set system clock to a specific value. In case above behavior is considered OK (hopefully not), another function is needed to query whether setting system clock with 2nd argument true would panic or not.

For automatically determining maximal overclock frequency, too high clock frequencies need to be tested (binary search). I stripped down my code to very short example demonstrating the inconsistent behavior for frequency 424000 KHz. Minicom output: ``` Press CTRL-A Z for help on special keys tud_cdc_connected() foobar ok 40_000,424000,440000 408000,424000,440000 *** PANIC *** System clock of 424000 kHz cannot be exactly achiev ``` The ok output shows that set_sys_clock_khz(424000, false) returned true. But calling set_sys_clock_khz(424000, true) afterwards results in assert(panic). Code: ``` #include "pico/stdlib.h" #include "hardware/vreg.h" #include <tusb.h> #define N 1000000 int main() { stdio_init_all(); while (!tud_cdc_connected()) { sleep_ms(100); } printf("tud_cdc_connected()\n"); gpio_init(3); gpio_set_dir(3, GPIO_OUT); vreg_set_voltage(VREG_VOLTAGE_1_30); sleep_ms(1); int mi, lo = 408000, hi = 440000; do { set_sys_clock_48mhz(); sleep_ms(200); printf("foobar\n"); mi = (lo+hi)/2; if (set_sys_clock_khz(mi, false)) printf("ok\n"); printf("%d,%d,%d\n",lo,mi,hi); printf("%d,%d,%d\n",lo,mi,hi); fflush(stdout); set_sys_clock_khz(mi, true); } while (hi-lo>1); printf("done\n"); } ``` I need to figure out if a I can set system clock to a specific value. In case above behavior is considered OK (hopefully not), another function is needed to query whether setting system clock with 2nd argument true would panic or not.

kilograham commented

2021-02-07 18:27:35 -06:00

(Migrated from github.com)

There is a function

/*! \brief Check if a given system clock frequency is valid/attainable
 *  \ingroup pico_stdlib
 *
 * \param freq_khz Requested frequency
 * \param vco_freq_out On success, the voltage controller oscillator frequeucny to be used by the SYS PLL
 * \param post_div1_out On success, The first post divider for the SYS PLL
 * \param post_div2_out On success, The second post divider for the SYS PLL.
 * @return true if the frequency is possible and the output parameters have been written.
 */
bool check_sys_clock_khz(uint32_t freq_khz, uint *vco_freq_out, uint *post_div1_out, uint *post_div2_out);

however this is what is called by set_sys_clock_khz(..., false)

It seems like set_sys_clock_khz() fails if it is called twice; this is a bug

There is a function ``` /*! \brief Check if a given system clock frequency is valid/attainable * \ingroup pico_stdlib * * \param freq_khz Requested frequency * \param vco_freq_out On success, the voltage controller oscillator frequeucny to be used by the SYS PLL * \param post_div1_out On success, The first post divider for the SYS PLL * \param post_div2_out On success, The second post divider for the SYS PLL. * @return true if the frequency is possible and the output parameters have been written. */ bool check_sys_clock_khz(uint32_t freq_khz, uint *vco_freq_out, uint *post_div1_out, uint *post_div2_out); ``` however this is what is called by `set_sys_clock_khz(..., false)` It seems like set_sys_clock_khz() fails if it is called twice; this is a bug

Wren6991 commented

2021-02-09 10:13:31 -06:00

(Migrated from github.com)

I need to figure out if a I can set system clock to a specific value. In case above behavior is considered OK (hopefully not), another function is needed to query whether setting system clock with 2nd argument true would panic or not.

Your repro is running the system at 424 MHz the second time the function is called. See the function description here:

/*! \brief Attempt to set a system clock frequency in khz
 *  \ingroup pico_stdlib
 *
 * Note that not all clock frequencies are possible; it is preferred that you
 * use src/rp2_common/hardware_clocks/scripts/vcocalc.py to calculate the parameters
 * for use with set_sys_clock_pll
 *
 * \param freq_khz Requested frequency
 * \param required if true then this function will assert if the frequency is not attainable.
 * \return true if the clock was configured
 */
static inline bool set_sys_clock_khz(uint32_t freq_khz, bool required) {

When I run your code unmodified (i.e. trying to run the system at 424 MHz), it crashes immediately for me; I don't get the same failure you see. When I reduce the frequency to something within the range given in our datasheet, the function works as documented and as expected.

Please post a complete reproducible example that does not run the chip faster than 133 MHz.

> I need to figure out if a I can set system clock to a specific value. **In case above behavior is considered OK (hopefully not),** another function is needed to query whether setting system clock with 2nd argument true would panic or not. Your repro is **running the system at 424 MHz** the second time the function is called. See the function description here: ``` /*! \brief Attempt to set a system clock frequency in khz * \ingroup pico_stdlib * * Note that not all clock frequencies are possible; it is preferred that you * use src/rp2_common/hardware_clocks/scripts/vcocalc.py to calculate the parameters * for use with set_sys_clock_pll * * \param freq_khz Requested frequency * \param required if true then this function will assert if the frequency is not attainable. * \return true if the clock was configured */ static inline bool set_sys_clock_khz(uint32_t freq_khz, bool required) { ``` When I run your code unmodified (i.e. trying to run the system at 424 MHz), it crashes immediately for me; I don't get the same failure you see. When I reduce the frequency to something within the range given in our datasheet, the function works as documented and as expected. Please post a complete reproducible example that does not run the chip faster than 133 MHz.

Wren6991 commented

2021-02-11 06:03:14 -06:00

(Migrated from github.com)

Please open another issue if you are able to reproduce this behaviour whilst running within the frequency range given in the datasheet.

Hermann-SW commented

2021-02-16 08:05:23 -06:00

(Migrated from github.com)

I found a way to deal with the high frequencies and automatically determine the highest safe frequency per voltage:
https://www.raspberrypi.org/forums/viewtopic.php?f=145&t=301902&start=50#p1820197

I had to call "set_sys_clock_khz(_, false)" twice, because calling 2nd time after 1st call confirmed that frequency is valid crashed.
I can understand that you close this bug, I will have to see whether I get the crash as well with <=133MHz.
In that case I would reopen this bug, because your closing argument was running above rater 133MHz frequency.

The Pico is a nice piece of hardware, running safely with >420MHz with four different Picos with working GPIO at 1.3V is so cool (for applications needing overclocking). Even USB output at such high frequencies is possible, one has just to clock down (I use "set_sys_clock_48MHz()") and wait a bit (I do "sleep_ms(2)", then printf works, and afterwards overclocking again works as well.

I found a way to deal with the high frequencies and automatically determine the highest safe frequency per voltage: https://www.raspberrypi.org/forums/viewtopic.php?f=145&t=301902&start=50#p1820197 I had to call "set_sys_clock_khz(_, false)" twice, because calling 2nd time after 1st call confirmed that frequency is valid crashed. I can understand that you close this bug, I will have to see whether I get the crash as well with <=133MHz. In that case I would reopen this bug, because your closing argument was running above rater 133MHz frequency. The Pico is a nice piece of hardware, running safely with >420MHz with four different Picos with working GPIO at 1.3V is so cool (for applications needing overclocking). Even USB output at such high frequencies is possible, one has just to clock down (I use "set_sys_clock_48MHz()") and wait a bit (I do "sleep_ms(2)", then printf works, and afterwards overclocking again works as well.