Re: [PATCH v2] mmc: sdhci: fix incorrect command used in tuning

[Aaron Lu <aaron.lu@amd.com>]

Hi,

On Tue, Jul 03, 2012 at 05:32:36PM +0530, Girish K S wrote:
> >>> > @@ -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).

So do you have any other problems with this patch?
If not, can I have your ack on this patch?

Thanks,
Aaron