mmc: block: Fixes for CQE error recovery recovery
Adrian Hunter
maintainerv2 cover mmc: block: Fixes for CQE error recovery recovery
Hi
Some issues have been found with CQE error recovery. Here are some fixes.
As of V2, the alternative implementation for the patch from Kornel Dulęba:
https://lore.kernel.org/linux-mmc/[email protected]/T/#u
is now included, see patch 6 "mmc: cqhci: Fix task clearing in CQE error recovery")
Please also note ->post_disable() seems to be missing from cqhci_recovery_start(). It would be good if ->post_disable() users could check if this needs attention.
Changes in V2:
mmc: cqhci: Fix task clearing in CQE error recovery
New patch
mmc: cqhci: Warn of halt or task clear failure
Add fixes and stable tags
Adrian Hunter (6):
mmc: block: Do not lose cache flush during CQE error recovery
mmc: cqhci: Increase recovery halt timeout
mmc: block: Be sure to wait while busy in CQE error recovery
mmc: block: Retry commands in CQE error recovery
mmc: cqhci: Warn of halt or task clear failure
mmc: cqhci: Fix task clearing in CQE error recovery
drivers/mmc/core/block.c | 2 ++ drivers/mmc/core/core.c | 9 +++++++-- drivers/mmc/host/cqhci-core.c | 44 +++++++++++++++++++++---------------------- 3 files changed, 31 insertions(+), 24 deletions(-)
Regards
Adrian
Adrian Hunter
maintainerPATCH v2 1/6 mmc: block: Do not lose cache flush during CQE error recovery
During CQE error recovery, error-free data commands get requeued if there is any data left to transfer, but non-data commands are completed even though they have not been processed. Requeue them instead.
Note the only non-data command is cache flush, which would have resulted in a cache flush being lost if it was queued at the time of CQE recovery.
--- drivers/mmc/core/block.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/mmc/core/block.c +2 -0
@@ -1482,6 +1482,8 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)blk_mq_requeue_request (req, true); else__blk_mq_end_request (req ,BLK_STS_OK ); + }else if (mq->in_recovery) { +blk_mq_requeue_request (req, true); }else {blk_mq_end_request (req,BLK_STS_OK ); }
Signature · 1 line
2.34.1
RE: [PATCH V2 1/6] mmc: block: Do not lose cache flush during CQE error recovery
Quoted 11 lines
During CQE error recovery, error-free data commands get requeued if there is any data left to transfer, but non-data commands are completed even though they have not been processed. Requeue them instead.
Note the only non-data command is cache flush, which would have resulted in a cache flush being lost if it was queued at the time of CQE recovery.
Fixes 1e8e55b67030 ("mmc: block: Add CQE support")Signed-off-by Adrian Hunter <[email protected]>
Quoted 20 lines
--- drivers/mmc/core/block.c | 2 ++ 1 file changed, 2 insertions(+)drivers/mmc/core/block.c index renamed from drivers/mmc/core/block.c +0
-0
3a8f27c3e310..4a32b756b7d8100644drivers/mmc/core/block.c +2
-0@@ -1482,6 +1482,8 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req)blk_mq_requeue_request (req ,true ); else__blk_mq_end_request (req ,BLK_STS_OK ); + }else if (mq->in_recovery) { +blk_mq_requeue_request (req, true); }else {blk_mq_end_request (req,BLK_STS_OK );}--
2.34.1
Adrian Hunter
maintainerPATCH v2 2/6 mmc: cqhci: Increase recovery halt timeout
In reply to Adrian HunterFailing to halt complicates the recovery. Additionally, unless the card or controller are stuck, which is expected to be very rare, then the halt should succeed, so it is better to wait. Set a large timeout.
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/mmc/host/cqhci-core.c +3 -3
@@ -984,10 +984,10 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout)/* * After halting we expect to be able to use the command line. We interpret the * failure to halt to mean the data lines might still be in use (and the upper -* layers will need to send a STOP command), so we set the timeout based on a -* generous command timeout. +* layers will need to send a STOP command), however failing to halt complicates +* the recovery, so set a timeout that would reasonably allow I/O to complete. */ -#define CQHCI_START_HALT_TIMEOUT 5 +#define CQHCI_START_HALT_TIMEOUT 500static void cqhci_recovery_start (struct mmc_host *mmc) {
Signature · 1 line
2.34.1
RE: [PATCH V2 2/6] mmc: cqhci: Increase recovery halt timeout
Quoted 3 lines
Failing to halt complicates the recovery. Additionally, unless the card or controller are stuck, which is expected to be very rare, then the halt should succeed, so it is better to wait. Set a large timeout.
Maybe also explain that If task queuing is in progress, CQE needs to complete the operation, sending both commands and processing the responses.
Quoted 5 lines
Fixes a4080225f51d ("mmc: cqhci: support for command queue enabledhost")
Signed-off-by Adrian Hunter <[email protected]>
Quoted 28 lines
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/mmc/host/cqhci-core.c +0
-0@@ -984,10 +984,10 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout)/* * After halting we expect to be able to use the command line. We interpret the
* failure to halt to mean the data lines might still be in use (and the upper
- * layers will need to send a STOP command), so we set the timeout based on a
- * generous command timeout.
+ * layers will need to send a STOP command), however failing to halt
+ complicates
+ * the recovery, so set a timeout that would reasonably allow I/O to complete.
*/drivers/mmc/host/cqhci-core.c +1
-2-#define CQHCI_START_HALT_TIMEOUT 5 +#define CQHCI_START_HALT_TIMEOUT 500static void cqhci_recovery_start (struct mmc_host *mmc) { --2.34.1
Adrian Hunter
maintainerRe: [PATCH V2 2/6] mmc: cqhci: Increase recovery halt timeout
Quoted 4 lines · Avri Altman
Quoted 3 lines
Failing to halt complicates the recovery. Additionally, unless the card or controller are stuck, which is expected to be very rare, then the halt should succeed, so it is better to wait. Set a large timeout.
Maybe also explain that If task queuing is in progress, CQE needs to complete the operation, sending both commands and processing the responses.
True, although those commands should be quite quick.
Quoted 37 lines
Quoted 6 lines
Fixes a4080225f51d ("mmc: cqhci: support for command queue enabledhost")
Signed-off-by Adrian Hunter <[email protected]>Reviewed-by Avri Altman <[email protected]>Quoted 28 lines
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/mmc/host/cqhci-core.c +0
-0@@ -984,10 +984,10 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout)/* * After halting we expect to be able to use the command line. We interpret the
* failure to halt to mean the data lines might still be in use (and the upper
- * layers will need to send a STOP command), so we set the timeout based on a
- * generous command timeout.
+ * layers will need to send a STOP command), however failing to halt
+ complicates
+ * the recovery, so set a timeout that would reasonably allow I/O to complete.
*/drivers/mmc/host/cqhci-core.c +1
-2-#define CQHCI_START_HALT_TIMEOUT 5 +#define CQHCI_START_HALT_TIMEOUT 500static void cqhci_recovery_start (struct mmc_host *mmc) { --2.34.1
Adrian Hunter
maintainerPATCH v2 3/6 mmc: block: Be sure to wait while busy in CQE error recovery
In reply to Adrian HunterSTOP command does not guarantee to wait while busy, but subsequent command MMC_CMDQ_TASK_MGMT to discard the queue will fail if the card is busy, so be sure to wait by employing mmc_poll_for_busy().
--- drivers/mmc/core/core.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/mmc/core/core.c +2 -0
@@ -553,6 +553,8 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ;mmc_wait_for_cmd (host, &cmd,0 ); +mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true,MMC_BUSY_IO ); +memset (&cmd,0 ,sizeof (cmd)); cmd.opcode =MMC_CMDQ_TASK_MGMT ; cmd.arg =1 ;/* Discard entire queue */
Signature · 1 line
2.34.1
RE: [PATCH V2 3/6] mmc: block: Be sure to wait while busy in CQE error recovery
Quoted 3 lines
STOP command does not guarantee to wait while busy, but subsequent command
MMC_CMDQ_TASK_MGMTto discard the queue will fail if the card is busy, so be sure to wait by employingmmc_poll_for_busy().
Doesn't the Task Discard Sequence expects you to check CQDPT[i]==1 before sending MMC_CMDQ_TASK_MGMT to discard task id i?
Thanks,
Avri
Quoted 25 lines
Fixes 72a5af554df8 ("mmc: core: Add support for handling CQE requests")Signed-off-by Adrian Hunter <[email protected]>--- drivers/mmc/core/core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/mmc/core/core.c index renamed from drivers/mmc/core/core.c +0
-0
3d3e0ca52614..befde2bd26d3100644drivers/mmc/core/core.c +3
-1@@ -553,6 +553,8 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ;mmc_wait_for_cmd (host, &cmd,0 ); +mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true, +MMC_BUSY_IO ); +memset (&cmd,0 ,sizeof (cmd)); cmd.opcode =MMC_CMDQ_TASK_MGMT ; cmd.arg =1 ;/* Discard entire queue */ --2.34.1
Adrian Hunter
maintainerRe: [PATCH V2 3/6] mmc: block: Be sure to wait while busy in CQE error recovery
Quoted 5 lines · Avri Altman
Quoted 3 lines
STOP command does not guarantee to wait while busy, but subsequent command
MMC_CMDQ_TASK_MGMTto discard the queue will fail if the card is busy, so be sure to wait by employingmmc_poll_for_busy().Doesn't the Task Discard Sequence expects you to check CQDPT[i]==1 before sending
MMC_CMDQ_TASK_MGMTto discard task id i?
We do not clear individual tasks. Instead the MMC_CMDQ_TASK_MGMT is sent with the op-code to "discard entire queue", which will also work even if the queue is empty. Refer JESD84-B51A, 6.6.39.6 CMDQ_TASK_MGMT and Table 43 — Task Management op-codes.
Quoted 30 lines
Thanks,
AvriQuoted 25 lines
Fixes 72a5af554df8 ("mmc: core: Add support for handling CQE requests")Signed-off-by Adrian Hunter <[email protected]>--- drivers/mmc/core/core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/mmc/core/core.c index renamed from drivers/mmc/core/core.c +0
-0
3d3e0ca52614..befde2bd26d3100644drivers/mmc/core/core.c +3
-1@@ -553,6 +553,8 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ;mmc_wait_for_cmd (host, &cmd,0 ); +mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true, +MMC_BUSY_IO ); +memset (&cmd,0 ,sizeof (cmd)); cmd.opcode =MMC_CMDQ_TASK_MGMT ; cmd.arg =1 ;/* Discard entire queue */ --2.34.1
RE: [PATCH V2 3/6] mmc: block: Be sure to wait while busy in CQE error recovery
In reply to Adrian HunterQuoted 8 lines
STOP command does not guarantee to wait while busy, but subsequent command
MMC_CMDQ_TASK_MGMTto discard the queue will fail if the card is busy, so be sure to wait by employingmmc_poll_for_busy().Fixes 72a5af554df8 ("mmc: core: Add support for handling CQE requests")Signed-off-by Adrian Hunter <[email protected]>
Quoted 21 lines
--- drivers/mmc/core/core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/mmc/core/core.c index renamed from drivers/mmc/core/core.c +0
-0
3d3e0ca52614..befde2bd26d3100644drivers/mmc/core/core.c +3
-1@@ -553,6 +553,8 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ;mmc_wait_for_cmd (host, &cmd,0 ); +mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true, +MMC_BUSY_IO ); +memset (&cmd,0 ,sizeof (cmd)); cmd.opcode =MMC_CMDQ_TASK_MGMT ; cmd.arg =1 ;/* Discard entire queue */ --2.34.1
Re: [PATCH V2 3/6] mmc: block: Be sure to wait while busy in CQE error recovery
In reply to Adrian HunterQuoted 7 lines · Adrian Hunter
STOP command does not guarantee to wait while busy, but subsequent command
MMC_CMDQ_TASK_MGMTto discard the queue will fail if the card is busy, so be sure to wait by employingmmc_poll_for_busy().Fixes 72a5af554df8 ("mmc: core: Add support for handling CQE requests")Signed-off-by Adrian Hunter <[email protected]>
Quoted 17 lines
--- drivers/mmc/core/core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/mmc/core/core.c +2
-0@@ -553,6 +553,8 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ;mmc_wait_for_cmd (host, &cmd,0 ); +mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true,MMC_BUSY_IO ); +memset (&cmd,0 ,sizeof (cmd)); cmd.opcode =MMC_CMDQ_TASK_MGMT ; cmd.arg =1 ;/* Discard entire queue */
Adrian Hunter
maintainerPATCH v2 4/6 mmc: block: Retry commands in CQE error recovery
In reply to Adrian HunterIt is important that MMC_CMDQ_TASK_MGMT command to discard the queue is successful because otherwise a subsequent reset might fail to flush the cache first. Retry it and the previous STOP command.
--- drivers/mmc/core/core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/mmc/core/core.c +5 -2
@@ -551,7 +551,7 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.flags =MMC_RSP_R1B |MMC_CMD_AC ; cmd.flags &= ~MMC_RSP_CRC ;/* Ignore CRC */ cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ; -mmc_wait_for_cmd (host, &cmd,0 ); +mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES );mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true,MMC_BUSY_IO );
@@ -561,10 +561,13 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.flags =MMC_RSP_R1B |MMC_CMD_AC ; cmd.flags &= ~MMC_RSP_CRC ;/* Ignore CRC */ cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ; - err =mmc_wait_for_cmd (host, &cmd,0 ); + err =mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES ); host->cqe_ops->cqe_recovery_finish (host); +if (err) + err =mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES ); +mmc_retune_release (host);return err;
Signature · 1 line
2.34.1
RE: [PATCH V2 4/6] mmc: block: Retry commands in CQE error recovery
Quoted 7 lines
It is important that
MMC_CMDQ_TASK_MGMTcommand to discard the queue is successful because otherwise a subsequent reset might fail to flush the cache first. Retry it and the previous STOP command.Fixes 72a5af554df8 ("mmc: core: Add support for handling CQE requests")Signed-off-by Adrian Hunter <[email protected]>
Quoted 35 lines
--- drivers/mmc/core/core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/mmc/core/core.c index renamed from drivers/mmc/core/core.c +0
-0
befde2bd26d3..a8c17b4cd737100644drivers/mmc/core/core.c +5
-3@@ -551,7 +551,7 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.flags =MMC_RSP_R1B |MMC_CMD_AC ; cmd.flags &= ~MMC_RSP_CRC ;/* Ignore CRC */ cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ; -mmc_wait_for_cmd (host, &cmd,0 ); +mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES );mmc_poll_for_busy (host->card,MMC_CQE_RECOVERY_TIMEOUT , true,MMC_BUSY_IO );@@ -561,10 +561,13 @@ int mmc_cqe_recovery(struct mmc_host *host) cmd.flags =MMC_RSP_R1B |MMC_CMD_AC ; cmd.flags &= ~MMC_RSP_CRC ;/* Ignore CRC */ cmd.busy_timeout =MMC_CQE_RECOVERY_TIMEOUT ; - err =mmc_wait_for_cmd (host, &cmd,0 ); + err =mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES ); host->cqe_ops->cqe_recovery_finish (host); +if (err) + err =mmc_wait_for_cmd (host, &cmd,MMC_CMD_RETRIES ); +mmc_retune_release (host);return err; --2.34.1
Adrian Hunter
maintainerPATCH v2 5/6 mmc: cqhci: Warn of halt or task clear failure
In reply to Adrian HunterA correctly operating controller should successfully halt and clear tasks. Failure may result in errors elsewhere, so promote messages from debug to warnings.
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/mmc/host/cqhci-core.c +3 -3
@@ -942,8 +942,8 @@ static bool cqhci_clear_all_tasks(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_tasks_cleared (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to clear tasks\n" , -mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to clear tasks\n" , +mmc_hostname (mmc));return ret; }
@@ -976,7 +976,7 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_halted (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to halt\n" ,mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to halt\n" ,mmc_hostname (mmc));return ret; }
Signature · 1 line
2.34.1
RE: [PATCH V2 5/6] mmc: cqhci: Warn of halt or task clear failure
Quoted 8 lines
A correctly operating controller should successfully halt and clear tasks. Failure may result in errors elsewhere, so promote messages from debug to warnings.
Fixes a4080225f51d ("mmc: cqhci: support for command queue enabledhost")
Signed-off-by Adrian Hunter <[email protected]>
Quoted 33 lines
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/mmc/host/cqhci-core.c +2
-2@@ -942,8 +942,8 @@ static bool cqhci_clear_all_tasks(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_tasks_cleared (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to clear tasks\n" , -mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to clear tasks\n" , +mmc_hostname (mmc));return ret;}
drivers/mmc/host/cqhci-core.c +2
-2@@ -976,7 +976,7 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_halted (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to halt\n" ,mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to halt\n" , +mmc_hostname (mmc));return ret; } --2.34.1
RE: [PATCH V2 5/6] mmc: cqhci: Warn of halt or task clear failure
In reply to Adrian HunterQuoted 8 lines
A correctly operating controller should successfully halt and clear tasks. Failure may result in errors elsewhere, so promote messages from debug to warnings.
Fixes a4080225f51d ("mmc: cqhci: support for command queue enabledhost")
Signed-off-by Adrian Hunter <[email protected]>
Quoted 33 lines
--- drivers/mmc/host/cqhci-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/mmc/host/cqhci-core.c +2
-2@@ -942,8 +942,8 @@ static bool cqhci_clear_all_tasks(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_tasks_cleared (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to clear tasks\n" , -mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to clear tasks\n" , +mmc_hostname (mmc));return ret;}
drivers/mmc/host/cqhci-core.c +2
-2@@ -976,7 +976,7 @@ static bool cqhci_halt(struct mmc_host *mmc, unsigned int timeout) ret =cqhci_halted (cq_host);if (!ret) -pr_debug ("%s: cqhci: Failed to halt\n" ,mmc_hostname (mmc)); +pr_warn ("%s: cqhci: Failed to halt\n" , +mmc_hostname (mmc));return ret; } --2.34.1
Adrian Hunter
maintainerPATCH v2 6/6 mmc: cqhci: Fix task clearing in CQE error recovery
In reply to Adrian HunterIf a task completion notification (TCN) is received when there is no outstanding task, the cqhci driver issues a "spurious TCN" warning. This was observed to happen right after CQE error recovery.
When an error interrupt is received the driver runs recovery logic. It halts the controller, clears all pending tasks, and then re-enables it. On some platforms, like Intel Jasper Lake, a stale task completion event was observed, regardless of the CQHCI_CLEAR_ALL_TASKS bit being set.
This results in either:
a) Spurious TC completion event for an empty slot.
b) Corrupted data being passed up the stack, as a result of premature completion for a newly added task.
Rather than add a quirk for affected controllers, ensure tasks are cleared by toggling CQHCI_ENABLE, which would happen anyway if cqhci_clear_all_tasks() timed out. This is simpler and should be safe and effective for all controllers.
--- drivers/mmc/host/cqhci-core.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
drivers/mmc/host/cqhci-core.c +16 -16
@@ -1075,28 +1075,28 @@ static void cqhci_recovery_finish(struct mmc_host *mmc) ok =cqhci_halt (mmc,CQHCI_FINISH_HALT_TIMEOUT ); -if (!cqhci_clear_all_tasks (mmc,CQHCI_CLEAR_TIMEOUT )) - ok = false; -/* * The specification contradicts itself, by saying that tasks cannot be * cleared if CQHCI does not halt, but if CQHCI does not halt, it should * be disabled/re-enabled, but not to disable before clearing tasks. * Have a go anyway. */ -if (!ok) { -pr_debug ("%s: cqhci: disable / re-enable\n" ,mmc_hostname (mmc)); - cqcfg =cqhci_readl (cq_host,CQHCI_CFG ); - cqcfg &= ~CQHCI_ENABLE ; -cqhci_writel (cq_host, cqcfg,CQHCI_CFG ); - cqcfg |=CQHCI_ENABLE ; -cqhci_writel (cq_host, cqcfg,CQHCI_CFG ); -/* Be sure that there are no tasks */ - ok =cqhci_halt (mmc,CQHCI_FINISH_HALT_TIMEOUT ); -if (!cqhci_clear_all_tasks (mmc,CQHCI_CLEAR_TIMEOUT )) - ok = false; -WARN_ON (!ok); - } +if (!cqhci_clear_all_tasks (mmc,CQHCI_CLEAR_TIMEOUT )) + ok = false; + +/* Disable to make sure tasks really are cleared */ + cqcfg =cqhci_readl (cq_host,CQHCI_CFG ); + cqcfg &= ~CQHCI_ENABLE ; +cqhci_writel (cq_host, cqcfg,CQHCI_CFG ); + + cqcfg =cqhci_readl (cq_host,CQHCI_CFG ); + cqcfg |=CQHCI_ENABLE ; +cqhci_writel (cq_host, cqcfg,CQHCI_CFG ); + +cqhci_halt (mmc,CQHCI_FINISH_HALT_TIMEOUT ); + +if (!ok) +cqhci_clear_all_tasks (mmc,CQHCI_CLEAR_TIMEOUT );cqhci_recover_mrqs (cq_host);
Signature · 1 line
2.34.1
RE: [PATCH V2 6/6] mmc: cqhci: Fix task clearing in CQE error recovery
Quoted 19 lines
If a task completion notification (TCN) is received when there is no outstanding task, the cqhci driver issues a "spurious TCN" warning. This was observed to happen right after CQE error recovery.
When an error interrupt is received the driver runs recovery logic. It halts the controller, clears all pending tasks, and then re-enables it. On some platforms, like Intel Jasper Lake, a stale task completion event was observed, regardless of the
CQHCI_CLEAR_ALL_TASKSbit being set.This results in either:
a) Spurious TC completion event for an empty slot.
b) Corrupted data being passed up the stack, as a result of premature completion for a newly added task.Rather than add a quirk for affected controllers, ensure tasks are cleared by toggling
CQHCI_ENABLE, which would happen anyway ifcqhci_clear_all_tasks()timed out. This is simpler and should be safe and effective for all controllers.
Quoted 8 lines
Fixes a4080225f51d ("mmc: cqhci: support for command queue enabledhost")
Reported-by Kornel Dulęba <[email protected]>Tested-by Kornel Dulęba <[email protected]>Co-developed-by Kornel Dulęba <[email protected]>Signed-off-by Kornel Dulęba <[email protected]>Signed-off-by Adrian Hunter <[email protected]>
Does the double "recovery" in the subject intentional?
Thanks,
Avri
Quoted 43 lines
Hi
Some issues have been found with CQE error recovery. Here are some fixes.
As of V2, the alternative implementation for the patch from Kornel Dulęba:
https://lore.kernel.org/linux-mmc/e7c12e07-7540-47ea-8891-[email protected]/T/#u
is now included, see patch 6 "mmc: cqhci: Fix task clearing in CQE error recovery")
Please also note ->post_disable() seems to be missing from
cqhci_recovery_start(). It would be good if ->post_disable() users could check if this needs attention.Changes in V2:
mmc: cqhci: Fix task clearing in CQE error recovery
New patchmmc: cqhci: Warn of halt or task clear failure
Add fixes and stable tagsAdrian Hunter (6):
mmc: block: Do not lose cache flush during CQE error recovery
mmc: cqhci: Increase recovery halt timeout
mmc: block: Be sure to wait while busy in CQE error recovery
mmc: block: Retry commands in CQE error recovery
mmc: cqhci: Warn of halt or task clear failure
mmc: cqhci: Fix task clearing in CQE error recoverydrivers/mmc/core/block.c | 2 ++ drivers/mmc/core/core.c | 9 +++++++-- drivers/mmc/host/cqhci-core.c | 44 +++++++++++++++++++++----------------- ----- 3 files changed, 31 insertions(+), 24 deletions(-)Regards
Adrian
Adrian Hunter
maintainerQuoted 1 line · Avri Altman
Does the double "recovery" in the subject intentional?
No, must be an echo in here
Quoted 47 lines
Thanks,
AvriQuoted 43 lines
Hi
Some issues have been found with CQE error recovery. Here are some fixes.
As of V2, the alternative implementation for the patch from Kornel Dulęba:
https://lore.kernel.org/linux-mmc/e7c12e07-7540-47ea-8891-[email protected]/T/#u
is now included, see patch 6 "mmc: cqhci: Fix task clearing in CQE error recovery")
Please also note ->post_disable() seems to be missing from
cqhci_recovery_start(). It would be good if ->post_disable() users could check if this needs attention.Changes in V2:
mmc: cqhci: Fix task clearing in CQE error recovery
New patchmmc: cqhci: Warn of halt or task clear failure
Add fixes and stable tagsAdrian Hunter (6):
mmc: block: Do not lose cache flush during CQE error recovery
mmc: cqhci: Increase recovery halt timeout
mmc: block: Be sure to wait while busy in CQE error recovery
mmc: block: Retry commands in CQE error recovery
mmc: cqhci: Warn of halt or task clear failure
mmc: cqhci: Fix task clearing in CQE error recoverydrivers/mmc/core/block.c | 2 ++ drivers/mmc/core/core.c | 9 +++++++-- drivers/mmc/host/cqhci-core.c | 44 +++++++++++++++++++++----------------- ----- 3 files changed, 31 insertions(+), 24 deletions(-)Regards
Adrian
Quoted 34 lines · Adrian Hunter
Hi
Some issues have been found with CQE error recovery. Here are some fixes.
As of V2, the alternative implementation for the patch from Kornel Dulęba:
https://lore.kernel.org/linux-mmc/[email protected]/T/#uis now included, see patch 6 "mmc: cqhci: Fix task clearing in CQE error recovery")
Please also note ->post_disable() seems to be missing from
cqhci_recovery_start(). It would be good if ->post_disable() users could check if this needs attention.Changes in V2:
mmc: cqhci: Fix task clearing in CQE error recovery
New patchmmc: cqhci: Warn of halt or task clear failure
Add fixes and stable tagsAdrian Hunter (6):
mmc: block: Do not lose cache flush during CQE error recovery
mmc: cqhci: Increase recovery halt timeout
mmc: block: Be sure to wait while busy in CQE error recovery
mmc: block: Retry commands in CQE error recovery
mmc: cqhci: Warn of halt or task clear failure
mmc: cqhci: Fix task clearing in CQE error recovery
Quoted 6 lines
drivers/mmc/core/block.c | 2 ++ drivers/mmc/core/core.c | 9 +++++++-- drivers/mmc/host/cqhci-core.c | 44 +++++++++++++++++++++---------------------- 3 files changed, 31 insertions(+), 24 deletions(-)
Applied for fixes, thanks!
Kind regards
Uffe