Skip to content

I2C: add clock configuration#5606

Merged
playfulFence merged 7 commits into
esp-rs:mainfrom
playfulFence:fix/misc
May 28, 2026
Merged

I2C: add clock configuration#5606
playfulFence merged 7 commits into
esp-rs:mainfrom
playfulFence:fix/misc

Conversation

@playfulFence
Copy link
Copy Markdown
Member

closes #2601


Changelog

esp-hal

  • Added: - I2C: clock source configuration via ClockSource and Config::with_clock_source

@MabezDev
Copy link
Copy Markdown
Member

We need some tests to test that it works on some level with the different clock sources.

@playfulFence playfulFence marked this pull request as draft May 26, 2026 16:23
@playfulFence
Copy link
Copy Markdown
Member Author

/hil full --test i2c

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

Triggered full HIL run for #5606.

Run: https://github.com/esp-rs/esp-hal/actions/runs/26495704750

Status update: ❌ HIL (full) run failed (conclusion: failure).

@playfulFence playfulFence marked this pull request as ready for review May 27, 2026 08:46
Copy link
Copy Markdown
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Thanks!

@MabezDev MabezDev added this pull request to the merge queue May 27, 2026
@bugadani bugadani removed this pull request from the merge queue due to a manual request May 27, 2026
@bugadani
Copy link
Copy Markdown
Contributor

If we're doing anything, can we do this through the clock tree system?

Comment thread esp-hal/src/i2c/master/low_level/mod.rs Outdated
Comment on lines 342 to 350
pub(super) fn clock_instance(&self) -> crate::soc::clocks::I2cInstance {
use crate::soc::clocks::I2cInstance;
#[cfg(soc_has_i2c1)]
if self.id == 1 {
return I2cInstance::I2c1;
}
I2cInstance::I2c0
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be part of Info

Comment thread esp-hal/src/i2c/master/low_level/mod.rs Outdated
Comment on lines +1864 to +1875
clock_instance: {
cfg_if::cfg_if! {
if #[cfg(soc_has_i2c1)] {
match $id {
0 => crate::soc::clocks::I2cInstance::I2c0,
_ => crate::soc::clocks::I2cInstance::I2c1,
}
} else {
crate::soc::clocks::I2cInstance::I2c0
}
}
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
clock_instance: {
cfg_if::cfg_if! {
if #[cfg(soc_has_i2c1)] {
match $id {
0 => crate::soc::clocks::I2cInstance::I2c0,
_ => crate::soc::clocks::I2cInstance::I2c1,
}
} else {
crate::soc::clocks::I2cInstance::I2c0
}
}
},
clock_instance: paste::paste! { crate::soc::clocks::I2cInstance::[<I2c $id>] },

I think this should be enough? But we can rename the clock tree instances to I2cExtN and use $peri directly, too

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I decided to go for paste as you suggested, I tried to rename the clock tree instances and didn't like it, 1-liner paste is just way more elegant IMO

Comment thread esp-hal/src/i2c/master/low_level/mod.rs Outdated
Comment on lines +363 to +369
cfg_if::cfg_if! {
if #[cfg(i2c_master_version = "3")] {
let config = I2cFunctionClockConfig::new(Default::default(), 0);
} else {
let config = I2cFunctionClockConfig::new(Default::default());
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can do crazy stuff like:

Suggested change
cfg_if::cfg_if! {
if #[cfg(i2c_master_version = "3")] {
let config = I2cFunctionClockConfig::new(Default::default(), 0);
} else {
let config = I2cFunctionClockConfig::new(Default::default());
}
}
let config = I2cFunctionClockConfig::new(Default::default(), #[cfg(i2c_master_version = "3")] 0);

Copy link
Copy Markdown
Contributor

@bugadani bugadani left a comment

Choose a reason for hiding this comment

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

I think the set_frequency functions could use some further cleanup but this does what I wanted.

@playfulFence playfulFence enabled auto-merge May 28, 2026 10:26
@playfulFence playfulFence added this pull request to the merge queue May 28, 2026
Merged via the queue into esp-rs:main with commit 4dee0c2 May 28, 2026
24 of 25 checks passed
MabezDev pushed a commit that referenced this pull request May 29, 2026
* I2C: add clock configuration

* Add test, add RefTick to V2 I2C

* Use clock tree

* use clocktree

* Clock instance -> part of Info

fmt

* address review, simplify clock instance selection

* Simplify cfg_if
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.

I2C: missing clock source configuration

3 participants