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
>
> -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
