Skip to content

Conversation

ianrrees
Copy link
Contributor

Resurrecting @bradleyharden's work on clockv2 for thumbv6m. Will begin by rebasing on to current master, then try to finish things up!

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch 2 times, most recently from 00dc127 to 82875e2 Compare June 18, 2025 07:13
@ianrrees
Copy link
Contributor Author

Note to self: #908

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch 3 times, most recently from 8367534 to b169704 Compare July 21, 2025 00:33
@ianrrees
Copy link
Contributor Author

I've spent some more time on this today, and have moved the thumbv6 clock v2 support up to just before #728 . From a not-very-close inspection of the newer history of the project, this seems like the last major snag to resurrect this work.

It seems like using diff/patch (IOW Git rebase) isn't a good approach to update the thumbv6 clock v2 work on to the new config scheme, there are tons of small changes spread through the codebase. I'm thinking that manually replicating the changes from the thumbv6 clock v2 work is the way forward, directly on to the current master.

If anyone else feels like taking a crack at this, please feel free!

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch 2 times, most recently from c2282e3 to 7676db0 Compare July 22, 2025 00:32
@ianrrees
Copy link
Contributor Author

I've saved the branch that attempted to do this with rebase as clock_v2_thumbv6m_rebase and started manually transcribing the work on top of current master here.

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch from 7676db0 to 08e9ec1 Compare July 22, 2025 04:08
@ianrrees
Copy link
Contributor Author

I've advanced to somewhere between Bradley's second and third "stash" commits, the code at the current head (commit message "WIP stash 2 enable v2 for thumbv6m") comments out some important stuff but builds much of the clock v2 code for thumbv6m (with some warnings that should be addressed), and the commit before that cleanly builds for thumbv7em (though isn't tested to still work). Some of the changes I've made are a little bit hacky; I'm trying to create a finer-grained history to keep things compiling in intermediate steps so that it'll be easier to go back and clean up that stuff once the bulk of the work is done.

@ianrrees
Copy link
Contributor Author

ianrrees commented Jul 22, 2025

  • Need to build with stable Rust - currently one of the earlier commits here introduces a couple nightly features just to get a compile.
  • There's a pattern in the new work, where a type alias is used to gain access to the peripheral register block. We should standardize that across all peripherals.
  • In several places, I've made fairly crude alternate implementations for thumbv6m and thumb7em chips, where Bradley's original work had finer conditionals on eg one a member of a struct. Use #[hal_macro_helper] to deal with this instead
  • The DPLL in smaller chips doesn't support DCO filtering options, have put a hacky conditional in to get a build but need to tidy this up - remove the API for changing DCO filter in smaller chips, etc.
  • There are quite a few things gated on eg clock-d5x that should instead use more specific hal_cfg arguments.
  • I2S clocking needs attention - just turned it off
  • Macro define_apb_types takes two booleans, but there are only two valid combinations of input - look in to cleaning that up.
  • Think up a naming scheme/structure to replace the (old|new)-clock-system, if it's not OK to keep using sysctrl and mclk-oscctrl. This is an internal-only thing, so probably not too important, but old/new is a bit ambiguous in light of PIC32C series discussion thread #915 (which describes new chips, some of which use SYSCTRL) and of course we might come across a third scheme!
  • The GCLK implementation currently is a bit repetitive, perhaps atsamd-hal-macros provides (or could be extended to provide) a variable like gclk-max so that we could use seq!().
  • Update T1 BSPs
  • Ensure flash wait states are (or, can be) managed correctly
  • How to handle TC/TCC instances sharing clocks? Maybe split this out to a separate issue
  • Should the DPLL enable() method call wait_enabled() at the end?

@ianrrees
Copy link
Contributor Author

Have made a bit more progress on this today, largely splitting up the last few commits in clock_v2_thumbv6m_rebase to make them a bit easier to digest

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch from f3d9269 to 8931976 Compare July 27, 2025 23:43
@ianrrees
Copy link
Contributor Author

The bulk of the clock v2 support is now in place for SAMD21, I've not been exercising SAMD11 builds and those currently error. There are a couple of Bradley's changes that still need to be ported, and the above list of TODOs, I'll try to chip away at it more tomorrow.

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch from d03a44b to fa7de36 Compare July 29, 2025 02:08
@ianrrees
Copy link
Contributor Author

Changes to clock v2 API for thumbv7 introduced by this PR:

  • Removed nvmctrl argument from clock_system_at_reset()
  • Changed field in reset::Clocks from osculp32k_base: Enabled<osculp32k::OscUlp32kBase> to pub osculp: OscUlpClocks,. OscUlpClocks has pub base: osculp32k::EnabledOscUlp32kBase,

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch 5 times, most recently from 36f5b4d to a8d4920 Compare August 3, 2025 08:43
@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch from 8d8f126 to bd81455 Compare August 9, 2025 08:26
@ianrrees
Copy link
Contributor Author

ianrrees commented Aug 9, 2025

I've done some basic testing on both Metro M0 and Metro M4, seems to work OK. Will chip away at updating the tier-1 BSPs for thumbv6m chips, but this seems ready for wider testing and feedback if anyone's got spare cycles to work on it.

@ianrrees ianrrees force-pushed the clock_v2_thumbv6m branch 2 times, most recently from 7ce8fb9 to 7a2a60a Compare August 15, 2025 20:09
This makes a `hal_cfg()` argument available for xosc1 on bigger chips
Decided not to split the file up, doesn't seem necessary
Couldn't deduplicate with the feather_m4 and metro_m4 examples, because
the alias used for the LED is different.  Sorting that out seems like a
separate project.
Reading the GCLK GENCTRL register isn't possible on the thumbv6m chips,
on these there is only one GENCTRL register for the whole GCLK
peripheral, in contrast to one GENCTRL per generator on the thumbv7em
chips.  On the thumbv6m chips, modifying GENCTRL is done by atomically
writing the whole register, which has a field for the generator ID to be
modified.

The GENCTRL register had been modified from the GclkToken<> impl,
per-field, but the state required to fill the other fields for the
atomic register write wasn't available in the GclkToken<> scope.  So,
these GENCTRL accesses were moved up in to the Gclk<> impl.

One more bit of state was added to the settings to handle the Output
Enable bit.
@ianrrees
Copy link
Contributor Author

Woohoo! Green tick from the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants