linux-kernel - RE: [PATCH] mmc: sd: Add a variable to check a faulty device

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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <BL0PR04MB65642D0389544F022A1D2222FC4F2@BL0PR04MB6564.namprd04.prod.outlook.com>
Date: Tue, 13 Feb 2024 08:42:09 +0000
From: Avri Altman <Avri.Altman@....com>
To: Seunghui Lee <sh043.lee@...sung.com>, "linux-mmc@...r.kernel.org"
	<linux-mmc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "ulf.hansson@...aro.org"
	<ulf.hansson@...aro.org>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>
CC: "grant.jung@...sung.com" <grant.jung@...sung.com>, "jt77.jang@...sung.com"
	<jt77.jang@...sung.com>, "dh0421.hwang@...sung.com"
	<dh0421.hwang@...sung.com>, "junwoo80.lee@...sung.com"
	<junwoo80.lee@...sung.com>, "jangsub.yi@...sung.com"
	<jangsub.yi@...sung.com>, "cw9316.lee@...sung.com" <cw9316.lee@...sung.com>,
	"sh8267.baek@...sung.com" <sh8267.baek@...sung.com>, "wkon.kim@...sung.com"
	<wkon.kim@...sung.com>
Subject: RE: [PATCH] mmc: sd: Add a variable to check a faulty device

> In mobile devices, suspend/resume situations are frequent.
> In the case of a defective SD card in which initialization fails, unnecessary
> initialization time is consumed for each resume.
> A field is needed to check that SD card initialization has failed on the host. It
> could be used to remove unnecessary initialization.
I don't see where you are using this new init_failed field?
Maybe instead, elaborate the logic to free_card: to detect a broken sd.
e.g. instead of just if (!oldcard), if (!oldcard || ! mmc_sd_alive(host)) or something.

Thanks,
Avri

> 
> Signed-off-by: Seunghui Lee <sh043.lee@...sung.com>
> ---
>  drivers/mmc/core/sd.c        | 12 +++++++++++-
>  drivers/mmc/core/slot-gpio.c |  1 +
>  include/linux/mmc/host.h     |  1 +
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index
> c3e554344c99..f0eb3864dc24 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1410,6 +1410,7 @@ static int mmc_sd_init_card(struct mmc_host *host,
> u32 ocr,
>         bool v18_fixup_failed = false;
> 
>         WARN_ON(!host->claimed);
> +       host->init_failed = false;
>  retry:
>         err = mmc_sd_get_cid(host, ocr, cid, &rocr);
>         if (err)
> @@ -1752,6 +1753,8 @@ static int _mmc_sd_resume(struct mmc_host *host)
> 
>         mmc_power_up(host, host->card->ocr);
>         err = mmc_sd_init_card(host, host->card->ocr, host->card);
> +       if (err)
> +               host->init_failed = true;
>         mmc_card_clr_suspended(host->card);
> 
>  out:
> @@ -1803,8 +1806,12 @@ static int mmc_sd_runtime_resume(struct
> mmc_host *host)
> 
>  static int mmc_sd_hw_reset(struct mmc_host *host)  {
> +       int err;
>         mmc_power_cycle(host, host->card->ocr);
> -       return mmc_sd_init_card(host, host->card->ocr, host->card);
> +       err = mmc_sd_init_card(host, host->card->ocr, host->card);
> +       if (err)
> +               host->init_failed = true;
> +       return err;
>  }
> 
>  static const struct mmc_bus_ops mmc_sd_ops = { @@ -1891,5 +1898,8 @@ int
> mmc_attach_sd(struct mmc_host *host)
>         pr_err("%s: error %d whilst initialising SD card\n",
>                 mmc_hostname(host), err);
> 
> +       if (err)
> +               host->init_failed = true;
> +
>         return err;
>  }
> diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c index
> 2a2d949a9344..93d081c7dd53 100644
> --- a/drivers/mmc/core/slot-gpio.c
> +++ b/drivers/mmc/core/slot-gpio.c
> @@ -33,6 +33,7 @@ static irqreturn_t mmc_gpio_cd_irqt(int irq, void *dev_id)
>         struct mmc_gpio *ctx = host->slot.handler_priv;
> 
>         host->trigger_card_event = true;
> +       host->init_failed = false;
>         mmc_detect_change(host, msecs_to_jiffies(ctx->cd_debounce_delay_ms));
> 
>         return IRQ_HANDLED;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index
> 2f445c651742..1d75cfdbf981 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -467,6 +467,7 @@ struct mmc_host {
>         struct timer_list       retune_timer;   /* for periodic re-tuning */
> 
>         bool                    trigger_card_event; /* card_event necessary */
> +       bool                    init_failed;    /* check if failed to initialize */
> 
>         struct mmc_card         *card;          /* device attached to this host */
> 
> --
> 2.29.0