8000 arch/arm/rp23xx: fix watchdog, enable TICK blocks by shtirlic · Pull Request #16299 · apache/nuttx · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

arch/arm/rp23xx: fix watchdog, enable TICK blocks #16299

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 5, 2025
Merged

arch/arm/rp23xx: fix watchdog, enable TICK blocks #16299

merged 1 commit into from
May 5, 2025

Conversation

shtirlic
Copy link
Contributor
@shtirlic shtirlic commented May 2, 2025 •
edited
Loading

Summary

On the RP2350, the watchdog receives its tick input from the system-level ticks block, unlike the RP2040, where the watchdog includes its own tick generator. Enable the TICK blocks, update the watchdog routines.

Impact

Working watchdog timer and TICK blocks.

Testing

picocalc-> wdog
[   14.430000] [  INFO] wdog: wdog_open: crefs: 0
[   14.430000] [  INFO] wdog: wdog_ioctl: cmd: 516 arg: 2000
[   14.430000] [  INFO] wdog: rp23xx_wdt_settimeout: Entry: timeout=2000
[   14.440000] [  INFO] wdog: wdog_ioctl: cmd: 513 arg: 0
[   14.450000] [  INFO] wdog: rp23xx_wdt_start: Entry
[   14.950000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   14.950000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   14.950000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   14.960000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   14.970000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   14.970000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1502
wdog_main[   14.980000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   14.990000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1502
  ping elapsed=0
[   15.490000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   15.490000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   15.490000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   15.500000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   15.510000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   15.510000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   15.520000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   15.530000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=540
[   16.030000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   16.030000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   16.030000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   16.040000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   16.050000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   16.050000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   16.060000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   16.070000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=1080
[   16.570000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   16.570000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   16.570000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   16.580000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   16.590000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   16.590000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   16.600000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   16.610000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=1620
[   17.110000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   17.110000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   17.110000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   17.120000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   17.130000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   17.130000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   17.140000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   17.150000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=2160
[   17.650000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   17.650000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   17.650000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   17.660000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   17.670000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   17.670000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   17.680000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   17.690000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=2700
[   18.190000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   18.190000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   18.190000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   18.200000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   18.210000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   18.210000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   18.220000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   18.230000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=3240
[   18.730000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   18.730000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   18.730000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   18.740000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   18.750000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   18.750000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   18.760000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   18.770000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=3780
[   19.270000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   19.270000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   19.270000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   19.280000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   19.290000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   19.290000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   19.300000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   19.310000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=4320
[   19.810000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   19.810000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   19.810000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   19.820000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   19.830000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   19.830000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main[   19.840000] [  INFO] wdog: wdog_ioctl: cmd: 518 arg: 0
[   19.850000] [  INFO] wdog: rp23xx_wdt_keepalive: Entry
: flags=00000001 timeout=2000 timeleft=1501
  ping elapsed=4860
[   20.350000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   20.350000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   20.350000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   20.360000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   20.370000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   20.370000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 1501
wdog_main: flags=00000001 timeout=2000 timeleft=1501
  NO ping elapsed=5400
[   20.880000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   20.880000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   20.880000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   20.890000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   20.900000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   20.900000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeleft : 971
wdog_main: flags=00000001 timeout=2000 timeleft=971
  NO ping elapsed=5930
[   21.410000] [  INFO] wdog: wdog_ioctl: cmd: 515 arg: 537193744
[   21.410000] [  INFO] wdog: rp23xx_wdt_getstatus: Entry
[   21.410000] [  INFO] wdog: rp23xx_wdt_getstatus: Status     :
[   21.420000] [  INFO] wdog: rp23xx_wdt_getstatus:   flags    : 00000001
[   21.430000] [  INFO] wdog: rp23xx_wdt_getstatus:   timeout  : 2000
[   21.430000] [  INFO] w
8000
dog: rp23xx_wdt_getstatus:   timeleft : 441
wdog_main: flags=00000001 timeout=2000 timeleft=441
  NO ping elapsed=6460
ABD

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium labels May 2, 2025
@nuttxpr
Copy link
nuttxpr commented May 2, 2025

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary of the changes and testing logs, it lacks crucial information required for proper review and integration.

Here's a breakdown of the missing information:

  • Summary: While the summary explains what was changed, it doesn't explain why. Was there a bug report? Is this a new feature request? It needs more context. It also lacks links to any related issues.
  • Impact: The provided impact statement is far too brief. It needs to explicitly address all the listed impact points with "NO" or "YES" and provide descriptions where necessary. Even if the answer is "NO," it's helpful to explicitly state it for each point. For example:
    • Impact on user: NO
    • Impact on build: YES (TICK block configuration needs to be enabled)
    • Impact on hardware: YES (Specifically affects RP2350)
    • Impact on documentation: YES (Documentation should be updated to reflect the changes to the watchdog and TICK block configuration)
    • Impact on security: NO
    • Impact on compatibility: Potentially YES if other boards use a similar watchdog setup; this needs investigation.
  • Testing: While testing logs are provided, there's no information about the Build Host or Target as requested. Specify the OS, CPU, Compiler used for building, and the target architecture and board configuration. Also, the logs themselves are not very descriptive. While they show the watchdog triggering, they don't show how the TICK block is verified to be functioning correctly. More comprehensive tests are needed. Before/After logs aren't particularly useful in this case; focusing on the positive verification of the fix is more important.

Example of a better Impact section:

  • Impact on user: NO (Users should not experience any functional changes besides a working watchdog timer)
  • Impact on build: YES (The RP2350_TICK config option must be enabled in the board configuration file. Example: CONFIG_RP2350_TICK=y)
  • Impact on hardware: YES (This change specifically addresses the RP2350 watchdog timer, which differs from the RP2040 implementation. It utilizes the system TICK blocks.)
  • Impact on documentation: YES (Documentation should be updated to explain the RP2350 watchdog timer configuration and the requirement for enabling the TICK blocks.)
  • Impact on security: NO (This change does not have any direct security implications.)
  • Impact on compatibility: YES (This change is specific to the RP2350. It needs to be verified that this doesn't negatively impact other boards that might share similar hardware blocks. A review of other board configurations is recommended.)

By providing more complete information in these sections, the PR will be much easier to review, understand, and ultimately merge.

@shtirlic shtirlic marked this pull request as draft May 2, 2025 06:32
@shtirlic
Copy link
Contributor Author
shtirlic commented May 2, 2025

Converted to Draft for more testing

@shtirlic shtirlic force-pushed the fixwd branch 4 times, most recently from 467b76b to 5c00dab Compare May 3, 2025 05:01
@shtirlic shtirlic marked this pull request as ready for review May 3, 2025 08:07
@shtirlic
Copy link
Contributor Author
shtirlic commented May 4, 2025

@acassis could you review it? I can't request or notify the reviewers after converting from the draft.

@shtirlic
arch/arm/rp23xx: fix watchdog, enable TICK blocks
557cd17 
On the RP2350, the watchdog receives its tick input from the
system-level ticks block, unlike the RP2040, where the watchdog includes
its own tick generator. Enable the TICK blocks, update the watchdog
routines.

Signed-off-by: Serg Podtynnyi <serg@podtynnyi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 95cc862 into apache:master May 5, 2025
25 checks passed
@shtirlic shtirlic deleted the fixwd branch May 6, 2025 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acassis acassis acassis approved these changes

@jerpelea jerpelea jerpelea approved these changes

@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

Assignees
No one assigned
Labels
Arch: arm Issues related to ARM (32-bit) architecture Size: M The size of the change in this PR is medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@shtirlic @nuttxpr @acassis @jerpelea @xiaoxiang781216
0