Skip to content

Conversation

@dkarnikis
Copy link

Check commits.

@dkarnikis dkarnikis requested review from a team and Emil-Juhl April 14, 2025 09:22
Copy link

@Emil-Juhl Emil-Juhl left a comment

Choose a reason for hiding this comment

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

When the system is crashing, the I2C lines (SDA/SCL) are not being
reset and might stay LOW.

I don't believe this claim to be correct. The i2c lines are released just fine by the mspm0 reset handler. Otherwise I believe you could face the same issue with a deliberate panic, no?

I think what you wanted to write was something about certain i2c controllers, such as the a2b chip, ad2428 (I think), can impose limitations for clock stretching duration - and this limitation can be enforced by the watchdog to prevent triggering erroneous behavior in the i2c controller (ad2428) itself.

@dkarnikis
Copy link
Author

When the system is crashing, the I2C lines (SDA/SCL) are not being
reset and might stay LOW.

I don't believe this claim to be correct. The i2c lines are released just fine by the mspm0 reset handler. Otherwise I believe you could face the same issue with a deliberate panic, no?

I think what you wanted to write was something about certain i2c controllers, such as the a2b chip, ad2428 (I think), can impose limitations for clock stretching duration - and this limitation can be enforced by the watchdog to prevent triggering erroneous behavior in the i2c controller (ad2428) itself.

image

static void i2c_mspm0_panic(const struct device *dev, uint8_t channel, uint32_t ticks,
                             void *user_data)
{
       struct i2c_mspm0_data *data = user_data;
       const struct i2c_mspm0_config *config = data->cfg;
       
       z_except_reason(CONFIG_I2C_MSPM0_WATCHDOG_PANIC_CODE);
}

tldr; the lines are not released

@dkarnikis dkarnikis requested a review from a team April 23, 2025 10:25
Copy link

@tbursztyka tbursztyka left a comment

Choose a reason for hiding this comment

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

lgtm

add properties to configure the I2C watchdog reset timeout and and
panic error code. Also, add a new kconfig property to define the driver
init priority. Since the i2c driver has a dependency to the counter
driver, we need to adjust for that.

Signed-off-by: Dimitris Karnikis <dika@bang-olufsen.dk>
describe the watchdog timer property that will reset the mcu and the
i2c lines (to high) to avoid potential SCL stuck low issues

Signed-off-by: Dimitris Karnikis <dika@bang-olufsen.dk>
When the system is crashing, the I2C lines (SDA/SCL) are not being
reset and might stay LOW. To address this issue, we need to ensure that
none of the user i2c driver callbacks (that are doing reads or writes)
are taking longer than CONFIG_I2C_MSPM0_WATCHDOG_TIMEOUT. So we
fire a timer before each call. If the callback doesn't finish fast
enough, the watchdog callback will reset the i2c lines, NACK the i2c
message and panic with the CONFIG_MSPM0_I2C_WATCHDOG_PANIC_CODE
error code.

The timeout is configured with CONFIG_I2C_MSPM0_WATCHDOG_TIMEOUT
The panic error code is configured with
CONFIG_MSPM0_I2C_WATCHDOG_PANIC_CODE.

To make use of this reboot feature, the device tree needs to provide
a reference to a hardware timer. Let's say we want to use timer timg7
as a watchdog:

We need to configure that timer in the dts:

```
&timg7 {
	status = "okay";
	mode = <ONE_SHOT_DOWN>;
	prescaler = <0>;
	divide-ratio = <RATE_1>;
};
```

and then reference in the i2c block:

```
&i2c0 {
	watchdog-timer = <&timg7>;
	extras ...
```

Since we are invoking a software panic on the mcu, make sure to disable
the COREDUMP since the default backend is the "logging" one and it takes
too much of the cpu resources, thus delaying the mcu reboot.

This can be done with:
```
CONFIG_DEBUG_COREDUMP=n
```

Of course for now, the implementation is using a counter timer since
we don't have a mspm0 watchdog.

We need to also use I2C_MSPM0_INIT_PRIORITY as the driver init
priority.

Signed-off-by: Dimitris Karnikis <dika@bang-olufsen.dk>
@dkarnikis dkarnikis merged commit a054b20 into beo/4.1.0 Apr 23, 2025
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.

4 participants