Added EWM Driver #88389

EmilioCBen · 2025-04-09T20:03:33Z

Adding the EWM Driver, tested with wdt_basic_none_reset test using the included overlay.

DerekSnell

Overall, this LGTM. But I am blocking for the hard-coded clock name in driver

drivers/watchdog/wdt_nxp_ewm.c

DerekSnell · 2025-04-10T19:21:18Z

tests/drivers/watchdog/wdt_basic_reset_none/src/main.c

+#define WDT_MAX_WINDOW		0xFE
+#define WDT_TIMEOUT		K_TICKS(0x100)
+#define SLEEP_TIME		K_TICKS(0x45)


should these be a Kconfig for this test app so other platforms can change these?

At first I had these as Kconfigs but I went back on it after realizing the big difference is the function being used e.g. K_TICKS vs K_MSEC, but I went ahead and changed the values passed in to Kconfigs as requested, let me know if this is what was expected or if you had another change in mind here.

tests/drivers/watchdog/wdt_basic_reset_none/src/main.c

drivers/watchdog/wdt_nxp_ewm.c

decsny · 2025-04-14T17:25:24Z

drivers/watchdog/wdt_nxp_ewm.c

+#define LOG_LEVEL CONFIG_WDT_LOG_LEVEL
+#include <zephyr/logging/log.h>
+LOG_MODULE_REGISTER(wdt_nxp_ewm);


log level can be an argument to the register macro, no need for LOG_LEVEL macro

@EmilioCBen you removed the LOG_MODULE_REGISTER instead of the LOG_LEVEL, now there will be no logging

Thanks, I have adjusted the change

drivers/watchdog/wdt_nxp_ewm.c

decsny · 2025-04-14T17:28:27Z

drivers/watchdog/wdt_nxp_ewm.c

