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

imx9/flexcan: Add disable/enable cycle #14685

Merged

Conversation

jlaitine
Copy link
Contributor

@jlaitine jlaitine commented Nov 7, 2024

Summary

This corrects the imx9 can bus driver in case of warm reboot, where the flexcan block might be already initialized

Impact

Impacts only imx9 canbus when doing warm reboot

Testing

Tested on a imx93 board

Add enable/disable cycle to initialize
function to speed up soft reset

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 7, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 7, 2024

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the basic requirements, but it is lacking crucial details. While it addresses the summary and testing sections, it's far too brief and omits important information.

Here's why it's incomplete and how to improve it:

  • Summary: While it states what is changed, it lacks the how and why. How does the fix correct the initialization issue? What was the root cause of the problem in the first place? A related NuttX issue should be linked if one exists.
  • Impact: While it mentions the impact on the i.MX9 CAN bus, it lacks specificity. Does this fix a bug? Improve performance? Add a feature? The other impact categories are completely unanswered (build, hardware, documentation, security, compatibility). Even if the answer is "NO," it should be explicitly stated.
  • Testing: "Tested on an i.MX93 board" is insufficient. Provide details:
    • Which i.MX93 board specifically?
    • Which NuttX configuration?
    • What tests were run?
    • Crucially: Include the requested "before" and "after" logs. These demonstrate that the change has the intended effect. Without these logs, the testing claim is unsubstantiated.

In short: The PR provides a starting point but needs significantly more detail to be considered complete. The missing information makes it difficult to review and assess the impact of the change.

@xiaoxiang781216 xiaoxiang781216 merged commit d260e7f into apache:master Nov 7, 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.

4 participants