* [PATCH 2/2] mmc: sdhci-pci: Allow deferred probe for sd card detect gpio
From: Adrian Hunter @ 2016-11-22 9:03 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, David E. Box
In-Reply-To: <1479805418-19603-1-git-send-email-adrian.hunter@intel.com>
From: "David E. Box" <david.e.box@linux.intel.com>
With commit f35bbf61ab77 ("gpio / ACPI: Return -EPROBE_DEFER if the
gpiochip was not found"), a gpio descriptor request can now be deferred if
the providing gpio host controller driver hasn't been loaded yet. Allow use
in mmc slot probe in order to prevent card detect gpio setup from failing
in this case.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/mmc/host/sdhci-pci-core.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 501098e65b0e..2d20fb60ce83 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1760,11 +1760,16 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
host->mmc->slotno = slotno;
host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
- if (slot->cd_idx >= 0 &&
- mmc_gpiod_request_cd(host->mmc, slot->cd_con_id, slot->cd_idx,
- slot->cd_override_level, 0, NULL)) {
- dev_warn(&pdev->dev, "failed to setup card detect gpio\n");
- slot->cd_idx = -1;
+ if (slot->cd_idx >= 0) {
+ ret = mmc_gpiod_request_cd(host->mmc, slot->cd_con_id, slot->cd_idx,
+ slot->cd_override_level, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ goto remove;
+
+ if (ret) {
+ dev_warn(&pdev->dev, "failed to setup card detect gpio\n");
+ slot->cd_idx = -1;
+ }
}
ret = sdhci_add_host(host);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] RFC: mmc: block: replace semaphore with freezing
From: Russell King - ARM Linux @ 2016-11-22 9:10 UTC (permalink / raw)
To: Linus Walleij
Cc: Arnd Bergmann, Binoy Jayan, Rafael J. Wysocki,
linux-mmc@vger.kernel.org, Ulf Hansson, Chunyan Zhang,
Baolin Wang, Linux PM, Rafael J . Wysocki, Jiri Kosina
In-Reply-To: <CACRpkdagS-7kaYCd2Sq0vLR8+19Mmckx=CuCqPv+HheRk8Cm7w@mail.gmail.com>
On Tue, Nov 22, 2016 at 09:54:18AM +0100, Linus Walleij wrote:
> On Wed, Nov 16, 2016 at 5:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday, November 16, 2016 4:20:47 PM CET Linus Walleij wrote:
> >> On Wed, Nov 16, 2016 at 1:46 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>
> >> > Well, we had a session at the KS regarding usage of the freezer on
> >> > kernel threads and the conclusion was to get rid of that (as opposed
> >> > to freezing user space, which is necessary IMO). So this change would
> >> > go in the opposite direction.
> >>
> >> Aha so I should not make this thread look like everyone else, instead
> >> everyone else should look like this thread, haha
> >>
> >> Ah well, I'll just drop it.
> >
> > It would still be good to remove the semaphore and do something else,
> > as we also want to remove all semaphores. ;-)
> >
> > We could check "mq->flags & MMC_QUEUE_SUSPENDED" in the kthread to see
> > if the queue is currently suspended, and otherwise go to sleep there,
> > and then call wake_up() in the resume function.
>
> Hm... so simply:
>
> if (mq->flags & MMC_QUEUE_SUSPENDED)
> schedule();
>
> ?
No, the schedule() is required when there are no more requests to
process, so the thread doesn't spin waiting for the next request to
appear.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH] RFC: mmc: block: replace semaphore with freezing
From: Arnd Bergmann @ 2016-11-22 9:16 UTC (permalink / raw)
To: Linus Walleij
Cc: Binoy Jayan, Rafael J. Wysocki, linux-mmc@vger.kernel.org,
Ulf Hansson, Chunyan Zhang, Baolin Wang, Linux PM,
Rafael J . Wysocki, Russell King, Jiri Kosina
In-Reply-To: <CACRpkdagS-7kaYCd2Sq0vLR8+19Mmckx=CuCqPv+HheRk8Cm7w@mail.gmail.com>
On Tuesday, November 22, 2016 9:54:18 AM CET Linus Walleij wrote:
> On Wed, Nov 16, 2016 at 5:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Wednesday, November 16, 2016 4:20:47 PM CET Linus Walleij wrote:
> >> On Wed, Nov 16, 2016 at 1:46 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >>
> >> > Well, we had a session at the KS regarding usage of the freezer on
> >> > kernel threads and the conclusion was to get rid of that (as opposed
> >> > to freezing user space, which is necessary IMO). So this change would
> >> > go in the opposite direction.
> >>
> >> Aha so I should not make this thread look like everyone else, instead
> >> everyone else should look like this thread, haha
> >>
> >> Ah well, I'll just drop it.
> >
> > It would still be good to remove the semaphore and do something else,
> > as we also want to remove all semaphores.
> >
> > We could check "mq->flags & MMC_QUEUE_SUSPENDED" in the kthread to see
> > if the queue is currently suspended, and otherwise go to sleep there,
> > and then call wake_up() in the resume function.
>
> Hm... so simply:
>
> if (mq->flags & MMC_QUEUE_SUSPENDED)
> schedule();
>
> ?
Something like that.
> This whole kthread business is pretty messy. I would prefer if I could
> just convert it to a workqueue. Just that it's not very simple the way
> it speculates around in the request queue from the block layer.
I don't see how that would work, but might be worth trying.
After doing that, a simple flush_workqueue() might be enough
to take care of the suspend operation.
> > While looking at that code, I just noticed that access to
> > mq->flags is racy and should be fixed as well.
>
> I guess we should take the queue lock &q->lock around accessing
> the flags.
Yes, either that, or use set_bit/test_bit/test_and_set_bit for
atomic access. For instance, this one
if (mq->flags & MMC_QUEUE_NEW_REQUEST) {
mq->flags &= ~MMC_QUEUE_NEW_REQUEST;
Could be
if (test_and_clear(MMC_QUEUE_NEW_REQUEST_BIT, &mq->flags))
...
Arnd
^ permalink raw reply
* [GIT PULL] DWMMC controller for next
From: Jaehoon Chung @ 2016-11-22 9:59 UTC (permalink / raw)
To: linux-mmc@vger.kernel.org; +Cc: Ulf Hansson, Jaehoon Chung
Dear Ulf,
Could you pull these patches on your next branch?
If there is a problem, let me know, plz.
The following changes since commit e2081b37d910c5e6c6925d9b4d0a7a6283f84ec5:
Merge branch 'fixes' into next (2016-11-21 11:09:18 +0100)
are available in the git repository at:
https://github.com/jh80chung/dw-mmc.git for-ulf
for you to fetch changes up to e25fd245b557482a8e0f7ab87841085f30706f3a:
Documentation: synopsys-dw-mshc: remove the unused properties (2016-11-22 10:34:05 +0900)
----------------------------------------------------------------
Colin Ian King (1):
mmc: dw_mmc: fix spelling mistake in dev_dbg message
Jaehoon Chung (9):
mmc: dw_mmc: display the real register value on debugfs
mmc: dw_mmc: fix the debug message for checking card's present
mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
mmc: dw_mmc: use the hold register when send stop command
mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
mmc: dw_mmc: use the cookie's enum values for post/pre_req()
mmc: dw_mmc: remove the unnecessary mmc_data structure
mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
Documentation: synopsys-dw-mshc: remove the unused properties
Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 ++------
drivers/mmc/host/dw_mmc.c | 92 ++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
include/linux/mmc/dw_mmc.h | 6 ++++++
3 files changed, 52 insertions(+), 54 deletions(-)
Best Regards,
Jaehoon Chung
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Linus Walleij @ 2016-11-22 10:20 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ritesh Harjani, Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Alex Lemberg, mateusz.nowak,
Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng, Asutosh Das,
Zhangfei Gao, Sujit Reddy Thumma, kdorfman, David Griego,
Sahitya Tummala, venkatg, Shawn Lin, Subhash Jadavani
In-Reply-To: <e18147ff-ac6b-a48f-4487-18ebd51bbc99@intel.com>
On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 21/11/16 17:34, Linus Walleij wrote:
>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>
>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>
>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
>>> command queue feature for such type of cards.
>>>
>>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>> [subhashj@codeaurora.org: fixed trivial merge conflicts]
>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Even if we don't merge the specific mechanism provided by the
>> rest of the patches, this patch just make us know more about
>> the capabilities of the hardware we're running on, which is good.
>
> I think SW CMDQ is a better starting point:
When I supplied the Reviewe-by, as stated below it I am not talking
about the patch set as a whole. I am talking about *this* patch.
As you can see in my reply to 00/10 I have serious concerns about
this patchset overall.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Adrian Hunter @ 2016-11-22 10:31 UTC (permalink / raw)
To: Linus Walleij
Cc: Ritesh Harjani, Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Alex Lemberg, mateusz.nowak,
Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng, Asutosh Das,
Zhangfei Gao, kdorfman, David Griego, Sahitya Tummala, venkatg,
Shawn Lin, Subhash Jadavani
In-Reply-To: <CACRpkdaHgN7XGdXjeE1dwQ2gn0z=VXfjVdVGYcT0gRfASNUKbQ@mail.gmail.com>
On 22/11/16 12:20, Linus Walleij wrote:
> On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 21/11/16 17:34, Linus Walleij wrote:
>>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>>
>>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>>
>>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>>> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
>>>> command queue feature for such type of cards.
>>>>
>>>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>> [subhashj@codeaurora.org: fixed trivial merge conflicts]
>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Even if we don't merge the specific mechanism provided by the
>>> rest of the patches, this patch just make us know more about
>>> the capabilities of the hardware we're running on, which is good.
>>
>> I think SW CMDQ is a better starting point:
>
> When I supplied the Reviewe-by, as stated below it I am not talking
> about the patch set as a whole. I am talking about *this* patch.
I should have been more explicit. SW CMDQ covers some of the same ground
and has basically the same patch:
https://marc.info/?l=linux-mmc&m=147729859922297&w=2
>
> As you can see in my reply to 00/10 I have serious concerns about
> this patchset overall.
I didn't mean to imply otherwise.
^ permalink raw reply
* Re: [PATCH 1/2] mmc: tegra: Support module reset
From: Adrian Hunter @ 2016-11-22 11:30 UTC (permalink / raw)
To: Thierry Reding
Cc: Ulf Hansson, Stephen Warren, Alexandre Courbot,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161117171855.20843-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 17/11/16 19:18, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> The device tree binding for the SDHCI controller found on Tegra SoCs
> specifies that a reset control can be provided by the device tree. No
> code was ever added to support the module reset, which can cause the
> driver to try and access registers from a module that's in reset. On
> most Tegra SoC generations doing so would cause a hang.
>
> Note that it's unlikely to see this happen because on most platforms
> these resets will have been deasserted by the bootloader. However the
> portability can be improved by making sure the driver deasserts the
> reset before accessing any registers.
>
> Since resets are synchronous on Tegra SoCs, the platform driver needs
> to implement a custom ->remove() callback now to make sure the clock
> is disabled after the reset is asserted.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-tegra.c | 38 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 20b6ff5b4af1..eac57bb161d3 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -21,6 +21,7 @@
> #include <linux/io.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/reset.h>
> #include <linux/mmc/card.h>
> #include <linux/mmc/host.h>
> #include <linux/mmc/mmc.h>
> @@ -65,6 +66,8 @@ struct sdhci_tegra {
> struct gpio_desc *power_gpio;
> bool ddr_signaling;
> bool pad_calib_required;
> +
> + struct reset_control *rst;
> };
>
> static u16 tegra_sdhci_readw(struct sdhci_host *host, int reg)
> @@ -489,6 +492,16 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
> clk_prepare_enable(clk);
> pltfm_host->clk = clk;
>
> + tegra_host->rst = devm_reset_control_get(&pdev->dev, "sdhci");
> + if (IS_ERR(tegra_host->rst)) {
> + rc = PTR_ERR(tegra_host->rst);
> + dev_err(&pdev->dev, "failed to get reset control: %d\n", rc);
> + goto err_clk_get;
> + }
> +
> + reset_control_deassert(tegra_host->rst);
> + usleep_range(2000, 4000);
> +
> rc = sdhci_add_host(host);
> if (rc)
> goto err_add_host;
Should you reset_control_assert() in the err_add_host path?
> @@ -504,6 +517,29 @@ static int sdhci_tegra_probe(struct platform_device *pdev)
> return rc;
> }
>
> +static int sdhci_tegra_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *platform = sdhci_priv(host);
> + struct sdhci_tegra *tegra = sdhci_pltfm_priv(platform);
> + int dead = 0;
> + u32 value;
> +
> + value = readl(host->ioaddr + SDHCI_INT_STATUS);
> + if (value == 0xffffffff)
I think this check of 0xffffffff is a PCI thing. Unless you know it is
meaningful for your platforms, you could leave it out.
> + dead = 1;
> +
> + sdhci_remove_host(host, dead);
> +
> + reset_control_assert(tegra->rst);
> + usleep_range(2000, 4000);
> + clk_disable_unprepare(platform->clk);
> +
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> static struct platform_driver sdhci_tegra_driver = {
> .driver = {
> .name = "sdhci-tegra",
> @@ -511,7 +547,7 @@ static struct platform_driver sdhci_tegra_driver = {
> .pm = &sdhci_pltfm_pmops,
> },
> .probe = sdhci_tegra_probe,
> - .remove = sdhci_pltfm_unregister,
> + .remove = sdhci_tegra_remove,
> };
>
> module_platform_driver(sdhci_tegra_driver);
>
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Linus Walleij @ 2016-11-22 12:30 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ritesh Harjani, Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Alex Lemberg, mateusz.nowak,
Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng, Asutosh Das,
Zhangfei Gao, kdorfman, David Griego, Sahitya Tummala, venkatg,
Shawn Lin, Subhash Jadavani
In-Reply-To: <0cf555d9-4e79-73fb-954d-8f022c90d4dc@intel.com>
On Tue, Nov 22, 2016 at 11:31 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 22/11/16 12:20, Linus Walleij wrote:
>> On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 21/11/16 17:34, Linus Walleij wrote:
>>>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>>>
>>>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>>>
>>>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>>>> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
>>>>> command queue feature for such type of cards.
>>>>>
>>>>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>>> [subhashj@codeaurora.org: fixed trivial merge conflicts]
>>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>
>>>> Even if we don't merge the specific mechanism provided by the
>>>> rest of the patches, this patch just make us know more about
>>>> the capabilities of the hardware we're running on, which is good.
>>>
>>> I think SW CMDQ is a better starting point:
>>
>> When I supplied the Reviewe-by, as stated below it I am not talking
>> about the patch set as a whole. I am talking about *this* patch.
>
> I should have been more explicit. SW CMDQ covers some of the same ground
> and has basically the same patch:
>
> https://marc.info/?l=linux-mmc&m=147729859922297&w=2
OK it seems to define more of the constants, I'll go and review it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Linus Walleij @ 2016-11-22 12:37 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ritesh Harjani, Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Alex Lemberg, mateusz.nowak,
Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng, Asutosh Das,
Zhangfei Gao, kdorfman, David Griego, Sahitya Tummala, venkatg,
Shawn Lin, Subhash Jadavani
In-Reply-To: <CACRpkdZkYVJmZ_M3oYhY3i4JrqEA_Xxc3ZALKaMdQq2ybxfvZA@mail.gmail.com>
On Tue, Nov 22, 2016 at 1:30 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Nov 22, 2016 at 11:31 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> I should have been more explicit. SW CMDQ covers some of the same ground
>> and has basically the same patch:
>>
>> https://marc.info/?l=linux-mmc&m=147729859922297&w=2
>
> OK it seems to define more of the constants, I'll go and review it.
Bah it's not in my INBOX for some reason, maybe I fell off the linux-mmc list
at some point :( I review it here:
Leave out this:
+ cmdq_en Command Queue enabled: 1 => enabled, 0 => not enabled
(...)
+MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
(...)
+ &dev_attr_cmdq_en.attr,
(...)
+ bool cmdq_en; /* Command Queue enabled */
Because that is about enabling the *use* of this mechnism rather than
detecting its presence.
With the above removed it is superior to $SUBJECT so you
can add:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Linus Walleij @ 2016-11-22 12:49 UTC (permalink / raw)
To: Ulf Hansson
Cc: Jaehoon Chung, Alex Lemberg, Arnd Bergmann,
linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang,
Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <CAPDyKFqVJeurr2jZcKRMxbvEuJfttnnFz_JWjqoCvP584h_byg@mail.gmail.com>
On Tue, Nov 22, 2016 at 9:49 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 22 November 2016 at 04:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Correct, in general there is no value in using packed for Read.
>>> But I can’t say this for all existing flash management solution.
>>> The eMMC spec allows to use it for Read as well.
>>
>> As i know, when packed command had implemented, early eMMC had the firmware problem
>> for Packed Read operation. but so I can't say Packed Read doesn't have the benefit for performance.
>> But Packed Write command can see the benefit for performance.
>
> Regarding "performance", are you merely thinking about increased
> throughput? With packed command we decrease the communication overhead
> with the card so less commands becomes sent/received.
>
> Or, did you also observed an improved behaviour of the card from a
> garbage collect point of view? In other words also a decreased latency
> when the device is becoming more and more used?
>
> Finally, did you compare the packed command, towards using the
> asynchronous request mechanisms (using the ->pre|post_req() mmc host
> ops)?
Packed write (and read, if we had implemented it) can only happen when
we get a number of requests to different areas of the card.
If they are to consecutive sectors (such as with a dd-test) it will not
improve performance, as that case is already optimized by the block
layer by front-merging of the requests into large chunks, I guess?
Indeed commit ce39f9d17c14 mentions improvement on lmdd but not
how it was invoked. I suspect it was invoked with random writes...
It is important to do everything we can to improve random small writes
though, and it seems packed command can do that.
(...)
>> Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
>> (For Android/Tizen OS and ARTIK boards)
>
> Thanks for sharing this information!
>
> It seems like we need to run another round of performance
> measurements, as to get some fresh number of the benefit of packed
> command.
> I would really appreciate if you could help out with that.
I would add: do it on the upstream kernel and submit the stuff needed
to make it work for you out-of-the box.
This is on Exynos I guess?
Can we have this flag set for the Exynos host controller, and/or
proper DT bindings to mark it as packed command enabled?
Hell I don't even know if this is a feature that needs anything special
from the host controller. Does it? Should we rather just enable it for all
host controllers if the card supports packed command? I'm lost.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Arnd Bergmann @ 2016-11-22 14:44 UTC (permalink / raw)
To: Ulf Hansson
Cc: Jaehoon Chung, Alex Lemberg, Linus Walleij,
linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang,
Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <CAPDyKFqVJeurr2jZcKRMxbvEuJfttnnFz_JWjqoCvP584h_byg@mail.gmail.com>
On Tuesday, November 22, 2016 9:49:26 AM CET Ulf Hansson wrote:
> On 22 November 2016 at 04:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> > On 11/21/2016 11:23 PM, Alex Lemberg wrote:
> >> On 11/21/16, 1:11 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
> >>> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
> >>>
> >>> Packed reads don't make a lot of sense, there is very little
> >>> for an MMC to optimize in reads that it can't already do without
> >>> the packing. For writes, packing makes could be an important
> >>> performance optimization, if the eMMC supports it.
> >>>
> >>> I've added Luca Porzio and Alex Lemberg to Cc. I think they
> >>> are subscribed to the list already, but it would be good to
> >>> get some estimate from them too about how common the packed
> >>> write support is on existing hardware from their respective
> >>> employers before we kill it off.
> >>
> >> Correct, in general there is no value in using packed for Read.
> >> But I can’t say this for all existing flash management solution.
> >> The eMMC spec allows to use it for Read as well.
> >
> > As i know, when packed command had implemented, early eMMC had the firmware problem
> > for Packed Read operation. but so I can't say Packed Read doesn't have the benefit for performance.
> > But Packed Write command can see the benefit for performance.
>
> Regarding "performance", are you merely thinking about increased
> throughput? With packed command we decrease the communication overhead
> with the card so less commands becomes sent/received.
>
> Or, did you also observed an improved behaviour of the card from a
> garbage collect point of view? In other words also a decreased latency
> when the device is becoming more and more used?
>
> Finally, did you compare the packed command, towards using the
> asynchronous request mechanisms (using the ->pre|post_req() mmc host
> ops)?
The main point of command packing is that the device can be smarter
about garbage collection as well as combine sub-page sized writes.
The communication overhead is nearly irrelevant in comparison,
and we would probably not have done anything for that.
> >> As far as I can say from reviewing the mobile (Android)
> >> platforms kernel source (from different vendors),
> >> many of them are enabling Packed Commands.
> >
> > Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
> > (For Android/Tizen OS and ARTIK boards)
>
> Thanks for sharing this information!
>
> It seems like we need to run another round of performance
> measurements, as to get some fresh number of the benefit of packed
> command.
> I would really appreciate if you could help out with that.
As far as I'm concerned, there are already two conclusions and
I don't think those measurements would help much:
- It's a problem that none of our upstream drivers support this
feature, and we really want them to do so, at least after the
blk_mq change.
- Linus' analysis is still valid: there is no regression in removing
it from the traditional blk code today, anyone who has a private
driver that uses it can simply revert the removal in their next
private product release.
If removing it helps us enable blk_mq support more easily, then
I think we can take out the packed command handling, but we have
to be prepared to put it back later on top of blk_mq.
Arnd
^ permalink raw reply
* [PATCH] mmc: sdhci-msm: Add sdhci_reset() implementation
From: Georgi Djakov @ 2016-11-22 15:50 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson
Cc: linux-mmc, linux-kernel, linux-arm-msm, riteshh, georgi.djakov
On apq8016, apq8084 and apq8074 platforms, when we want to do a
software reset, we need to poke some additional vendor specific
registers. If we don't do so, the following error message appears:
mmc0: Reset 0x1 never completed.
sdhci: =========== REGISTER DUMP (mmc0)===========
sdhci: Sys addr: 0x00000000 | Version: 0x00002e02
sdhci: Blk size: 0x00004000 | Blk cnt: 0x00000000
sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
sdhci: Present: 0x01f80000 | Host ctl: 0x00000000
sdhci: Power: 0x00000000 | Blk gap: 0x00000000
sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
sdhci: Timeout: 0x00000000 | Int stat: 0x00000000
sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
sdhci: Caps: 0x322dc8b2 | Caps_1: 0x00008007
sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
sdhci: Host ctl2: 0x00000000
sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x0000000000000000
sdhci: ===========================================
Fix it by implementing the custom sdhci_reset() function,
which does what is needed.
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
drivers/mmc/host/sdhci-msm.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a2fd9..87a124a37408 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -505,6 +505,23 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
return IRQ_HANDLED;
}
+void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+
+ if (mask & SDHCI_RESET_ALL) {
+ u32 val = readl_relaxed(msm_host->core_mem + CORE_POWER);
+
+ val |= CORE_SW_RST;
+ writel_relaxed(val, msm_host->core_mem + CORE_POWER);
+
+ sdhci_msm_voltage_switch(host);
+ }
+
+ sdhci_reset(host, mask);
+}
+
static const struct of_device_id sdhci_msm_dt_match[] = {
{ .compatible = "qcom,sdhci-msm-v4" },
{},
@@ -514,7 +531,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
static const struct sdhci_ops sdhci_msm_ops = {
.platform_execute_tuning = sdhci_msm_execute_tuning,
- .reset = sdhci_reset,
+ .reset = sdhci_msm_reset,
.set_clock = sdhci_set_clock,
.set_bus_width = sdhci_set_bus_width,
.set_uhs_signaling = sdhci_msm_set_uhs_signaling,
^ permalink raw reply related
* Re: [PATCH] mmc: block: delete packed command support
From: Ulf Hansson @ 2016-11-22 16:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jaehoon Chung, Alex Lemberg, Linus Walleij,
linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang,
Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <44199908.WIlGo4b061@wuerfel>
[...]
>>
>> Regarding "performance", are you merely thinking about increased
>> throughput? With packed command we decrease the communication overhead
>> with the card so less commands becomes sent/received.
>>
>> Or, did you also observed an improved behaviour of the card from a
>> garbage collect point of view? In other words also a decreased latency
>> when the device is becoming more and more used?
>>
>> Finally, did you compare the packed command, towards using the
>> asynchronous request mechanisms (using the ->pre|post_req() mmc host
>> ops)?
>
> The main point of command packing is that the device can be smarter
> about garbage collection as well as combine sub-page sized writes.
Ideally, yes! But I am not sure that is the case. Vendors would have to confirm.
By following the evolution of development of the eMMC spec one could
make some guesses. Let me try it :-).
In the eMMC 4.5 spec, "data tag", "context ID" and "packed command"
was added. It seems to me that "data tag" and "context ID" was
intended to help the device to be smarter about garbage collection,
while "packed command" is about reducing overhead.
The below is quoted from the packed command section in the spec:
"Read and write commands can be packed in groups of commands (either
all read or all write) that transfer the data for all commands in the
group in one transfer on the bus, to reduce overheads."
>
> The communication overhead is nearly irrelevant in comparison,
> and we would probably not have done anything for that.
I think we did.
And that's particularly why I am interested to see a fresh comparison
against the async request transfer mechanism.
>
>> >> As far as I can say from reviewing the mobile (Android)
>> >> platforms kernel source (from different vendors),
>> >> many of them are enabling Packed Commands.
>> >
>> > Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
>> > (For Android/Tizen OS and ARTIK boards)
>>
>> Thanks for sharing this information!
>>
>> It seems like we need to run another round of performance
>> measurements, as to get some fresh number of the benefit of packed
>> command.
>> I would really appreciate if you could help out with that.
>
> As far as I'm concerned, there are already two conclusions and
> I don't think those measurements would help much:
>
> - It's a problem that none of our upstream drivers support this
> feature, and we really want them to do so, at least after the
> blk_mq change.
>
> - Linus' analysis is still valid: there is no regression in removing
> it from the traditional blk code today, anyone who has a private
> driver that uses it can simply revert the removal in their next
> private product release.
>
> If removing it helps us enable blk_mq support more easily, then
> I think we can take out the packed command handling, but we have
> to be prepared to put it back later on top of blk_mq.
>
Thanks for the summary. This do seems like a nice approach.
Let's see what other people thinks about this.
Kind regards
Uffe
^ permalink raw reply
* Re: [GIT PULL] DWMMC controller for next
From: Ulf Hansson @ 2016-11-22 16:13 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <63a0b568-cd9e-d038-b7eb-08b7868f96f2@samsung.com>
On 22 November 2016 at 10:59, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Dear Ulf,
>
> Could you pull these patches on your next branch?
> If there is a problem, let me know, plz.
>
> The following changes since commit e2081b37d910c5e6c6925d9b4d0a7a6283f84ec5:
>
> Merge branch 'fixes' into next (2016-11-21 11:09:18 +0100)
>
> are available in the git repository at:
>
> https://github.com/jh80chung/dw-mmc.git for-ulf
>
> for you to fetch changes up to e25fd245b557482a8e0f7ab87841085f30706f3a:
>
> Documentation: synopsys-dw-mshc: remove the unused properties (2016-11-22 10:34:05 +0900)
>
> ----------------------------------------------------------------
> Colin Ian King (1):
> mmc: dw_mmc: fix spelling mistake in dev_dbg message
>
> Jaehoon Chung (9):
> mmc: dw_mmc: display the real register value on debugfs
> mmc: dw_mmc: fix the debug message for checking card's present
> mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
> mmc: dw_mmc: use the hold register when send stop command
> mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
> mmc: dw_mmc: use the cookie's enum values for post/pre_req()
> mmc: dw_mmc: remove the unnecessary mmc_data structure
> mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
> Documentation: synopsys-dw-mshc: remove the unused properties
>
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 ++------
> drivers/mmc/host/dw_mmc.c | 92 ++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
> include/linux/mmc/dw_mmc.h | 6 ++++++
> 3 files changed, 52 insertions(+), 54 deletions(-)
>
> Best Regards,
> Jaehoon Chung
Dear Jaehoon, thanks for the pull request. I have add queued it all up
for my next branch!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host
From: Zach Brown @ 2016-11-22 16:37 UTC (permalink / raw)
To: Ulf Hansson
Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAPDyKFrbfiTuU6zvQ8RR9pcO+x+9AZUejHrw396+Xsn2gCMsGA@mail.gmail.com>
On Tue, Nov 22, 2016 at 09:27:29AM +0100, Ulf Hansson wrote:
>
> Please try to not forget to bump the version number and to provide a
> history of the what changes between revisions. It makes life easier
> when reviewing and when I am about to apply patches.
>
Sorry, I'll make sure to include a history.
When switching from RFC to PATCH should the version number of the PATCH
be 1 or the lastest RFC version plus 1? Or does it not matter either way
as long as the history is present?
--
Zach
^ permalink raw reply
* Re: [PATCH 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host
From: Ulf Hansson @ 2016-11-22 17:30 UTC (permalink / raw)
To: Zach Brown
Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20161122163739.GA10111@zach-desktop>
On 22 November 2016 at 17:37, Zach Brown <zach.brown@ni.com> wrote:
> On Tue, Nov 22, 2016 at 09:27:29AM +0100, Ulf Hansson wrote:
>>
>> Please try to not forget to bump the version number and to provide a
>> history of the what changes between revisions. It makes life easier
>> when reviewing and when I am about to apply patches.
>>
>
> Sorry, I'll make sure to include a history.
> When switching from RFC to PATCH should the version number of the PATCH
> be 1 or the lastest RFC version plus 1? Or does it not matter either way
RFC + 1, that would be best. Although it doesn't matter that much.
> as long as the history is present?
Yes, this is the most important part.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-11-22 17:23 UTC (permalink / raw)
To: Rob Herring
Cc: Ziji Hu, Ulf Hansson, Adrian Hunter, linux-mmc, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, devicetree, Thomas Petazzoni,
linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
Wei(SOCP) Liu, Wilson Ding, Xueping Liu
In-Reply-To: <15b06a12-ed69-03a7-ccc7-0c133ce1ac1e@marvell.com>
Hi Rob,
On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
[...]
>>> +
>>> +- reg:
>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>> +
>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>> + PHY PAD Voltage Control register.
>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>> + in below.
>>> + Please also check property marvell,pad-type in below.
>>> +
>>> +Optional Properties:
>>> +- marvell,xenon-slotno:
>>
>> Multiple slots should be represented as child nodes IMO. I think some
>> other bindings already do this.
>>
>
> All the slots are entirely independent.
> I prefer to consider it as multiple independent SDHCs placed in
> a single IP, instead of that a IP contains multiple child slots.
It was indeed what I tried to show in my answer for the 1st version:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
Maybe you missed it.
You also mentioned other bindings using child nodes, but for this one
we have one controller with only one set of register with multiple slots
(Atmel is an example). Here each slot have it own set of register.
Actually giving the fact that each slot is controlled by a different set
of register I wonder why the hardware can't also deduce the slot number
from the address register. For me it looks like an hardware bug but we
have to deal with it.
Do you still think we needchild node here?
>
> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
> In my very own opinion, it is inconvenient and unnecessary.
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 7/7] mmc: core: Update CMD13 polling policy when switch to HS DDR mode
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
According to the JEDEC specification, during bus timing change operations
for mmc, sending a CMD13 could trigger CRC errors.
As switching to HS DDR mode indeed causes a bus timing change, polling with
CMD13 to detect card busy, may thus potentially trigger CRC errors.
Currently these errors are treated as the switch to HS DDR mode failed.
To improve this behaviour, let's instead tell __mmc_switch() to retry when
it encounters CRC errors during polling.
Moreover, when switching to HS DDR mode, let's make sure the CMD13 polling
is done by having the mmc host and the mmc card, being configured to
operate at the same selected bus speed timing. Fix this by providing
MMC_TIMING_MMC_DDR52 as the timing parameter to __mmc_switch().
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 15dd51c..3268fcd 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1040,10 +1040,12 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
ext_csd_bits = (bus_width == MMC_BUS_WIDTH_8) ?
EXT_CSD_DDR_BUS_WIDTH_8 : EXT_CSD_DDR_BUS_WIDTH_4;
- err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BUS_WIDTH,
- ext_csd_bits,
- card->ext_csd.generic_cmd6_time);
+ err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BUS_WIDTH,
+ ext_csd_bits,
+ card->ext_csd.generic_cmd6_time,
+ MMC_TIMING_MMC_DDR52,
+ true, true, true);
if (err) {
pr_err("%s: switch to bus width %d ddr failed\n",
mmc_hostname(host), 1 << bus_width);
@@ -1086,9 +1088,6 @@ static int mmc_select_hs_ddr(struct mmc_card *card)
if (err)
err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330);
- if (!err)
- mmc_set_timing(host, MMC_TIMING_MMC_DDR52);
-
return err;
}
--
1.9.1
^ permalink raw reply related
* [PATCH v2 3/7] mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
The ignore_crc parameter/variable name is used at a couple of places in the
mmc core. Let's rename it to retry_crc_err to reflect its new purpose.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc_ops.c | 10 +++++-----
drivers/mmc/core/mmc_ops.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0515748..214e734 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -461,7 +461,7 @@ int mmc_switch_status(struct mmc_card *card)
}
static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
- bool send_status, bool ignore_crc)
+ bool send_status, bool retry_crc_err)
{
struct mmc_host *host = card->host;
int err;
@@ -496,7 +496,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
busy = host->ops->card_busy(host);
} else {
err = mmc_send_status(card, &status);
- if (ignore_crc && err == -EILSEQ)
+ if (retry_crc_err && err == -EILSEQ)
busy = true;
else if (err)
return err;
@@ -528,13 +528,13 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
* timeout of zero implies maximum possible timeout
* @use_busy_signal: use the busy signal as response type
* @send_status: send status cmd to poll for busy
- * @ignore_crc: ignore CRC errors when sending status cmd to poll for busy
+ * @retry_crc_err: retry when CRC errors when polling with CMD13 for busy
*
* Modifies the EXT_CSD register for selected card.
*/
int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
unsigned int timeout_ms, bool use_busy_signal, bool send_status,
- bool ignore_crc)
+ bool retry_crc_err)
{
struct mmc_host *host = card->host;
int err;
@@ -590,7 +590,7 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
}
/* Let's try to poll to find out when the command is completed. */
- err = mmc_poll_for_busy(card, timeout_ms, send_status, ignore_crc);
+ err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err);
out:
mmc_retune_release(host);
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 7f6c0e9..9fccddb 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -30,7 +30,7 @@
int mmc_switch_status(struct mmc_card *card);
int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
unsigned int timeout_ms, bool use_busy_signal, bool send_status,
- bool ignore_crc);
+ bool retry_crc_err);
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
Changes in v2:
- From discussions around the difficulties for how to support CMD13
polling for HS200, HS400 HS400ES, I decided to drop those patches for
now. It's is particularly due to the need for tuning, after a speed mode
switch, that makes it hard to rely on CMD13 polling for these cases.
Perhaps we can allow CMD13 polling when switching to the intermediate
speed modes, but let's deal with that outside of this series.
- Folded in a new patch, which checks the SWITCH_ERROR bit in each
CMD13 response when polling with CMD13. Patch v2 4/7.
- No other changes made to the rest of the series.
Several changes has been made for how and when to use CMD13 as a
polling-method, to find out when the mmc cards stops signaling busy after
sending a CMD6. This particularly concerns the cases when switching to new bus
speed modes, such as HS (high-speed), HS DDR, HS200, HS400 and HS400ES.
Currently CMD13 polling is *not* allowed for these cases, but this was recently
changed - and which did cause regressions for card detection for some
platforms.
A simple fix was made to solve these regressions, simply by using a default
CMD6 generic timeout of 500ms, as to avoid using the fall-back timeout in
__mmc_switch() of 10min. That greatly improve the situation and one could
consider the regressions as "solved".
However, this simple fix is still causing unnecessary long card initialization
times, especially for those mmc hosts that doesn't support HW busy detection
(using MMC_CAP_WAIT_WHILE_BUSY and/or implements the ->card_busy() host ops).
This because we wait a fixed amount of time (CMD6 generic timeout) to make sure
the card is done signaling busy, while in practice this happens a lot sooner.
A couple of tests for HS and HS DDR eMMC cards, shows the card only need a few
ms to complete the bus speed mode changes, thus it's far less than the CMD6
generic timeout.
To improve this behaviour and shorten the card initialization time, we need to
allow using CMD13 as polling method to find out when the card stops signaling
busy. Although, enabling CMD13 polling may also introduce other issues as
according to the JEDEC spec, it's not the recommended method. Especially it
mentions that CRC errors may be triggered when sending a CMD13 command during a
bus timing change.
To deal with these issues, we need to change from ignoring those CRC errors but
instead continue to treat the card as being busy and continue to poll with
CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
method didn't work out well.
This series requires extensive testing, please help with that!
I have currently tested it with HS and HS DDR eMMC cards, and for combinations
of an mmc hosts (mmci) supporting HW busy detection and not (local hacks in
mmci.c).
Ulf Hansson (9):
mmc: core: Retry instead of ignore at CRC errors when polling for busy
mmc: core: Remove redundant __mmc_send_status()
mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
mmc: core: Enable __mmc_switch() to change bus speed timing for the
host
mmc: core: Allow CMD13 polling when switching to HS mode for mmc
mmc: core: Update CMD13 polling policy when switch to HS DDR mode
mmc: core: Allow CMD13 polling when switch to HS200 mode
mmc: core: Allow CMD13 polling when switch to HS400 mode
mmc: core: Allow CMD13 polling when switch to HS400ES mode
drivers/mmc/core/core.c | 2 +-
drivers/mmc/core/mmc.c | 156 ++++++++++++++-------------------------------
drivers/mmc/core/mmc_ops.c | 45 +++++++------
drivers/mmc/core/mmc_ops.h | 4 +-
4 files changed, 77 insertions(+), 130 deletions(-)
--
1.9.1
*** BLURB HERE ***
Ulf Hansson (7):
mmc: core: Retry instead of ignore at CRC errors when polling for busy
mmc: core: Remove redundant __mmc_send_status()
mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
mmc: core: Check SWITCH_ERROR bit from each CMD13 response when
polling
mmc: core: Enable __mmc_switch() to change bus speed timing for the
host
mmc: core: Allow CMD13 polling when switching to HS mode for mmc
mmc: core: Update CMD13 polling policy when switch to HS DDR mode
drivers/mmc/core/core.c | 2 +-
drivers/mmc/core/mmc.c | 53 +++++++++++++++++++++++-----------------------
drivers/mmc/core/mmc_ops.c | 51 ++++++++++++++++++++++++++------------------
drivers/mmc/core/mmc_ops.h | 4 ++--
4 files changed, 60 insertions(+), 50 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH v2 5/7] mmc: core: Enable __mmc_switch() to change bus speed timing for the host
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
In cases when a speed mode change is requested for mmc cards, a CMD6 is
sent by calling __mmc_switch() during the card initialization. The CMD6
leads to the card entering a busy period. When that is completed, the host
must parse the CMD6 status to find out whether the change of the speed mode
succeeded.
To enable the mmc core to poll the card by using CMD13 to find out when the
busy period is completed, it's reasonable to make sure polling is done by
having the mmc host and the mmc card, being configured to operate at the
same selected bus speed timing.
Therefore, let's extend __mmc_switch() to take yet another parameter, which
allow its callers to update the bus speed timing of the mmc host. In this
way, __mmc_switch() also becomes capable of reading and validating the CMD6
status by sending a CMD13, in cases when that's desired.
If __mmc_switch() encounters a failure, we make sure to restores the old
bus speed timing for the mmc host, before propagating the error code.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 2 +-
drivers/mmc/core/mmc.c | 18 +++++++++---------
drivers/mmc/core/mmc_ops.c | 20 +++++++++++++++-----
drivers/mmc/core/mmc_ops.h | 4 ++--
4 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 50bb9a1..060f767 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -380,7 +380,7 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
mmc_retune_hold(card->host);
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BKOPS_START, 1, timeout,
+ EXT_CSD_BKOPS_START, 1, timeout, 0,
use_busy_signal, true, false);
if (err) {
pr_warn("%s: Error %d starting bkops\n",
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 9355366..cb1cf4e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1012,7 +1012,7 @@ static int mmc_select_hs(struct mmc_card *card)
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
- card->ext_csd.generic_cmd6_time,
+ card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (!err) {
mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
@@ -1115,7 +1115,7 @@ static int mmc_select_hs400(struct mmc_card *card)
val = EXT_CSD_TIMING_HS;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, val,
- card->ext_csd.generic_cmd6_time,
+ card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err) {
pr_err("%s: switch to high-speed from hs200 failed, err:%d\n",
@@ -1150,7 +1150,7 @@ static int mmc_select_hs400(struct mmc_card *card)
card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, val,
- card->ext_csd.generic_cmd6_time,
+ card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err) {
pr_err("%s: switch to hs400 failed, err:%d\n",
@@ -1193,7 +1193,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
/* Switch HS400 to HS DDR */
val = EXT_CSD_TIMING_HS;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
- val, card->ext_csd.generic_cmd6_time,
+ val, card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err)
goto out_err;
@@ -1207,7 +1207,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
/* Switch HS DDR to HS */
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BUS_WIDTH,
EXT_CSD_BUS_WIDTH_8, card->ext_csd.generic_cmd6_time,
- true, false, true);
+ 0, true, false, true);
if (err)
goto out_err;
@@ -1221,7 +1221,7 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
val = EXT_CSD_TIMING_HS200 |
card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING,
- val, card->ext_csd.generic_cmd6_time,
+ val, card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err)
goto out_err;
@@ -1295,7 +1295,7 @@ static int mmc_select_hs400es(struct mmc_card *card)
card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, val,
- card->ext_csd.generic_cmd6_time,
+ card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err) {
pr_err("%s: switch to hs400es failed, err:%d\n",
@@ -1377,7 +1377,7 @@ static int mmc_select_hs200(struct mmc_card *card)
card->drive_strength << EXT_CSD_DRV_STR_SHIFT;
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, val,
- card->ext_csd.generic_cmd6_time,
+ card->ext_csd.generic_cmd6_time, 0,
true, false, true);
if (err)
goto err;
@@ -1841,7 +1841,7 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_POWER_OFF_NOTIFICATION,
- notify_type, timeout, true, false, false);
+ notify_type, timeout, 0, true, false, false);
if (err)
pr_err("%s: Power Off Notification timed out, %u\n",
mmc_hostname(card->host), timeout);
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index fba5d29..9b2617c 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -530,6 +530,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
* @value: value to program into EXT_CSD register
* @timeout_ms: timeout (ms) for operation performed by register write,
* timeout of zero implies maximum possible timeout
+ * @timing: new timing to change to
* @use_busy_signal: use the busy signal as response type
* @send_status: send status cmd to poll for busy
* @retry_crc_err: retry when CRC errors when polling with CMD13 for busy
@@ -537,13 +538,14 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
* Modifies the EXT_CSD register for selected card.
*/
int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
- unsigned int timeout_ms, bool use_busy_signal, bool send_status,
- bool retry_crc_err)
+ unsigned int timeout_ms, unsigned char timing,
+ bool use_busy_signal, bool send_status, bool retry_crc_err)
{
struct mmc_host *host = card->host;
int err;
struct mmc_command cmd = {0};
bool use_r1b_resp = use_busy_signal;
+ unsigned char old_timing = host->ios.timing;
mmc_retune_hold(host);
@@ -585,16 +587,24 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
if (!use_busy_signal)
goto out;
+ /* Switch to new timing before poll and check switch status. */
+ if (timing)
+ mmc_set_timing(host, timing);
+
/*If SPI or used HW busy detection above, then we don't need to poll. */
if (((host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) ||
mmc_host_is_spi(host)) {
if (send_status)
err = mmc_switch_status(card);
- goto out;
+ goto out_tim;
}
/* Let's try to poll to find out when the command is completed. */
err = mmc_poll_for_busy(card, timeout_ms, send_status, retry_crc_err);
+
+out_tim:
+ if (err && timing)
+ mmc_set_timing(host, old_timing);
out:
mmc_retune_release(host);
@@ -604,8 +614,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
int mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
unsigned int timeout_ms)
{
- return __mmc_switch(card, set, index, value, timeout_ms, true, true,
- false);
+ return __mmc_switch(card, set, index, value, timeout_ms, 0,
+ true, true, false);
}
EXPORT_SYMBOL_GPL(mmc_switch);
diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h
index 9fccddb..761cb69 100644
--- a/drivers/mmc/core/mmc_ops.h
+++ b/drivers/mmc/core/mmc_ops.h
@@ -29,8 +29,8 @@
int mmc_can_ext_csd(struct mmc_card *card);
int mmc_switch_status(struct mmc_card *card);
int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
- unsigned int timeout_ms, bool use_busy_signal, bool send_status,
- bool retry_crc_err);
+ unsigned int timeout_ms, unsigned char timing,
+ bool use_busy_signal, bool send_status, bool retry_crc_err);
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v2 6/7] mmc: core: Allow CMD13 polling when switching to HS mode for mmc
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
In cases when the mmc host doesn't support HW busy detection, polling for a
card being busy by using CMD13 is beneficial. That is because, instead of
waiting a fixed amount of time, 500ms or the generic CMD6 time from
EXT_CSD, we find out a lot sooner when the card stops signaling busy. This
leads to a significant decreased total initialization time for the mmc
card.
However, to allow polling with CMD13 during a bus timing change operation,
such as switching to HS mode, we first need to update the mmc host's bus
timing before starting to poll. Deal with that, simply by providing
MMC_TIMING_MMC_HS as the timing parameter to __mmc_switch() from
mmc_select_hs().
By telling __mmc_switch() to allow polling with CMD13, also makes it
validate the CMD6 status, thus we can remove the corresponding checks.
When switching to HS400ES, the mmc_select_hs() function is called in one of
the intermediate steps. To still prevent CMD13 polling for HS400ES, let's
call the __mmc_switch() function in this path as it enables us to keep
using the existing method.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index cb1cf4e..15dd51c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1012,13 +1012,8 @@ static int mmc_select_hs(struct mmc_card *card)
err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
- card->ext_csd.generic_cmd6_time, 0,
- true, false, true);
- if (!err) {
- mmc_set_timing(card->host, MMC_TIMING_MMC_HS);
- err = mmc_switch_status(card);
- }
-
+ card->ext_csd.generic_cmd6_time, MMC_TIMING_MMC_HS,
+ true, true, true);
if (err)
pr_warn("%s: switch to high-speed failed, err:%d\n",
mmc_hostname(card->host), err);
@@ -1268,16 +1263,23 @@ static int mmc_select_hs400es(struct mmc_card *card)
goto out_err;
/* Switch card to HS mode */
- err = mmc_select_hs(card);
- if (err)
+ err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS,
+ card->ext_csd.generic_cmd6_time, 0,
+ true, false, true);
+ if (err) {
+ pr_err("%s: switch to hs for hs400es failed, err:%d\n",
+ mmc_hostname(host), err);
goto out_err;
+ }
- mmc_set_clock(host, card->ext_csd.hs_max_dtr);
-
+ mmc_set_timing(host, MMC_TIMING_MMC_HS);
err = mmc_switch_status(card);
if (err)
goto out_err;
+ mmc_set_clock(host, card->ext_csd.hs_max_dtr);
+
/* Switch card to DDR with strobe bit */
val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE;
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/7] mmc: core: Retry instead of ignore at CRC errors when polling for busy
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
After a CMD6 command has been sent, the __mmc_switch() function might be
advised to poll the card for busy by using CMD13 and also by ignoring CRC
errors.
In the case of ignoring CRC errors, the mmc core tells the mmc host to also
ignore these errors via masking the MMC_RSP_CRC response flag. This seems
wrong, as it leads to that the mmc host could propagate an unreliable
response, instead of a proper error code.
What we really want, is not to ignore CRC errors but instead retry the
polling attempt. So, let's change this by treating a CRC error as the card
is still being busy and thus continue to run the polling loop.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc_ops.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 481bbdb..4773c56 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -503,10 +503,13 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
if (host->ops->card_busy) {
busy = host->ops->card_busy(host);
} else {
- err = __mmc_send_status(card, &status, ignore_crc);
- if (err)
+ err = mmc_send_status(card, &status);
+ if (ignore_crc && err == -EILSEQ)
+ busy = true;
+ else if (err)
return err;
- busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
+ else
+ busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
}
/* Timeout if the device still remains busy. */
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/7] mmc: core: Remove redundant __mmc_send_status()
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
There are only one users left which calls __mmc_send_status(). Moreover,
the ignore_crc parameter isn't being used, so let's just remove these
redundant parts.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/mmc_ops.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 4773c56..0515748 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -54,8 +54,7 @@
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
- bool ignore_crc)
+int mmc_send_status(struct mmc_card *card, u32 *status)
{
int err;
struct mmc_command cmd = {0};
@@ -67,8 +66,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
if (!mmc_host_is_spi(card->host))
cmd.arg = card->rca << 16;
cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
- if (ignore_crc)
- cmd.flags &= ~MMC_RSP_CRC;
err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
if (err)
@@ -83,11 +80,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
return 0;
}
-int mmc_send_status(struct mmc_card *card, u32 *status)
-{
- return __mmc_send_status(card, status, false);
-}
-
static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
struct mmc_command cmd = {0};
--
1.9.1
^ permalink raw reply related
* [PATCH v2 4/7] mmc: core: Check SWITCH_ERROR bit from each CMD13 response when polling
From: Ulf Hansson @ 2016-11-22 20:56 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
According to the JEDEC specification, the SWITCH_ERROR bit in the device
status from a R1 response, is an error bit which may be cleared as soon as
the response that reports the error is sent.
When polling with CMD13 to find out when the card stops signaling busy
after a CMD6 has been sent, we currently parse only the last CMD13 response
for the SWITCH_ERROR bit. Consequentially we could loose important
information about the card.
In worst case if the card stops signaling busy within the allowed timeout,
we could end up believing that the CMD6 command completed successfully,
when in fact it didn't.
To improve the behaviour, let's parse each CMD13 response to see if the
SWITCH_ERROR bit is set in the device status. In such case, we abort the
polling loop and report the error.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
Changes in v2:
- New patch.
---
drivers/mmc/core/mmc_ops.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 214e734..fba5d29 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -496,12 +496,16 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
busy = host->ops->card_busy(host);
} else {
err = mmc_send_status(card, &status);
- if (retry_crc_err && err == -EILSEQ)
+ if (retry_crc_err && err == -EILSEQ) {
busy = true;
- else if (err)
+ } else if (err) {
return err;
- else
+ } else {
+ err = mmc_switch_status_error(host, status);
+ if (err)
+ return err;
busy = R1_CURRENT_STATE(status) == R1_STATE_PRG;
+ }
}
/* Timeout if the device still remains busy. */
@@ -515,7 +519,7 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
if (host->ops->card_busy && send_status)
return mmc_switch_status(card);
- return mmc_switch_status_error(host, status);
+ return 0;
}
/**
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox