Re: [PATCH 2/3] mmc: core: also abort tuning with CMD12 for SD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 14 Sept 2021 at 20:21, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> We have various SanDisk cards which fail tuning to SDR104 unless we
> allow a CMD12 also to be sent to abort a broken tuning. It is true that
> the SD specs do not mention that CMD12 is allowed, but they also don't
> say it is forbidden. And now reality tells that it is needed to make
> some cards work. Other cards I tried did not regress.
>
> So, add a new callback to the bus_ops which allows us to send STOP for
> MMC and SD cards but not SDIO which does not support CMD12.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> Tested with a Renesas Salvator-XS board (R-Car H3 ES2.0) and a SanDisk
> Ultra 64GB card.
>
>  drivers/mmc/core/core.h    |  1 +
>  drivers/mmc/core/mmc.c     |  6 ++++++
>  drivers/mmc/core/mmc_ops.c | 21 +++------------------
>  drivers/mmc/core/sd.c      |  6 ++++++
>  4 files changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 7931a4f0137d..660873ba13ed 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -31,6 +31,7 @@ struct mmc_bus_ops {
>         int (*sw_reset)(struct mmc_host *);
>         bool (*cache_enabled)(struct mmc_host *);
>         int (*flush_cache)(struct mmc_host *);
> +       int (*send_abort_tuning)(struct mmc_host *);
>  };
>
>  void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops);
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 29e58ffae379..d638477e0a49 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -2227,6 +2227,11 @@ static int _mmc_hw_reset(struct mmc_host *host)
>         return mmc_init_card(host, card->ocr, card);
>  }
>
> +static int _mmc_send_abort_tuning(struct mmc_host *host)
> +{
> +       return mmc_send_stop(host, 150, 0);

Perhaps also add/copy the comment about why we use 150 ms as timeout here.

> +}
> +
>  static const struct mmc_bus_ops mmc_ops = {
>         .remove = mmc_remove,
>         .detect = mmc_detect,
> @@ -2239,6 +2244,7 @@ static const struct mmc_bus_ops mmc_ops = {
>         .hw_reset = _mmc_hw_reset,
>         .cache_enabled = _mmc_cache_enabled,
>         .flush_cache = _mmc_flush_cache,
> +       .send_abort_tuning = _mmc_send_abort_tuning,
>  };
>
>  /*
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 0c54858e89c0..bc794419d443 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -705,26 +705,11 @@ EXPORT_SYMBOL_GPL(mmc_send_tuning);
>
>  int mmc_send_abort_tuning(struct mmc_host *host, u32 opcode)
>  {
> -       struct mmc_command cmd = {};
> -
> -       /*
> -        * eMMC specification specifies that CMD12 can be used to stop a tuning
> -        * command, but SD specification does not, so do nothing unless it is
> -        * eMMC.
> -        */
> -       if (opcode != MMC_SEND_TUNING_BLOCK_HS200)
> -               return 0;
> +       if (host->bus_ops->send_abort_tuning)
> +               return host->bus_ops->send_abort_tuning(host);
>
> -       cmd.opcode = MMC_STOP_TRANSMISSION;
> -       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> -
> -       /*
> -        * For drivers that override R1 to R1b, set an arbitrary timeout based
> -        * on the tuning timeout i.e. 150ms.
> -        */
> -       cmd.busy_timeout = 150;
> +       return 0;
>
> -       return mmc_wait_for_cmd(host, &cmd, 0);
>  }
>  EXPORT_SYMBOL_GPL(mmc_send_abort_tuning);
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 4646b7a03db6..19c85e6088f4 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -1784,6 +1784,11 @@ static int mmc_sd_hw_reset(struct mmc_host *host)
>         return mmc_sd_init_card(host, host->card->ocr, host->card);
>  }
>
> +static int sd_send_abort_tuning(struct mmc_host *host)
> +{
> +       return mmc_send_stop(host, 150, 0);

Perhaps also add/copy the comment about why we use 150 ms as timeout here.

> +}
> +
>  static const struct mmc_bus_ops mmc_sd_ops = {
>         .remove = mmc_sd_remove,
>         .detect = mmc_sd_detect,
> @@ -1796,6 +1801,7 @@ static const struct mmc_bus_ops mmc_sd_ops = {
>         .hw_reset = mmc_sd_hw_reset,
>         .cache_enabled = sd_cache_enabled,
>         .flush_cache = sd_flush_cache,
> +       .send_abort_tuning = sd_send_abort_tuning,
>  };
>
>  /*
> --
> 2.30.2
>

Besides the minor nitpicks, this looks good to me!

Kind regards
Uffe



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux