Hi Adrian,
Thank you for response.
On 20/12/22 01:41, Shyam Saini wrote:
As per the JEDEC spec, tuning(command CMD21) is not allowed in
RPMB partition.
To avoid retuning while switching to RPMB, hold_retune variable was
introduced
No, mmc_retune_pause() is used:
/*
* Pause re-tuning for a small set of operations. The pause begins after the
* next command and after first doing re-tuning.
*/
void mmc_retune_pause(struct mmc_host *host)
{
if (!host->retune_paused) {
host->retune_paused = 1;
mmc_retune_needed(host);
mmc_retune_hold(host);
}
}
but it is not taken into account while making the tuning
decision. As a consequence of this, mmc driver aborts while switching to
RPMB partition:
mmc0: mmc_hs400_to_hs200 failed, error -84
Do you normally re-tune at all? It could just be that re-tuning
doesn't work.
Yes, we are able to retune.
In fact this bug occurs when we iteratively switch to RPMB partition.
May be related to this, we also observed that in __mmc_blk_ioctl_cmd
function in line 487[1] part_index is assigned target_part variable and
on the next line EXT_CSD_PART_CONFIG_ACC_RPMB OR'ed to the target_part
variable.
In mmc_blk_part_switch_pre function line 831 [2], part_type variable is
compared to EXT_CSD_PART_CONFIG_ACC_RPMB without taking into account that
the part_index variable is also OR'ed and its not separated before
the comparision.
Same thing happens in mmc_blk_part_switch_post function.
Is it expected to be this way?, please let me know.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git/tree/drivers/mmc/core/block.c?h=next#n487
[2] https://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git/tree/drivers/mmc/core/block.c?h=next#n831
To fix this, take hold_retune variable into account while making retune
decision in mmc_retune() function.
Fixes: 57da0c042f4a ("mmc: block: Pause re-tuning while switched to the RPMB partition")
Reported-by: Judy Wang <wangjudy@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Shyam Saini <shyamsaini@xxxxxxxxxxxxxxxxxxx>
---
drivers/mmc/core/host.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index b89dca1f15e9..342c1f5c256b 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -181,6 +181,9 @@ int mmc_retune(struct mmc_host *host)
bool return_to_hs400 = false;
int err;
+ if (host->hold_retune >= 1)
+ return 0;
No, hold_retune is set *before* the command *after* which
tuning is not permitted.
+
if (host->retune_now)
host->retune_now = 0;
else
--
2.34.1
On 20/12/22 01:41, Shyam Saini wrote:
Hi All,
Cc'ing Tyler
Please note that I am not 100 % sure about the fix.
This issue is reproducible without this patch and the patch at least fix the issue without
any regressions on our side.
We observed that hold_retune is always greater or equal to 1 in mmc_retune() function,
so it may always avoid re-tuning when it is actually needed and there may
be a better fix for the issue.
Please let me know your any feedback or comments.
Best Regards,
Shyam
