On 3 July 2012 17:27, Girish K S <girish.shivananjappa@xxxxxxxxxx> wrote: > On 3 July 2012 17:01, Aaron Lu <aaron.lu@xxxxxxx> wrote: >> Hi, >> >> On Tue, Jul 03, 2012 at 03:28:28PM +0530, Girish K S wrote: >>> On 3 July 2012 14:57, Aaron Lu <aaron.lu@xxxxxxx> wrote: >>> > V2: >>> > Fix for SDIO case: both SD and SDIO cards use cmd19 while eMMC use cmd21. >>> > >>> > V1: >>> > For SD hosts using retuning mode 1, when retuning timer expired, it will >>> > need to do retuning in sdhci_request before processing the actual >>> > request. But the retuning command is fixed: cmd19 for SD card and cmd21 >>> > for eMMC card, so we can't use the original request's command to do the >>> > tuning. >>> > >>> > And since the tuning command depends on the card type atteched to the >>> > host, we will need to know the card type to use the correct tuning >>> > command. >>> > >>> > Cc: stable <stable@xxxxxxxxxxxxxxx> [3.3+] >>> > Signed-off-by: Aaron Lu <aaron.lu@xxxxxxx> >>> > --- >>> > drivers/mmc/host/sdhci.c | 8 +++++++- >>> > 1 file changed, 7 insertions(+), 1 deletion(-) >>> > >>> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> > index f76736b..4e53e6b 100644 >>> > --- a/drivers/mmc/host/sdhci.c >>> > +++ b/drivers/mmc/host/sdhci.c >>> > @@ -27,6 +27,7 @@ >>> > >>> > #include <linux/mmc/mmc.h> >>> > #include <linux/mmc/host.h> >>> > +#include <linux/mmc/card.h> >>> > >>> > #include "sdhci.h" >>> > >>> > @@ -1245,6 +1246,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) >>> > struct sdhci_host *host; >>> > bool present; >>> > unsigned long flags; >>> > + u32 tuning_opcode; >>> > >>> > host = mmc_priv(mmc); >>> > >>> > @@ -1292,8 +1294,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq) >>> > */ >>> > if ((host->flags & SDHCI_NEEDS_RETUNING) && >>> > !(present_state & (SDHCI_DOING_WRITE | SDHCI_DOING_READ))) { >>> > + /* eMMC uses cmd21 while sd and sdio use cmd19 */ >>> > + tuning_opcode = mmc->card->type == MMC_TYPE_MMC ? >>> > + MMC_SEND_TUNING_BLOCK_HS200 : >>> > + MMC_SEND_TUNING_BLOCK; >>> > spin_unlock_irqrestore(&host->lock, flags); >>> > - sdhci_execute_tuning(mmc, mrq->cmd->opcode); >>> > + sdhci_execute_tuning(mmc, tuning_opcode); >>> dont you think the previous implementation does the same. It is >>> already handled by introducing the 2nd parameter. >> >> Suppose the following scenario: >> mmc_start_request (e.g. mrq->cmd->opcode is 18 for this call) >> -> host->ops->request >> (sdhci's retuning timer expired, the flag SDHCI_NEEDS_RETUNING is set) >> -> sdhci_request >> -> sdhci_execute_tuning will be called before processing the >> actual request due to retuning's requirement, but with the wrong command >> opcode(cmd18) instead of cmd19 for sd/sdio or cmd21 for emmc. >> >> The problem is with retuning, for normal explicit calls of >> sdhci_execute_tuning, there is no problem with the code. But when >> retuning is required, sdhci_execute_retuning will be executed implicitly >> to the above layer and we have to use the right tuning command instead of >> the current processing command, which can be any of the valid sd/sdio/mmc >> commands. > Thanks for the explanation. is it possible to make a separate local > function for this. Since mmc_host_ops has a member .execute_tuning > which takes 2 parameters that are called from respective sd/mmc/sdio > card files. there might be conflict Sorry i just read it wrongly (function has only 1 param). >> >> -Aaron >> >>> > spin_lock_irqsave(&host->lock, flags); >>> > >>> > /* Restore original mmc_request structure */ >>> > -- >>> > 1.7.11.1.3.g4c8a9db >>> > >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html