Jaehoon, On Fri, Mar 13, 2015 at 4:19 AM, Jaehoon Chung <jh80.ch...@samsung.com> wrote: > Hi Doug. > > This patch is a right process. Just i wonder something. > > On 03/10/2015 08:18 AM, Doug Anderson wrote: >> In the Designware databook's description of the "Voltage Switch Normal >> Scenario" it instructs us to set a timer and fail the voltage change >> if we don't see the voltage change interrupt within 2ms. Let's >> implement that. Without implementing this I have often been able to >> reproduce a hang while trying to send CMD11 on an rk3288-based board >> while constantly ejecting and inserting UHS cards. >> >> Signed-off-by: Doug Anderson <diand...@chromium.org> >> --- >> drivers/mmc/host/dw_mmc.c | 26 ++++++++++++++++++++++++++ >> include/linux/mmc/dw_mmc.h | 2 ++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 47dfd0e..d259662 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -1020,6 +1020,15 @@ static void __dw_mci_start_request(struct dw_mci >> *host, >> >> dw_mci_start_command(host, cmd, cmdflags); >> >> + if (cmd->opcode == SD_SWITCH_VOLTAGE) { >> + /* >> + * Databook says to fail after 2ms w/ no response; give an >> + * extra jiffy just in case we're about to roll over. >> + */ >> + mod_timer(&host->cmd11_timer, >> + jiffies + msecs_to_jiffies(2) + 1); > > What's "plus one"?
I tried to briefly describe it in the comment above with the "in case we're about to roll over". ...but more detail... * I expect HZ to be something like 100. ...so a jiffy will be 10ms. 2ms will be rounded up to 1 jiffy. * It's entirely possible that we're about to roll over jiffies. That is, we might make the mod_timer call when we're 1 nanosecond away from moving from 999 to 1000 jiffies. We'll still read "jiffies" as 999 and add "msecs_to_jiffies(2)" to get 1000 jiffies. ...but then it will roll over and we'll make the call mod_timer(1000) when jiffies is already 1000. That means that we really got a 1ns delay--not so good. ...if we add the extra 1 jiffy then we'll probably really delay for 10-20ms, but that should be fine in this case. If I misunderstood the above, please correct me. >> + } >> + >> if (mrq->stop) >> host->stop_cmdr = dw_mci_prepare_command(slot->mmc, mrq->stop); >> else >> @@ -2158,6 +2167,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void >> *dev_id) >> /* Check volt switch first, since it can look like an error */ >> if ((host->state == STATE_SENDING_CMD11) && >> (pending & SDMMC_INT_VOLT_SWITCH)) { >> + del_timer(&host->cmd11_timer); >> + >> mci_writel(host, RINTSTS, SDMMC_INT_VOLT_SWITCH); >> pending &= ~SDMMC_INT_VOLT_SWITCH; >> dw_mci_cmd_interrupt(host, pending); >> @@ -2571,6 +2582,18 @@ ciu_out: >> return ret; >> } >> >> +static void dw_mci_cmd11_timer(unsigned long arg) >> +{ >> + struct dw_mci *host = (struct dw_mci *)arg; >> + >> + if (host->state != STATE_SENDING_CMD11) >> + dev_info(host->dev, "Unexpected CMD11 timeout\n"); > > If Unexpected CMD11 timeout, can it do just" return"? > Well, I think Unexpected CMD11 timeout is an rare case. Duh, of course. I'm happy to respin this or I'm happy if you want to just add a "return;" Please let me know. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/