linux-kernel - Re: [PATCH 2/3] mmc: host: omap_hsmmc: use generic_cmd6

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]

Date:   Tue, 31 Jan 2017 11:12:33 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Ravikumar Kattekola <rk@...com>
Cc:     Peter Ujfalusi <peter.ujfalusi@...com>,
        Sekhar Nori <nsekhar@...com>, Tony Lindgren <tony@...mide.com>,
        Andreas Fenkart <afenkart@...il.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        linux-omap <linux-omap@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kishon <kishon@...com>
Subject: Re: [PATCH 2/3] mmc: host: omap_hsmmc: use generic_cmd6_time to
 program timeout value for CMD6

On 30 January 2017 at 11:11, Ravikumar Kattekola <rk@...com> wrote:
> From: Kishon Vijay Abraham I <kishon@...com>
>
> commit e2bf08d643a244ccb ("omap_hsmmc: set a large data timeout for
> commands with busy signal") sets an arbitrary timeout value (100ms) for
> commands like CMD6 (MMC SWITCH). However extended CSD register defined
> in the eMMC standard has a field for GENERIC_CMD6_TIME which indicates
> the default maximum timeout for a SWITCH command.
> Use busy_timeout of cmd structure (populated with GENERIC_CMD6_TIME
> in the case of SWITCH command) to program the data timeout value in
> omap_hsmmc driver.
> SWITCH command to turn the cache on took more than 100ms to complete
> with MICRON eMMC card present in AM572x IDK REV 1.3A resulting in
> timeout and failed enumeration. It is fixed here by programming the
> timeout with the value advertised in GENERIC_CMD6_TIME.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@...com>
> Signed-off-by: Sekhar Nori <nsekhar@...com>
> Signed-off-by: Ravikumar Kattekola <rk@...com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 0ee5650..51ca3d7 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1527,16 +1527,24 @@ static void omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host)
>  omap_hsmmc_prepare_data(struct omap_hsmmc_host *host, struct mmc_request *req)
>  {
>         int ret;
> +       unsigned int timeout;
> +
>         host->data = req->data;
>
>         if (req->data == NULL) {
>                 OMAP_HSMMC_WRITE(host->base, BLK, 0);
> -               /*
> -                * Set an arbitrary 100ms data timeout for commands with
> -                * busy signal.
> -                */
> -               if (req->cmd->flags & MMC_RSP_BUSY)
> -                       set_data_timeout(host, 100000000U, 0);
> +               if (req->cmd->flags & MMC_RSP_BUSY) {
> +                       timeout = req->cmd->busy_timeout * NSEC_PER_MSEC;
> +
> +                       /*
> +                        * Set an arbitrary 100ms data timeout for commands with
> +                        * busy signal and no indication of busy_timeout.
> +                        */
> +                       if (!timeout)

This is a bug in the mmc core if this ever happen.

Therefore I am particularly interested to find out if this is really
needed or it's just playing safe?

> +                               timeout = 100000000U;
> +
> +                       set_data_timeout(host, timeout, 0);
> +               }
>                 return 0;
>         }
>
> --
> 1.9.1
>

Kind regards
Uffe