From daf5cb1d6957dc570cee20f60f7307b7c88ce50d Mon Sep 17 00:00:00 2001 From: Dimitris Karnikis Date: Wed, 26 Mar 2025 12:58:43 +0100 Subject: [PATCH 1/3] drivers: i2c: Kconfig.mspm0g3xxx watchdog props 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 --- drivers/i2c/Kconfig.mspm0g3xxx | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/drivers/i2c/Kconfig.mspm0g3xxx b/drivers/i2c/Kconfig.mspm0g3xxx index e6ec24db1644a..81932caeb7a7e 100644 --- a/drivers/i2c/Kconfig.mspm0g3xxx +++ b/drivers/i2c/Kconfig.mspm0g3xxx @@ -24,4 +24,27 @@ config I2C_MSPM0G3XXX_TRANSFER_TIMEOUT 0 means that the driver should use the K_FOREVER value, i.e. it should wait as long as necessary. +config I2C_MSPM0G3XXX_WATCHDOG_TIMEOUT + int "I2C watchdog reset timeout (us)" + default 300 + help + Defines the timeout in microseconds after which the watchdog will + reset the I2C lines and trigger a system panic. + +config I2C_MSPM0G3XXX_WATCHDOG_PANIC_CODE + int "I2C watchdog reset panic code" + default 17 + help + Defines the panic code that will be triggered when the I2C watchdog + times out and resets the I2C lines. This code can be used for debugging + or logging the failure reason. + + +config I2C_MSPM0G3XXX_INIT_PRIORITY + int "TI I2C MSPM0G3XXX init priority" + default COUNTER_INIT_PRIORITY + help + Define the driver init priority. The driver has a dependency on the counter + component. + endif # I2C_MSPM0G3XXX From 10222e3706a7ea111ba9fee7bd85b5f34f5939fc Mon Sep 17 00:00:00 2001 From: Dimitris Karnikis Date: Wed, 26 Mar 2025 13:02:05 +0100 Subject: [PATCH 2/3] bindings: i2c: ti,mspm0g3xxx-i2c.yaml add watchdog 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 --- dts/bindings/i2c/ti,mspm0g3xxx-i2c.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dts/bindings/i2c/ti,mspm0g3xxx-i2c.yaml b/dts/bindings/i2c/ti,mspm0g3xxx-i2c.yaml index 8dcea99b5857f..1a6cf3037123d 100644 --- a/dts/bindings/i2c/ti,mspm0g3xxx-i2c.yaml +++ b/dts/bindings/i2c/ti,mspm0g3xxx-i2c.yaml @@ -24,3 +24,16 @@ properties: pinctrl-names: required: true + + watchdog-timer: + type: phandle + description: | + The type of the phandle object should be a counter device. + 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_MSPM0G3XXX_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_MSPM0G3XXX_I2C_WATCHDOG_PANIC_CODE + error code. From 9c273d7d8bc9e300c689c2711bcc5967dc797a8d Mon Sep 17 00:00:00 2001 From: Dimitris Karnikis Date: Wed, 26 Mar 2025 13:08:48 +0100 Subject: [PATCH 3/3] drivers: i2c: i2c_mspm0g3xxx: add watchdog timer 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_MSPM0G3XXX_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_MSPM0G3XXX_I2C_WATCHDOG_PANIC_CODE error code. The timeout is configured with CONFIG_I2C_MSPM0G3XXX_WATCHDOG_TIMEOUT The panic error code is configured with CONFIG_MSPM0G3XXX_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 = ; prescaler = <0>; divide-ratio = ; }; ``` 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_MSPM0G3XXX_INIT_PRIORITY as the driver init priority. Signed-off-by: Dimitris Karnikis --- drivers/i2c/i2c_mspm0g3xxx.c | 80 +++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/i2c_mspm0g3xxx.c b/drivers/i2c/i2c_mspm0g3xxx.c index 6e90294cc62fb..a45046bfe2d15 100644 --- a/drivers/i2c/i2c_mspm0g3xxx.c +++ b/drivers/i2c/i2c_mspm0g3xxx.c @@ -6,6 +6,7 @@ #include #include #include +#include #include /* Logging includes */ @@ -76,6 +77,7 @@ struct i2c_mspm0g3xxx_config { const struct pinctrl_dev_config *pinctrl; void (*interrupt_init_function)(const struct device *dev); uint32_t dt_bitrate; + const struct device *watchdog_timer; }; struct i2c_mspm0g3xxx_data { @@ -92,6 +94,7 @@ struct i2c_mspm0g3xxx_data { int target_rx_valid; struct k_sem i2c_busy_sem; struct k_sem transfer_timeout_sem; + struct counter_alarm_cfg watchdog_timer_cfg; }; #ifdef CONFIG_I2C_MSPM0G3XXX_TARGET_SUPPORT @@ -111,6 +114,58 @@ static K_KERNEL_STACK_DEFINE(i2c_mspm0g3xxx_target_stack, CONFIG_I2C_MSPM0G3XXX_TARGET_THREAD_STACK_SIZE); static struct k_thread i2c_mspm0g3xxx_target_thread; +static void i2c_mspm0g3xxx_target_stop_watchdog(struct i2c_mspm0g3xxx_data *data) { + const struct i2c_mspm0g3xxx_config *config = data->cfg; + if (!config->watchdog_timer) { + return; + } + + int err = counter_stop(config-> watchdog_timer); + if (err != 0) { + LOG_ERR("Failed to stop the timer, err: %d", err); + } + + err = counter_cancel_channel_alarm(config->watchdog_timer, 0); + if (err != 0) { + LOG_ERR("Failed to cancel the timer alarm, err: %d", err); + } +} + +static void i2c_mspm0g3xxx_panic(const struct device *dev, uint8_t channel, uint32_t ticks, + void *user_data) +{ + struct i2c_mspm0g3xxx_data *data = user_data; + const struct i2c_mspm0g3xxx_config *config = data->cfg; + + DL_I2C_setTargetACKOverrideValue((I2C_Regs *)config->base, + DL_I2C_TARGET_RESPONSE_OVERRIDE_VALUE_NACK); + DL_I2C_disableTargetClockStretching((I2C_Regs *)config->base); + DL_I2C_disablePower((I2C_Regs *)config->base); + z_except_reason(CONFIG_I2C_MSPM0G3XXX_WATCHDOG_PANIC_CODE); +} + +static void i2c_mspm0g3xxx_target_start_watchdog(struct i2c_mspm0g3xxx_data *data) +{ + const struct i2c_mspm0g3xxx_config *config = data->cfg; + struct counter_alarm_cfg *watchdog_timer_cfg = &data->watchdog_timer_cfg; + + if (!config->watchdog_timer) { + return; + } + + i2c_mspm0g3xxx_target_stop_watchdog(data); + + int err = counter_set_channel_alarm(config->watchdog_timer, 0, watchdog_timer_cfg); + if (err != 0) { + LOG_ERR("Failed to cancel the timer alarm, err: %d", err); + } + + err = counter_start(config->watchdog_timer); + if (err != 0) { + LOG_ERR("Failed to start the timer, err: %d", err); + } +} + void i2c_mspm0g3xxx_target_thread_work(void) { struct i2c_mspm0g3xxx_target_msg target_msg; @@ -151,6 +206,7 @@ void i2c_mspm0g3xxx_target_thread_work(void) uint8_t nextByte; while (DL_I2C_isTargetRXFIFOEmpty((I2C_Regs *)config->base) != true) { + i2c_mspm0g3xxx_target_start_watchdog(data); if (data->target_rx_valid == 0) { nextByte = DL_I2C_receiveTargetData( (I2C_Regs *)config->base); @@ -174,6 +230,7 @@ void i2c_mspm0g3xxx_target_thread_work(void) (I2C_Regs *)config->base, DL_I2C_TARGET_RESPONSE_OVERRIDE_VALUE_NACK); } + i2c_mspm0g3xxx_target_stop_watchdog(data); } } @@ -182,6 +239,7 @@ void i2c_mspm0g3xxx_target_thread_work(void) data->state = I2C_mspm0g3xxx_TARGET_TX_INPROGRESS; /* Fill TX FIFO if there are more bytes to send */ if (tconfig->callbacks->read_requested != NULL) { + i2c_mspm0g3xxx_target_start_watchdog(data); uint8_t nextByte; data->target_tx_valid = tconfig->callbacks->read_requested(tconfig, &nextByte); @@ -193,15 +251,19 @@ void i2c_mspm0g3xxx_target_thread_work(void) * 0's are transmitted */ DL_I2C_transmitTargetData((I2C_Regs *)config->base, 0x00); } + + i2c_mspm0g3xxx_target_stop_watchdog(data); } break; case DL_I2C_IIDX_TARGET_TXFIFO_EMPTY: if (tconfig->callbacks->read_processed != NULL) { + i2c_mspm0g3xxx_target_start_watchdog(data); /* still using the FIFO, we call read_processed in order to add * additional data rather than from a buffer. If the write-received * function chooses to return 0 (no more data present), then 0's * will be filled in */ uint8_t nextByte; + if (data->target_tx_valid == 0) { data->target_tx_valid = tconfig->callbacks->read_processed( tconfig, &nextByte); @@ -215,6 +277,8 @@ void i2c_mspm0g3xxx_target_thread_work(void) * 0's are transmitted */ DL_I2C_transmitTargetData((I2C_Regs *)config->base, 0x00); } + + i2c_mspm0g3xxx_target_stop_watchdog(data); } break; case DL_I2C_IIDX_TARGET_STOP: @@ -768,12 +832,25 @@ static int i2c_mspm0g3xxx_init(const struct device *dev) config->clock_frequency); return -EINVAL; } + LOG_DBG("DT bitrate %uHz (%u)", config->clock_frequency, config->dt_bitrate); k_sem_init(&data->i2c_busy_sem, 0, 1); k_sem_init(&data->transfer_timeout_sem, 1, 1); + if (config->watchdog_timer) { + if (!device_is_ready(config->watchdog_timer)) { + LOG_ERR("Watchdog timer is not ready"); + return -EINVAL; + } + + data->watchdog_timer_cfg.user_data = (void *)data; + data->watchdog_timer_cfg.callback = i2c_mspm0g3xxx_panic; + data->watchdog_timer_cfg.ticks = counter_us_to_ticks( + config->watchdog_timer, CONFIG_I2C_MSPM0G3XXX_WATCHDOG_TIMEOUT); + } + /* Init power */ DL_I2C_reset((I2C_Regs *)config->base); DL_I2C_enablePower((I2C_Regs *)config->base); @@ -895,6 +972,7 @@ static const struct i2c_driver_api i2c_mspm0g3xxx_driver_api = { .divideRatio = DL_I2C_CLOCK_DIVIDE_1}, \ .dt_bitrate = CALC_DT_BITRATE(DT_INST_PROP(index, clock_frequency), \ DT_INST_PROP_BY_PHANDLE(index, clocks, clock_frequency)), \ + .watchdog_timer = DEVICE_DT_GET_OR_NULL(DT_PHANDLE(DT_DRV_INST(index), watchdog_timer)),\ }; \ \ static struct i2c_mspm0g3xxx_data i2c_mspm0g3xxx_data_##index = { \ @@ -903,7 +981,7 @@ static const struct i2c_driver_api i2c_mspm0g3xxx_driver_api = { \ I2C_DEVICE_DT_INST_DEFINE(index, i2c_mspm0g3xxx_init, NULL, &i2c_mspm0g3xxx_data_##index, \ &i2c_mspm0g3xxx_cfg_##index, POST_KERNEL, \ - CONFIG_I2C_INIT_PRIORITY, &i2c_mspm0g3xxx_driver_api); \ + CONFIG_I2C_MSPM0G3XXX_INIT_PRIORITY, &i2c_mspm0g3xxx_driver_api); \ \ INTERRUPT_INIT_FUNCTION(index)