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

{bp-13585} riscv: use g_running_task store current regs #13788

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

jerpelea
Copy link
Contributor

@jerpelea jerpelea commented Oct 2, 2024

Summary

This commit fixes the regression from #13561

In order to determine whether a context switch has occurred, we can use g_running_task to store the current regs. This allows us to compare the current register state with the previously stored state to identify if a context switch has taken place.

Impact

RELEASE

Testing

CI

This commit fixes the regression from apache#13561

In order to determine whether a context switch has occurred,
we can use g_running_task to store the current regs.
This allows us to compare the current register state with the previously
stored state to identify if a context switch has taken place.

Signed-off-by: hujun5 <[email protected]>
@github-actions github-actions bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: S The size of the change in this PR is small labels Oct 2, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 2, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements.

Missing Information:

  • Summary:
    • What functional part of the code is being changed? Be specific about the module or subsystem.
    • How does the change exactly work? Provide a more detailed explanation of the technical implementation.
  • Impact:
    • Is existing feature changed? This seems likely given the mention of a regression.
    • Impact on user: Even if the impact is minimal, explain why a user wouldn't need to adapt.
    • Impact on hardware: Specify the architectures/boards affected by the original regression and thus fixed by this PR.
  • Testing:
    • Build Host(s): List the specific operating system, CPU architecture, and compiler used for testing.
    • Target(s): Provide the specific architecture(s), board(s), and configurations tested.
    • Testing logs: Placeholder logs are insufficient. Include actual logs demonstrating the issue before and the fix after the change.

Recommendations:

  1. Expand the Summary: Clearly describe the code section modified and the technical details of using g_running_task for context switch detection.
  2. Complete the Impact Section: Address all bullet points, even if the answer is "NO" with a brief justification. Be specific about affected architectures and boards.
  3. Provide Detailed Testing Information: List all build hosts and target environments. Include actual testing logs showing the problem before and the solution after your change.

By providing this missing information, your PR will be more understandable, reviewable, and likely to be merged quickly.

@pkarashchenko
Copy link
Contributor

There are strange errors reported by CI

@lupyuen
Copy link
Member

lupyuen commented Oct 2, 2024

There are strange errors reported by CI

https://github.com/apache/nuttx/actions/runs/11142136706/job/30964518546?pr=13788

Error: gpio_main.c:317:12: error: 'GPIO_INTERRUPT_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_PIN'?
  317 |       case GPIO_INTERRUPT_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_PIN
gpio_main.c:317:12: note: each undeclared identifier is reported only once for each function it appears in
Error: gpio_main.c:318:12: error: 'GPIO_INTERRUPT_HIGH_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_HIGH_PIN'?
  318 |       case GPIO_INTERRUPT_HIGH_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_HIGH_PIN
Error: gpio_main.c:319:12: error: 'GPIO_INTERRUPT_LOW_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_LOW_PIN'?
  319 |       case GPIO_INTERRUPT_LOW_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_LOW_PIN
Error: gpio_main.c:320:12: error: 'GPIO_INTERRUPT_RISING_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_RISING_PIN'?
  320 |       case GPIO_INTERRUPT_RISING_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_RISING_PIN
Error: gpio_main.c:321:12: error: 'GPIO_INTERRUPT_FALLING_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_FALLING_PIN'?
  321 |       case GPIO_INTERRUPT_FALLING_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_FALLING_PIN
Error: gpio_main.c:322:12: error: 'GPIO_INTERRUPT_BOTH_PIN_WAKEUP' undeclared (first use in this function); did you mean 'GPIO_INTERRUPT_BOTH_PIN'?
  322 |       case GPIO_INTERRUPT_BOTH_PIN_WAKEUP:
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |            GPIO_INTERRUPT_BOTH_PIN

I think GPIO_INTERRUPT_PIN_WAKEUP comes from this newer PR: #13675

Which is referenced by the newer examples/gpio: apache/nuttx-apps#2614

@xiaoxiang781216 xiaoxiang781216 merged commit d1fec65 into apache:releases/12.7 Oct 3, 2024
7 of 29 checks passed
@jerpelea jerpelea deleted the bp-13585 branch October 3, 2024 07:50
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 Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants