Skip to content
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

rtc_clk_init fix #2398

Closed
wants to merge 2 commits into from
Closed

rtc_clk_init fix #2398

wants to merge 2 commits into from

Conversation

negativekelvin
Copy link
Contributor

Actually set cpu frequency

@@ -119,6 +119,7 @@ void rtc_clk_init(rtc_clk_config_t cfg)

bool res = rtc_clk_cpu_freq_mhz_to_config(cfg.cpu_freq_mhz, &new_config);
assert(res && "invalid CPU frequency value");
rtc_clk_cpu_freq_set_config_fast(&new_config);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! This should probably be non-fast version, since _fast can't enable PLL, if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought _fast has fallback to full version if required? You are saying it will always fall back in this situation so just call it directly?

Copy link
Member

Choose a reason for hiding this comment

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

It does, but that relies on static variables which are not set at that point yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean s_cur_pll_freq will be zero so we won't be able to take shortcut anyway and if we happen to be setting a divided xtal freq we want to make sure pll is disabled? So no advantage in _fast.

Call non _fast version rtc_clk_cpu_freq_set_config
@negativekelvin
Copy link
Contributor Author

negativekelvin commented Sep 11, 2018

@igrr if we use _fast variant in esp_clk_init it can save ~10k cycles which at 80mhz is 130us do you think it is worthwhile?

ugly test code:

extern int s_cur_pll_freq;
uint8_t bbpll_endiv5_val = I2C_READREG_RTC(I2C_BBPLL, I2C_BBPLL_ENDIV5);
uint8_t bbpll_bbadc_dsmp_val = I2C_READREG_RTC(I2C_BBPLL, I2C_BBPLL_BBADC_DSMP);
if(bbpll_endiv5_val == BBPLL_ENDIV5_VAL_320M && bbpll_bbadc_dsmp_val == BBPLL_BBADC_DSMP_VAL_320M)
    s_cur_pll_freq = 320;
else if(bbpll_endiv5_val == BBPLL_ENDIV5_VAL_480M && bbpll_bbadc_dsmp_val == BBPLL_BBADC_DSMP_VAL_480M)
    s_cur_pll_freq = 480;
rtc_clk_cpu_freq_set_config_fast(&new_config);

save another ~1k cycles by giving the value of s_cur_pll_freq from bootloader to app

@igrr
Copy link
Member

igrr commented Sep 12, 2018

@negativekelvin Good point on the possible optimization. To resolve the issue faster, I'll take the current version of the fix, and then confirm about using I2C_READREG_RTC.

@igrr igrr added the Status: Pending blocked by some other factor label Sep 12, 2018
igrr pushed a commit that referenced this pull request Sep 17, 2018
Add missing call to rtc_clk_cpu_freq_set_config

Merges #2398
@igrr
Copy link
Member

igrr commented Sep 19, 2018

Squashed and cherry-picked as 8e2856b, thanks @negativekelvin!

@igrr igrr closed this Sep 19, 2018
catalinio pushed a commit to catalinio/pycom-esp-idf that referenced this pull request Jun 28, 2019
Add missing call to rtc_clk_cpu_freq_set_config

Merges espressif/esp-idf#2398
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending blocked by some other factor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants