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

kernel/os: Fix issue that causes WDOG reset reason to be lost #3094

Closed

Conversation

benmccrea
Copy link
Contributor

In the default irq handler, hal_watchdog_tickle() was called before doing a core dump but on Dialog da1469x this causes the reset reason to be lost, and reboot log was recording the reset reason as HAL_RESET_SOFT instead of HAL_RESET_WDOG.

The issue is fixed by calling hal_watchdog_disable() instead of hal_watchdog_tickle() before doing a core dump.

This PR also adds hal_watchdog_disable() to the HAL API for all MCUs (for now it is implemented only for Dialog da1469x and CMAC).

In the default irq handler, hal_watchdog_tickle() was called before doing a core dump
but on Dialog da1469x this causes the reset reason to be lost, and reboot log was
recording the reset reason as HAL_RESET_SOFT instead of HAL_RESET_WDOG.

The issue is fixed by calling hal_watchdog_disable() instead of hal_watchdog_tickle()
before doing a core dump.

This PR also adds hal_watchdog_disable() to the HAL API for all MCUs (for now it
is implemented only for Dialog da1469x and CMAC).
Copy link
Contributor

@vrahane vrahane left a comment

Choose a reason for hiding this comment

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

I tried it out. It generates a coredump fine on a watchdog with the correct reboot reason. I will let others comment as well. If there are no issues, will merge it in. FYI, this is what caused the issue: #2440

@vrahane vrahane requested review from wes3 and kasjer October 13, 2023 20:36
@andrzej-kaczmarek
Copy link
Contributor

If you replace tickle with disable and leave disable not implemented for all platforms but dialog, it will possibly break those platforms as in mentioned issue. Also watchdog cannot be disabled on some platforms, in particular nrf52, so this won't work.

So perhaps what can be done instead is to make call to tickle optional via syscfg and then use coredump callback to disable watchdog via registers to avoid having an API that is not implemented on any platform.

@benmccrea
Copy link
Contributor Author

@andrzej-kaczmarek Thanks, I agree with you. We'll find a different way. We can use os_coredump_cb as you suggest, or we might be able to modify how watchdog reboot reason is detected for da1469x so that it isn't lost by the tickle.
Closing this unmerged.

@benmccrea benmccrea closed this Oct 16, 2023
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.

3 participants