add PICO_USE_FASTEST_SUPPORTED_CLOCK, and support vreg setting and SYS_CLOCK_MHZ=200 for rp2040 #2285

2025-02-17T10:35:27-06:00

kilograham commented

2025-02-17 10:35:27 -06:00

(Migrated from github.com)

Also adds vreg_get_voltage()

Also adds `vreg_get_voltage()`

Wren6991 (Migrated from github.com) reviewed

2025-02-17 10:35:27 -06:00

peterharperuk (Migrated from github.com) reviewed

2025-02-17 10:35:27 -06:00

P33M (Migrated from github.com) reviewed

2025-02-17 10:46:10 -06:00

src/rp2_common/hardware_vreg/vreg.c

					
				@ -47,2 +48,3 @@

				    // The voltage limit can't be disabled on RP2040 (was implemented by hard-wiring the LDO controls)

				#else

				    panic_unsupported();

				    hw_set_bits(&powman_hw->vreg_ctrl, POWMAN_PASSWORD_BITS | POWMAN_VREG_CTRL_DISABLE_VOLTAGE_LIMIT_BITS);

else if PICO_RP2350 / else panic?

src/rp2_common/pico_runtime_init/runtime_init_clocks.c

Why is 1ms enough?

lurch (Migrated from github.com) reviewed

2025-02-17 10:51:05 -06:00

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

					
				@ -64,0 +65,4 @@

				#ifndef PICO_USE_FASTEST_SUPPORTED_CLOCK

				#define PICO_USE_FASTEST_SUPPORTED_CLOCK 0

				#endif

Given that this setting is used by code within rp2_common, would it make sense to also add this PICO_CONFIG: to one of the rp2350 header files?

Given that this setting is used by code within `rp2_common`, would it make sense to also add this `PICO_CONFIG:` to one of the rp2350 header files?

P33M commented

2025-02-17 11:02:16 -06:00

(Migrated from github.com)

IMO SYS_CLK_VREG_VOLTAGE_MIN is misnamed - it's a frequency scaling operating point, not a minimum. If you're not wanting to boost clk_sys then the "minimum" shouldn't be applied, as it just wastes power. Such OPPs are usually referred to as turbo/boost settings.

IMO `SYS_CLK_VREG_VOLTAGE_MIN` is misnamed - it's a frequency scaling operating point, not a minimum. If you're not wanting to boost clk_sys then the "minimum" shouldn't be applied, as it just wastes power. Such OPPs are usually referred to as turbo/boost settings.

lurch commented

2025-02-17 11:03:18 -06:00

(Migrated from github.com)

There are a couple of small tweaks needed to stop tools/extract_configs.py getting upset...

diff --git a/src/rp2040/hardware_regs/include/hardware/platform_defs.h b/src/rp2040/hardware_regs/include/hardware/platform_defs.h
index e1d368e..9566e1f 100644
--- a/src/rp2040/hardware_regs/include/hardware/platform_defs.h
+++ b/src/rp2040/hardware_regs/include/hardware/platform_defs.h
@@ -69,6 +69,9 @@
 #define SYS_CLK_HZ ((SYS_CLK_MHZ) * _u(1000000))
 #else
 // PICO_CONFIG: PICO_USE_FASTEST_SUPPORTED_CLOCK, Use the fastest officially supported clock by default, type=bool, default=0, group=hardware_base
+#ifndef PICO_USE_FASTEST_SUPPORTED_CLOCK
+#define PICO_USE_FASTEST_SUPPORTED_CLOCK 0
+#endif
 #if PICO_USE_FASTEST_SUPPORTED_CLOCK
 #define SYS_CLK_HZ _u(200000000)
 #else
