Skip to content

clarify clock_configure and make it return the correct value achieved for bad clock inputs #2225

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

Merged
merged 1 commit into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/rp2_common/hardware_clocks/clocks.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ bool clock_configure(clock_handle_t clock, uint32_t src, uint32_t auxsrc, uint32
return false;

uint32_t div = (uint32_t)((((uint64_t) src_freq) << CLOCKS_CLK_GPOUT0_DIV_INT_LSB) / freq);
// only clock divider of 1, or >= 2 are supported
if (div < (2u << CLOCKS_CLK_GPOUT0_DIV_INT_LSB)) {
div = (1u << CLOCKS_CLK_GPOUT0_DIV_INT_LSB);
}
uint32_t actual_freq = (uint32_t) ((((uint64_t) src_freq) << CLOCKS_CLK_GPOUT0_DIV_INT_LSB) / div);

clock_configure_internal(clock, src, auxsrc, actual_freq, div);
Expand Down
13 changes: 10 additions & 3 deletions src/rp2_common/hardware_clocks/include/hardware/clocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,18 @@ extern "C" {

typedef clock_num_t clock_handle_t;

/*! \brief Configure the specified clock
/*! \brief Configure the specified clock with automatic clock divisor setup
* \ingroup hardware_clocks
*
* This method allows both the src_frequency of the input clock source AND the desired
* frequency to be specified, and will set the clock divider to achieve the exact or higher frequency
* achievable, with the maximum being the src_freq.
*
* Note: That the clock hardware only support divisors of exactly 1 or 2.0->65535.0
*
* See the tables in the description for details on the possible values for clock sources.
*
*
* \param clock The clock to configure
* \param src The main clock source, can be 0.
* \param auxsrc The auxiliary clock source, which depends on which clock is being set. Can be 0
Expand All @@ -275,7 +282,7 @@ typedef clock_num_t clock_handle_t;
*/
bool clock_configure(clock_handle_t clock, uint32_t src, uint32_t auxsrc, uint32_t src_freq, uint32_t freq);

/*! \brief Configure the specified clock to use the undividded input source
/*! \brief Configure the specified clock to use the undivided input source
* \ingroup hardware_clocks
*
* See the tables in the description for details on the possible values for clock sources.
Expand All @@ -287,7 +294,7 @@ bool clock_configure(clock_handle_t clock, uint32_t src, uint32_t auxsrc, uint32
*/
void clock_configure_undivided(clock_handle_t clock, uint32_t src, uint32_t auxsrc, uint32_t src_freq);

/*! \brief Configure the specified clock to use the undividded input source
/*! \brief Configure the specified clock to use the undivided input source
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, same 'brief' description for both clock_configure_undivided and clock_configure_int_divider - is that correct, or was it a copy'n'paste error?

* \ingroup hardware_clocks
*
* See the tables in the description for details on the possible values for clock sources.
Expand Down
Loading