Skip to content

Conversation

@bespsm
Copy link
Contributor

@bespsm bespsm commented Jul 21, 2025

  • lib: add i2c mod to lib
  • lib: add bind_interrupts macros for async workflow
  • config: add I2C SDA,SCA pin traits code generation
  • config: add clock source for the I2C
  • config: add clock divider for the I2C
  • config: add I2C BusSpeed configuration
  • I2C: add blocking API: blocking_read, blocking_write, blocking_write_read
  • I2C: add async API: async_write, async_read, async_write_read
  • I2C: add embedded-hal (v0.2) API for blocking & async impl.
  • I2C: add embedded-hal (v1.0) API for blocking & async impl.
  • I2C-tests: checks for timer_period & check_clock_rate fn's
  • mspm0l1306 examples: add I2C blocking & async examples
  • mspm0l1306 examples: add -O2 optimization due to Flash limitations
  • mspm0g3507 examples: add I2C blocking & async examples

Looking forward for a feedback: @i509VCB

Copy link
Member

@i509VCB i509VCB left a comment

Choose a reason for hiding this comment

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

Still going through this.

@bespsm bespsm force-pushed the mspm0-i2c branch 3 times, most recently from 669590c to f0eb466 Compare July 23, 2025 15:39
@bespsm
Copy link
Contributor Author

bespsm commented Jul 24, 2025

After switching to I2C standard mode, driver start working incorrectly (write-read test of 17 bytes test failed). I will do the research

@Iooon
Copy link
Contributor

Iooon commented Jul 25, 2025

After switching to I2C standard mode, driver start working incorrectly (write-read test of 17 bytes test failed). I will do the research

Seems very likely that this is because of the wrong timer period. When executing the tests, a specific case with FastMode seems to work but all other cases fail. Probably the new calculation is a little bit wrong. Make sure you really work periods instead of frequencies (or account for that). That was my error at the beginning.

@bespsm
Copy link
Contributor Author

bespsm commented Jul 28, 2025

After switching to I2C standard mode, driver start working incorrectly (write-read test of 17 bytes test failed). I will do the research

Seems very likely that this is because of the wrong timer period. When executing the tests, a specific case with FastMode seems to work but all other cases fail. Probably the new calculation is a little bit wrong. Make sure you really work periods instead of frequencies (or account for that). That was my error at the beginning.

This bug is reproducible even with old timer period calculation, I will dig into it more.

Added: this is expected behavior according TI. Blocking API is capable to transfer max 8 bytes in one transaction. It's explicitly mentioned in the same TI example and it shows the same faulty behavior. Async (with interrupts) API works well with different i2c speeds

@bespsm bespsm changed the title MSPM0: Add I2C Controller (blocking & async) + examples for mspm0l1306, mspm0g3507 (tested MCUs) [WIP] MSPM0: Add I2C Controller (blocking & async) + examples for mspm0l1306, mspm0g3507 (tested MCUs) Jul 30, 2025
@i509VCB
Copy link
Member

i509VCB commented Aug 2, 2025

Checking in, I assume the driver is still being worked on? (and whether it is working?)

@bespsm
Copy link
Contributor Author

bespsm commented Aug 2, 2025

Checking in, I assume the driver is still being worked on? (and whether it is working?)

yep, still working on.

Added: @i509VCB I guess I addressed all issues

bespsm added 9 commits August 4, 2025 10:19
- lib: add i2c mod to lib
- lib: add `bind_interrupts` mod for async workflow
- lib: set SYSOSCBASE as system oscillator
- config: add I2C SDA,SCA pin traits code generation
- config: add clock source for the I2C
- config: add clock divider for the I2C
- config: add I2C BusSpeed configuration
- I2C: add blocking API: blocking_read, blocking_write, blocking_write_read
- I2C: add async API: async_write, async_read, async_write_read
- I2C: add embedded-hal (v0.2) API for blocking & async impl.
- I2C: add embedded-hal	(v1.0) API for blocking & async impl.
- I2C-tests: checks for timer_period & check_clock_rate fn's
- mspm0l1306 examples: add I2C blocking & async examples
- mspm0l1306 examples: add -O2 optimization due to Flash limitations
- mspm0g3507 examples: add I2C blocking & async examples
- i2c-config: automatically defines clock source based on input I2C rate
- i2c: proper config functions naming
- i2c-examples: adapt to changed API
- i2c: save initialization pf cctr register
- blocking API for transfering max 8 bytes
- async API has no such limitations
@bespsm bespsm changed the title [WIP] MSPM0: Add I2C Controller (blocking & async) + examples for mspm0l1306, mspm0g3507 (tested MCUs) MSPM0: Add I2C Controller (blocking & async) + examples for mspm0l1306, mspm0g3507 (tested MCUs) Aug 4, 2025
@i509VCB
Copy link
Member

i509VCB commented Aug 6, 2025

Implementation looks fine. I'll give the errata sheet a look just in case there is something possibly sneaky there we need to know about.

