Skip to content

Conversation

kyp44
Copy link
Contributor

@kyp44 kyp44 commented Jul 19, 2025

Summary

As part of the clock::v2 effort tracked in Issue #912, this PR updates the icm to use the clock::v2 API by requiring ownership of its AhbClk and ApbClk. Note that this peripheral is only on thumbv7 targets.

No examples on any Tier 1 BSPs use the ICM so none were affected.

Checklist

  • All new or modified code is well documented, especially public items
  • No new warnings or clippy suggestions have been introduced - CI will deny clippy warnings by default! You may #[allow] certain lints where reasonable, but ideally justify those with a short comment.

@rnd-ash
Copy link
Contributor

rnd-ash commented Jul 19, 2025

Looks good to me! Appreciate the documentation updates as well.

Copy link
Contributor

@ianrrees ianrrees left a comment

Choose a reason for hiding this comment

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

Just a minor naming suggestion, but otherwise LGTM

@@ -877,23 +910,19 @@ impl<I: RegionNum> Region<I> {
pub struct Icm {
/// ICM pac register providing hardware access
icm: crate::pac::Icm,
_ahb_clk: IcmAhbClk,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove the leading underscore from these, they do get used in free().

pub fn new(icm: crate::pac::Icm, ahb_clk: IcmAhbClk, apb_clk: IcmApbClk) -> Self {
Self {
icm,
_ahb_clk: ahb_clk,
Copy link
Contributor

Choose a reason for hiding this comment

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

With the underscore removed as above, clippy will want this to look like ahb_clk, r/t ahb_clk: ahb_clk,

kyp44 added a commit to kyp44/atsamd that referenced this pull request Jul 21, 2025
kyp44 added a commit to kyp44/atsamd that referenced this pull request Jul 21, 2025
kyp44 added a commit to kyp44/atsamd that referenced this pull request Jul 21, 2025
@kyp44
Copy link
Contributor Author

kyp44 commented Jul 22, 2025

@ianrrees Removed the underscores in the latest commit, and also did so in PR #925 for the same reason.

kyp44 added a commit to kyp44/atsamd that referenced this pull request Jul 30, 2025
kyp44 added a commit to kyp44/atsamd that referenced this pull request Aug 4, 2025
kyp44 added a commit to kyp44/atsamd that referenced this pull request Aug 13, 2025
kyp44 added 2 commits August 13, 2025 13:47
    * `Icm::new` now requires ownership of its `AhbClk` and `ApbClk`.
    * Renames `Icm::destroy` to the standard `Icm::free` method to free the `pac::Icm` and bus clocks.
    * Updates the `icm` module documentation example to include the above changes.
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.

3 participants