-
Notifications
You must be signed in to change notification settings - Fork 356
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
Added overclock feature #78
Conversation
@@ -75,7 +77,7 @@ void systemInit(void) | |||
|
|||
// Configure the System clock frequency, HCLK, PCLK2 and PCLK1 prescalers | |||
// Configure the Flash Latency cycles and enable prefetch buffer | |||
SetSysClock(); | |||
SetSysClock(feature(FEATURE_OVERCLOCK)); |
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.
Drivers do not import things from multiwii , especially stuff like feature()
This is a bit hacky (other than the obvious part in setSysClock(). I don't think there's a reason to do all these modifications. You can simply set a flag and re-run setSysClock(). I would suggest to simply implement a way to pass a flag to setSysclock, then read configuration, and if overclock flag is set, re-run setSysClock again with it. This would also remove all the other changes to configuration loading/etc. Also I'm not really sure this is a "feature", this is already more of a 'debug' kind of thing, so I would have preferred this to be a 'set' variable. to add to obscurity, it shold not be called overclock, but something like "emf_avoidance". so set emf_avoidance=1 would run at 80, and otherwise at default of 72. Thoughts? |
In the in Reference Manual Section 7.2.3 it says: The PLL configuration (selection of HSI oscillator divided by 2 or HSE oscillator for PLL input clock, and multiplication factor) must be done before enabling the PLL. Once the PLL enabled, these parameters cannot be changed. Therefore I implemented the configuration in the first call. As far as I understand the reference it would be necessary to switch back to the internal clock source/change the pll/switch to hse. This seems more complicated to do than to load the config before the first call. I wanted to hide the complexity of pll multipliers/hse sources from the user and a feature can just be activated so it seemed well suited. But emf_avoidance=1 does the same thing. |
Hm, I missed the part about not being able to reconfigure PLL. Not sure if I like splitting config reads like this tho. I'd have to study On Tue, Apr 15, 2014 at 4:34 PM, Johannes notifications@github.com wrote:
|
I changed the feature to a set variable. The splitting isn't necessary since the pid controler selection/... does not involve any frequency specific work. The idea was to preserve the order of the init sequence as far as possible. |
Any updates? |
currently flying (not rc), I will have time to review this this weekend.
|
This branch adds the overclock feature. If activated and the hse clock source is used the PLL is configured to use a higher multiplier and the board runs at 80 or 84 MHz (depending on the hse frequency).
The readEEPROM function is split in the actual eeprom reading and activation of the loaded configuration. The config is read before the systemInit function is called. Now the status of the overclock feature is accessible during the systemInit. The remaining configuration is activated after the systemInit.
I did not extensively test the changes yet. If activated the higher frequency is displayed in the status screen and I did not notice any negative side effects. I just have a acro naze rev 5 so I cannot test if the mag/baro/older revisions still work.
The looptime value is too low if this feature is activated. It would be possible to scale it internally to the actual frequency.
Did I miss anything else?