From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Mon, 28 Jan 2013 08:02:20 +0100 Subject: [U-Boot] [RFC] mmc:fix: Increase the timeout value for SDHCI_send_command() In-Reply-To: <510323EE.8070100@samsung.com> References: <1357665792-8141-1-git-send-email-l.majewski@samsung.com> <20130109201201.5F4E9202B9C@gemini.denx.de> <20130111161929.6ecd75e9@amdc308.digital.local> <510323EE.8070100@samsung.com> Message-ID: <20130128080220.4ff50ede@amdc308.digital.local> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Jaehoon, > On 01/25/2013 08:44 PM, Jagan Teki wrote: > > Hi, > > > > On Fri, Jan 11, 2013 at 8:49 PM, Lukasz Majewski > > wrote: > >> Hi Wolfgang, > >> > >>> Dear Lukasz Majewski, > >>> > >>> In message > >>> <1357665792-8141-1-git-send-email-l.majewski@samsung.com> you > >>> wrote: > >>>> I'd like to ask for your opinion about the following problem: > >>> > >>> I cannot comment on the problem - only a bit about the proposed > >>> patch ;-) > >>> > >>>> From a brief checking I can say that it happens when we are doing > >>>> consecutive MMC operations (i.e. many reads), and the 10ms > >>>> timeout might be too short when eMMC firmware is forced to do > >>>> some internal time consuming operations (e.g. flash blocks > >>>> management, wear leveling). > >>>> In this situation, the SDHCI_CMD_INHIBIT bit is set, which means > >>>> that SDHCI controller didn't received response from eMMC. > >>>> > >>>> One proposition would be to define the per device/per memory chip > >>>> specific timeouts, to replace those defined > >>>> at ./drivers/mmc/sdhci.c file. > >>> > >>> Is there no way to ask the device and/or controller when it is > >>> done, so we can poll for ready state instead of adding delays, > >>> which will always have to be tailored for the so far known worst > >>> case, i. e. they will be always too long on all almost all > >>> systems. > >> > >> We are doing this already - the SDHCI_PRESENT_STATE register's bit > >> 0 (SDHCI_CMD_INHIBIT) and bit 1 (DATA_INHIBIT) are for this > >> purpose. Those indicate when host controller can send further > >> command/data to the card. > >> > >> Moreover, there are also timeouts in the case when someone pull > >> out SD card inserted to the slot (or any other use case which I'm > >> not aware). > >> > >> > >>> > >>>> --- a/drivers/mmc/sdhci.c > >>>> +++ b/drivers/mmc/sdhci.c > >>>> @@ -137,8 +137,8 @@ int sdhci_send_command(struct mmc *mmc, > >>>> struct mmc_cmd *cmd, unsigned int timeout, start_addr = 0; > >>>> unsigned int retry = 10000; > >>>> > >>>> - /* Wait max 10 ms */ > >>>> - timeout = 10; > >>>> + /* Wait max 100 ms */ > >>>> + timeout = 100; > >>> > >>> We have cases where we struggle for sub-second boot times. Adding > >>> 100 ms delay here is clearly prohbitive. [Even the 10 ms are way > >>> too long IMHO.] There must be a better way to handle this. > >> > >> That's why I'm asking. > >> > >> It is strange that, when I'm increasing delay it works. > >> > >> Maybe we will find some areas of optimization? > > > > BTW: I am also finding the similar issue. > > > > But when I enabled CONFIG_MMC_TRACE for log traces, i never see the > > issue..it's pretty much working fine. > It's not important to enable the MMC_TRACE. It should be increased > the delay. You don't see problem, since CONFIG_MMC_TRACE causes extra delays to write log information to serial console. > > As per my latest debug, the issue is fire for CMD6 (SWITCH_FUNC). > Right, i also find the error log for CMD6. > Could you test this point? > > sdhci_writel(host, SDHCI_INT_ALL_MASK, SDHCI_INT_STATUS); > - mask = SDHCI_CMD_INHIBIT | SDHCI_DATA_INHIBIT; > + mask = SDHCI_CMD_INHIBIT; > + > + if ((data != NULL) || (cmd->resp_type & MMC_RSP_BUSY)) > + mask |= SDHCI_DATA_INHIBIT; I've tested it on trats, but mentioned errors also appear (with lower frequency though). > > Best Regards, > Jaehoon Chung > > > > May be we need to update the logic on this while loop... Yep. > > > > Thanks, > > Jagan. > > > >> > >>> > >>> Best regards, > >>> > >>> Wolfgang Denk > >>> > >> > >> > >> > >> -- > >> Best regards, > >> > >> Lukasz Majewski > >> > >> Samsung R&D Poland (SRPOL) | Linux Platform Group > >> _______________________________________________ > >> U-Boot mailing list > >> U-Boot at lists.denx.de > >> http://lists.denx.de/mailman/listinfo/u-boot > > _______________________________________________ > > U-Boot mailing list > > U-Boot at lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot > > > -- Best regards, Lukasz Majewski Samsung R&D Poland (SRPOL) | Linux Platform Group