mmc: core: apply SD quirks earlier during probe

PATCH Superseded Keita Aihara 4 participants 7 messages Aug 2, 2024 – Sep 18, 2024 MMC, SD and SDIO

Share & tools

Avri Altman

Reviewed-by on v1 1/1

2024-08-02 06:53 UTC #1 lore raw

Quoted 11 lines

Applying MMC_QUIRK_BROKEN_SD_CACHE is broken, as the card's extended registers are parsed prior to the quirk being applied in mmc_blk.
Split this out into an SD-specific list of quirks and apply in mmc_sd_init_card instead.
Fixes c467c8f08185 ("mmc: Add MMC_QUIRK_BROKEN_SD_CACHE for
Kingston Canvas Go Plus from 11/2019")
Authored-by Jonathan Bell <[email protected]>
Signed-off-by Jonathan Bell <[email protected]>
Signed-off-by Keita Aihara <[email protected]>

Reviewed-by Avri Altman <[email protected]>

Keita Aihara

author

2024-08-06 01:46 UTC #3 lore raw

Quoted 6 lines · Ulf Hansson

Quoted 3 lines · Keita Aihara
Applying MMC_QUIRK_BROKEN_SD_CACHE is broken, as the card's extended registers are parsed prior to the quirk being applied in mmc_blk.
In what way is it a problem to read the extended registers first?

SD quirks are referenced by mmc_card_broken_sd_cache() in sd_parse_ext_reg_perf(). If the quirk is set, SD_EXT_PERF_CACHE is not set to card->ext_perf.feature_support and the cache support will not be enabled.

Therefore, SD quirks should be initialized before parsing the extension registers.

Quoted 5 lines

Quoted 5 lines
Split this out into an SD-specific list of quirks and apply in mmc_sd_init_card instead.

Quoted 7 lines

Quoted 4 lines
Fixes c467c8f08185 ("mmc: Add MMC_QUIRK_BROKEN_SD_CACHE for Kingston Canvas Go Plus from 11/2019")
Authored-by Jonathan Bell <[email protected]>
Signed-off-by Jonathan Bell <[email protected]>
Signed-off-by Keita Aihara <[email protected]>
Kind regards
Uffe

Best regards,
Keita Aihara

Quoted 71 lines

---
 drivers/mmc/core/quirks.h | 22 +++++++++++++---------
 drivers/mmc/core/sd.c     |  4 ++++
 2 files changed, 17 insertions(+), 9 deletions(-)

drivers/mmc/core/quirks.h +13 -9

index cca71867bc4a..92905fc46436 100644
@@ -15,6 +15,19 @@
 
 #include "card.h"
 
+static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = {
+       /*
+        * Kingston Canvas Go! Plus microSD cards never finish SD cache flush.
+        * This has so far only been observed on cards from 11/2019, while new
+        * cards from 2023/05 do not exhibit this behavior.
+        */
+       _FIXUP_EXT("SD64G", CID_MANFID_KINGSTON_SD, 0x5449, 2019, 11,
+                  0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd,
+                  MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY),
+
+       END_FIXUP
+};
+
 static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = {
 #define INAND_CMD38_ARG_EXT_CSD  113
 #define INAND_CMD38_ARG_ERASE    0x00

@@ -53,15 +66,6 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = {
        MMC_FIXUP("MMC32G", CID_MANFID_TOSHIBA, CID_OEMID_ANY, add_quirk_mmc,
                  MMC_QUIRK_BLK_NO_CMD23),
 
-       /*
-        * Kingston Canvas Go! Plus microSD cards never finish SD cache flush.
-        * This has so far only been observed on cards from 11/2019, while new
-        * cards from 2023/05 do not exhibit this behavior.
-        */
-       _FIXUP_EXT("SD64G", CID_MANFID_KINGSTON_SD, 0x5449, 2019, 11,
-                  0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd,
-                  MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY),
-
        /*
         * Some SD cards lockup while using CMD23 multiblock transfers.
         */

drivers/mmc/core/sd.c +4 -1

index 1c8148cdda50..ee37ad14e79e 100644
@@ -26,6 +26,7 @@
 #include "host.h"
 #include "bus.h"
 #include "mmc_ops.h"
+#include "quirks.h"
 #include "sd.h"
 #include "sd_ops.h"

@@ -1475,6 +1476,9 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
                        goto free_card;
        }
 
+       /* Apply quirks prior to card setup */
+       mmc_fixup_device(card, mmc_sd_fixups);
+
        err = mmc_sd_setup_card(host, card, oldcard != NULL);
        if (err)
                goto free_card;
--

2.43.2

Ulf Hansson

2024-08-20 10:11 UTC #4 lore raw

Quoted 9 lines · Keita Aihara

Quoted 6 lines · Ulf Hansson
Quoted 3 lines · Keita Aihara
Applying MMC_QUIRK_BROKEN_SD_CACHE is broken, as the card's extended registers are parsed prior to the quirk being applied in mmc_blk.
In what way is it a problem to read the extended registers first?

Quoted 7 lines

SD quirks are referenced by mmc_card_broken_sd_cache() in sd_parse_ext_reg_perf(). If the quirk is set, SD_EXT_PERF_CACHE is not set to card->ext_perf.feature_support and the cache support will not be enabled.
Therefore, SD quirks should be initialized before parsing the extension registers.

Makes perfect sense! Please include some of this information in the commit message to make this clear.

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Split this out into an SD-specific list of quirks and apply in mmc_sd_init_card instead.
Fixes c467c8f08185 ("mmc: Add MMC_QUIRK_BROKEN_SD_CACHE for Kingston Canvas Go Plus from 11/2019")
Authored-by Jonathan Bell <[email protected]>

This tag isn't normally what we use. I suggest you change the author of the patch to Jonathan and keep his sob-tag.

Then add yourself with a "Co-developed-by" tag and keep your sob-tag.

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Signed-off-by Jonathan Bell <[email protected]>
Signed-off-by Keita Aihara <[email protected]>

[...]

Kind regards
Uffe

Keita Aihara

author

2024-08-20 23:29 UTC #5 lore raw

Quoted 15 lines · Ulf Hansson

Quoted 14 lines · Keita Aihara
Quoted 6 lines · Ulf Hansson
Quoted 3 lines · Keita Aihara
Applying MMC_QUIRK_BROKEN_SD_CACHE is broken, as the card's extended registers are parsed prior to the quirk being applied in mmc_blk.
In what way is it a problem to read the extended registers first?
SD quirks are referenced by mmc_card_broken_sd_cache() in sd_parse_ext_reg_perf(). If the quirk is set, SD_EXT_PERF_CACHE is not set to card->ext_perf.feature_support and the cache support will not be enabled.

Quoted 5 lines

Quoted 2 lines
Therefore, SD quirks should be initialized before parsing the extension registers.
Makes perfect sense! Please include some of this information in the commit message to make this clear.

Sure.

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Split this out into an SD-specific list of quirks and apply in mmc_sd_init_card instead.

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fixes c467c8f08185 ("mmc: Add MMC_QUIRK_BROKEN_SD_CACHE for Kingston Canvas Go Plus from 11/2019")
Authored-by Jonathan Bell <[email protected]>
This tag isn't normally what we use. I suggest you change the author of the patch to Jonathan and keep his sob-tag.
Then add yourself with a "Co-developed-by" tag and keep your sob-tag.

Thank you for your suggestion.
Removed "Authored-by" tag and added "Co-developed-by" tag to right after the first "Signed-off-by" tag by Jonathan.

[PATCH v2] mmc: core: apply SD quirks earlier during probe

I hope the v2 patch meets the format to treat Jonathan as the patch author.

Quoted 8 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Signed-off-by Jonathan Bell <[email protected]>
Signed-off-by Keita Aihara <[email protected]>
[...]
Kind regards
Uffe

Best regards,
Keita Aihara

Olexa Bilaniuk

2024-09-18 14:52 UTC #6 lore raw

PATCH 1/1 mmc: core: apply SD quirks earlier during probe

In reply to Keita Aihara

Quoted 11 lines

drivers/mmc/core/quirks.h +3 -0

index cca71867bc4a..92905fc46436 100644
@@ -15,6 +15,19 @@
 
 #include "card.h"
 
+static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = {
+	/*
+	 * Kingston Canvas Go! Plus microSD cards never finish SD cache flush.

drivers/mmc/core/quirks.h line 30

Quoted 10 lines

drivers/mmc/core/quirks.h +10 -0

+	 * This has so far only been observed on cards from 11/2019, while new
+	 * cards from 2023/05 do not exhibit this behavior.
+	 */
+	_FIXUP_EXT("SD64G", CID_MANFID_KINGSTON_SD, 0x5449, 2019, 11,
+		   0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd,
+		   MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY),
+
+	END_FIXUP
+};
+

By happenstance I have just such a Kingston Canvas Go! Plus card, but in standard SD form-factor and not microSD, and dating from *08/2021*. I had heretofore-inexplicable filesystem corruption problems with this card that may well be attributable to a failure to flush the cache. Despite diligence in issuing an explicit "sync" before ejecting, the filesystem would reliably appear corrupt after reconnecting it, and would require a "fsck", but no bad blocks could be identified.

The vitals of my card are:

  type:   SD
  name:   SD64G
  date:   08/2021
  manfid: 0x00009f (Kingston)
  oemid:  0x5449
  fwrev:  0x1
  hwrev:  0x6
  csd:    400e00325b590001cf9f7f800a400001
  ocr:    0x00300000

I think the quirk here can be safely broadened to all variants of this card and to the date range 11/2019 - 08/2021 inclusive.

Sincerely,
Olexa Bilaniuk