diff --git a/src/rp2_common/hardware_clocks/include/hardware/clocks.h b/src/rp2_common/hardware_clocks/include/hardware/clocks.h
index 217bb7d..1a598cf 100644
--- a/src/rp2_common/hardware_clocks/include/hardware/clocks.h
+++ b/src/rp2_common/hardware_clocks/include/hardware/clocks.h
@@ -225,6 +225,10 @@ extern "C" {
 #ifndef PLL_SYS_POSTDIV2
 #define PLL_SYS_POSTDIV2                    1
 #endif
+#else
+#ifndef SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST
+#define SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST 0
+#endif
 #endif // PICO_RP2040 && SYS_CLK_KHZ == 200000 && XOSC_KHZ == 12000 && PLL_COMMON_REFDIV == 1
 
 #if !defined(PLL_SYS_VCO_FREQ_HZ) || !defined(PLL_SYS_POSTDIV1) || !defined(PLL_SYS_POSTDIV2)

There are a couple of small tweaks needed to stop `tools/extract_configs.py` getting upset... ```diff diff --git a/src/rp2040/hardware_regs/include/hardware/platform_defs.h b/src/rp2040/hardware_regs/include/hardware/platform_defs.h index e1d368e..9566e1f 100644 --- a/src/rp2040/hardware_regs/include/hardware/platform_defs.h +++ b/src/rp2040/hardware_regs/include/hardware/platform_defs.h @@ -69,6 +69,9 @@ #define SYS_CLK_HZ ((SYS_CLK_MHZ) * _u(1000000)) #else // PICO_CONFIG: PICO_USE_FASTEST_SUPPORTED_CLOCK, Use the fastest officially supported clock by default, type=bool, default=0, group=hardware_base +#ifndef PICO_USE_FASTEST_SUPPORTED_CLOCK +#define PICO_USE_FASTEST_SUPPORTED_CLOCK 0 +#endif #if PICO_USE_FASTEST_SUPPORTED_CLOCK #define SYS_CLK_HZ _u(200000000) #else diff --git a/src/rp2_common/hardware_clocks/include/hardware/clocks.h b/src/rp2_common/hardware_clocks/include/hardware/clocks.h index 217bb7d..1a598cf 100644 --- a/src/rp2_common/hardware_clocks/include/hardware/clocks.h +++ b/src/rp2_common/hardware_clocks/include/hardware/clocks.h @@ -225,6 +225,10 @@ extern "C" { #ifndef PLL_SYS_POSTDIV2 #define PLL_SYS_POSTDIV2 1 #endif +#else +#ifndef SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST +#define SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST 0 +#endif #endif // PICO_RP2040 && SYS_CLK_KHZ == 200000 && XOSC_KHZ == 12000 && PLL_COMMON_REFDIV == 1 #if !defined(PLL_SYS_VCO_FREQ_HZ) || !defined(PLL_SYS_POSTDIV1) || !defined(PLL_SYS_POSTDIV2) ```

kilograham (Migrated from github.com) reviewed

2025-02-17 11:37:17 -06:00

src/rp2_common/pico_runtime_init/runtime_init_clocks.c

Empirically this seems to work for most people, but I'll make it a define

kilograham commented

2025-02-17 11:39:22 -06:00

(Migrated from github.com)

IMO SYS_CLK_VREG_VOLTAGE_MIN is misnamed - it's a frequency scaling operating point, not a minimum. If you're not wanting to boost clk_sys then the "minimum" shouldn't be applied, as it just wastes power. Such OPPs are usually referred to as turbo/boost settings.

It is the minimum voltage for the requested sys_clock; i.e. its value is dependent on the sys clock you choose, and the clock setup will increase the current voltage if it is less than this

> IMO `SYS_CLK_VREG_VOLTAGE_MIN` is misnamed - it's a frequency scaling operating point, not a minimum. If you're not wanting to boost clk_sys then the "minimum" shouldn't be applied, as it just wastes power. Such OPPs are usually referred to as turbo/boost settings. It is the minimum voltage for the requested sys_clock; i.e. its value is dependent on the sys clock you choose, and the clock setup will increase the current voltage if it is less than this

kilograham (Migrated from github.com) reviewed

2025-02-17 11:51:58 -06:00

src/rp2_common/hardware_vreg/vreg.c

					
				@ -47,2 +48,3 @@

				    // The voltage limit can't be disabled on RP2040 (was implemented by hard-wiring the LDO controls)

				#else

				    panic_unsupported();

				    hw_set_bits(&powman_hw->vreg_ctrl, POWMAN_PASSWORD_BITS | POWMAN_VREG_CTRL_DISABLE_VOLTAGE_LIMIT_BITS);

stylistically, we don't do that - might never be needed

lurch (Migrated from github.com) reviewed

2025-02-17 12:00:29 -06:00

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

					
				@ -209,0 +228,4 @@

				#else

				#ifndef SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST

				#define SYS_CLK_VREG_VOLTAGE_AUTO_ADJUST 0

				#endif

"regular voltage" -> "regulator voltage" ?

lurch (Migrated from github.com) reviewed

2025-02-17 17:52:00 -06:00

src/rp2_common/pico_runtime_init/runtime_init_clocks.c

This might be a stupid question, but given that there's a division by 1000000 in there, would it be possible to rearrange this calculation to not use 64-bit arithmetic? (given that we've already got at least one PR that is attempting to reduce the amount of 64-bit maths)