Futher review comments and move to specify system freqs in kHz (not MHz)

Author: andygpz11

src/host/pico_platform/include/hardware/platform_defs.h

@@ -17,7 +17,7 @@
 
 #define NUM_SPIN_LOCKS 32u
 
-#define XOSC_MHZ 12
+#define XOSC_KHZ 12000u
 
 #define NUM_SPIN_LOCKS 32u
 

src/rp2040/hardware_regs/include/hardware/platform_defs.h

@@ -37,19 +37,23 @@
 
 #define PIO_INSTRUCTION_COUNT _u(32)
 
-// For USB operation this *has* to be 48 MHz
-#define USB_CLK_MHZ _u(48)
-
-// PICO_CONFIG: SYS_CLK_MHZ, The system operating frequency in MHz, type=int, default=125, advanced=true, group=hardware_base
-#ifndef SYS_CLK_MHZ
-#define SYS_CLK_MHZ _u(125)
+// PICO_CONFIG: XOSC_KHZ, The crystal oscillator frequency in kHz, type=int, default=12000, advanced=true, group=hardware_base
+// NOTE:  The system and USB clocks are generated from the frequency using two PLLs.
+// If you override this define, or SYS_CLK_KHZ below, you will *also* need to add your own adjusted PLL set-up defines to
+// overide the defaults which live in src/rp2_common/hardware_clocks/include/hardware/clocks.h
+// Please see the comments there about calculating the new PLL setting values.
+#ifndef XOSC_KHZ
+#define XOSC_KHZ _u(12000)
 #endif
 
-// PICO_CONFIG: XOSC_MHZ, The crystal oscillator frequency in MHz, type=int, default=12, advanced=true, group=hardware_base
-#ifndef XOSC_MHZ
-#define XOSC_MHZ _u(12)
+// PICO_CONFIG: SYS_CLK_KHZ, The system operating frequency in kHz, type=int, default=125000, advanced=true, group=hardware_base
+#ifndef SYS_CLK_KHZ
+#define SYS_CLK_KHZ _u(125000)
 #endif
 
+// For USB operation this *has* to be 48MHz
+#define USB_CLK_KHZ _u(48000)
+
 #define FIRST_USER_IRQ (NUM_IRQS - NUM_USER_IRQS)
 #define VTABLE_FIRST_IRQ 16
 

src/rp2_common/hardware_clocks/clocks.c

@@ -18,7 +18,7 @@
 // division ratio will be used in the clocks block).  A divisor of 1024 generates
 // an RTC clock tick of 46875Hz.  This frequency is relatively close to the
 // customary 32 or 32.768kHz 'slow clock' crystals and provides good timing resolution.
-#define RTC_CLOCK_FREQ_HZ       (USB_CLK_MHZ * MHZ / 1024)
+#define RTC_CLOCK_FREQ_HZ       (USB_CLK_KHZ * KHZ / 1024)
 
 check_hw_layout(clocks_hw_t, clk[clk_adc].selected, CLOCKS_CLK_ADC_SELECTED_OFFSET);
 check_hw_layout(clocks_hw_t, fc0.result, CLOCKS_FC0_RESULT_OFFSET);
