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

Documentation: how to use auto monitor feature to keep the chip alive #2543

Merged
merged 5 commits into from Dec 18, 2020

Conversation

saramonteiro
Copy link
Contributor

Summary

It describes how to use each auto monitor feature and briefly how it works.

Impact

N/A

Testing

N/A

Copy link
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

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

Just a question

This command resets the watchdog timer ("ping", "pet the dog", "feed the dog").

Using the Auto-monitor feature to keep the chip alive
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused by this. Is this named correctly? A HW watchdog needs to be "reset" if not when it times out it is "triggered".

So what is Auto-monitor? Is the generating the keep-alive? Then it is not a monitor,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @davids5
What name do you refer to?
The Auto-monitor is an option that is under the Watchdog Timer Support option.
The Auto-monitor provides 4 mechanisms to feed the dog automatically.
I think the auto prefix stands for automatically,i.e., without the developer configuring it in execution code and the monitor word means that it will monitor your code because if one task gets stuck the timer will expire and the chip will be reseted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I get it: It is really built in monitoring the system and resetting the watchdog. "Enable Built in System Monitoring to reset the watchdog."

Is the timer one a HW timer off an ISR? Because WD kicking done off an ISR is a really bad idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Enable Built in System Monitoring to reset the watchdog." is a better title indeed.

The timer option calls wd_start in the beginning.

This function adds a watchdog timer to the active timer queue.

and registers a function that will be called from the interrupt level after the specified number of ticks has elapsed.
When this option is selected, the registered function resets the watchdog and calls wd_start again registering itself again.

It is implemented in drivers/timers/watchdog.c

Copy link
Contributor

@davids5 davids5 Dec 15, 2020

Choose a reason for hiding this comment

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

"Enable Built in System Monitoring to reset the watchdog." is a better title indeed.

I am easily confused by what I read a lot so I am glad it makes sense,

registers a function that will be called from the interrupt level after the specified number of ticks has elapsed.

Yikes, I would put a caution all over that to not to not use it, less one really knows what they are doing.,,,,,

Documentation/components/drivers/character/watchdog.rst Outdated Show resolved Hide resolved
Documentation/components/drivers/character/watchdog.rst Outdated Show resolved Hide resolved
This command resets the watchdog timer ("ping", "pet the dog", "feed the dog").

Using the Auto-monitor feature to keep the chip alive
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I get it: It is really built in monitoring the system and resetting the watchdog. "Enable Built in System Monitoring to reset the watchdog."

Is the timer one a HW timer off an ISR? Because WD kicking done off an ISR is a really bad idea.

Documentation/components/drivers/character/watchdog.rst Outdated Show resolved Hide resolved
Co-authored-by: David Sidrane <David.Sidrane@Nscdg.com>
``Timer callback``: This choice also uses a timer callback to reset the watchdog, but it will reset the watchdog every "keep a live interval".

``Worker callback``: This choice uses the Low Priority Work Queue to reset the watchog every "keep a live interval". This choice depends on having the Low Priority Work Queue enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

not really need Low Priority Work Queue:

  1. If only high work enable, the high work queue is used
  2. otherwise the low work queue is used

This is because, low work queue will map to low work queue if only high work queue is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum, so I'll change and add it as follows:

Worker callback: This choice uses a Work Queue to reset the watchdog every "keep a live interval". This choice depends on having the Low or High Priority Work Queue enabled.
If only the High Priority Work Queue is enabled, this one will be used, otherwise Low Priority Work Queue is used.

What do you think?

BTW, I thought it was only LPWORK because of the configuration:

      work_queue(LPWORK, &upper->work, watchdog_automonitor_worker,
                 upper, WATCHDOG_AUTOMONITOR_PING_INTERVAL);

I didn't know the LPWORK was replaced by the HPWORK in case the latter was enabled and the former was not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why invent terms? Keep alive -> Watchdog Reset interval

Copy link
Contributor

Choose a reason for hiding this comment

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

@saramonteiro yes, it's accurate now, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davids5 I did not create terms like "Keep alive" and "pet the dog" or "ping".
Actually I just used the terms I saw in drivers/timers/watchdog.c and in include/nuttx/timers/watchdog.h
For example, the ioctl that feeds the dog is actually named WDIOC_KEEPALIVE.
My main goal with this PR is to document the WDT related features sticking to the current terms used in the source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xiaoxiang781216 thank you! =)

Copy link
Contributor

Choose a reason for hiding this comment

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

@davids5 I did not create terms like "Keep alive" and "pet the dog" or "ping".
Actually I just used the terms I saw in drivers/timers/watchdog.c and in include/nuttx/timers/watchdog.h
For example, the ioctl that feeds the dog is actually named WDIOC_KEEPALIVE.
My main goal with this PR is to document the WDT related features sticking to the current terms used in the source code.

Fare enough.

Documentation/components/drivers/character/watchdog.rst Outdated Show resolved Hide resolved
``Timer callback``: This choice also uses a timer callback to reset the watchdog, but it will reset the watchdog every "keep a live interval".

``Worker callback``: This choice uses the Low Priority Work Queue to reset the watchog every "keep a live interval". This choice depends on having the Low Priority Work Queue enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

@davids5 I did not create terms like "Keep alive" and "pet the dog" or "ping".
Actually I just used the terms I saw in drivers/timers/watchdog.c and in include/nuttx/timers/watchdog.h
For example, the ioctl that feeds the dog is actually named WDIOC_KEEPALIVE.
My main goal with this PR is to document the WDT related features sticking to the current terms used in the source code.

Fare enough.

@xiaoxiang781216
Copy link
Contributor

Is it ready for merge?

@davids5 davids5 merged commit 8de9cba into apache:master Dec 18, 2020
@btashton btashton added this to To-Add in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from To-Add to Minor in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from Minor to DOCS in Release Notes - 10.1.0 Apr 16, 2021
@jerpelea jerpelea moved this from DOCS to Added in Release Notes - 10.1.0 Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants