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

mmcsd: move block driver register to probe stage in order to read ext_csd reg #13705

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

gneworld
Copy link

Summary

mmcsd: move block driver register to probe stage in order to read ext_csd reg

Impact

mmcsd

Testing

cortext M55

@github-actions github-actions bot added the Size: S The size of the change in this PR is small label Sep 28, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 28, 2024

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX Requirements.

Missing Information:

  • Summary:
    • A clear explanation of why this change is necessary is missing. What problem does moving the block driver registration solve?
    • How does moving the registration to the probe stage enable reading the ext_csd register? Provide a technical explanation.
  • Impact:
    • The impact section is far too brief. Address all the bullet points (user impact, build impact, etc.) with "YES" or "NO" and provide details where applicable. Even if there's no impact, state "NO" explicitly for clarity.
  • Testing:
    • Insufficient Detail: "cortext M55" is not enough information. Specify:
      • Host OS: (e.g., Linux Ubuntu 20.04)
      • Compiler Version: (e.g., GCC 10.3.0)
      • NuttX Configuration: (e.g., configs/cortex-m55/nsh)
    • Missing Logs: Provide actual testing logs from before and after the change to demonstrate the issue and the fix.

Recommendations:

  1. Elaborate on the "Why": Clearly state the problem that this change addresses. Why is reading the ext_csd register important, and why was it not possible before?
  2. Explain the Technical Details: Describe the code changes and how they enable reading the ext_csd register.
  3. Complete the Impact Section: Carefully consider and address all impact points, even if the answer is "NO".
  4. Provide Detailed Testing Information: Include specific host and target details, along with actual testing logs demonstrating the problem and the solution.

@xiaoxiang781216
Copy link
Contributor

@gneworld please fix(https://github.com/apache/nuttx/actions/runs/11084304188/job/30799485530?pr=13705):

mmcsd/mmcsd_sdio.c: In function 'mmcsd_slotinitialize':
Error: mmcsd/mmcsd_sdio.c:4367:47: error: 'devname' undeclared (first use in this function); did you mean 'rename'?
 4367 |   finfo("MMC: %s %" PRIu64 "KB %s %s mode\n", devname,
      |                                               ^~~~~~~

@gneworld
Copy link
Author

gneworld commented Oct 4, 2024

@gneworld please fix(https://github.com/apache/nuttx/actions/runs/11084304188/job/30799485530?pr=13705):

mmcsd/mmcsd_sdio.c: In function 'mmcsd_slotinitialize':
Error: mmcsd/mmcsd_sdio.c:4367:47: error: 'devname' undeclared (first use in this function); did you mean 'rename'?
 4367 |   finfo("MMC: %s %" PRIu64 "KB %s %s mode\n", devname,
      |                                               ^~~~~~~

@xiaoxiang781216
done

@xiaoxiang781216 xiaoxiang781216 merged commit e16c785 into apache:master Oct 5, 2024
29 checks passed
@gneworld gneworld deleted the work1 branch October 7, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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