Fixbug: Fix the bug emmc bug #11139
Fixbug: Fix the bug emmc bug #11139
Conversation
…the card is not queried, which can cause failure in switching to the high-speed mode in DDR mode.
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
|
unknown seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-01-16 15:35 CST)
📝 Review Instructions
|
|
For more details, please refer to: https://club.rt-thread.org/ask/question/26ae04a2e1a47f0d.html |
There was a problem hiding this comment.
Pull request overview
This PR fixes an eMMC bug where CMD6 (SWITCH command) is sent to modify the EXT_CSD register, but the card status is not subsequently polled to verify the operation completed successfully. This can cause failures when switching to high-speed DDR mode. The fix adds two new helper functions (mmc_send_status and mmc_poll_for_busy) that implement CMD13 status polling with timeout handling, and integrates this polling into the mmc_switch function.
Changes:
- Added
mmc_send_status()function to send CMD13 and retrieve card status - Added
mmc_poll_for_busy()function to poll card status until ready or timeout - Modified
mmc_switch()to callmmc_poll_for_busy()after sending CMD6
| if (out) | ||
| { | ||
| LOG_E("wait card busy timeout"); | ||
| return -RT_ETIMEOUT; | ||
| } |
There was a problem hiding this comment.
[Bug/错误]: Timeout check logic is incorrect / 超时检查逻辑错误
English: The timeout check is performed AFTER sending the status command. This means:
- If the card is already ready (status & R1_READY_FOR_DATA is true), the timeout check is skipped entirely due to the while loop condition at line 330
- The timeout is only checked after each status query, which means the loop could run one extra iteration beyond the timeout
The timeout check should be done BEFORE checking status or at the beginning of the loop to ensure accurate timeout behavior.
中文：超时检查在发送状态命令之后执行。这意味着：
- 如果卡已经就绪（status & R1_READY_FOR_DATA 为真），由于第 330 行的 while 循环条件，超时检查会被完全跳过
- 超时仅在每次状态查询后检查，这意味着循环可能会在超时后多运行一次迭代
超时检查应该在检查状态之前或在循环开始时执行，以确保准确的超时行为。
| start = rt_tick_get(); | ||
| do | ||
| { | ||
| rt_bool_t out = (int)(rt_tick_get() - start) > timeout; |
There was a problem hiding this comment.
[Naming/命名]: Variable name 'out' is unclear / 变量名 'out' 不清晰
English: The variable name out at line 308 is ambiguous. It's not clear whether it means "output", "timeout occurred", or something else.
A clearer name would be timeout_expired, is_timeout, or timed_out to explicitly indicate that it represents the timeout condition.
中文：第 308 行的变量名 out 含义模糊。不清楚它是表示 "output"、"超时发生" 还是其他含义。
更清晰的名称应该是 timeout_expired、is_timeout 或 timed_out，以明确表示它代表超时条件。
| return err; | ||
|
|
||
| /* | ||
| * Poll the status of the cmd13 card with a timeout of 500ms and a polling interval of 1ms. |
There was a problem hiding this comment.
[Spelling/拼写]: Typo in comment / 注释中的拼写错误
English: The comment says "Poll the status of the cmd13 card" but should say "Poll the card status using CMD13" or "Poll the card status with CMD13". The phrase "cmd13 card" is grammatically incorrect - CMD13 is the command, not a type of card.
中文：注释说 "Poll the status of the cmd13 card"，但应该是 "Poll the card status using CMD13" 或 "Poll the card status with CMD13"。短语 "cmd13 card" 语法不正确 - CMD13 是命令，不是卡的类型。
| * Poll the status of the cmd13 card with a timeout of 500ms and a polling interval of 1ms. | |
| * Poll the card status using CMD13 with a timeout of 500ms and a polling interval of 1ms. |
| err = mmc_send_status(card, &status, retries); | ||
| if (err) | ||
| { | ||
| LOG_E("error %d requesting status", err); | ||
| return err; | ||
| } | ||
|
|
||
| if (out) | ||
| { | ||
| LOG_E("wait card busy timeout"); | ||
| return -RT_ETIMEOUT; | ||
| } | ||
|
|
||
| rt_thread_mdelay(1); | ||
| /* | ||
| * Some cards mishandle the status bits, | ||
| * so make sure to check both the busy | ||
| * indication and the card state. | ||
| */ | ||
| } | ||
| while (!(status & R1_READY_FOR_DATA)); |
There was a problem hiding this comment.
[Bug/错误]: Missing error status check in card status response / 卡状态响应中缺少错误状态检查
English: The function mmc_poll_for_busy() only checks for R1_READY_FOR_DATA bit but doesn't check for error bits in the card status response. According to the MMCSD specification, the R1 response contains various error status bits (defined in dev_mmcsd_core.h, such as R1_ERROR, R1_CC_ERROR, R1_CARD_ECC_FAILED, etc.) that should be checked.
If the card reports an error status, the function should detect and return an error instead of continuing to poll. Consider adding a check like:
- Check
R1_STATUS(status)macro to detect error conditions - Return appropriate error if card reports an error state
中文：函数 mmc_poll_for_busy() 只检查 R1_READY_FOR_DATA 位，但不检查卡状态响应中的错误位。根据 MMCSD 规范，R1 响应包含各种错误状态位（在 dev_mmcsd_core.h 中定义，如 R1_ERROR、R1_CC_ERROR、R1_CARD_ECC_FAILED 等）应该被检查。
如果卡报告错误状态，函数应该检测并返回错误，而不是继续轮询。考虑添加检查，例如：
- 检查
R1_STATUS(status)宏以检测错误条件 - 如果卡报告错误状态则返回相应的错误
| /* | ||
| * Poll Busy. | ||
| */ | ||
| static int mmc_poll_for_busy(struct rt_mmcsd_card *card, unsigned int timeout_ms, int retries) |
There was a problem hiding this comment.
[Best Practices/最佳实践]: Inconsistent parameter type usage / 参数类型使用不一致
English: The function mmc_poll_for_busy() uses unsigned int timeout_ms for the timeout parameter, while mmc_send_status() uses unsigned retries. This inconsistency in using unsigned int vs unsigned reduces code readability.
For consistency with RT-Thread coding style, consider using explicit types like rt_uint32_t for timeout_ms, or use unsigned int consistently throughout the function parameters.
中文：函数 mmc_poll_for_busy() 对超时参数使用 unsigned int timeout_ms，而 mmc_send_status() 使用 unsigned retries。这种在 unsigned int 和 unsigned 之间的不一致降低了代码可读性。
为了与 RT-Thread 编码风格保持一致，考虑对 timeout_ms 使用显式类型如 rt_uint32_t，或在整个函数参数中一致使用 unsigned int。
| static int mmc_poll_for_busy(struct rt_mmcsd_card *card, unsigned int timeout_ms, int retries) | |
| static int mmc_poll_for_busy(struct rt_mmcsd_card *card, unsigned timeout_ms, int retries) |
| static int mmc_send_status(struct rt_mmcsd_card *card, rt_uint32_t *status, unsigned retries) | ||
| { | ||
| int err; | ||
| struct rt_mmcsd_cmd cmd; |
There was a problem hiding this comment.
[Best Practices/最佳实践]: Inconsistent structure initialization pattern / 结构体初始化模式不一致
English: In mmc_send_status(), the rt_mmcsd_cmd cmd structure is declared without initialization, and then individual fields are set. However, in mmc_switch() at line 349, the structure is initialized with { 0 }.
For consistency and to avoid potential uninitialized field issues, consider using the same initialization pattern. The safer approach is to initialize with { 0 } as done in mmc_switch().
中文：在 mmc_send_status() 中，rt_mmcsd_cmd cmd 结构体声明时没有初始化，然后设置各个字段。然而，在第 349 行的 mmc_switch() 中，结构体用 { 0 } 初始化。
为了保持一致性并避免潜在的未初始化字段问题，考虑使用相同的初始化模式。更安全的方法是像 mmc_switch() 中那样使用 { 0 } 初始化。
| struct rt_mmcsd_cmd cmd; | |
| struct rt_mmcsd_cmd cmd = (struct rt_mmcsd_cmd){ 0 }; |
| rt_thread_mdelay(1); | ||
| /* | ||
| * Some cards mishandle the status bits, | ||
| * so make sure to check both the busy | ||
| * indication and the card state. | ||
| */ | ||
| } | ||
| while (!(status & R1_READY_FOR_DATA)); |
There was a problem hiding this comment.
[Performance/性能]: Potential unnecessary delay after card is ready / 卡已就绪后可能有不必要的延迟
English: The code calls rt_thread_mdelay(1) at line 323 before checking if the card is ready at line 330. This means even when the card is already ready (status indicates R1_READY_FOR_DATA), the function will still delay for 1ms unnecessarily before exiting the loop.
Consider checking the status condition before the delay to avoid the unnecessary wait when the card is already ready.
中文：代码在第 323 行调用 rt_thread_mdelay(1)，然后在第 330 行检查卡是否就绪。这意味着即使卡已经就绪（状态指示 R1_READY_FOR_DATA），函数在退出循环之前仍然会不必要地延迟 1ms。
考虑在延迟之前检查状态条件，以避免在卡已经就绪时不必要的等待。
| int timeout = rt_tick_from_millisecond(timeout_ms); | ||
| int err = 0; | ||
| rt_uint32_t status; | ||
| rt_tick_t start; | ||
|
|
||
| start = rt_tick_get(); | ||
| do | ||
| { | ||
| rt_bool_t out = (int)(rt_tick_get() - start) > timeout; |
There was a problem hiding this comment.
[Bug/错误]: Type mismatch and incorrect timeout calculation / 类型不匹配和错误的超时计算
English: There are two issues with the timeout calculation:
rt_tick_from_millisecond()returnsrt_tick_t(typically unsigned), but it's assigned toint timeout, which could cause issues if the value exceeds INT_MAX- The comparison
(int)(rt_tick_get() - start) > timeoutuses signed integer comparison for tick differences, which could produce incorrect results due to potential overflow in the subtraction or type conversion
For proper tick-based timeout checking in RT-Thread, consider using the pattern: (rt_tick_get() - start) >= timeout with consistent unsigned types, or check if rt_tick_get() >= (start + timeout) to avoid overflow issues.
中文：超时计算存在两个问题：
rt_tick_from_millisecond()返回rt_tick_t（通常是无符号类型），但它被赋值给int timeout，如果值超过 INT_MAX 可能会导致问题- 比较
(int)(rt_tick_get() - start) > timeout使用有符号整数比较来处理 tick 差值，由于减法或类型转换中的潜在溢出，可能会产生不正确的结果
为了在 RT-Thread 中进行正确的基于 tick 的超时检查，考虑使用模式：(rt_tick_get() - start) >= timeout 并使用一致的无符号类型，或检查 rt_tick_get() >= (start + timeout) 以避免溢出问题。
| * indication and the card state. | ||
| */ | ||
| } | ||
| while (!(status & R1_READY_FOR_DATA)); |
There was a problem hiding this comment.
[Documentation/文档]: Comment mentions checking card state but implementation doesn't / 注释提到检查卡状态但实现未检查
English: The comment at lines 324-328 states "Some cards mishandle the status bits, so make sure to check both the busy indication and the card state." However, the implementation at line 330 only checks R1_READY_FOR_DATA (the busy indication) but does not check the card state using R1_CURRENT_STATE(status).
According to the comment's intent, the code should also verify that the card is in the correct state (e.g., transfer state). Consider adding a check for the card state if this is indeed necessary for proper operation, or update the comment to accurately reflect what the code does.
中文：第 324-328 行的注释说明 "Some cards mishandle the status bits, so make sure to check both the busy indication and the card state."（某些卡会错误处理状态位，因此要确保同时检查忙指示和卡状态）。然而，第 330 行的实现只检查了 R1_READY_FOR_DATA（忙指示），但没有使用 R1_CURRENT_STATE(status) 检查卡状态。
根据注释的意图，代码还应该验证卡处于正确的状态（例如传输状态）。如果这对于正确操作确实是必要的，请考虑添加卡状态检查，或者更新注释以准确反映代码的实际行为。
| while (!(status & R1_READY_FOR_DATA)); | |
| while (!(status & R1_READY_FOR_DATA) || | |
| (R1_CURRENT_STATE(status) == R1_STATE_PRG)); |
2 participants
拉取/合并请求描述：(PR description)
Fix the bug where the cmd6 command is sent but the status of the card is not queried, which can cause failure in switching to the high-speed mode in DDR mode.
为什么提交这份PR (why to submit this PR)
Solve the issue of normal code execution in the eMMC DDR mode.
你的解决方案是什么 (what is your solution)
Add command 13 polling in mmc_switch.
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up