@@ -125,8 +125,8 @@ bool clock_configure(enum clock_index clk_index, uint32_t src, uint32_t auxsrc,
 /// \end::clock_configure[]
 
 void clocks_init(void) {
-    // Start tick in watchdog
-    watchdog_start_tick(XOSC_MHZ);
+    // Start tick in watchdog, the argument is in 'cycles per microsecond' i.e. MHz
+    watchdog_start_tick(XOSC_KHZ / KHZ);
 
     // Everything is 48MHz on FPGA apart from RTC. Otherwise set to 0 and will be set in clock configure
     if (running_on_fpga()) {
@@ -152,55 +152,55 @@ void clocks_init(void) {
         tight_loop_contents();
 
     /// \tag::pll_init[]
-    pll_init(pll_sys, PLL_COMMON_REFDIV, PLL_SYS_VCO_FREQ_MHZ * MHZ, PLL_SYS_POSTDIV1, PLL_SYS_POSTDIV2);
-    pll_init(pll_usb, PLL_COMMON_REFDIV, PLL_USB_VCO_FREQ_MHZ * MHZ, PLL_USB_POSTDIV1, PLL_USB_POSTDIV2);
+    pll_init(pll_sys, PLL_COMMON_REFDIV, PLL_SYS_VCO_FREQ_KHZ * KHZ, PLL_SYS_POSTDIV1, PLL_SYS_POSTDIV2);
+    pll_init(pll_usb, PLL_COMMON_REFDIV, PLL_USB_VCO_FREQ_KHZ * KHZ, PLL_USB_POSTDIV1, PLL_USB_POSTDIV2);
     /// \end::pll_init[]
 
     // Configure clocks
     // CLK_REF = XOSC (usually) 12MHz / 1 = 12MHz
     clock_configure(clk_ref,
                     CLOCKS_CLK_REF_CTRL_SRC_VALUE_XOSC_CLKSRC,
                     0, // No aux mux
-                    XOSC_MHZ * MHZ,
-                    XOSC_MHZ * MHZ);
+                    XOSC_KHZ * KHZ,
+                    XOSC_KHZ * KHZ);
 
     /// \tag::configure_clk_sys[]
     // CLK SYS = PLL SYS (usually) 125MHz / 1 = 125MHz
     clock_configure(clk_sys,
                     CLOCKS_CLK_SYS_CTRL_SRC_VALUE_CLKSRC_CLK_SYS_AUX,
                     CLOCKS_CLK_SYS_CTRL_AUXSRC_VALUE_CLKSRC_PLL_SYS,
-                    SYS_CLK_MHZ * MHZ,
-                    SYS_CLK_MHZ * MHZ);
+                    SYS_CLK_KHZ * KHZ,
+                    SYS_CLK_KHZ * KHZ);
     /// \end::configure_clk_sys[]
 
     // CLK USB = PLL USB 48MHz / 1 = 48MHz
     clock_configure(clk_usb,
                     0, // No GLMUX
                     CLOCKS_CLK_USB_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                    USB_CLK_MHZ * MHZ,
-                    USB_CLK_MHZ * MHZ);
+                    USB_CLK_KHZ * KHZ,
+                    USB_CLK_KHZ * KHZ);
 
     // CLK ADC = PLL USB 48MHZ / 1 = 48MHz
     clock_configure(clk_adc,
                     0, // No GLMUX
                     CLOCKS_CLK_ADC_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                    USB_CLK_MHZ * MHZ,
-                    USB_CLK_MHZ * MHZ);
+                    USB_CLK_KHZ * KHZ,
+                    USB_CLK_KHZ * KHZ);
 
     // CLK RTC = PLL USB 48MHz / 1024 = 46875Hz
     clock_configure(clk_rtc,
                     0, // No GLMUX
                     CLOCKS_CLK_RTC_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                    USB_CLK_MHZ * MHZ,
+                    USB_CLK_KHZ * KHZ,
                     RTC_CLOCK_FREQ_HZ);
 
     // CLK PERI = clk_sys. Used as reference clock for Peripherals. No dividers so just select and enable
     // Normally choose clk_sys or clk_usb
     clock_configure(clk_peri,
                     0,
                     CLOCKS_CLK_PERI_CTRL_AUXSRC_VALUE_CLK_SYS,
-                    SYS_CLK_MHZ * MHZ,
-                    SYS_CLK_MHZ * MHZ);
+                    SYS_CLK_KHZ * KHZ,
+                    SYS_CLK_KHZ * KHZ);
 }
 
 /// \tag::clock_get_hz[]

src/rp2_common/hardware_clocks/include/hardware/clocks.h

