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

[Chaotian Jing <chaotian.jing@mediatek.com>]

On Mon, 2016-10-31 at 15:09 +0200, Adrian Hunter wrote:
> On 27/10/16 13:04, Ulf Hansson wrote:
> > On 20 October 2016 at 09:06, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >> On 20 October 2016 at 04:22, Chaotian Jing <chaotian.jing@mediatek.com> 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@intel.com> wrote:
> >>>>> On 18/10/16 11:36, Linus Walleij wrote:
> >>>>>> On Mon, Oct 17, 2016 at 4:32 PM, Linus Walleij <linus.walleij@linaro.org> 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@samsung.com>
> > 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@samsung.com>
> >     Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> >     Signed-off-by: Chris Ball <cjb@laptop.org>
> > 
> > 
> > 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 agree: ignoring CRC errors and then expecting the status in the response
> to be correct doesn't make sense.
> 
> However, it raises the question of what to do if there are always CRC errors
> e.g. if it only works without CRC errors once the mode and frequency are
> changed in the host controller.
> 
> > 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?
> 
> Does Chaotian have a real problem since his driver has busy detection anyway?

In fact, I have not encounter CRC errors of CMD13, I have tried several
eMMC cards, after mode switch, CMD13 will only gets 0x800 response and
we don't know if card is busy by 0x800 response.