-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add clock measurement support, fix VCO min freq & add GPin0/1 clock source support. #683
base: main
Are you sure you want to change the base?
Changes from all commits
08504b6
4dbadef
8445d1f
442b662
6cbfae2
fc6a348
8b48e81
3766cde
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,7 @@ | |
//! See [Chapter 2 Section 15](https://datasheets.raspberrypi.org/rp2040/rp2040_datasheet.pdf) for more details | ||
use core::{convert::Infallible, marker::PhantomData}; | ||
use fugit::{HertzU32, RateExtU32}; | ||
use pac::clocks::fc0_src::FC0_SRC_A; | ||
|
||
use crate::{ | ||
pac::{self, CLOCKS, PLL_SYS, PLL_USB, RESETS, XOSC}, | ||
|
@@ -79,9 +80,34 @@ use crate::{ | |
mod macros; | ||
mod clock_sources; | ||
|
||
use clock_sources::PllSys; | ||
|
||
use self::clock_sources::{GPin0, GPin1, PllUsb, Rosc, Xosc}; | ||
pub use clock_sources::{GPin0, GPin1}; | ||
|
||
use clock_sources::{PllSys, PllUsb, Rosc, Xosc}; | ||
|
||
/// Frequency counter accuracy | ||
/// | ||
/// See: https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf#table-fc-test-interval | ||
#[repr(u8)] | ||
#[allow(missing_docs)] | ||
#[allow(clippy::enum_variant_names)] | ||
pub enum FCAccuracy { | ||
_2048kHz = 0, | ||
_1024kHz, | ||
_512kHz, | ||
_256kHz, | ||
_128kHz, | ||
_64kHz, | ||
_32kHz, | ||
_16kHz, | ||
_8kHz, | ||
_4kHz, | ||
_2kHz, | ||
_1kHz, | ||
_500Hz, | ||
_250Hz, | ||
_125Hz, | ||
_62_5Hz, | ||
} | ||
|
||
#[derive(Copy, Clone)] | ||
/// Provides refs to the CLOCKS block. | ||
|
@@ -111,6 +137,11 @@ pub enum ClockError { | |
FrequencyTooLow, | ||
} | ||
|
||
/// The clock stopped while its frequency was being measured. | ||
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] | ||
#[cfg_attr(feature = "defmt", derive(defmt::Format))] | ||
pub struct ClockDiedError; | ||
|
||
/// For clocks | ||
pub trait Clock: Sealed + Sized { | ||
/// Enum with valid source clocks register values for `Clock` | ||
|
@@ -167,6 +198,9 @@ pub trait StoppableClock: Sealed { | |
|
||
/// Trait for things that can be used as clock source | ||
pub trait ClockSource: Sealed { | ||
/// Associated Frequency counter source. | ||
const FCOUNTER_SRC: FC0_SRC_A; | ||
|
||
/// Get the operating frequency for this source | ||
/// | ||
/// Used to determine the divisor | ||
|
@@ -301,6 +335,63 @@ impl ClocksManager { | |
.configure_clock(&self.system_clock, self.system_clock.freq()) | ||
} | ||
|
||
/// Measure the frequency of the given clock source by approximation. | ||
pub fn measure_frequency<C: ClockSource>( | ||
&mut self, | ||
_trg_clk: C, | ||
accuracy: FCAccuracy, | ||
) -> Result<HertzU32, ClockDiedError> { | ||
// Wait for the frequency counter to be ready | ||
while self.clocks.fc0_status.read().running().bit_is_set() { | ||
core::hint::spin_loop() | ||
} | ||
|
||
// Set the speed of the reference clock in kHz. | ||
self.clocks.fc0_ref_khz.write(|w| unsafe { | ||
w.fc0_ref_khz() | ||
.bits(self.reference_clock.get_freq().to_kHz()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not obvious if this is safe: |
||
}); | ||
|
||
// > The test interval is 0.98us * 2**interval, but let's call it 1us * 2**interval. | ||
// > The default gives a test interval of 250us | ||
// | ||
// https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf#reg-clocks-FC0_INTERVAL | ||
// https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf#table-fc-test-interval | ||
self.clocks | ||
.fc0_interval | ||
.write(|w| unsafe { w.fc0_interval().bits(accuracy as u8) }); | ||
|
||
// We don't really care about the min/max, so these are just set to min/max values. | ||
self.clocks | ||
.fc0_min_khz | ||
.write(|w| unsafe { w.fc0_min_khz().bits(0) }); | ||
self.clocks | ||
.fc0_max_khz | ||
.write(|w| unsafe { w.fc0_max_khz().bits(0xffffffff) }); | ||
|
||
// Select which clock to measure. | ||
self.clocks | ||
.fc0_src | ||
.write(|w| w.fc0_src().variant(C::FCOUNTER_SRC)); | ||
|
||
// Wait until the measurement is ready | ||
let mut status; | ||
loop { | ||
status = self.clocks.fc0_status.read(); | ||
if status.done().bit_is_set() { | ||
break; | ||
} | ||
} | ||
|
||
if status.fail().bit_is_set() { | ||
Err(ClockDiedError) | ||
} else { | ||
let result = self.clocks.fc0_result.read(); | ||
let speed_hz = result.khz().bits() * 1000 + u32::from(result.frac().bits()); | ||
Ok(speed_hz.Hz()) | ||
} | ||
} | ||
|
||
/// Releases the CLOCKS block | ||
pub fn free(self) -> CLOCKS { | ||
self.clocks | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,10 +127,10 @@ pub mod common_configs { | |
|
||
/// Default, nominal configuration for PLL_USB. | ||
pub const PLL_USB_48MHZ: PLLConfig = PLLConfig { | ||
vco_freq: HertzU32::MHz(480), | ||
vco_freq: HertzU32::MHz(960), | ||
refdiv: 1, | ||
post_div1: 5, | ||
post_div2: 2, | ||
post_div2: 4, | ||
}; | ||
} | ||
|
||
|
@@ -141,18 +141,22 @@ impl<D: PhaseLockedLoopDevice> PhaseLockedLoop<Disabled, D> { | |
xosc_frequency: HertzU32, | ||
config: PLLConfig, | ||
) -> Result<PhaseLockedLoop<Disabled, D>, Error> { | ||
const VCO_FREQ_RANGE: RangeInclusive<HertzU32> = HertzU32::MHz(400)..=HertzU32::MHz(1_600); | ||
const VCO_FREQ_RANGE: RangeInclusive<HertzU32> = HertzU32::MHz(750)..=HertzU32::MHz(1_600); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we consider this a breaking change and defer it to version 0.10.0? In any case: This will break with the current value of IMHO we should change the value of PLL_USB_48MHZ now (as it's not a breaking change), and change VCO_FREQ_RANGE with version 0.10.0. PLL_USB_48MHZ should become:
(New values taken from https://github.com/raspberrypi/pico-sdk/pull/869/files) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (BTW: This is the error I get when trying to run it without fixing PLL_USB_48MHZ:
) |
||
const POSTDIV_RANGE: Range<u8> = 1..7; | ||
const FBDIV_RANGE: Range<u16> = 16..320; | ||
|
||
let vco_freq = config.vco_freq; | ||
let PLLConfig { | ||
vco_freq, | ||
refdiv, | ||
post_div1, | ||
post_div2, | ||
} = config; | ||
|
||
if !VCO_FREQ_RANGE.contains(&vco_freq) { | ||
return Err(Error::VcoFreqOutOfRange); | ||
} | ||
|
||
if !POSTDIV_RANGE.contains(&config.post_div1) || !POSTDIV_RANGE.contains(&config.post_div2) | ||
{ | ||
if !POSTDIV_RANGE.contains(&post_div1) || !POSTDIV_RANGE.contains(&post_div2) { | ||
return Err(Error::PostDivOutOfRage); | ||
} | ||
|
||
|
@@ -161,7 +165,7 @@ impl<D: PhaseLockedLoopDevice> PhaseLockedLoop<Disabled, D> { | |
|
||
let ref_freq_hz: HertzU32 = xosc_frequency | ||
.to_Hz() | ||
.checked_div(u32::from(config.refdiv)) | ||
.checked_div(u32::from(refdiv)) | ||
.ok_or(Error::BadArgument)? | ||
.Hz(); | ||
|
||
|
@@ -180,9 +184,6 @@ impl<D: PhaseLockedLoopDevice> PhaseLockedLoop<Disabled, D> { | |
return Err(Error::FeedbackDivOutOfRange); | ||
} | ||
|
||
let refdiv = config.refdiv; | ||
let post_div1 = config.post_div1; | ||
let post_div2 = config.post_div2; | ||
let frequency: HertzU32 = ((ref_freq_hz / u32::from(refdiv)) * u32::from(fbdiv)) | ||
/ (u32::from(post_div1) * u32::from(post_div2)); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had some problems calling this function at all:
How is it meant to be used?
(I worked around that by changing the signature:
Which works, but I guess the
&mut self
is desirable to guarantee exclusive access.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh, let me remove that part of the PR then. It's causing more troubles than anything.
I thought that'd be an incremental step on the clock update (rather than the monolithic thing that GPIO were and PIO will be :( )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't get me wrong, I like the feature. But it would be nice to be able to call the function :-)
Keeping it separate from fixing the VCO frequency is a good idea, though. The minimal fix, updating the settings for the USB PLL, should be in its own PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could work:
Little bit ugly at the call site, but no ownership issues: