-
Notifications
You must be signed in to change notification settings - Fork 241
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 on-chip voltage regulator (VREG) voltage setting function #757
Conversation
PR sent to rp2040-pac. |
The core voltage is now changed using VSEL_A, which was added in pac v0.6.0. It is a trivial code, but I think it makes sense as a variation of the example and on-target-tests. |
} | ||
|
||
#[test] | ||
fn change_onchip_regulator_voltage(state: &mut State) { |
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.
How do we know that this test works reliably on every device?
The datasheet says: "Note that RP2040 may not operate reliably with its digital core supply (DVDD) at a voltage other than 1.1V."
Perhaps this test should only cover a smaller range of voltages, eg. 1.05 - 1.15V?
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.
In my experience, I have set the core voltage to 1.2V on more than 50 RP2040s and have never had a problem. Also, in the thread about overclocking on the RP2040, there is no example of a failure to increase the core voltage.
However, this is not normal usage, there is no guarantee that it will work reliably on all RP2040s.
I noticed that with the default system clock, lowering the core voltage too much can cause problems. It should be a smaller test case range, as you mentioned. Especially in the direction of lowering the core voltage.
1.20V should be fine, so i'll reduce it to 1.05 - 1.20V.
If you are concerned, I would reduce it more. Reducing it is easy:)
Fix doc test for vreg_set_voltage() Rename vreg_set/get_voltage() to set/get_voltage()
Reduced test cases and improved documentation comments. And rebased commit log to tidy it up. |
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'm ok with the change on the hal, but I have doubt about the tests.
I'm not sure they add any value. IE what would you expect to possibly fail ?
As one might expect from the slope of this graph, 0.80V may not work at the default system clock frequency. However, the RP2040 C/C++ SDK has a minimum voltage of 0.85V, so 0.80V seems like an unusual value. |
What I mean is that the tests as implemented there do not provide anyproof that things are:
Writing to and Reading from a register is trivial enough that I don’t think it requires an on-target tests. For example, if there’s a possibility to actually verify that the voltage is applied as expected that would make a good test case. |
With the Raspberry Pi Pico, that is impossible. |
Added an on-chip voltage regulator voltage setting function.
Increasing core voltage may be necessary for overclocking. Implemented with reference to hardware_vreg in pico-sdk.