mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrei Warkentin <andreiw@motorola.com>
To: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: linux-mmc@vger.kernel.org, cjb@laptop.org,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	adrian.hunter@nokia.com
Subject: Re: [PATCH v4 2/3]mmc: enable TRIM/ERASE caps for SDHCI host
Date: Sun, 3 Apr 2011 23:33:52 -0500	[thread overview]
Message-ID: <BANLkTik0ijdeccTgqxf8Mu87s8jxevaOhA@mail.gmail.com> (raw)
In-Reply-To: <BANLkTikGW3sYjW1HjSJfdBV22DK+Y0zi1Q@mail.gmail.com>

On Sun, Apr 3, 2011 at 11:06 PM, Andrei Warkentin <andreiw@motorola.com> wrote:
> On Sat, Feb 12, 2011 at 12:22 AM, Chuanxiao Dong
> <chuanxiao.dong@intel.com> wrote:
>> SDHCI host controller has TRIM/ERASE capability, enable these caps for erasing
>> purpose.
>>
>> ERASE command needs R1B response. So for such command with busy signal, before
>> sending command, SDHCI host driver will simply set a maximum value for timeout
>> control register which is safe to use.
>>
>
> Awesome. I took a look at the eMMC spec, and the following commands
> also have R1B responses:
>
> 1) CMD5 (SLEEP_AWAKE)
> 2) CMD6 (SWITCH)
> 3) CMD7 (Select/Deselect - for Disconnected->Programming transitions)
> 4) CMD12 (STOP_TRANSMISSION - for write case)
> 5) CMD28 (SET_WRITE_PROT)
> 6) CMD29 (CLR_WRITE_PROT)
>
> ...so this should help with timeouts for those commands as well. Thanks!
>
> A
>

On a second thought (and look), I see the following problems with this
patch, which are otherwise going to make the MMC code confusing (and
invalid).

1) Erase timeout value is defined by spec (eMMC or SD) and already
calculated in mmc_set_erase_timeout within core/core.c. You should
honor that value instead of picking the max timeout.
2) The trim/erase commands are not the only commands with variable
timeouts. For example, certain CMD6 operations take different amounts
of time (Partition switch takes ext_csd:PARTITION_SWITCH_TIME) Right
now the command structure contains the field "erase_timeout", which
curiously enough is used nowhere other than getting set in
core/core.c. I propose renaming it to cmd_timeout. If cmd.data is
NULL, then cmd_timeout is used to set timeout for busy-type commands.

A

      reply	other threads:[~2011-04-04  4:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-12  6:22 Chuanxiao Dong
2011-02-12  8:34 ` Arnd Bergmann
2011-04-03  9:41 ` Sascha Silbe
2011-04-04  4:06 ` Andrei Warkentin
2011-04-04  4:33   ` Andrei Warkentin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BANLkTik0ijdeccTgqxf8Mu87s8jxevaOhA@mail.gmail.com \
    --to=andreiw@motorola.com \
    --cc=adrian.hunter@nokia.com \
    --cc=akpm@linux-foundation.org \
    --cc=chuanxiao.dong@intel.com \
    --cc=cjb@laptop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®