drivers: add i2c driver support for MSPM0 #94627

d-philpot · 2025-08-19T03:21:51Z

Adds I2C controller and target support for MSPM0. Includes driver changes, dts and board updates.

drivers/i2c/i2c_mspm0.c

comment addressed

parthitce

This driver needs lot of re-factor. In summary,

ISR context is handling all the data path, which needs to be abstracted.
Target and Controller support needs to be split.
Multi target support needs to be compile time based on devicetree target nodes.
Current usage of data section is high and needs to be optimized based on rodata / data place-holding.

Current code base is not compilable as well. So IMO it makes sense to put in DRAFT until it's ready for review.

teburd

There's some confusing things going on, maybe because the HAL is handling interrupts? It's unclear, but those should be handled by Zephyr not by the HAL if that's the case.

teburd · 2025-08-21T15:36:14Z

drivers/i2c/i2c_mspm0.c

+	DL_I2C_startControllerTransfer(config->base, data->addr, DL_I2C_CONTROLLER_DIRECTION_TX,
+				       data->msg.len);
+
+	while (!(DL_I2C_getControllerStatus(config->base) & DL_I2C_CONTROLLER_STATUS_IDLE)) {


Tight polling loops like this aren't inherently wrong but typically there's an interrupt signal available.

Would recommend changing the driver, if possible, to use an interrupt (event driven) state machine that is kicked off by the transfer function, and completed when the state machine signals back to the thread that had called transfer blocking on a k_sem_take by giving a semaphore.

If you really want the tight poll loop here instead, or there's no other option, use a WAIT_FOR() macro with a timeout such that even given a faulty i2c transfer the caller isn't blocked forever and instead may get an error.

https://docs.zephyrproject.org/latest/doxygen/html/group__sys-util.html#ga68eb35df6b4715714312df90209accee

Looking into refactoring transfer to use semaphores

teburd · 2025-08-21T15:36:38Z

drivers/i2c/i2c_mspm0.c

+	while (!(DL_I2C_getControllerStatus(config->base) & DL_I2C_CONTROLLER_STATUS_IDLE)) {
+	}
+
+	if (data->state == I2C_MSPM0_TIMEOUT) {


And I maybe spoke too soon if there's a hardware timeout available.

There is a configurable SCL low hardware timeout available

teburd · 2025-08-21T15:39:03Z

dts/bindings/i2c/ti,mspm0-i2c.yaml

+  reg:
+    required: true
+
+  interrupts:


I don't see the interrupts being used?

teburd · 2025-08-21T15:39:13Z

dts/arm/ti/mspm0/l/mspm0lx22x.dtsi

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x400f2000 0x2000>;
+			interrupts = <26 0>;


It doesn't seem like the driver supports interrupts

Interrupts are used for controller and target logic

d-philpot · 2025-08-22T19:22:32Z

This driver needs lot of re-factor. In summary,

ISR context is handling all the data path, which needs to be abstracted.

Target and Controller support needs to be split.

Multi target support needs to be compile time based on devicetree target nodes.

Current usage of data section is high and needs to be optimized based on rodata / data place-holding.

Current code base is not compilable as well. So IMO it makes sense to put in DRAFT until it's ready for review.

The code base is compliable. Target support is split and optional from controller via kconfig define. Multi target support is compile time based on Kconfig defines

d-philpot · 2025-08-27T19:27:47Z

@teburd Ping for re-review. Refactored transmit logic to use semaphores instead of busy wait looping

tbursztyka

ISR handler would gain in clarity being split: irq types handled in dedicated inline functions and as noted already: target stuff being on their own as well.

drivers/i2c/i2c_mspm0.c

github-actions · 2025-09-11T21:47:55Z

The following west manifest projects have changed revision in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_ti	zephyrproject-rtos/hal_ti@`d062844`	zephyrproject-rtos/hal_ti#66	zephyrproject-rtos/hal_ti#66/files

⛔ DNM label due to: 1 project with PR revision

Note: This message is automatically posted and updated by the Manifest GitHub Action.

In flight PR changes default pin properties for I2C. Signed-off-by: Dylan Philpot <[email protected]>

tbursztyka

I'll try to find some time to dig into the logic details later

drivers/i2c/i2c_mspm0.c

rriveramcrus · 2025-09-17T14:30:59Z

I don't see anything here to review on the charger side of things, so I am going to remove the charger label if no one has any objections.

d-philpot · 2025-09-24T18:41:03Z

@teburd @tbursztyka @parthitce @ssekar15 ping for review

teburd · 2025-09-24T20:01:26Z

@d-philpot you need to correct the issues pointed out and push your changed here so we can see them, it doesn't seem you've done that yet

I don't see the comments from @tbursztyka addressed yet, and they are valid

d-philpot · 2025-09-24T20:10:13Z

@d-philpot you need to correct the issues pointed out and push your changed here so we can see them, it doesn't seem you've done that yet

I don't see the comments from @tbursztyka addressed yet, and they are valid

Addressed in latest push

Includes driver and dts binding changes for I2C support Signed-off-by: Dylan Philpot <[email protected]>

Adds devicetree entries for different I2C peripherals on MSPM0 devices. Signed-off-by: Dylan Philpot <[email protected]>

Adds I2C peripheral for launchpad use. Signed-off-by: Dylan Philpot <[email protected]>

sonarqubecloud · 2025-09-24T21:00:47Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zephyrbot added platform: Texas Instruments MSPM0 area: I2C platform: TI SimpleLink Texas Instruments SimpleLink MCU labels Aug 19, 2025

zephyrbot requested review from ssekar15, teburd, vaishnavachath and vanti August 19, 2025 03:22

zephyrbot assigned teburd Aug 19, 2025

pdgendt previously requested changes Aug 19, 2025

View reviewed changes

drivers/i2c/i2c_mspm0.c Outdated Show resolved Hide resolved

d-philpot force-pushed the i2c_dev branch from 72c1da1 to cda6ccf Compare August 19, 2025 14:37

parthitce linked an issue Aug 20, 2025 that may be closed by this pull request

I2C linumiz/zephyr#70

Open

parthitce requested changes Aug 20, 2025

View reviewed changes

teburd requested changes Aug 21, 2025

View reviewed changes

d-philpot force-pushed the i2c_dev branch 3 times, most recently from 1f122c6 to b708dc5 Compare August 27, 2025 18:27

d-philpot requested review from teburd and pdgendt August 27, 2025 19:27

tbursztyka requested changes Sep 9, 2025

View reviewed changes

drivers/i2c/i2c_mspm0.c Outdated Show resolved Hide resolved

drivers/i2c/i2c_mspm0.c Outdated Show resolved Hide resolved

d-philpot force-pushed the i2c_dev branch from b708dc5 to 02e52af Compare September 11, 2025 21:46

zephyrbot added the area: Charger label Sep 11, 2025

zephyrbot requested a review from rriveramcrus September 11, 2025 21:47

github-actions bot added manifest manifest-hal_ti DNM (manifest) This PR should not be merged (controlled by action-manifest) labels Sep 11, 2025

west: update to in flight PR

485ce3e

In flight PR changes default pin properties for I2C. Signed-off-by: Dylan Philpot <[email protected]>

zephyrbot requested review from jvasanth1, nandojve and VenkatKotakonda September 15, 2025 01:56

d-philpot force-pushed the i2c_dev branch 2 times, most recently from 3b796f7 to 23c36c9 Compare September 15, 2025 02:55

d-philpot requested a review from tbursztyka September 15, 2025 03:38

tbursztyka requested changes Sep 16, 2025

View reviewed changes

nandojve removed request for nandojve and jvasanth1 September 16, 2025 08:44

rriveramcrus removed the area: Charger label Sep 17, 2025

d-philpot changed the title ~~Add I2C driver support for MSPM0~~ drivers: add i2c driver support for MSPM0 Sep 24, 2025

d-philpot force-pushed the i2c_dev branch from 23c36c9 to dc0883c Compare September 24, 2025 20:08

zephyrbot added platform: ADI Analog Devices, Inc. area: Charger labels Sep 24, 2025

zephyrbot requested review from jvasanth1, MaureenHelm, ozersa and scottwcpg September 24, 2025 20:10

d-philpot added 3 commits September 24, 2025 15:34

drivers: add I2C mspm0 driver

88a3dc7

Includes driver and dts binding changes for I2C support Signed-off-by: Dylan Philpot <[email protected]>

dts: add I2C nodes for MSPM0

cfa57f5

Adds devicetree entries for different I2C peripherals on MSPM0 devices. Signed-off-by: Dylan Philpot <[email protected]>

boards: add i2c nodes for boards

4b6403f

Adds I2C peripheral for launchpad use. Signed-off-by: Dylan Philpot <[email protected]>

d-philpot force-pushed the i2c_dev branch from dc0883c to 4b6403f Compare September 24, 2025 20:35

d-philpot requested review from tbursztyka and parthitce October 1, 2025 01:01

tbursztyka approved these changes Oct 1, 2025

View reviewed changes

drivers: add i2c driver support for MSPM0 #94627

Are you sure you want to change the base?

drivers: add i2c driver support for MSPM0 #94627

Conversation

d-philpot commented Aug 19, 2025

Uh oh!

Uh oh!

parthitce left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teburd left a comment

Choose a reason for hiding this comment

Uh oh!

teburd Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

d-philpot Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teburd Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

d-philpot Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

teburd Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

teburd Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

d-philpot Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

d-philpot commented Aug 22, 2025

Uh oh!

d-philpot commented Aug 27, 2025

Uh oh!

tbursztyka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tbursztyka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rriveramcrus commented Sep 17, 2025

Uh oh!

d-philpot commented Sep 24, 2025

Uh oh!

teburd commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

d-philpot commented Sep 24, 2025

Uh oh!

sonarqubecloud bot commented Sep 24, 2025

Quality Gate passed

Uh oh!

Uh oh!

parthitce left a comment •

edited

Loading

d-philpot Aug 22, 2025 •

edited

Loading

github-actions bot commented Sep 11, 2025 •

edited

Loading

teburd commented Sep 24, 2025 •

edited

Loading