| Message ID | 20210914182023.8103-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mmc: also abort tuning with CMD12 for SD | expand |
Hi Wolfram, On Tue, Sep 14, 2021 at 8:22 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > After my hackish RFC patch, here is a small series implementing > (hopefully) the solution we discussed. It will make > mmc_send_abort_tuning() also send CMD12 for SD cards which makes more SD > cards work for us. Details are in the patch descriptions. > > Please let me know what you think. > > Thanks, and happy hacking! > > > Wolfram Sang (3): > mmc: core: add helper to send STOP > mmc: core: also abort tuning with CMD12 for SD > mmc: core: remove obsolete parameter from mmc_send_abort_tuning Thanks for your series! I gave this a quick boot test on the Gose and ALT in Magnus' farm, but the issues are still there. Gr{oetje,eeting}s, Geert
> I gave this a quick boot test on the Gose and ALT in Magnus' farm, but > the issues are still there. Pity :( It doesn't fix all cards, but it fixes the SanDisk cards which the BSP team and I have.
I did not test the patch but want to make you aware of the comment in dw_mmc:
/*
* During UHS tuning sequence, sending the stop
* command after the response CRC error would
* throw the system into a confused state
* causing all future tuning phases to report
* failure.
*
* In such case controller will move into a data
* transfer state after a response error or
* response CRC error. Let's let that finish
* before trying to send a stop, so we'll go to
* STATE_SENDING_DATA.
*
* Although letting the data transfer take place
* will waste a bit of time (we already know
* the command was bad), it can't cause any
* errors since it's possible it would have
* taken place anyway if this tasklet got
* delayed. Allowing the transfer to take place
* avoids races and keeps things simple.
*/
The message in 46d179525a1f6d16957dcb4624517bc04142b3e7
does not mention which card was causing problem, unfortunately.
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Sent: Tuesday, September 14, 2021 8:20 PM
To: linux-mmc@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org; Yoshihiro Shimoda; Wolfram Sang
Subject: [PATCH 0/3] mmc: also abort tuning with CMD12 for SD
After my hackish RFC patch, here is a small series implementing
(hopefully) the solution we discussed. It will make
mmc_send_abort_tuning() also send CMD12 for SD cards which makes more SD
cards work for us. Details are in the patch descriptions.
Please let me know what you think.
Thanks, and happy hacking!
Wolfram Sang (3):
mmc: core: add helper to send STOP
mmc: core: also abort tuning with CMD12 for SD
mmc: core: remove obsolete parameter from mmc_send_abort_tuning
drivers/mmc/core/block.c | 14 +-------------
drivers/mmc/core/core.h | 1 +
drivers/mmc/core/mmc.c | 6 ++++++
drivers/mmc/core/mmc_ops.c | 23 ++++-------------------
drivers/mmc/core/mmc_ops.h | 14 ++++++++++++++
drivers/mmc/core/sd.c | 6 ++++++
drivers/mmc/host/renesas_sdhi_core.c | 2 +-
drivers/mmc/host/sdhci.c | 2 +-
include/linux/mmc/host.h | 2 +-
9 files changed, 35 insertions(+), 35 deletions(-)
On Wed, Sep 15, 2021 at 08:50:21AM +0000, Christian Löhle wrote: > I did not test the patch but want to make you aware of the comment in dw_mmc: > /* > * During UHS tuning sequence, sending the stop > * command after the response CRC error would > * throw the system into a confused state > * causing all future tuning phases to report > * failure. > * > * In such case controller will move into a data > * transfer state after a response error or > * response CRC error. Let's let that finish > * before trying to send a stop, so we'll go to > * STATE_SENDING_DATA. > * > * Although letting the data transfer take place > * will waste a bit of time (we already know > * the command was bad), it can't cause any > * errors since it's possible it would have > * taken place anyway if this tasklet got > * delayed. Allowing the transfer to take place > * avoids races and keeps things simple. > */ > The message in 46d179525a1f6d16957dcb4624517bc04142b3e7 > does not mention which card was causing problem, unfortunately. Thank you for the pointer! This is interesting, in deed. As I understand, it is a limitation of the controller which always goes into data transfer state even after CRC errors. However, because the controller driver is not using mmc_send_abort_data(), it will not be affected by my patch series. My patch series only extends the optional MMC core helper to be used for SD cards, too, in addition to eMMC. If I missed something else, please let me know.
Hi, > After my hackish RFC patch, here is a small series implementing > (hopefully) the solution we discussed. It will make > mmc_send_abort_tuning() also send CMD12 for SD cards which makes more > SD > cards work for us. Details are in the patch descriptions. > > Please let me know what you think. > > Thanks, and happy hacking! I made note of your patch series to our SD (hw) guys, and here is what they say: "We are ok with host sending CMD12 to abort data transfer when they discover failure with response / incoming data. In both SD/eMMC spec, stop transmission command is allowed during data transfer phase ('data' state). Sometimes, the CMD12 would have been received by card while in 'tran' state. As long host is able to handle the 'illegal command' error indication for this situation, we don't see any other problem. Per SD Spec, CMD12 is allowed in 'tran' state only for SDUC card. In non SDUC cards, CMD12 received while in 'tran' state will be treated as illegal command. However we could not understand how aborting the data transfer is helping host to complete the tuning scheme and have successful read / write operations." They also think that : " we believe this hack was added to avoid the data transfer after response crc error... Receiving CRC error with the tuning pattern would be normal as long as the tuning was not complete." My 5 cents are, maybe you should try retries > 0 in sd_send_abort_tuning, If indeed it's a crc while tuning is not complete. Thanks, Avri > > > Wolfram Sang (3): > mmc: core: add helper to send STOP > mmc: core: also abort tuning with CMD12 for SD > mmc: core: remove obsolete parameter from mmc_send_abort_tuning > > drivers/mmc/core/block.c | 14 +------------- > drivers/mmc/core/core.h | 1 + > drivers/mmc/core/mmc.c | 6 ++++++ > drivers/mmc/core/mmc_ops.c | 23 ++++------------------- > drivers/mmc/core/mmc_ops.h | 14 ++++++++++++++ > drivers/mmc/core/sd.c | 6 ++++++ > drivers/mmc/host/renesas_sdhi_core.c | 2 +- > drivers/mmc/host/sdhci.c | 2 +- > include/linux/mmc/host.h | 2 +- > 9 files changed, 35 insertions(+), 35 deletions(-) > > -- > 2.30.2
Hi Avri, first of all, thank you very much for asking your SD experts and providing this valuable information! This is much appreciated. Sadly, I have only indirect access to the SD specs and more advanced measurement techniques. > "We are ok with host sending CMD12 to abort data transfer when they > discover failure with response / incoming data. In both SD/eMMC spec, > stop transmission command is allowed during data transfer phase > ('data' state). Yes. The spec is clear about the data state. > Sometimes, the CMD12 would have been received by card while in 'tran' > state. As long host is able to handle the 'illegal command' error > indication for this situation, we don't see any other problem. Confirmed. The call to mmc_send_abort_tuning() returns -EILSEQ. But it still makes the card work. Leaving out the call to mmc_send_abort_tuning() gives: mmc1: error -5 whilst initialising SD card > Per SD Spec, CMD12 is allowed in 'tran' state only for SDUC card. In > non SDUC cards, CMD12 received while in 'tran' state will be treated > as illegal command. I didn't know about SDUC. The advantage of the approach Ulf suggested is that we can distinguish the type of cards in the SD callback if needed. > However we could not understand how aborting the data transfer is > helping host to complete the tuning scheme and have successful read / > write operations." I also didn't know why. But read on, there is a pointer now. > They also think that : > " we believe this hack was added to avoid the data transfer after response crc error... > Receiving CRC error with the tuning pattern would be normal as long as the tuning was not complete." Yes, I get CRC and CMD_IDX errors for CMD19. > My 5 cents are, maybe you should try retries > 0 in sd_send_abort_tuning, > If indeed it's a crc while tuning is not complete. Interesting. If I do this, I get a timeout error from mmc_send_abort_tuning(). And bingo! If I replace mmc_send_abort_tuning() with mdelay(100) the card also probes fine. Also, if I skip this patch series (so abort_tuning is only for MMC) and add the delay in my host driver, then the card also comes up. I don't think, though, I should fix it in the host driver. Is there a delay defined somewhere which ones need to wait after a CRC error in tuning? I couldn't find it in the simplified spec. Thanks for the help again, Wolfram