[PR] GreenHills support: fix the naked_function function non-compatible issue with arm_svcall [nuttx]

["extinguish (via GitHub)" <gi...@apache.org>]

extinguish opened a new pull request, #14718:
URL: https://github.com/apache/nuttx/pull/14718

   ## Summary
   
   when build with greenhills_202354, the naked_function should only contains the basic asm statements, so if the naked_function contains the statements that using to accept params from C lang runtime, then following build error will reported:
   ```
   error #101112: Only basic asm expressions are allowed for functions
   with __attribute__((naked)): dispatch_syscall
   ```
   
   ## Impact
   
   has no impact on other toolchain
   
   ## Testing
   
   1. has passed the ostest
   2. has tested on ghs and gcc toolchains
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GreenHills support: fix the naked_function function non-compatible issue with arm_svcall [nuttx]

["nuttxpr (via GitHub)" <gi...@apache.org>]

nuttxpr commented on PR #14718:
URL: https://github.com/apache/nuttx/pull/14718#issuecomment-2467415130

   [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
   
   
   
   Yes, this PR appears to meet the basic NuttX requirements, but it could be improved with more detail.  Here's a breakdown:
   
   **Strengths:**
   
   * **Clear problem statement:** The summary explains the issue encountered with the Green Hills compiler and why the change is necessary.
   * **Specific impact:** It correctly notes the limited impact to only the Green Hills toolchain.
   * **Testing performed:**  Confirmation of testing with both Green Hills and GCC is good.
   
   **Weaknesses & Suggestions for Improvement:**
   
   * **Missing details in Summary:**
       * **What functional part of the code is changed?**  Be explicit.  e.g., "The `dispatch_syscall` function in `arch/arm/src/armv7-m/up_svcall.c`".
       * **How does the change exactly work?** Briefly describe the code modification.  e.g., "Removed parameter handling code from the naked function and implemented a workaround using assembly to access registers directly."
       * **Issue references:** If there's a related NuttX issue, link it.
   
   * **Missing details in Impact:** While stating "no impact on other toolchains" is good, explicitly stating "NO" for all other impact categories (user, build process for other toolchains, hardware, documentation, security, compatibility) would improve clarity and completeness.
   
   * **Insufficient Testing Detail:**
       * **Build Host(s):**  Provide more detail about your build host environment for *both* Green Hills and GCC testing. Include OS, CPU architecture, and the *specific* versions of the compilers used.
       * **Target(s):** Specify the target architecture and board configuration used for testing.  "sim" is too vague.  Be specific (e.g., "qemu-armv7m", "stm32f4discovery").
       * **Missing logs:** The template requests "Testing logs before change" and "Testing logs after change."  Even if the logs are lengthy, include a concise snippet demonstrating the error before the change and its absence after the change. This provides concrete evidence of the fix.
   
   
   **Example of Improved Testing Section:**
   
   ```
   ## Testing
   
   I confirm that changes are verified on local setup and works as intended:
   
   * Build Host(s):
       * Green Hills: Windows 10, Intel Core i7, Green Hills MULTI 2023.5.4
       * GCC: Ubuntu 20.04, x86_64, GCC 10.3.0
   
   * Target(s):
       * qemu-armv7m: cortex-m7, default configuration
   
   Testing logs before change:
   
   ```
   arm-none-eabi-gcc: error: Only basic asm expressions are allowed for functions with __attribute__((naked)): dispatch_syscall
   ...
   ```
   
   Testing logs after change:
   
   ```
   ostest results:
   ... (Relevant successful test output demonstrating correct syscall handling) ...
   ```
   
   
   
   By providing this additional information, the PR review process will be much smoother and faster.  Reviewers will have a clearer understanding of the change, its impact, and the verification performed, leading to quicker acceptance.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] GreenHills support: fix the naked_function function non-compatible issue with arm_svcall [nuttx]

["anchao (via GitHub)" <gi...@apache.org>]

anchao merged PR #14718:
URL: https://github.com/apache/nuttx/pull/14718


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org