@@ -89,36 +89,45 @@ extern "C" {
 #define KHZ 1000
 #define MHZ 1000000
 
-#if (XOSC_MHZ == 12)
+/// \tag::pll_settings[]
+// There are two PLLs in RP2040:
+// The 'SYS PLL' generates the 125MHz system clock.
+// The 'USB PLL' generates the 48MHz USB clock.
+// The defines below are correct for the above two frequencies and a 12MHz crystal
+// frequency, defined as `XOSC_KHZ`.
+//
+// If you override `XOSC_KHZ`, the settings below will need to be checked and possibly
+// revised.  If you override `SYS_CLK_KHZ`, the settings below will need revised!
+// Use `vcocalc.py` to check and calculate new values if you change any of these frequencies.
+/// \end::pll_settings[]
+
 #ifndef PLL_COMMON_REFDIV
 // Software requires that the same 'reference divider' setting is used for both PLLs (although each has its own register).
-// In general, this is not an issue and 1 is the best setting - change with caution!
 #define PLL_COMMON_REFDIV                   1
 #endif
 
- // Pll settings for 48 MHz USB clock are based on being able to get an exact
- // 1200 MHz VCO, they _may_ need revising with a different reference frequency.
-#define PLL_USB_VCO_FREQ_MHZ                1200
+// NOTE:  PLL settings for a USB clock of 48MHz  are based on being able to get an
+// exact 1200MHz VCO, they may need revising with a different reference frequency.
+#ifndef PLL_USB_VCO_FREQ_KHZ
+#define PLL_USB_VCO_FREQ_KHZ                (1200 * KHZ)
+#endif
+#ifndef PLL_USB_POSTDIV1
 #define PLL_USB_POSTDIV1                    5
+#endif
+#ifndef PLL_USB_POSTDIV2
 #define PLL_USB_POSTDIV2                    5
-#else
-#error Use vcocalc.py to calculate correct values for the revised reference clock frequency and define here.
-#endif // XOSC_MHZ == 12
+#endif
 
- // Pll settings for standard system clock
-#if (SYS_CLK_MHZ == 125)
-/// \tag::pll_settings[]
-// Configure PLLs
-//          VCO        POSTDIV 1 & 2
-// PLL SYS: 1500MHz    / 6 / 2 = 125MHz
-// PLL USB: 1200MHz    / 5 / 5 =  48MHz
-/// \end::pll_settings[]
-#define PLL_SYS_VCO_FREQ_MHZ                1500
+// PLL settings for standard system clock with SYS_CLK_KHZ == 125,000
+#ifndef PLL_SYS_VCO_FREQ_KHZ
+#define PLL_SYS_VCO_FREQ_KHZ                (1500 * KHZ)
+#endif
+#ifndef PLL_SYS_POSTDIV1
 #define PLL_SYS_POSTDIV1                    6
+#endif
+#ifndef PLL_SYS_POSTDIV2
 #define PLL_SYS_POSTDIV2                    2
-#else
-#error Use vcocalc.py to calculate correct values for the revised system clock frequency and define here.
-#endif // SYS_CLK_MHZ == 125
+#endif
 
 // PICO_CONFIG: PARAM_ASSERTIONS_ENABLED_CLOCKS, Enable/disable assertions in the clocks module, type=bool, default=0, group=hardware_clocks
 #ifndef PARAM_ASSERTIONS_ENABLED_CLOCKS

src/rp2_common/hardware_pll/include/hardware/pll.h

@@ -31,12 +31,12 @@ typedef pll_hw_t *PLL;
 #define pll_sys pll_sys_hw
 #define pll_usb pll_usb_hw
 
-#ifndef PICO_PLL_VCO_MIN_FREQ_MHZ
-#define PICO_PLL_VCO_MIN_FREQ_MHZ 750
+#ifndef PICO_PLL_VCO_MIN_FREQ_KHZ
+#define PICO_PLL_VCO_MIN_FREQ_KHZ (750 * KHZ)
 #endif
 
-#ifndef PICO_PLL_VCO_MAX_FREQ_MHZ
-#define PICO_PLL_VCO_MAX_FREQ_MHZ 1600
+#ifndef PICO_PLL_VCO_MAX_FREQ_KHZ
+#define PICO_PLL_VCO_MAX_FREQ_KHZ (1600 * KHZ)
 #endif
 
 /*! \brief Initialise specified PLL.

src/rp2_common/hardware_pll/pll.c

@@ -4,17 +4,17 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-// For MHZ definitions etc
+// For frequency and PLL definitions etc.
 #include "hardware/clocks.h"
 #include "hardware/pll.h"
 #include "hardware/resets.h"
 
 /// \tag::pll_init_calculations[]
 void pll_init(PLL pll, uint refdiv, uint vco_freq, uint post_div1, uint post_div2) {
-    uint32_t ref_freq = XOSC_MHZ * MHZ / refdiv;
+    uint32_t ref_freq = XOSC_KHZ * KHZ / refdiv;
 
     // Check vco freq is in an acceptable range
-    assert(vco_freq >= (PICO_PLL_VCO_MIN_FREQ_MHZ * MHZ) && vco_freq <= (PICO_PLL_VCO_MAX_FREQ_MHZ * MHZ));
+    assert(vco_freq >= (PICO_PLL_VCO_MIN_FREQ_KHZ * KHZ) && vco_freq <= (PICO_PLL_VCO_MAX_FREQ_KHZ * KHZ));
 
     // What are we multiplying the reference clock by to get the vco freq
     // (The regs are called div, because you divide the vco output and compare it to the refclk)

src/rp2_common/hardware_xosc/xosc.c

@@ -6,18 +6,18 @@
 
 #include "pico.h"
 
-// For MHZ definitions etc
+// For frequency related definitions etc
 #include "hardware/clocks.h"
 
 #include "hardware/platform_defs.h"
 #include "hardware/regs/xosc.h"
 #include "hardware/xosc.h"
 
-#if XOSC_MHZ < 1 || XOSC_MHZ > 50
-#error XOSC_MHZ must be in the range 1-50
+#if XOSC_KHZ < (1 * KHZ) || XOSC_KHZ > (15 * KHZ)
+#error XOSC_KHZ must be in the range 1,000-15,000KHz i.e. 1-15MHz
 #endif
 
-#define STARTUP_DELAY (((((XOSC_MHZ * MHZ) / 1000) + 128) / 256) * PICO_XOSC_STARTUP_DELAY_MULTIPLIER)
+#define STARTUP_DELAY (((((XOSC_KHZ * KHZ) / 1000) + 128) / 256) * PICO_XOSC_STARTUP_DELAY_MULTIPLIER)
 
 // The DELAY field in xosc_hw->startup is 14 bits wide.
 #if STARTUP_DELAY >= (1 << 13)

src/rp2_common/pico_stdlib/stdlib.c

@@ -21,8 +21,8 @@ void set_sys_clock_48mhz() {
         clock_configure(clk_sys,
                         CLOCKS_CLK_SYS_CTRL_SRC_VALUE_CLKSRC_CLK_SYS_AUX,
                         CLOCKS_CLK_SYS_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                        USB_CLK_MHZ * MHZ,
-                        USB_CLK_MHZ * MHZ);
+                        USB_CLK_KHZ * KHZ,
+                        USB_CLK_KHZ * KHZ);
 
         // Turn off PLL sys for good measure
         pll_deinit(pll_sys);
@@ -31,8 +31,8 @@ void set_sys_clock_48mhz() {
         clock_configure(clk_peri,
                         0,
                         CLOCKS_CLK_PERI_CTRL_AUXSRC_VALUE_CLK_SYS,
-                        USB_CLK_MHZ * MHZ,
-                        USB_CLK_MHZ * MHZ);
+                        USB_CLK_KHZ * KHZ,
+                        USB_CLK_KHZ * KHZ);
     }
 }
 
@@ -41,8 +41,8 @@ void set_sys_clock_pll(uint32_t vco_freq, uint post_div1, uint post_div2) {
         clock_configure(clk_sys,
                         CLOCKS_CLK_SYS_CTRL_SRC_VALUE_CLKSRC_CLK_SYS_AUX,
                         CLOCKS_CLK_SYS_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                        USB_CLK_MHZ * MHZ,
-                        USB_CLK_MHZ * MHZ);
+                        USB_CLK_KHZ * KHZ,
+                        USB_CLK_KHZ * KHZ);
 
         pll_init(pll_sys, PLL_COMMON_REFDIV, vco_freq, post_div1, post_div2);
         uint32_t freq = vco_freq / (post_div1 * post_div2);
@@ -52,8 +52,8 @@ void set_sys_clock_pll(uint32_t vco_freq, uint post_div1, uint post_div2) {
         clock_configure(clk_ref,
                         CLOCKS_CLK_REF_CTRL_SRC_VALUE_XOSC_CLKSRC,
                         0, // No aux mux
-                        XOSC_MHZ * MHZ,
-                        XOSC_MHZ * MHZ);
+                        XOSC_KHZ * KHZ,
+                        XOSC_KHZ * KHZ);
 
         // CLK SYS = PLL SYS (usually) 125MHz / 1 = 125MHz
         clock_configure(clk_sys,
@@ -64,16 +64,16 @@ void set_sys_clock_pll(uint32_t vco_freq, uint post_div1, uint post_div2) {
         clock_configure(clk_peri,
                         0, // Only AUX mux on ADC
                         CLOCKS_CLK_PERI_CTRL_AUXSRC_VALUE_CLKSRC_PLL_USB,
-                        USB_CLK_MHZ * MHZ,
-                        USB_CLK_MHZ * MHZ);
+                        USB_CLK_KHZ * KHZ,
+                        USB_CLK_KHZ * KHZ);
     }
 }
 
 bool check_sys_clock_khz(uint32_t freq_khz, uint *vco_out, uint *postdiv1_out, uint *postdiv2_out) {
-    uint reference_freq_khz = XOSC_MHZ * KHZ / PLL_COMMON_REFDIV;
+    uint reference_freq_khz = XOSC_KHZ / PLL_COMMON_REFDIV;
     for (uint fbdiv = 320; fbdiv >= 16; fbdiv--) {
         uint vco = fbdiv * reference_freq_khz;
-        if (vco < PICO_PLL_VCO_MIN_FREQ_MHZ * KHZ  || vco > PICO_PLL_VCO_MAX_FREQ_MHZ * KHZ) continue;
+        if (vco < PICO_PLL_VCO_MIN_FREQ_KHZ  || vco > PICO_PLL_VCO_MAX_FREQ_KHZ) continue;
         for (uint postdiv1 = 7; postdiv1 >= 1; postdiv1--) {
             for (uint postdiv2 = postdiv1; postdiv2 >= 1; postdiv2--) {
                 uint out = vco / (postdiv1 * postdiv2);