linux-kernel - Re: [RFC PATCH] mmc: core: Add SD card quirk for broken poweroff notification

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPDyKFpiM5O06YEjyE22jFY2vV3RDPnR=uHmUPUEvRF7AQhxQw@mail.gmail.com>
Date: Thu, 3 Oct 2024 01:35:57 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Keita Aihara <keita.aihara@...y.com>
Cc: Shingo.Takeuchi@...y.com, Masaya.Takahashi@...y.com, 
	linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] mmc: core: Add SD card quirk for broken poweroff notification

On Fri, 13 Sept 2024 at 11:44, Keita Aihara <keita.aihara@...y.com> wrote:
>
> GIGASTONE Gaming Plus microSD cards manufactured on 02/2022 report that
> they support poweroff notification and cache, but they are not working
> correctly.
>
> Flush Cache bit never gets cleared in sd_flush_cache() and Poweroff
> Notification Ready bit also never gets set to 1 within 1 second from the
> end of busy of CMD49 in sd_poweroff_notify().
>
> This leads to I/O error and runtime PM error state.
>
> I observed that the same card manufactured on 01/2024 works as expected.
>
> This problem seems similar to the Kingston cards fixed with
> commit c467c8f08185 ("mmc: Add MMC_QUIRK_BROKEN_SD_CACHE for Kingston
> Canvas Go Plus from 11/2019") and should be handled using quirks.
>
> CID for the problematic card is here.
> 12345641535443002000000145016200
>
> Manufacturer ID is 0x12 and defined as CID_MANFID_GIGASTONE as of now,
> but would like comments on what naming is appropriate because MID list
> is not public and not sure it's right.
>
> Signed-off-by: Keita Aihara <keita.aihara@...y.com>

Since the MI list isn't public and there is nobody using 0x12 for now,
we might as well use it. At least I don't have a better suggestion.

That said, I have applied this for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/core/card.h   | 7 +++++++
>  drivers/mmc/core/quirks.h | 9 +++++++++
>  drivers/mmc/core/sd.c     | 2 +-
>  include/linux/mmc/card.h  | 1 +
>  4 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/card.h b/drivers/mmc/core/card.h
> index b7754a1b8d97..8476754b1b17 100644
> --- a/drivers/mmc/core/card.h
> +++ b/drivers/mmc/core/card.h
> @@ -82,6 +82,7 @@ struct mmc_fixup {
>  #define CID_MANFID_SANDISK_SD   0x3
>  #define CID_MANFID_ATP          0x9
>  #define CID_MANFID_TOSHIBA      0x11
> +#define CID_MANFID_GIGASTONE    0x12
>  #define CID_MANFID_MICRON       0x13
>  #define CID_MANFID_SAMSUNG      0x15
>  #define CID_MANFID_APACER       0x27
> @@ -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_sd_poweroff_notify(const struct mmc_card *c)
> +{
> +       return c->quirks & MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY;
> +}
> +
>  #endif
> diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h
> index 92905fc46436..89b512905be1 100644
> --- a/drivers/mmc/core/quirks.h
> +++ b/drivers/mmc/core/quirks.h
> @@ -25,6 +25,15 @@ static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = {
>                    0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd,
>                    MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY),
>
> +       /*
> +        * GIGASTONE Gaming Plus microSD cards manufactured on 02/2022 never
> +        * clear Flush Cache bit and set Poweroff Notification Ready bit.
> +        */
> +       _FIXUP_EXT("ASTC", CID_MANFID_GIGASTONE, 0x3456, 2022, 2,
> +                  0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd,
> +                  MMC_QUIRK_BROKEN_SD_CACHE | MMC_QUIRK_BROKEN_SD_POWEROFF_NOTIFY,
> +                  EXT_CSD_REV_ANY),
> +
>         END_FIXUP
>  };
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index ee37ad14e79e..0ec550ad5651 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1118,7 +1118,7 @@ static int sd_parse_ext_reg_power(struct mmc_card *card, u8 fno, u8 page,
>         card->ext_power.rev = reg_buf[0] & 0xf;
>
>         /* Power Off Notification support at bit 4. */
> -       if (reg_buf[1] & BIT(4))
> +       if ((reg_buf[1] & BIT(4)) && !mmc_card_broken_sd_poweroff_notify(card))
>                 card->ext_power.feature_support |= SD_EXT_POWER_OFF_NOTIFY;
>
>         /* Power Sustenance support at bit 5. */
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index f34407cc2788..543446392776 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -294,6 +294,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_SD_POWEROFF_NOTIFY    (1<<17) /* Disable broken SD poweroff notify support */
>
>         bool                    written_flag;   /* Indicates eMMC has been written since power on */
>         bool                    reenable_cmdq;  /* Re-enable Command Queue */
> --
> 2.46.0
>