Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support rtwdog on rt1180 #82456

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SuperHeroAbner
Copy link

Added the rtwdog Zephyr driver for RT1180 EVK.

@SuperHeroAbner
Copy link
Author

Related hal_nxp PR: zephyrproject-rtos/hal_nxp#482

@lucien-nxp
Copy link
Contributor

lucien-nxp commented Dec 3, 2024

Related hal_nxp PR: zephyrproject-rtos/hal_nxp#482

Please record your hal_nxp PR in west.yml like : pull/482/head.
After added, please inform Hake Huang help test your PR on RT1180 platform. Thank you.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 3, 2024

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

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@c42d70d zephyrproject-rtos/hal_nxp#482 zephyrproject-rtos/hal_nxp#482/files

DNM label due to: 1 project with PR revision

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

@zephyrbot zephyrbot added manifest manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Dec 3, 2024
Port NXP rtwdog driver to Zephyr.

Signed-off-by: Ruijia Wang <[email protected]>
RT1180 takes reset event mask feature which should be unmasked when
watch dog is enabled.

Signed-off-by: Ruijia Wang <[email protected]>
Sync pr for test.

Signed-off-by: Ruijia Wang <[email protected]>
Copy link
Collaborator

@danieldegrasse danieldegrasse left a comment

Choose a reason for hiding this comment

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

Please take a look at the compliance errors, the github review UI highlights the issues that need fixing nicely


description: imxRT watchdog

compatible: "nxp,imx-rtwdog"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's avoid using imx in the compatible name, this IP will probably be reused outside of the i.MX RT line at some point.

I'd suggest the compatible nxp,rtwdog

@@ -0,0 +1,28 @@
# Copyright (c) 2018, NXP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Copyright (c) 2018, NXP
# Copyright 2024, NXP

@@ -0,0 +1,222 @@
/*
* Copyright (C) 2024, NXP
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Copyright (C) 2024, NXP
* Copyright 2024, NXP

#define MSEC_TO_RTWDOG_TICKS(clock_freq, divider, msec) \
((uint32_t)(clock_freq * msec / 1000U / divider))

#ifndef U16_MAX
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not include <stdint.h> and use UINT16_MAX


config->irq_config_func(dev);

ret = pinctrl_apply_state(config->pcfg, PINCTRL_STATE_DEFAULT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need a pin setting for this watchdog? It seems to have no external signals


static struct mcux_rtwdog_data mcux_rtwdog_data;

DEVICE_DT_INST_DEFINE(0, &mcux_rtwdog_init, NULL, &mcux_rtwdog_data, &mcux_rtwdog_config,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not use this syntax here, this driver can easily be multi instance. Take a look at this driver for an example of a multi-instance watchdog setup:

#define GECKO_WDT_INIT(index) \

clk-divider:
type: int
description: Watchdog counter clock divider
required: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing newline at end of file

@@ -56,6 +59,14 @@ static const clock_arm_pll_config_t armPllConfig_BOARD_BootClockRUN = {
};
#endif

#if defined(CONFIG_WDT_MCUX_IMX_RTWDOG)
#define RTWDOG_IF_SET_SRC(n, i) \
if (strcmp(DT_PROP(DT_NODELABEL(rtwdog##n), status), "okay") == 0) { \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use if (IS_ENABLED(DT_NODE_HAS_COMPAT_STATUS(DT_NODELABEL(rtwdog#nn), nxp_imx_rtwdog, okay)) here, instead of the runtime check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Watchdog Watchdog DNM This PR should not be merged (Do Not Merge) manifest manifest-hal_nxp platform: NXP Drivers NXP Semiconductors, drivers platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants