atsamd
atsamd copied to clipboard
atsamd-rs
thumbv6m: fix USB clock recovery
Summary
USB clock recovery was disabled because it caused USB resets and instability. This occurred because the DFFLMUL.MUL was set to the wrong value. The multiplier must be 48000 to obtain a 48 MHz clock from the 1 kHz USB SOF signal.
There appears to be no harm in unconditionally enabling USB clock recovery when there is no external crystal, since it will just operate in open loop mode (as before) when USB is not connected.
Checklist
- [x]
CHANGELOG.mdfor the BSP or HAL updated - [ ] All new or modified code is well documented, especially public items
- [ ] No new warnings or clippy suggestions have been introduced (see CI or check locally)
If Adding a new Board
- [ ] Board CI added to
crates.json - [ ] Board is properly following "Tier 2" conventions, unless otherwise decided to be "Tier 1"
Thanks for the contribution. Can you go through the checklist? specifically adding a changing entry. Also, is this change valid for samd11 too? Why were they working fine up til now?
The SAMD11 datasheet contains the exact same sentence as the SAMD21:
When the USB device is connected, a SOF will be sent every 1ms, thus DFLLVAL.MUX bits should be written to 0xBB80 to obtain a 48MHz clock.
Note that DFLLVAL.MUX doesn't exist, and the only sensible thing they could have meant is DFLLMUL.MUL.
Do you know that the SAMD11 actually worked correctly? The comment doesn't seem too certain.
I barely used this chip but seem to remember its cursed. A quick look at the errata shows only three entries - theres no way thats right, its more likely that everyone else gave up too.
Given the obvious copypasta, its likely they used similar IP for the USB/DFLL peripheral on the d11 as the d21, and I don't see any catastrophic errata entries on the other chips in the series. So this is probably fine.
At the end of the day, if you've got it working fine on real hardware (both with and without USB connected), then I'm okay with this going in. But perhaps mention which revision of the chip you have on this PR?
@lopsided98 What has to happen yet to complete the PR? I'm more than happy to help close this item as I'm working on a project which would benefit from this fix.
@n8tlarsen, this looks good to me. A changelog entry is there, and I don't see anything else blocking. It's a pretty simple change. Seems like a pure bug fix.
