| Message ID | 1480497614-3501-3-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30 November 2016 at 10:20, Adrian Hunter <adrian.hunter@intel.com> wrote: > Clearing the tuning bits should reset the tuning circuit. However there is > more to do. Reset the command and data lines for good measure, and then > for eMMC ensure the card is not still trying to process a tuning command by > sending a stop command. > > Note the JEDEC eMMC specification says the stop command (CMD12) can be used > to stop a tuning command (CMD21) whereas the SD specification is silent on > the subject with respect to the SD tuning command (CMD19). Considering that > CMD12 is not a valid SDIO command, the stop command is sent only when the > tuning command is CMD21 i.e. for eMMC. That addresses cases seen so far > which have been on eMMC. > > Note that this replaces the commit fe5fb2e3b58f ("mmc: sdhci: Reset cmd and > data circuits after tuning failure") which is being reverted for v4.9+. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > Tested-by: Dan O'Donovan <dan@emutex.com> > Cc: stable@vger.kernel.org > --- > drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index e761fe2aa99e..1d72a51287d4 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2095,7 +2095,27 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) > ctrl &= ~SDHCI_CTRL_EXEC_TUNING; > sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); > > + sdhci_do_reset(host, SDHCI_RESET_CMD); > + sdhci_do_reset(host, SDHCI_RESET_DATA); > + > err = -EIO; > + > + if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200) > + goto out; > + > + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); > + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); > + > + spin_unlock_irqrestore(&host->lock, flags); > + > + memset(&cmd, 0, sizeof(cmd)); > + cmd.opcode = MMC_STOP_TRANSMISSION; > + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; > + cmd.busy_timeout = 50; > + mmc_wait_for_cmd(mmc, &cmd, 0); No, please don't add more hacks to send commands internally from sdhci. Maybe even before you start fix the problems for tuning, perhaps you try to clean up the current code when sending CMD21/19 in sdhci_execute_tuning()? Moreover, according to the change log above, it seems like a generic thing to send CMD12 to abort tuning. In such case, we could either make the core deal with it in the error path - or we could implement a "mmc_abort_tuning()" function, host drivers may call when needed. > + > + spin_lock_irqsave(&host->lock, flags); > + > goto out; > } > > -- > 1.9.1 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/11/16 12:06, Ulf Hansson wrote: > On 30 November 2016 at 10:20, Adrian Hunter <adrian.hunter@intel.com> wrote: >> Clearing the tuning bits should reset the tuning circuit. However there is >> more to do. Reset the command and data lines for good measure, and then >> for eMMC ensure the card is not still trying to process a tuning command by >> sending a stop command. >> >> Note the JEDEC eMMC specification says the stop command (CMD12) can be used >> to stop a tuning command (CMD21) whereas the SD specification is silent on >> the subject with respect to the SD tuning command (CMD19). Considering that >> CMD12 is not a valid SDIO command, the stop command is sent only when the >> tuning command is CMD21 i.e. for eMMC. That addresses cases seen so far >> which have been on eMMC. >> >> Note that this replaces the commit fe5fb2e3b58f ("mmc: sdhci: Reset cmd and >> data circuits after tuning failure") which is being reverted for v4.9+. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >> Tested-by: Dan O'Donovan <dan@emutex.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index e761fe2aa99e..1d72a51287d4 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2095,7 +2095,27 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) >> ctrl &= ~SDHCI_CTRL_EXEC_TUNING; >> sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >> >> + sdhci_do_reset(host, SDHCI_RESET_CMD); >> + sdhci_do_reset(host, SDHCI_RESET_DATA); >> + >> err = -EIO; >> + >> + if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200) >> + goto out; >> + >> + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); >> + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); >> + >> + spin_unlock_irqrestore(&host->lock, flags); >> + >> + memset(&cmd, 0, sizeof(cmd)); >> + cmd.opcode = MMC_STOP_TRANSMISSION; >> + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >> + cmd.busy_timeout = 50; >> + mmc_wait_for_cmd(mmc, &cmd, 0); > > No, please don't add more hacks to send commands internally from sdhci. > > Maybe even before you start fix the problems for tuning, perhaps you > try to clean up the current code when sending CMD21/19 in > sdhci_execute_tuning()? > > Moreover, according to the change log above, it seems like a generic > thing to send CMD12 to abort tuning. In such case, we could either > make the core deal with it in the error path - or we could implement a > "mmc_abort_tuning()" function, host drivers may call when needed. I am not sure a cleanup would apply cleanly to stable trees. It would be nicer to have these patches for stable and then a cleanup on top. Would that be acceptable? -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 November 2016 at 11:17, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 30/11/16 12:06, Ulf Hansson wrote: >> On 30 November 2016 at 10:20, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> Clearing the tuning bits should reset the tuning circuit. However there is >>> more to do. Reset the command and data lines for good measure, and then >>> for eMMC ensure the card is not still trying to process a tuning command by >>> sending a stop command. >>> >>> Note the JEDEC eMMC specification says the stop command (CMD12) can be used >>> to stop a tuning command (CMD21) whereas the SD specification is silent on >>> the subject with respect to the SD tuning command (CMD19). Considering that >>> CMD12 is not a valid SDIO command, the stop command is sent only when the >>> tuning command is CMD21 i.e. for eMMC. That addresses cases seen so far >>> which have been on eMMC. >>> >>> Note that this replaces the commit fe5fb2e3b58f ("mmc: sdhci: Reset cmd and >>> data circuits after tuning failure") which is being reverted for v4.9+. >>> >>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >>> Tested-by: Dan O'Donovan <dan@emutex.com> >>> Cc: stable@vger.kernel.org >>> --- >>> drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++ >>> 1 file changed, 20 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index e761fe2aa99e..1d72a51287d4 100644 >>> --- a/drivers/mmc/host/sdhci.c >>> +++ b/drivers/mmc/host/sdhci.c >>> @@ -2095,7 +2095,27 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) >>> ctrl &= ~SDHCI_CTRL_EXEC_TUNING; >>> sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >>> >>> + sdhci_do_reset(host, SDHCI_RESET_CMD); >>> + sdhci_do_reset(host, SDHCI_RESET_DATA); >>> + >>> err = -EIO; >>> + >>> + if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200) >>> + goto out; >>> + >>> + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); >>> + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); >>> + >>> + spin_unlock_irqrestore(&host->lock, flags); >>> + >>> + memset(&cmd, 0, sizeof(cmd)); >>> + cmd.opcode = MMC_STOP_TRANSMISSION; >>> + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >>> + cmd.busy_timeout = 50; >>> + mmc_wait_for_cmd(mmc, &cmd, 0); >> >> No, please don't add more hacks to send commands internally from sdhci. >> >> Maybe even before you start fix the problems for tuning, perhaps you >> try to clean up the current code when sending CMD21/19 in >> sdhci_execute_tuning()? >> >> Moreover, according to the change log above, it seems like a generic >> thing to send CMD12 to abort tuning. In such case, we could either >> make the core deal with it in the error path - or we could implement a >> "mmc_abort_tuning()" function, host drivers may call when needed. > > I am not sure a cleanup would apply cleanly to stable trees. It would be > nicer to have these patches for stable and then a cleanup on top. Would > that be acceptable? > Yes. That's ok. Can you please re-spin with cleanups on top then. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index e761fe2aa99e..1d72a51287d4 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2095,7 +2095,27 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) ctrl &= ~SDHCI_CTRL_EXEC_TUNING; sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); + sdhci_do_reset(host, SDHCI_RESET_CMD); + sdhci_do_reset(host, SDHCI_RESET_DATA); + err = -EIO; + + if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200) + goto out; + + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE); + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE); + + spin_unlock_irqrestore(&host->lock, flags); + + memset(&cmd, 0, sizeof(cmd)); + cmd.opcode = MMC_STOP_TRANSMISSION; + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; + cmd.busy_timeout = 50; + mmc_wait_for_cmd(mmc, &cmd, 0); + + spin_lock_irqsave(&host->lock, flags); + goto out; }
Clearing the tuning bits should reset the tuning circuit. However there is more to do. Reset the command and data lines for good measure, and then for eMMC ensure the card is not still trying to process a tuning command by sending a stop command. Note the JEDEC eMMC specification says the stop command (CMD12) can be used to stop a tuning command (CMD21) whereas the SD specification is silent on the subject with respect to the SD tuning command (CMD19). Considering that CMD12 is not a valid SDIO command, the stop command is sent only when the tuning command is CMD21 i.e. for eMMC. That addresses cases seen so far which have been on eMMC. Note that this replaces the commit fe5fb2e3b58f ("mmc: sdhci: Reset cmd and data circuits after tuning failure") which is being reverted for v4.9+. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Tested-by: Dan O'Donovan <dan@emutex.com> Cc: stable@vger.kernel.org --- drivers/mmc/host/sdhci.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)