| Message ID | 20231211165500.310097-1-jorge@foundries.io (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: rpmb: do not force a retune before RPMB switch | expand |
On 11/12/23 18:55, Jorge Ramirez-Ortiz 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. > > 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> > --- > 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
On Mon, 11 Dec 2023 at 17:55, 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. > > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> > Acked-by: Avri Altman <avri.altman@wdc.com> This seems reasonable, but I would like to see some justification from a performance point of view in the commit message too. Moreover, please bump the version number of the patch at each iteration and add a version summary of what has changed. That helps the review process. Kind regards Uffe > --- > 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
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); } }