* Re: [PATCH] mmc: rpmb: add quirk MMC_QUIRK_BROKEN_RPMB_RETUNE
[not found] ` <e159b627-22d4-489d-89a0-4de3be9af99b@arm.com>
@ 2023-11-29 14:42 ` Jorge Ramirez-Ortiz, Foundries
2023-11-29 15:04 ` Christian Loehle
0 siblings, 1 reply; 2+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2023-11-29 14:42 UTC (permalink / raw)
To: Christian Loehle
Cc: Jorge Ramirez-Ortiz, ulf.hansson, adrian.hunter, CLoehle,
linux-mmc
On 29/11/23 10:32:41, Christian Loehle wrote:
> On 29/11/2023 09:43, Jorge Ramirez-Ortiz wrote:
> > On the eMMC SanDisk iNAND 7250 configured with HS200, requesting a
> > re-tune before switching to the RPMB partition would randomly cause
> > subsequent RPMB requests to fail with EILSEQ:
> > * data error -84, tigggered in __mmc_blk_ioctl_cmd()
> >
> > This commit skips the retune when switching to RPMB.
> > Tested over several days with per minute RPMB reads.
> >
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > ---
> > drivers/mmc/core/block.c | 6 +++++-
> > drivers/mmc/core/card.h | 7 +++++++
> > drivers/mmc/core/quirks.h | 7 +++++++
> > include/linux/mmc/card.h | 1 +
> > 4 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > index 152dfe593c43..9b7ba6562a3b 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -860,6 +860,11 @@ static int mmc_blk_part_switch_pre(struct mmc_card *card,
> > return ret;
> > }
> > mmc_retune_pause(card->host);
> > +
> > + /* Do not force retune before RPMB switch */
> > + if (mmc_can_retune(card->host) &&
> > + mmc_card_broken_rpmb_retune(card))
> > + card->host->need_retune = 0;
> > }
>
> Is this just an issue for rpmb switches or did you try other non-rpmb
> partition switches, too?
I only tried the reported failure case (ie, RPMB switch).This failure
case came from a product verification team so I believe all the other
scenarios should be functional - other than RPMB accesses (these tests
started a few months ago), no other issues have been reported from eMMC
validation in the last couple of years.
Is there some additional check/test you would like to see done? if so,
please could you let me know how to trigger them?
> And only HS200 or that's just what you tested with?
Yes that is just the product configuration; so the only one I tested.
The host controller is sdhci-of-arasan for xlnx,zynqmp-8.9a. I inspected
the driver history and auto-tuning was extended to support ZynqMP (DLL
reset) some years ago so I believe tuning itself should be fine.
I am referring to commit 8d2e334377dbe645415fbe031711324bc2281907 "mmc:
sdhci-of-arasan: Add support for DLL reset for ZynqMP platforms "
>
> >
> > return ret;
> > @@ -3143,4 +3148,3 @@ module_exit(mmc_blk_exit);
> >
> > MODULE_LICENSE("GPL");
> > MODULE_DESCRIPTION("Multimedia Card (MMC) block device driver");
> > -
> > diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> > index b7754a1b8d97..1e1555a15de9 100644
> > --- a/drivers/mmc/core/card.h
> > +++ b/drivers/mmc/core/card.h
> > @@ -85,6 +85,7 @@ struct mmc_fixup {
> > #define CID_MANFID_MICRON 0x13
> > #define CID_MANFID_SAMSUNG 0x15
> > #define CID_MANFID_APACER 0x27
I had to add another ID for SanDisk - maybe 0x45 is for Industrial
controllers? do you know?
> > +#define CID_MANFID_SANDISK2 0x45
> > #define CID_MANFID_KINGSTON 0x70
> > #define CID_MANFID_HYNIX 0x90
> > #define CID_MANFID_KINGSTON_SD 0x9F
> > @@ -284,4 +285,10 @@ static inline int mmc_card_broken_cache_flush(const struct mmc_card *c)
> > {
> > return c->quirks & MMC_QUIRK_BROKEN_CACHE_FLUSH;
> > }
> > +
> > +static inline int mmc_card_broken_rpmb_retune(const struct mmc_card *c)
> > +{
> > + return c->quirks & MMC_QUIRK_BROKEN_RPMB_RETUNE;
> > +}
> > +
> > #endif
> > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> > index cca71867bc4a..35dfc8437d29 100644
> > --- a/drivers/mmc/core/quirks.h
> > +++ b/drivers/mmc/core/quirks.h
> > @@ -130,6 +130,13 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = {
> > MMC_FIXUP(CID_NAME_ANY, CID_MANFID_SANDISK_SD, 0x5344, add_quirk_sd,
> > MMC_QUIRK_BROKEN_SD_DISCARD),
> >
> > + /*
> > + * SanDisk iNAND 7250 DDG4064, this quirk shall disable the retune
> > + * operation enforced by default when switching to RPMB.
> > + */
> > + MMC_FIXUP("DG4064", CIF_MANFID_SANDISK2, 0x100, add_quirk_mmc,
> > + MMC_QUIRK_BROKEN_RPMB_RETUNE),
> > +
> > END_FIXUP
> > };
> >
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index 7b12eebc5586..bd6986189e8b 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -296,6 +296,7 @@ struct mmc_card {
> > #define MMC_QUIRK_BROKEN_SD_DISCARD (1<<14) /* Disable broken SD discard support */
> > #define MMC_QUIRK_BROKEN_SD_CACHE (1<<15) /* Disable broken SD cache support */
> > #define MMC_QUIRK_BROKEN_CACHE_FLUSH (1<<16) /* Don't flush cache until the write has occurred */
> > +#define MMC_QUIRK_BROKEN_RPMB_RETUNE (1<<17) /* Don't force a retune before switching to RPMB */
> >
> > bool written_flag; /* Indicates eMMC has been written since power on */
> > bool reenable_cmdq; /* Re-enable Command Queue */
> > --
> > 2.34.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread