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

arch/arm64/imx9/imx9_flexspi: Replace memcpy by while loop #14973

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

joukkone
Copy link
Contributor

libc memcpy cannot access fspi memory space correctly remove unnecessary debugassert and cache operations

Note: Please adhere to Contributing Guidelines.

Summary

Update this section with information on why change is necessary,
what it exactly does and how, if new feature shows up, provide
references (dependencies, similar problems and solutions), etc.

Impact

Update this section, where applicable, on how change affects users,
build process, hardware, documentation, security, compatibility, etc.

Testing

Update this section with details on how did you verify the change,
what Host was used for build (OS, CPU, compiler, ..), what Target was
used for verification (arch, board:config, ..), etc. Providing build
and runtime logs from before and after change is highly appreciated.

libc memcpy cannot access fspi memory space correctly
remove unnecessary debugassert and cache operations

Signed-off-by: Jouni Ukkonen <[email protected]>
@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small labels Nov 28, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 28, 2024

[Experimental Bot, please feedback here]

This PR appears incomplete and does not meet the NuttX requirements. It contains placeholder text in the Impact and Testing sections, indicating the author hasn't provided the necessary information. The Summary section also needs more detail.

Here's a breakdown of what's missing:

  • Summary: The title "libc memcpy cannot access fspi memory space correctly remove unnecessary debugassert and cache operations" is a good start, but it needs expansion. Explain why memcpy was failing to access FSPI memory. What was the root cause? How does removing the debugassert and cache operations fix the problem? What were the specific cache operations removed? Why were they unnecessary? If there's a related NuttX issue, include the link.

  • Impact: All the impact sub-sections are placeholders. The author needs to fill these in. Consider:

    • User impact: Does this fix a bug that users were encountering? Does it improve performance?
    • Build impact: Does this change require any build system modifications?
    • Hardware impact: Does this change affect specific architectures, boards, or drivers? Specifically, which ones related to FSPI?
    • Documentation impact: Does the documentation need to be updated to reflect this change?
    • Security impact: Does this fix a security vulnerability? Does it introduce any new ones?
    • Compatibility impact: Does this change break compatibility with any existing code or hardware configurations?
  • Testing: The Testing section is completely empty. The author must provide:

    • Details of the build host (OS, CPU architecture, compiler version).
    • Details of the target hardware (architecture, board, configuration).
    • Logs demonstrating the issue before the change. Show how memcpy was failing.
    • Logs demonstrating the fix after the change. Show how memcpy now works correctly.

Without this information, it's impossible to review the PR effectively and determine if it's suitable for merging. The author needs to thoroughly complete these sections before the PR can be considered.

@xiaoxiang781216 xiaoxiang781216 merged commit 1d23baa into apache:master Nov 28, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm64 Issues related to ARM64 (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.

3 participants