[PATCHv3] mmc: rpmb: do not force a retune before RPMB switch

Requesting a retune before switching to the RPMB partition has been
observed to cause CRC errors on the RPMB reads (-EILSEQ).

Since RPMB reads can not be retried, the clients would be directly
affected by the errors.

This commit disables the retune request prior to switching to the RPMB
partition: mmc_retune_pause() no longer triggers a retune before the
pause period begins.

This was verified with the sdhci-of-arasan driver (ZynqMP) configured
for HS200 using two separate eMMC cards (DG4064 and 064GB2). In both
cases, the error was easy to reproduce triggering every few tenths of
reads.

With this commit, systems that were utilizing OP-TEE to access RPMB
variables will experience an enhanced performance. Specifically, when
OP-TEE is configured to employ RPMB as a secure storage solution, it not
only writes the data but also the secure filesystem within the
partition. As a result, retrieving any variable involves multiple RPMB
reads, typically around five.

For context, on ZynqMP, each retune request consumed approximately
8ms. Consequently, reading any RPMB variable used to take at the very
minimum 40ms.

After droping the need to retune before switching to the RPMB partition,
this is no longer the case.

Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
Acked-by: Avri Altman <avri.altman@wdc.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
 v3:
    Added some performance information to the commit message
 v2:
    mmc_retune_pause() no longer can trigger a retune.
    Keeping Avri Altman Acked-by since they are functionally equivalent.
 v1:
    modify mmc_retune_pause to optionally trigger a retune.


 drivers/mmc/core/host.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.34.1

On Wed, 3 Jan 2024 at 12:29, Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
>
> Requesting a retune before switching to the RPMB partition has been
> observed to cause CRC errors on the RPMB reads (-EILSEQ).
>
> Since RPMB reads can not be retried, the clients would be directly
> affected by the errors.
>
> This commit disables the retune request prior to switching to the RPMB
> partition: mmc_retune_pause() no longer triggers a retune before the
> pause period begins.
>
> This was verified with the sdhci-of-arasan driver (ZynqMP) configured
> for HS200 using two separate eMMC cards (DG4064 and 064GB2). In both
> cases, the error was easy to reproduce triggering every few tenths of
> reads.
>
> With this commit, systems that were utilizing OP-TEE to access RPMB
> variables will experience an enhanced performance. Specifically, when
> OP-TEE is configured to employ RPMB as a secure storage solution, it not
> only writes the data but also the secure filesystem within the
> partition. As a result, retrieving any variable involves multiple RPMB
> reads, typically around five.
>
> For context, on ZynqMP, each retune request consumed approximately
> 8ms. Consequently, reading any RPMB variable used to take at the very
> minimum 40ms.
>
> After droping the need to retune before switching to the RPMB partition,
> this is no longer the case.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> Acked-by: Avri Altman <avri.altman@wdc.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks for updating the commit message, very nice! And thanks Adrian
for reviewing this!

Applied for next, thanks!

Kind regards
Uffe

> ---
>  v3:
>     Added some performance information to the commit message
>  v2:
>     mmc_retune_pause() no longer can trigger a retune.
>     Keeping Avri Altman Acked-by since they are functionally equivalent.
>  v1:
>     modify mmc_retune_pause to optionally trigger a retune.
>
>
>  drivers/mmc/core/host.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 096093f7be00..ed44920e92df 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -119,13 +119,12 @@ void mmc_retune_enable(struct mmc_host *host)
>
>  /*
>   * Pause re-tuning for a small set of operations.  The pause begins after the
> - * next command and after first doing re-tuning.
> + * next command.
>   */
>  void mmc_retune_pause(struct mmc_host *host)
>  {
>         if (!host->retune_paused) {
>                 host->retune_paused = 1;
> -               mmc_retune_needed(host);
>                 mmc_retune_hold(host);
>         }
>  }
> --
> 2.34.1