u-boot: Fix the SPI and eMMC hang issue #570
Conversation
Refer to 1 and 2, the root cause is the wrong data type of the timer value. Thanks [email protected] for the patches. 1. https://lists.denx.de/pipermail/u-boot/2024-December/574817.html 2. https://lists.denx.de/pipermail/u-boot/2024-December/574818.html Signed-off-by: Baocheng Su <[email protected]>
BaochengSu
force-pushed
the
su/20241226-spi
branch
from
5cef3f6 to
1e9172a
Compare
| file://0012-spi-cadence-quadspi-Fix-error-message-on-stuck-busy-.patch \ | ||
| file://0013-spi-cadence-quadspi-fix-potential-malfunction-after-.patch \ | ||
| file://0014-mmc-Fix-potential-timer-value-truncation.patch \ | ||
| file://0015-driver-iot2050-Add-a-temporary-workaround-for-the-SD.patch \ |
There was a problem hiding this comment.
Do we still need patch 15, given patch 14?
There was a problem hiding this comment.
Actually during my speeded-up reproducing (by using a u16 start), I have not observed the SD card issue, this echoes the code exactly, no wrong sized timer variables are used in the SD driver part.
So I think the SD issue is a different one, so I keep the patch 15.
There was a problem hiding this comment.
Wait, I actually tested the SD on an advanced device, IIRC, the SD card issue is reported on a basic device, let me try it on the basic device then let's see.
There was a problem hiding this comment.
Just tested on both PG1 & 2 basic devices, SD card booting does not have such issue pattern. So better to keep the patch 15.
There was a problem hiding this comment.
- the mmc fails when also SPI failed
- patch 15 also deals with timeouts
- the patched code also used an "int" for holding the timeout
- patch 14 changes some uint to and ulong in the same code file to address an overflow issue
Isn't all that screaming for a closer look? I suspect sd_send_op_cond is just missing the same fix as __mmc_switch and mmc_send_op_cond receive via patch 14. Please do so, upstream first.
There was a problem hiding this comment.
The int timeout in sd_send_op_cond does not bind to any get_timer.
There was a problem hiding this comment.
This is correct, but it does not change the key point: Either, patch 15 is no obsolete because the root cause was found and fixed, or we should look for any remaining timer overflows that caused that issues. It is more than likely that the same root cause applies to "our" mmc issue as well.
There was a problem hiding this comment.
That's also what I thought, however, after a deep dive into the SD card calling stack (not only, I've checked almost every get_timer calling in the u-boot -- indeed we are in lucky, there are few u32 cases, and most of them are command which does not matter in our issue...), I found there is no other place to overflow the timer, besides the reported place, so this SD card issue is either a false alarm or a different issue, in this case, better to keep the patch 15, since that patch won't cause anything harmful even if it does not fix the issue.
There was a problem hiding this comment.
Tune get_time to return an overflowing value until the mmc issue reoccurs and debug the root cause / validate that it is fixed by patch 14. Just keeping the hack of patch 15 is not really the way forward anymore, I would say.
Another fix for the infamous #440 , should be the right one.
Refer to 1 and 2, the root cause is the wrong data type of the timer value.
Thanks [email protected] for the patches.
The SD patch is kept, however most probably the SD card issue is a different one.