Skip to content

Use small lock to protect resources related to irq in arch risc-V and…#15634

Merged
xiaoxiang781216 merged 1 commit into
apache:masterfrom
wangzhi16:lock_13
Jan 22, 2025
Merged

Use small lock to protect resources related to irq in arch risc-V and…#15634
xiaoxiang781216 merged 1 commit into
apache:masterfrom
wangzhi16:lock_13

Conversation

@wangzhi16
Copy link
Copy Markdown
Contributor

… xtensa.

Note: Please adhere to Contributing Guidelines.

Summary

Use small lock to protect resources related to irq in arch risc-V and xtensa.

Impact

The underlying IRQ logic of arch risc-V and xtensa.

Testing

CI

@github-actions github-actions Bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: M The size of the change in this PR is medium labels Jan 22, 2025
@nuttxpr
Copy link
Copy Markdown

nuttxpr commented Jan 22, 2025

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details.

Here's a breakdown of what's missing:

  • Summary: While it states what is changed, it doesn't explain why (what problem does this solve?). It also lacks detail on how the small lock is used. There's no mention of related issues.
  • Impact: The description under "Impact" describes what code is changed, not the impact of the change. All the YES/NO questions are unanswered. This is the most significant deficiency. Consider the impact on users, build process, hardware support, documentation needs, security implications, compatibility, etc. Even if the answer is NO, explicitly state it.
  • Testing: Simply stating "CI" is insufficient. Provide specific details about the build host (OS, CPU, compiler version) and target(s) (architecture, board, configuration). The "Testing logs before change" and "Testing logs after change" sections are empty. Real logs demonstrating the issue before the change and the corrected behavior after the change are required.

The PR needs to be significantly revised to provide the required information before it can be properly reviewed. Simply listing the requirements and writing one-line descriptions doesn't fulfill the intent of having clear and comprehensive information for reviewers.

Comment thread arch/xtensa/src/esp32s2/esp32s2_irq.c Outdated
@wangzhi16 wangzhi16 force-pushed the lock_13 branch 2 times, most recently from 4dc8b63 to 54bf214 Compare January 22, 2025 05:16
… xtensa.

Signed-off-by: wangzhi16 <wangzhi16@xiaomi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crtical section should be replaced with spinlock as much as we can to improve SMP performance

4 participants