From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pantelis Antoniou Date: Tue, 1 Sep 2015 19:22:08 +0300 Subject: [U-Boot] [PATCH 1/2] mmc: dw_mmc: Increase timeout to 20 seconds In-Reply-To: <201508292119.11094.marex@denx.de> References: <1440769821-24005-1-git-send-email-l.majewski@samsung.com> <201508291552.10117.marex@denx.de> <20150829183848.53f7d79b@jawa> <201508292119.11094.marex@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Marek, > On Aug 29, 2015, at 22:19 , Marek Vasut wrote: > > On Saturday, August 29, 2015 at 06:38:48 PM, Lukasz Majewski wrote: >> Hi Marek, > > Hi Lukasz, > >>> On Saturday, August 29, 2015 at 01:55:36 PM, Lukasz Majewski wrote: >>>> On Fri, 28 Aug 2015 23:55:17 +0200 >>> >>> Hi! >>> >>>> Marek Vasut wrote: >>>>> On Friday, August 28, 2015 at 03:50:20 PM, Lukasz Majewski wrote: >>>>>> The commit: d9dbb97be0e4a550457aec5f11afefb446169c90 >>>>>> "mmc: dw_mmc: Zap endless timeout" removed endless loop waiting >>>>>> for end of dw mmc transfer. >>>>>> >>>>>> For some workloads - dfu test @ Odroid XU3 (sending 8MiB file) - >>>>>> and SD cards (e.g. MicroSD Kingston 4GiB, Adata 4GiB) >>>>>> the default timeout is to short. >>>>>> >>>>>> The new value - 20 seconds - takes into account the situation >>>>>> when SD card triggers internal clean up. Such process may take >>>>>> more than 10 seconds on some cards. >>>>> >>>>> What happens if you pull the SD card out of the slot during such a >>>>> process? >>>> >>>> Then we would wait 20 seconds :-) to proceed. >>> >>> Oops, I think I was not clear here. I was wondering what happens to >>> the card if you yank it out of the slot whole it's performing it's >>> internal cleanup or whatever. Is it possible that the card suffers >>> data corruption, effectively trashing user data ? >> >> I think that only the card manufacturer may reveal what can happen with >> the card (what policy have been implemented in their FW). >> >> I guess that you may lost data in such case. > > Uuuurgh, that's seriously shitty. > Welcome to the world of managed flash. Manufacturers tend not to follow the spec to the letter. What matters is standard consumer usage benchmarks. >>> Is this behavior >>> specific to Samsung SD cards ? >> >> I've experienced the problem with Kingston (brand new one) and Adata >> MicroSD HC (4GiB) cards. > > I had bad previous experience with Kingston too. > >>>> To be clear - the mentioned patch introduced regression. >>> >>> That's a bug, not a regression, but anyway, >>> that's not the point. I do >>> agree with you that we do have a problem and I'm inclined to Ack this >>> patch, I'd like to understand what the real implications of such a >>> behavior of these cards are. >>> >>>> It works for >>>> small files on a commonly available SD cards (like 4 GiB >>>> Kingston/Adata). >>>> >>>> When I ran DFU tests I've discovered that there is a problem with >>>> storing 8MiB file (dat_8M.img). >>>> >>>> Even worse - when one wants to store Image.itb file (which might be >>>> 4-6 MiB) it sometimes works and sometimes not. Nightmare for >>>> debugging. >>> >>> Ew, that's one crappy card you have there. I'm reading the SD card >>> "Physical Layer Simplified Specification Version 4.10" (part1_410.pdf) >>> section 4.6.2.2 and it states that for SDHC cards, the write operation >>> should take at most 250mS, for SDXC it's 500mS. Could it be that your >>> card is violating the spec ? >> >> I'll look into the spec and then comment :-). >> >> For my boards the time was 1.2 seconds for storing 8 MiB file. > > OK, but that's weird. The transfer should always be up to 512B long and not > any longer, unless it's a multiblock transfer. > >>>> Please correct me if I'm wrong - but is seems like we are now using >>>> 1 second timeout for detection if SD card has been removed? >>>> >>>> Shouldn't we use polling on the card detect IO pin instead? We >>>> could add such polling in several places in the MMC subsystem (like >>>> we do it with watchdog). >>>> >>>> Marek, Pantelis, what do you think about this? >>> >>> If you implement board_mmc_getcd(), you can check if the card is >>> present this way instead of waiting for command to time out. The >>> infrastructure for that is already in place. Right ? >> >> So you suggest adding board_mmc_getcd() in several places in the mmc >> subsystem driver to detect removal of the SD card? > > Hmmmm, I'm not sure about this one. Panto ? > I?m fine with this. Perhaps we can avoid spamming this all over the place, and that would be better. >>> It'd be cool if the MMC subsystem could pull the wp-gpios and cd-gpios >>> from DT though :) >> >> +1 >> Indeed. I would expect this to be a per-board thing. I would not expect all platforms to provide that capability. >>>>> Also, where did you find out there is such "cleanup" mechanism >>>>> please ? >>>> >>>> Internally we did some tests with several SD cards. We were stunned >>>> when it turned out that for some workloads it took up to 15 seconds >>>> to end write operation for small data. >>>> >>>> The culprit is the SD Card embedded controller responsible for FTL - >>>> flash translation layer. >>>> It allows NAND memory on the card to be visible as the block device. >>>> More importantly it also takes care of wear leveling and bad block >>>> management. >>>> >>>> Hence, we don't know when it would start housekeeping operations. >>>> We can only poll/wait until this controller finishes it work. >>>> The code as it was (with the indefinite loop) was taking this >>>> situation into account. >>>> >>>> The 1 second timeout is apparently too short and makes using SD card >>>> non-deterministic and error prone in u-boot. >>>> >>>> Even worse, many devices use SD card as the only storage device. >>> >>> Yes, horrible. >> >> Good that we have agreed. > > Heh :) > Horrible closed source flash management algorithm is horrible, film at 11 :) > Best regards, > Marek Vasut Regards ? Pantelis