Re: [PATCH] mmc: core: Add a card quirk for broken boot partitions

[Ulf Hansson <ulf.hansson@linaro.org>]

On Tue, 29 Jun 2021 at 01:32, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Some boot partitions on the Samsung 4GB KLM4G1YE4C "4YMD1R" card appear
> broken when accessed randomly. CMD6 to switch back to the main partition
> randomly stalls after CMD18 access to the boot partition 1, and the card
> never comes back online. The accesses to the boot partitions work several
> times before this happens.
>
> I tried using only single blocks with CMD17 on the boot partitions with the
> result that it crashed even faster.
>
> The card may survive on older kernels, but this seems to be because recent
> kernel partition parsers are accessing the boot partitions more, and we get
> more block traffic to the boot partitions during kernel startup.
>
> My only conclusion is that the boot partitions are somehow broken on this
> card, and we add a quirk for this.

You may very well be right on the above conclusion. However, I would
prefer some more details before I am ready to apply this. For example,
more exactly, what happens when the stall is triggered? Can you
provide some more debug info, perhaps with the sequence of commands
that triggers the error?

Moreover, it would be nice to know exactly where in the code the stall
happens. Could it be that it's the mmc_wait_for_cmd() that never
returns in __mmc_switch(), when sending the CMD6 to switch the
partition?

My main worry is that we should not set a card quirk for an eMMC that
could be working fine (on some other platforms), that's why I want us
to be sure.

>
> After applying this patch, the main partition can be accessed and mounted
> without problems.
>
> This eMMC card is found in the Samsung GT-S7710 mobile phone.
>
> Cc: phone-devel@vger.kernel.org
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Reported-by: newbyte@disroot.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Kind regards
Uffe

> ---
>  drivers/mmc/core/block.c  | 6 ++++++
>  drivers/mmc/core/quirks.h | 7 +++++++
>  include/linux/mmc/card.h  | 1 +
>  3 files changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 689eb9afeeed..e6cde68cda0e 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2621,6 +2621,12 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
>                                 card->part[idx].name);
>                         if (ret)
>                                 return ret;
> +               } else if((card->part[idx].area_type & MMC_BLK_DATA_AREA_BOOT) &&
> +                         (card->quirks & MMC_QUIRK_BROKEN_BOOT_PARTITIONS)) {
> +                       pr_info("%s: skipping broken boot partition %s\n",
> +                               mmc_hostname(card->host),
> +                               card->part[idx].name);
> +                       continue;
>                 } else if (card->part[idx].size) {
>                         ret = mmc_blk_alloc_part(card, md,
>                                 card->part[idx].part_cfg,
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index d68e6e513a4f..aa4060c10aa9 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -99,6 +99,13 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = {
>         MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
>                   MMC_QUIRK_TRIM_BROKEN),
>
> +       /*
> +        * Some Samsung eMMCs have broken boot partitions, accessing them
> +        * randomly will make the device lock up and crash.
> +        */
> +       MMC_FIXUP("4YMD1R", CID_MANFID_SAMSUNG, CID_OEMID_ANY, add_quirk_mmc,
> +                 MMC_QUIRK_BROKEN_BOOT_PARTITIONS),
> +
>         END_FIXUP
>  };
>
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index f9ad35dd6012..4006736f59dd 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -259,6 +259,7 @@ struct mmc_card {
>                                                 /* for byte mode */
>  #define MMC_QUIRK_NONSTD_SDIO  (1<<2)          /* non-standard SDIO card attached */
>                                                 /* (missing CIA registers) */
> +#define MMC_QUIRK_BROKEN_BOOT_PARTITIONS (1<<3)        /* Disable broken boot partitions */
>  #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4)                /* SDIO card has nonstd function interfaces */
>  #define MMC_QUIRK_DISABLE_CD   (1<<5)          /* disconnect CD/DAT[3] resistor */
>  #define MMC_QUIRK_INAND_CMD38  (1<<6)          /* iNAND devices have broken CMD38 */
> --
> 2.31.1
>