+	base->CTRL &= (~EWM_CTRL_INTEN_MASK);
+	if (data->timeout_cfg.callback) {


nit please separate with newline

well now you just put it without a newline next to the above code, which should be triggering check compliance, I feel like check compliance has been really lenient lately, I wonder if someone changed it

decsny · 2025-04-14T17:29:58Z

drivers/watchdog/wdt_nxp_ewm.c

+	EWM_Type *base = config->base;
+	unsigned int key = irq_lock();
+
+	base->SERV = EWM_SERV_SERVICE(NXP_EWM_FEED_MAGIC_NUMBER);


is this macro going to convert the 16 bit integer literal to 8 bit ?

It should but extra precaution I added a cast to 8 bit

drivers/watchdog/wdt_nxp_ewm.c

mmahadevan108 · 2025-04-14T20:49:03Z

tests/drivers/watchdog/wdt_basic_reset_none/src/main.c

+
+#if defined(CONFIG_WDT_NXP_EWM)
+#define WDT_SETUP_FLAGS		0
+#define WDT_TIMEOUT		K_TICKS(WDT_TIMEOUT_VALUE)


Why does the NXP_EWM get the value in ticks?

The watchdog window to feed the watchdog is in the Microsecond range. The Tick value is the most accurate speed to test this EWM without having to worry about the test unintentionally missing a watchdog feed.

dleach02 · 2025-04-21T18:59:40Z

drivers/watchdog/wdt_nxp_ewm.c

+	struct nxp_ewm_data *data = dev->data;
+
+	if (!data->is_watchdog_setup) {
+		-ENOMEM;


I'll note that this is just a free standing ENOMEM?

Good catch, I added the return

dleach02 · 2025-04-21T19:01:03Z

drivers/watchdog/wdt_nxp_ewm.c

+
+	if (cfg && cfg->window.max <= NXP_EWM_MAX_TIMEOUT_WINDOW &&
+		cfg->window.min <= cfg->window.max		 &&
+		cfg->window.max > 0				 &&


nit, but I don't think you should use tabs to align the '&&'

I have removed the tabs.

dleach02 · 2025-04-21T19:02:46Z

drivers/watchdog/wdt_nxp_ewm.c

+	}
+#else
+
+	if (cfg->flags) {


Should this test be at the beginning of this routine? Seems odd to do configuration of the timeoue_cfg.window above but then here decide to return an error.

Sure since it is a passed in argument I will give this check the priority and put it on the top of the routine.

dleach02 · 2025-04-21T19:07:50Z

drivers/watchdog/wdt_nxp_ewm.c

+	unsigned int key = irq_lock();
+
+	base->SERV = EWM_SERV_SERVICE(NXP_EWM_FEED_MAGIC_NUMBER);
+	base->SERV = EWM_SERV_SERVICE((int8_t)(NXP_EWM_FEED_MAGIC_NUMBER >> 8));


If you are going to cast then you should cast in the target value. SERV is a uint8_t.

decsny · 2025-04-22T13:54:22Z

drivers/watchdog/wdt_nxp_ewm.c

+	EWM_Type *base = config->base;
+	base->CTRL &= (~EWM_CTRL_INTEN_MASK);


im surprised if the check compliance doesnt complain, declarations should be separated from following code by newline

decsny · 2025-04-22T13:55:56Z

drivers/watchdog/wdt_nxp_ewm.c

+		return -ENOTSUP;
+	}
+
+


nit: extra newline

dleach02 · 2025-05-01T22:35:57Z

@DerekSnell do you still have a pending change request?

DerekSnell

Thanks for updating @EmilioCBen

drivers/watchdog/wdt_nxp_ewm.c

tests/drivers/watchdog/wdt_basic_reset_none/Kconfig

Added a driver for the External Watchdog Driver Signed-off-by: Emilio Benavente <[email protected]>

Added EWM Support for MCXW71 and MCXW72 Signed-off-by: Emilio Benavente <[email protected]>

Enabled the wdt_basic_reset_none to test boards using the EWM. Signed-off-by: Emilio Benavente <[email protected]>

EmilioCBen added area: Watchdog Watchdog platform: NXP NXP labels Apr 9, 2025

EmilioCBen requested review from mmahadevan108, DerekSnell and decsny April 9, 2025 20:03

github-actions bot added the platform: NXP Drivers NXP Semiconductors, drivers label Apr 9, 2025

github-actions bot requested review from butok, dbaluta, dleach02, katsuster, manuargue, martinjaeger and Raymond0225 April 9, 2025 20:04

github-actions bot assigned dleach02 and mmahadevan108 Apr 9, 2025

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch 2 times, most recently from caaa6da to ec8cb41 Compare April 10, 2025 14:29

DerekSnell requested changes Apr 10, 2025

View reviewed changes

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch 2 times, most recently from bd996d5 to a09c68a Compare April 14, 2025 14:53

decsny reviewed Apr 14, 2025

View reviewed changes

mmahadevan108 reviewed Apr 14, 2025

View reviewed changes

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch from a09c68a to eaa9ebe Compare April 20, 2025 15:41

dleach02 reviewed Apr 21, 2025

View reviewed changes

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch 2 times, most recently from 05d8f98 to 2d11a04 Compare April 21, 2025 19:26

decsny reviewed Apr 22, 2025

View reviewed changes

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch 5 times, most recently from 6926b10 to 7b064a2 Compare April 29, 2025 22:55

DerekSnell previously approved these changes May 2, 2025

View reviewed changes

drivers/watchdog/wdt_nxp_ewm.c Outdated Show resolved Hide resolved

tests/drivers/watchdog/wdt_basic_reset_none/Kconfig Show resolved Hide resolved

dleach02 previously approved these changes May 5, 2025

View reviewed changes

EmilioCBen added 3 commits May 5, 2025 09:15

drivers: watchdog: Added Driver for the EWM

a2b5af0

Added a driver for the External Watchdog Driver Signed-off-by: Emilio Benavente <[email protected]>

dts: arm: nxp: mcxw71_common: Added EWM Support

670e5ea

Added EWM Support for MCXW71 and MCXW72 Signed-off-by: Emilio Benavente <[email protected]>

tests: drivers: watchdog: Enable reset none testing for ewm

1a10121

Enabled the wdt_basic_reset_none to test boards using the EWM. Signed-off-by: Emilio Benavente <[email protected]>

EmilioCBen dismissed stale reviews from dleach02 and DerekSnell via 1a10121 May 5, 2025 14:16

EmilioCBen force-pushed the feature/mcxw7x_ewm_support branch from 7b064a2 to 1a10121 Compare May 5, 2025 14:16

DerekSnell approved these changes May 6, 2025

View reviewed changes

decsny approved these changes May 8, 2025

View reviewed changes

dleach02 approved these changes May 8, 2025

View reviewed changes

kartben merged commit 417b902 into zephyrproject-rtos:main May 8, 2025
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added EWM Driver #88389

Added EWM Driver #88389

EmilioCBen commented Apr 9, 2025

DerekSnell left a comment

DerekSnell Apr 10, 2025

EmilioCBen Apr 14, 2025

decsny Apr 14, 2025

EmilioCBen Apr 20, 2025

decsny Apr 22, 2025

EmilioCBen Apr 25, 2025

decsny Apr 14, 2025

EmilioCBen Apr 20, 2025

decsny Apr 22, 2025

decsny Apr 14, 2025

EmilioCBen Apr 20, 2025

mmahadevan108 Apr 14, 2025 •

edited

Loading

EmilioCBen Apr 20, 2025

dleach02 Apr 21, 2025

EmilioCBen Apr 21, 2025

dleach02 Apr 21, 2025

EmilioCBen Apr 21, 2025

dleach02 Apr 21, 2025

EmilioCBen Apr 21, 2025

dleach02 Apr 21, 2025

EmilioCBen Apr 21, 2025

decsny Apr 22, 2025

EmilioCBen Apr 25, 2025

decsny Apr 22, 2025

EmilioCBen Apr 25, 2025

dleach02 commented May 1, 2025

DerekSnell left a comment

		base->CTRL &= (~EWM_CTRL_INTEN_MASK);
		if (data->timeout_cfg.callback) {

		EWM_Type *base = config->base;
		base->CTRL &= (~EWM_CTRL_INTEN_MASK);

Added EWM Driver #88389

Added EWM Driver #88389

Conversation

EmilioCBen commented Apr 9, 2025

DerekSnell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmahadevan108 Apr 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dleach02 commented May 1, 2025

DerekSnell left a comment

Choose a reason for hiding this comment

mmahadevan108 Apr 14, 2025 •

edited

Loading