Comment on lines +817 to +819
for byte in chunk {
*byte = self.info.regs.controller(0).crxdata().read().value();
}
Copy link
Member

Choose a reason for hiding this comment

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

We should verify this does not cause problems with I2C_ERR_08 errata.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@i509VCB I couldn't add errata to the project due to compiling issues. could you point, what exactly should I verify here?

Copy link
Member

@i509VCB i509VCB Aug 8, 2025

Choose a reason for hiding this comment

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

https://www.ti.com/lit/pdf/slaz742, I2C_ERR_08 states:

FIFO Read directly after RXDONE interrupt will cause erroneous data to be read

The workaround listed is:

Wait 2 I2C CLK cycles for the FIFO to guarantee to have the latest data. I2C CLK will
based on the CLKSEL register in the I2C registers.

Steps:

  1. IRQ handler wakes the tasking which is pending
  2. poll_fn closure is run, when RXDONE is set then return from poll_fn.
  3. Check a branch and maybe stop the controller transmission.
  4. Read from the fifo

The time gap between step 1 and 4 is non-deterministic. To properly address the errata we would need to insert a wait for 2 I2C clock cycles. Although giving that a thought we would need to know the core clock speed to issue the correct number of cycles to delay. But that is going to depend on #4445.

I'll open an issue about that errata, as we will need to deal with it later.

@i509VCB
Copy link
Member

i509VCB commented Aug 7, 2025

Giving the errata a check:

  • I2C_ERR_01 - SMBUS related issue. We don't implement SMBUS mode yet so we can deal with that later.
  • I2C_ERR_02 - Quick command related issue. Only applicable in target mode, so ignore it.
  • I2C_ERR_03 - I2C peripheral mode cannot wake up device when sourced from MFCLK. Maybe something to keep in mind when we allow entering STOP/STANDBY. Ignore for now.
  • I2C_ERR_04 - Only relevant in target mode
  • I2C_ERR_05 - We don't touch active bit during transmission, not applicable.
  • I2C_ERR_06 - SMBus timeout again, so ignore.
  • I2C_ERR_07 - We do single CCTR writes. So this is worked around indirectly.
  • I2C_ERR_08 - Maybe an issue, see comment
  • I2C_ERR_09 - Target mode again
  • I2C_ERR_10 - Low power, doesn't need to be done yet.

@i509VCB i509VCB added this pull request to the merge queue Aug 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 8, 2025
@i509VCB
Copy link
Member

i509VCB commented Aug 8, 2025

Flaky tests... Lets hope another run works.

@i509VCB i509VCB added this pull request to the merge queue Aug 8, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 8, 2025
@i509VCB
Copy link
Member

i509VCB commented Aug 8, 2025

@Dirbaio when you can please merge. 5340dk tests are flaking.

@bespsm
Copy link
Contributor Author

bespsm commented Aug 11, 2025

@Dirbaio kind reminder to check this one

@Dirbaio when you can please merge. 5340dk tests are flaking.

@i509VCB i509VCB added this pull request to the merge queue Aug 11, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 11, 2025
@bespsm
Copy link
Contributor Author

bespsm commented Aug 13, 2025

@i509VCB @Dirbaio I see that more PR get merged recently, Could we try again?

Copy link
Contributor

@Iooon Iooon left a comment

Choose a reason for hiding this comment

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

Sorry to throws this in now, but I noticed that the tests in the i2c.rs file don't work. If the implementation seems to be correct we should at least change the tests.
Failing tests are:
i2c::tests::ti_calculate_timer_period
i2c::tests::ti_calculate_timer_period_2
i2c::tests::ti_calculate_timer_period_3
i2c::tests::ti_calculate_timer_period_4

@bespsm
Copy link
Contributor Author

bespsm commented Aug 14, 2025

Sorry to throws this in now, but I noticed that the tests in the i2c.rs file don't work. If the implementation seems to be correct we should at least change the tests. Failing tests are: i2c::tests::ti_calculate_timer_period i2c::tests::ti_calculate_timer_period_2 i2c::tests::ti_calculate_timer_period_3 i2c::tests::ti_calculate_timer_period_4

fixed and tested with oscillator

@bespsm bespsm requested a review from Iooon August 14, 2025 08:40
@bespsm
Copy link
Contributor Author

bespsm commented Aug 14, 2025

@i509VCB @Dirbaio I see that more PR get merged recently, Could we try again?

kind reminder for another attempt 🥲

@i509VCB
Copy link
Member

i509VCB commented Aug 14, 2025

I'll open a pull request to disable the 5340 tests, as those seem to have broken. I can only use the merge queue myself.

@i509VCB i509VCB mentioned this pull request Aug 14, 2025
@i509VCB i509VCB added this pull request to the merge queue Aug 14, 2025
Merged via the queue into embassy-rs:main with commit 32f142d Aug 14, 2025
8 checks passed
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