Re: Regression after "do not use CMD13 to get status after speed mode switch"

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

 



On 20 October 2016 at 09:06, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On 20 October 2016 at 04:22, Chaotian Jing <chaotian.jing@xxxxxxxxxxxx> wrote:
>> On Wed, 2016-10-19 at 18:41 +0200, Ulf Hansson wrote:
>>> Adrian, Linus,
>>>
>>> Thanks for looking into this and reporting!
>>>
>>> On 18 October 2016 at 15:23, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>> > On 18/10/16 11:36, Linus Walleij wrote:
>>> >> On Mon, Oct 17, 2016 at 4:32 PM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>>> >>
>>> >>> Before this patch the eMMC is detected and all partitions enumerated
>>> >>> immediately, but after the patch it doesn't come up at all, except
>>> >>> sometimes, when it appears minutes (!) after boot, all of a sudden.
>>> >>
>>> >> FYI this is what it looks like when it eventually happens:
>>> >> root@msm8660:/ [  627.710175] mmc0: new high speed MMC card at address 0001
>>> >> [  627.711641] mmcblk0: mmc0:0001 SEM04G 3.69 GiB
>>> >> [  627.715485] mmcblk0boot0: mmc0:0001 SEM04G partition 1 1.00 MiB
>>> >> [  627.736654] mmcblk0boot1: mmc0:0001 SEM04G partition 2 1.00 MiB
>>> >> [  627.747397] mmcblk0rpmb: mmc0:0001 SEM04G partition 3 128 KiB
>>> >> [  627.756326]  mmcblk0: p1 p2 p3 p4 < p5 p6 p7 p8 p9 p10 p11 p12 p13
>>> >> p14 p15 p16 p17 p18 p19 p20 p21 >
>>> >>
>>> >> So after 627 seconds, a bit hard for users to wait this long for their
>>> >> root filesystem.
>>> >
>>> > If the driver does not support busy detection and the eMMC card provides
>>> > zero as the cmd6 generic timeout (which it may especially as cmd6 generic
>>> > timeout wasn't added until eMMCv4.5), then __mmc_switch() defaults to
>>> > waiting 10 minutes i.e.
>>> >
>>> > #define MMC_OPS_TIMEOUT_MS      (10 * 60 * 1000) /* 10 minute timeout */
>>>
>>> Urgh! Yes, I have verified that this is exactly what happens.
>>>
>>> >
>>> > So removal of CMD13 polling for HS mode (as per commit
>>> > 08573eaf1a70104f83fdbee9b84e5be03480e9ed) is going to be a problem for some
>>> > combinations of eMMC cards and host drivers.
>>>
>>> I was looking in the __mmc_switch() function, it's just a pain to walk
>>> trough it :-) So first out I decided to clean it up and factor out the
>>> polling parts. I will post the patches first out tomorrow morning,
>>> running some final test right now.
>>>
>>> Although, that of course doesn't solve our problem. As I see it we
>>> only have a few options here.
>>>
>>> 1) In case when cmd6 generic timeout isn't available, let's assign
>>> another empirically selected value.
>>> 2) Use a specific timeout when switching to HS mode.
>>> 3) Even if we deploy 1 (and 2), perhaps we still should allow polling
>>> with CMD13 for switching to HS mode - unless it causes issues for some
>>> cards/drivers combination?
>>>
>>> BTW, I already tried 2) and it indeed solves the problem, although
>>> depending on the selected timeout, it might delay the card detection
>>> to process.
>>>
>>> Thoughts?
>>
>> I just have a try of switching to HS mode with Hynix EMMC, the first
>> CMD13 gets response of 0x900, but the EMMC is still pull-low DAT0. so
>> that CMD13 cannot indicate current card status in this case.
>
> Thanks for sharing that. Okay, so clearly we have some cards that
> don't supports polling with CMD13 when switching to HS mode.
> One could of course add quirks for these kind of cards and do a fixed
> delay for them, but then to find out which these cards are is going to
> be hard.
>
> It seems like we are left with using a fixed delay. Any ideas of what
> such delay should be? And should we have one specific for switch to
> the various speed modes and a different one that overrides the CMD6
> generic timout, when it doesn't exist?
>

Replying to my own earlier response, as I believe the problem could
also be related to another old commit, see below.

commit a27fbf2f067b0cd6f172c8b696b9a44c58bfaa7a
Author: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
Date:   Wed Sep 4 21:21:05 2013 +0900

    mmc: add ignorance case for CMD13 CRC error

    While speed mode is changed, CMD13 cannot be guaranteed.
    According to the spec., it is not recommended to use CMD13
    to check the busy completion of the timing change.
    If CMD13 is used in this case, CRC error must be ignored.

    Signed-off-by: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
    Acked-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
    Signed-off-by: Chris Ball <cjb@xxxxxxxxxx>


The intent with this commit was not really correct. We don't want to
ignore CRC errors, but instead we should *re-try* sending CMD13 once
we get a CRC error.

Unfortunate since this commit, instead we tell the host driver to
*ignore* CRC errors and instead reads the status and returns 0
(indicating success). In the mmc core, in __mmc_switch(), it will thus
parse the status reply, even for a reply that might have been received
with a CRC error. Not good!

I am wondering whether this actually is the main problem to why we
think polling isn't working for some cases. And perhaps that was the
original problem Chaotian was trying to solve?

Thoughts?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux