| Message ID | 20250314195832.1588159-1-erick.shepherd@ni.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: Add quirk to disable DDR50 tuning | expand |
On Fri, 14 Mar 2025 at 20:58, Erick Shepherd <erick.shepherd@ni.com> wrote: > > Adds the MMC_QUIRK_NO_UHS_DDR50_TUNING quirk and updates > mmc_execute_tuning() to return 0 if that quirk is set. This fixes an > issue on certain Swissbit SD cards that do not support DDR50 tuning > where tuning requests caused I/O errors to be thrown. > > Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> > --- > drivers/mmc/core/card.h | 1 + > drivers/mmc/core/core.c | 4 ++++ > drivers/mmc/core/quirks.h | 10 ++++++++++ > include/linux/mmc/card.h | 1 + > 4 files changed, 16 insertions(+) > > diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h > index 3205feb1e8ff..756f80024635 100644 > --- a/drivers/mmc/core/card.h > +++ b/drivers/mmc/core/card.h > @@ -89,6 +89,7 @@ struct mmc_fixup { > #define CID_MANFID_MICRON 0x13 > #define CID_MANFID_SAMSUNG 0x15 > #define CID_MANFID_APACER 0x27 > +#define CID_MANFID_SWISSBIT 0x5D > #define CID_MANFID_KINGSTON 0x70 > #define CID_MANFID_HYNIX 0x90 > #define CID_MANFID_KINGSTON_SD 0x9F > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 5241528f8b90..8962992f05aa 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -937,6 +937,10 @@ int mmc_execute_tuning(struct mmc_card *card) > if (!host->ops->execute_tuning) > return 0; > > + if ((card->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING) && > + host->ios.timing == MMC_TIMING_UHS_DDR50) > + return 0; > + Please move this to mmc_sd_init_uhs_card() instead. Moreover, please add a helper in drivers/mmc/core/card.h for MMC_QUIRK_NO_UHS_DDR50_TUNING, similar to other quirks. > if (host->cqe_on) > host->cqe_ops->cqe_off(host); > > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h > index 89b512905be1..7f893bafaa60 100644 > --- a/drivers/mmc/core/quirks.h > +++ b/drivers/mmc/core/quirks.h > @@ -34,6 +34,16 @@ static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = { > MMC_QUIRK_BROKEN_SD_CACHE | MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY, > EXT_CSD_REV_ANY), > > + /* > + * Swissbit series S46-u cards throw I/O errors during tuning requests > + * after the initial tuning request expectedly times out. This has > + * only been observed on cards manufactured on 01/2019 that are using > + * Bay Trail host controllers. > + */ > + _FIXUP_EXT("0016G", CID_MANFID_SWISSBIT, 0x5342, 2019, 1, > + 0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd, > + MMC_QUIRK_NO_UHS_DDR50_TUNING, EXT_CSD_REV_ANY), > + > END_FIXUP > }; > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 526fce581657..ddcdf23d731c 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -329,6 +329,7 @@ struct mmc_card { > #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_SD_POWEROFF_NOTIFY (1<<17) /* Disable broken SD poweroff notify support */ > +#define MMC_QUIRK_NO_UHS_DDR50_TUNING (1<<18) /* Disable DDR50 tuning */ > > bool written_flag; /* Indicates eMMC has been written since power on */ > bool reenable_cmdq; /* Re-enable Command Queue */ > -- > 2.43.0 > Other than the minor things above, this seems reasonable to me! Kind regards Uffe
> Please move this to mmc_sd_init_uhs_card() instead. Moreover, please > add a helper in drivers/mmc/core/card.h for > MMC_QUIRK_NO_UHS_DDR50_TUNING, similar to other quirks. Sorry for the late response, how does this look? I can change the MMC_QUIRK_NO_UHS_DDR50_TUNING check to be before the tuning if-statement instead of within it if that seems more appropriate. --- a/drivers/mmc/core/card.h +++ b/drivers/mmc/core/card.h @@ -89,6 +89,7 @@ struct mmc_fixup { #define CID_MANFID_MICRON 0x13 #define CID_MANFID_SAMSUNG 0x15 #define CID_MANFID_APACER 0x27 +#define CID_MANFID_SWISSBIT 0x5D #define CID_MANFID_KINGSTON 0x70 #define CID_MANFID_HYNIX 0x90 #define CID_MANFID_KINGSTON_SD 0x9F @@ -294,4 +295,9 @@ static inline int mmc_card_broken_sd_poweroff_notify(const struct mmc_card *c) return c->quirks & MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY; } +static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c) +{ + return c->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING; +} + --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -664,6 +664,10 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) if (!mmc_host_is_spi(card->host) && (card->host->ios.timing == MMC_TIMING_UHS_SDR50 || card->host->ios.timing == MMC_TIMING_UHS_DDR50 || card->host->ios.timing == MMC_TIMING_UHS_SDR104)) { + if ((card->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING) && + card->host->ios.timing == MMC_TIMING_UHS_DDR50) + goto out; + err = mmc_execute_tuning(card); /* Regards, Erick
On 24/03/25 21:21, Erick Shepherd wrote: >> Please move this to mmc_sd_init_uhs_card() instead. Moreover, please >> add a helper in drivers/mmc/core/card.h for >> MMC_QUIRK_NO_UHS_DDR50_TUNING, similar to other quirks. > > Sorry for the late response, how does this look? I can change the > MMC_QUIRK_NO_UHS_DDR50_TUNING check to be before the tuning > if-statement instead of within it if that seems more appropriate. > > --- a/drivers/mmc/core/card.h > +++ b/drivers/mmc/core/card.h > @@ -89,6 +89,7 @@ struct mmc_fixup { > #define CID_MANFID_MICRON 0x13 > #define CID_MANFID_SAMSUNG 0x15 > #define CID_MANFID_APACER 0x27 > +#define CID_MANFID_SWISSBIT 0x5D > #define CID_MANFID_KINGSTON 0x70 > #define CID_MANFID_HYNIX 0x90 > #define CID_MANFID_KINGSTON_SD 0x9F > @@ -294,4 +295,9 @@ static inline int mmc_card_broken_sd_poweroff_notify(const struct mmc_card *c) > return c->quirks & MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY; > } > > +static inline int mmc_card_no_uhs_ddr50_tuning(const struct mmc_card *c) > +{ > + return c->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING; > +} > + > > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -664,6 +664,10 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card) > if (!mmc_host_is_spi(card->host) && > (card->host->ios.timing == MMC_TIMING_UHS_SDR50 || > card->host->ios.timing == MMC_TIMING_UHS_DDR50 || > card->host->ios.timing == MMC_TIMING_UHS_SDR104)) { > + if ((card->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING) && > + card->host->ios.timing == MMC_TIMING_UHS_DDR50) > + goto out; Perhaps make a helper function (untested): static bool mmc_sd_use_tuning(struct mmc_card *card) { if (mmc_host_is_spi(card->host)) return false; switch (card->host->ios.timing) { case MMC_TIMING_UHS_SDR50: case MMC_TIMING_UHS_SDR104: return true; case MMC_TIMING_UHS_DDR50: return !mmc_card_no_uhs_ddr50_tuning(card); } return false; } Then change to: if (mmc_sd_use_tuning(card)) { err = mmc_execute_tuning(card); etc } > + > err = mmc_execute_tuning(card); > > /* > > Regards, > Erick
> Perhaps make a helper function (untested):
Making a helper function seems like a good idea to me. How does this
look? I tested it using the SD card with our issue to confirm the I/O
errors no longer appear. I also tested it with another DDR50 SD card
to make sure the tuning is still executed in that case. I don't have a
SDR50 or SDR104 card on hand but could try to find them if you believe
the helper function needs to be tested further.
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index cc757b850e79..fc3416027033 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -613,6 +613,29 @@ static int sd_set_current_limit(struct mmc_card *card, u8 *status)
return 0;
}
+/*
+ * Determine if the card should tune or not.
+ */
+static bool mmc_sd_use_tuning(struct mmc_card *card)
+{
+ /*
+ * SPI mode doesn't define CMD19 and tuning is only valid for SDR50 and
+ * SDR104 mode SD-cards. Note that tuning is mandatory for SDR104.
+ */
+ if (mmc_host_is_spi(card->host))
+ return false;
+
+ switch (card->host->ios.timing) {
+ case MMC_TIMING_UHS_SDR50:
+ case MMC_TIMING_UHS_SDR104:
+ return true;
+ case MMC_TIMING_UHS_DDR50:
+ return !mmc_card_no_uhs_ddr50_tuning(card);
+ }
+
+ return false;
+}
+
/*
* UHS-I specific initialization procedure
*/
@@ -656,14 +679,7 @@ static int mmc_sd_init_uhs_card(struct mmc_card *card)
if (err)
goto out;
- /*
- * SPI mode doesn't define CMD19 and tuning is only valid for SDR50 and
- * SDR104 mode SD-cards. Note that tuning is mandatory for SDR104.
- */
- if (!mmc_host_is_spi(card->host) &&
- (card->host->ios.timing == MMC_TIMING_UHS_SDR50 ||
- card->host->ios.timing == MMC_TIMING_UHS_DDR50 ||
- card->host->ios.timing == MMC_TIMING_UHS_SDR104)) {
+ if (mmc_sd_use_tuning(card)) {
err = mmc_execute_tuning(card);
/*
Regards,
Erick
diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h index 3205feb1e8ff..756f80024635 100644 --- a/drivers/mmc/core/card.h +++ b/drivers/mmc/core/card.h @@ -89,6 +89,7 @@ struct mmc_fixup { #define CID_MANFID_MICRON 0x13 #define CID_MANFID_SAMSUNG 0x15 #define CID_MANFID_APACER 0x27 +#define CID_MANFID_SWISSBIT 0x5D #define CID_MANFID_KINGSTON 0x70 #define CID_MANFID_HYNIX 0x90 #define CID_MANFID_KINGSTON_SD 0x9F diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5241528f8b90..8962992f05aa 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -937,6 +937,10 @@ int mmc_execute_tuning(struct mmc_card *card) if (!host->ops->execute_tuning) return 0; + if ((card->quirks & MMC_QUIRK_NO_UHS_DDR50_TUNING) && + host->ios.timing == MMC_TIMING_UHS_DDR50) + return 0; + if (host->cqe_on) host->cqe_ops->cqe_off(host); diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index 89b512905be1..7f893bafaa60 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -34,6 +34,16 @@ static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = { MMC_QUIRK_BROKEN_SD_CACHE | MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY, EXT_CSD_REV_ANY), + /* + * Swissbit series S46-u cards throw I/O errors during tuning requests + * after the initial tuning request expectedly times out. This has + * only been observed on cards manufactured on 01/2019 that are using + * Bay Trail host controllers. + */ + _FIXUP_EXT("0016G", CID_MANFID_SWISSBIT, 0x5342, 2019, 1, + 0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd, + MMC_QUIRK_NO_UHS_DDR50_TUNING, EXT_CSD_REV_ANY), + END_FIXUP }; diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 526fce581657..ddcdf23d731c 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -329,6 +329,7 @@ struct mmc_card { #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_SD_POWEROFF_NOTIFY (1<<17) /* Disable broken SD poweroff notify support */ +#define MMC_QUIRK_NO_UHS_DDR50_TUNING (1<<18) /* Disable DDR50 tuning */ bool written_flag; /* Indicates eMMC has been written since power on */ bool reenable_cmdq; /* Re-enable Command Queue */
Adds the MMC_QUIRK_NO_UHS_DDR50_TUNING quirk and updates mmc_execute_tuning() to return 0 if that quirk is set. This fixes an issue on certain Swissbit SD cards that do not support DDR50 tuning where tuning requests caused I/O errors to be thrown. Signed-off-by: Erick Shepherd <erick.shepherd@ni.com> --- drivers/mmc/core/card.h | 1 + drivers/mmc/core/core.c | 4 ++++ drivers/mmc/core/quirks.h | 10 ++++++++++ include/linux/mmc/card.h | 1 + 4 files changed, 16 insertions(+)