On Wed, Mar 16, 2011 at 2:20 AM, Nath, Arindam <Arindam.Nath@xxxxxxx> wrote: > Hi Zhangfei, > > >> -----Original Message----- >> From: zhangfei gao [mailto:zhangfei.gao@xxxxxxxxx] >> Sent: Wednesday, March 16, 2011 8:21 AM >> To: Nath, Arindam >> Cc: cjb@xxxxxxxxxx; prakity@xxxxxxxxxxx; subhashj@xxxxxxxxxxxxxx; >> linux-mmc@xxxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; anath.amd@xxxxxxxxx >> Subject: Re: [PATCH v2 09/12] mmc: sd: add support for tuning during >> uhs initialization >> >> On Tue, Mar 15, 2011 at 6:43 AM, Nath, Arindam <Arindam.Nath@xxxxxxx> >> wrote: >> > Hi Zhangfei, >> > >> > >> >> -----Original Message----- >> >> From: zhangfei gao [mailto:zhangfei.gao@xxxxxxxxx] >> >> Sent: Tuesday, March 15, 2011 4:02 PM >> >> To: Nath, Arindam >> >> Cc: cjb@xxxxxxxxxx; prakity@xxxxxxxxxxx; subhashj@xxxxxxxxxxxxxx; >> >> linux-mmc@xxxxxxxxxxxxxxx; Su, Henry; Lu, Aaron; anath.amd@xxxxxxxxx >> >> Subject: Re: [PATCH v2 09/12] mmc: sd: add support for tuning during >> >> uhs initialization >> >> >> >> On Fri, Mar 4, 2011 at 6:32 AM, Arindam Nath <arindam.nath@xxxxxxx> >> >> wrote: >> >> > Host Controller needs tuning during initialization to operate >> SDR50 >> >> > and SDR104 UHS-I cards. Whether SDR50 mode actually needs tuning >> is >> >> > indicated by bit 45 of the Host Controller Capabilities register. >> >> > A new command CMD19 has been defined in the Physical Layer spec >> >> > v3.01 to request the card to send tuning pattern. >> >> > >> >> > We enable Buffer Read Ready interrupt at the very begining of >> tuning >> >> > procedure, because that is the only interrupt generated by the >> Host >> >> > Controller during tuning. The tuning block is sent by the card to >> the >> >> > Host Controller using DAT lines, so we set Data Present Select >> (bit >> >> 5) >> >> > in the Command register. The Host Controller is responsible for >> doing >> >> > the verfication of tuning block sent by the card at the hardware >> >> level. >> >> > After sending CMD19, we wait for Buffer Read Ready interrupt. In >> case >> >> > we don't receive an interrupt after the specified timeout value, >> we >> >> > fall back on fixed sampling clock by setting Execute Tuning (bit >> 6) >> >> > and Sampling Clock Seletc (bit 7) of Host Control2 register to 0. >> >> > Before exiting the tuning procedure, we disable Buffer Read Ready >> >> > interrupt. >> >> > >> >> > Signed-off-by: Arindam Nath <arindam.nath@xxxxxxx> >> >> > --- >> >> > drivers/mmc/core/sd.c | 4 + >> >> > drivers/mmc/host/sdhci.c | 137 >> >> ++++++++++++++++++++++++++++++++++++++++++++- >> >> > drivers/mmc/host/sdhci.h | 3 + >> >> > include/linux/mmc/host.h | 1 + >> >> > include/linux/mmc/mmc.h | 1 + >> >> > include/linux/mmc/sdhci.h | 4 + >> >> > 6 files changed, 149 insertions(+), 1 deletions(-) >> >> > >> >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> >> > index be01397..1e2d157 100644 >> >> > --- a/drivers/mmc/core/sd.c >> >> > +++ b/drivers/mmc/core/sd.c >> >> > @@ -637,6 +637,10 @@ static int mmc_sd_init_uhs_card(struct >> mmc_card >> >> *card) >> >> > /* Set current limit for the card */ >> >> > err = sd_set_current_limit(card, status); >> >> > >> >> > + /* SPI mode doesn't define CMD19 */ >> >> > + if (!mmc_host_is_spi(card->host) && card->host->ops- >> >> >execute_tuning) >> >> > + card->host->ops->execute_tuning(card->host); >> >> > + >> >> > out: >> >> > kfree(status); >> >> > >> >> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> >> > index 245cc39..8f4f102 100644 >> >> > --- a/drivers/mmc/host/sdhci.c >> >> > +++ b/drivers/mmc/host/sdhci.c >> >> > @@ -39,6 +39,8 @@ >> >> > #define SDHCI_USE_LEDS_CLASS >> >> > #endif >> >> > >> >> > +#define MAX_TUNING_LOOP 40 >> >> > + >> >> > static unsigned int debug_quirks = 0; >> >> > >> >> > static void sdhci_prepare_data(struct sdhci_host *, struct >> mmc_data >> >> *); >> >> > @@ -985,7 +987,9 @@ static void sdhci_send_command(struct >> sdhci_host >> >> *host, struct mmc_command *cmd) >> >> > flags |= SDHCI_CMD_CRC; >> >> > if (cmd->flags & MMC_RSP_OPCODE) >> >> > flags |= SDHCI_CMD_INDEX; >> >> > - if (cmd->data) >> >> > + >> >> > + /* CMD19 is special in that the Data Present Select should >> be >> >> set */ >> >> > + if (cmd->data || (cmd->opcode == MMC_SEND_TUNING_BLOCK)) >> >> > flags |= SDHCI_CMD_DATA; >> >> > >> >> > sdhci_writew(host, SDHCI_MAKE_CMD(cmd->opcode, flags), >> >> SDHCI_COMMAND); >> >> > @@ -1485,6 +1489,119 @@ static int >> sdhci_get_max_current_180(struct >> >> mmc_host *mmc) >> >> > return max_current_180; >> >> > } >> >> > >> >> > +static void sdhci_execute_tuning(struct mmc_host *mmc) >> >> > +{ >> >> > + struct sdhci_host *host; >> >> > + u16 ctrl; >> >> > + int tuning_loop_counter = MAX_TUNING_LOOP; >> >> > + unsigned long flags; >> >> > + unsigned long timeout; >> >> > + >> >> > + host = mmc_priv(mmc); >> >> > + >> >> > + spin_lock_irqsave(&host->lock, flags); >> >> > + >> >> > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> >> > + >> >> > + /* >> >> > + * Host Controller needs tuning only in case of SDR104 >> mode >> >> > + * and for SDR50 mode when Use Tuning for SDR50 is set in >> >> > + * Capabilities register. >> >> > + */ >> >> > + if ((ctrl & SDHCI_CTRL_UHS_SDR104) || >> >> > + ((ctrl & SDHCI_CTRL_UHS_SDR50) && >> >> > + (host->flags & SDHCI_SDR50_NEEDS_TUNING))) >> >> > + ctrl |= SDHCI_CTRL_EXEC_TUNING; >> >> > + else { >> >> > + spin_unlock_irqrestore(&host->lock, flags); >> >> > + return; >> >> > + } >> >> > + >> >> > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >> >> > + >> >> > + /* >> >> > + * As per the Host Controller spec v3.00, tuning command >> >> > + * generates Buffer Read Ready interrupt, so enable that. >> >> > + */ >> >> > + sdhci_unmask_irqs(host, SDHCI_INT_DATA_AVAIL); >> >> > + >> >> > + /* >> >> > + * Issue CMD19 repeatedly till Execute Tuning is set to 0 >> or >> >> the number >> >> > + * of loops reaches 40 times or a timeout of 150ms occurs. >> >> > + */ >> >> > + timeout = 150; >> >> > + do { >> >> > + struct mmc_command cmd; >> >> > + struct mmc_request mrq; >> >> > + >> >> > + if (!tuning_loop_counter && !timeout) >> >> > + break; >> >> > + >> >> > + memset(&cmd, 0, sizeof(struct mmc_command)); >> >> > + cmd.opcode = MMC_SEND_TUNING_BLOCK; >> >> > + cmd.arg = 0; >> >> > + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC; >> >> > + >> >> > + memset(&cmd.resp, 0, sizeof(cmd.resp)); >> >> > + cmd.retries = 0; >> >> > + >> >> > + cmd.data = NULL; >> >> > + cmd.error = 0; >> >> > + >> >> > + memset(&mrq, 0, sizeof(struct mmc_request)); >> >> > + mrq.cmd = &cmd; >> >> > + host->mrq = &mrq; >> >> >> >> Tune may fail since SDHCI_TRNS_DMA is set in SDHCI_TRANSFER_MODE, >> >> SDHCI_TRNS_DMA required to be cleared before send_command, such as >> >> sdhci_writew(host, 0x12, SDHCI_TRANSFER_MODE); >> > >> > Can you please tell me why and under what condition can tuning fail >> when SDHCI_TRANS_DMA is set? Since cmd->data is set to NULL inside >> sdhci_execute_tuning(), sdhci_set_transfer_mode() will return at the >> very beginning of the function before setting SDHCI_TRNS_DMA. Moreover, >> as per the Host Controller v3.00 spec, during tuning procedure, the >> host controller does not generate interrupts other than Buffer Read >> Ready. >> > >> >> Hi, Arindam >> >> The Present Status Register describes Buffer Read Enable (bit 11) and >> Buffer Write Enable (bit 10) only used for non-DMA transfer. >> If set SDHCI_TRANS_DMA during tuning process, controller may get no >> irq at all including Buffer Read Enable. >> Could you help check, thanks > > In the current implementation of tuning procedure, data is set to NULL before calling sdhci_send_command(), which in turn will make sure that SDHCI_TRNS_DMA is not set. So can you give me some specific scenario when the condition you mentioned above can occur? Hi, Arindam Since sdhci_send_command as well as sdhci_send_command is called many times before, the SDHCI_TRNS_DMA have already been set, if driver does not clear SDHCI_TRNS_DMA, I am afraid the tune can not get any irq and fail. Thanks > > Thanks, > Arindam > >> >> > Thanks, >> > Arindam >> > >> >> >> >> > + sdhci_send_command(host, &cmd); >> >> > + >> >> > + host->cmd = NULL; >> >> > + host->mrq = NULL; >> >> > + >> >> > + spin_unlock_irqrestore(&host->lock, flags); >> >> > + >> >> > + /* Wait for Buffer Read Ready interrupt */ >> >> > + wait_event_interruptible_timeout(host- >> >buf_ready_int, >> >> > + (host->tuning_done == 1), >> >> > + msecs_to_jiffies(50)); >> >> > + spin_lock_irqsave(&host->lock, flags); >> >> > + >> >> > + if (!host->tuning_done) { >> >> > + printk(KERN_INFO DRIVER_NAME ": Tuning >> >> procedure" >> >> > + " failed, falling back to fixed >> >> sampling" >> >> > + " clock\n"); >> >> > + ctrl &= ~SDHCI_CTRL_TUNED_CLK; >> >> > + ctrl &= ~SDHCI_CTRL_EXEC_TUNING; >> >> > + sdhci_writew(host, ctrl, >> >> SDHCI_HOST_CONTROL2); >> >> > + >> >> > + goto out; >> >> > + } >> >> > + >> >> > + host->tuning_done = 0; >> >> > + >> >> > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> >> > + tuning_loop_counter--; >> >> > + timeout--; >> >> > + mdelay(1); >> >> > + } while (ctrl & SDHCI_CTRL_EXEC_TUNING); >> >> > + >> >> > + /* >> >> > + * The Host Driver has exhausted the maximum number of >> loops >> >> allowed, >> >> > + * so use fixed sampling frequency. >> >> > + */ >> >> > + if (!tuning_loop_counter || !timeout) { >> >> > + ctrl &= ~SDHCI_CTRL_TUNED_CLK; >> >> > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >> >> > + } else { >> >> > + if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) >> >> > + printk(KERN_INFO DRIVER_NAME ": Tuning >> >> procedure" >> >> > + " failed, falling back to fixed >> >> sampling" >> >> > + " clock\n"); >> >> > + } >> >> > + >> >> > +out: >> >> > + sdhci_mask_irqs(host, SDHCI_INT_DATA_AVAIL); >> >> > + spin_unlock_irqrestore(&host->lock, flags); >> >> > +} >> >> > + >> >> > static const struct mmc_host_ops sdhci_ops = { >> >> > .request = sdhci_request, >> >> > .set_ios = sdhci_set_ios, >> >> > @@ -1492,6 +1609,7 @@ static const struct mmc_host_ops sdhci_ops = >> { >> >> > .enable_sdio_irq = sdhci_enable_sdio_irq, >> >> > .start_signal_voltage_switch = >> >> sdhci_start_signal_voltage_switch, >> >> > .get_max_current_180 = >> sdhci_get_max_current_180, >> >> > + .execute_tuning = sdhci_execute_tuning, >> >> > }; >> >> > >> >> > >> >> >> /********************************************************************* >> >> ********\ >> >> > @@ -1703,6 +1821,16 @@ static void sdhci_data_irq(struct >> sdhci_host >> >> *host, u32 intmask) >> >> > { >> >> > BUG_ON(intmask == 0); >> >> > >> >> > + /* CMD19 generates _only_ Buffer Read Ready interrupt */ >> >> > + if (intmask & SDHCI_INT_DATA_AVAIL) { >> >> > + if (SDHCI_GET_CMD(sdhci_readw(host, >> SDHCI_COMMAND)) >> >> > + == MMC_SEND_TUNING_BLOCK) { >> >> > + host->tuning_done = 1; >> >> > + wake_up(&host->buf_ready_int); >> >> > + return; >> >> > + } >> >> > + } >> >> > + >> >> > if (!host->data) { >> >> > /* >> >> > * The "data complete" interrupt is also used to >> >> > @@ -2126,6 +2254,10 @@ int sdhci_add_host(struct sdhci_host *host) >> >> > if (caps[1] & SDHCI_SUPPORT_DDR50) >> >> > mmc->caps |= MMC_CAP_UHS_DDR50; >> >> > >> >> > + /* Does the host needs tuning for SDR50? */ >> >> > + if (caps[1] & SDHCI_USE_SDR50_TUNING) >> >> > + host->flags |= SDHCI_SDR50_NEEDS_TUNING; >> >> > + >> >> > /* Driver Type(s) (A, C, D) supported by the host */ >> >> > if (caps[1] & SDHCI_DRIVER_TYPE_A) >> >> > mmc->caps |= MMC_CAP_DRIVER_TYPE_A; >> >> > @@ -2269,6 +2401,9 @@ int sdhci_add_host(struct sdhci_host *host) >> >> > >> >> > setup_timer(&host->timer, sdhci_timeout_timer, (unsigned >> >> long)host); >> >> > >> >> > + if (host->version >= SDHCI_SPEC_300) >> >> > + init_waitqueue_head(&host->buf_ready_int); >> >> > + >> >> > ret = request_irq(host->irq, sdhci_irq, IRQF_SHARED, >> >> > mmc_hostname(mmc), host); >> >> > if (ret) >> >> > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >> >> > index 5bf244d..4746879 100644 >> >> > --- a/drivers/mmc/host/sdhci.h >> >> > +++ b/drivers/mmc/host/sdhci.h >> >> > @@ -160,6 +160,8 @@ >> >> > #define SDHCI_CTRL_DRV_TYPE_A 0x0010 >> >> > #define SDHCI_CTRL_DRV_TYPE_C 0x0020 >> >> > #define SDHCI_CTRL_DRV_TYPE_D 0x0030 >> >> > +#define SDHCI_CTRL_EXEC_TUNING 0x0040 >> >> > +#define SDHCI_CTRL_TUNED_CLK 0x0080 >> >> > #define SDHCI_CTRL_PRESET_VAL_ENABLE 0x8000 >> >> > >> >> > #define SDHCI_CAPABILITIES 0x40 >> >> > @@ -187,6 +189,7 @@ >> >> > #define SDHCI_DRIVER_TYPE_A 0x00000010 >> >> > #define SDHCI_DRIVER_TYPE_C 0x00000020 >> >> > #define SDHCI_DRIVER_TYPE_D 0x00000040 >> >> > +#define SDHCI_USE_SDR50_TUNING 0x00002000 >> >> > >> >> > #define SDHCI_CAPABILITIES_1 0x44 >> >> > >> >> > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> >> > index e84cd05..651e40b 100644 >> >> > --- a/include/linux/mmc/host.h >> >> > +++ b/include/linux/mmc/host.h >> >> > @@ -129,6 +129,7 @@ struct mmc_host_ops { >> >> > >> >> > int (*start_signal_voltage_switch)(struct mmc_host >> *host); >> >> > int (*get_max_current_180)(struct mmc_host *mmc); >> >> > + void (*execute_tuning)(struct mmc_host *host); >> >> > }; >> >> > >> >> > struct mmc_card; >> >> > diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >> >> > index 612301f..9194f9e 100644 >> >> > --- a/include/linux/mmc/mmc.h >> >> > +++ b/include/linux/mmc/mmc.h >> >> > @@ -50,6 +50,7 @@ >> >> > #define MMC_SET_BLOCKLEN 16 /* ac [31:0] block len >> R1 >> >> */ >> >> > #define MMC_READ_SINGLE_BLOCK 17 /* adtc [31:0] data addr >> R1 >> >> */ >> >> > #define MMC_READ_MULTIPLE_BLOCK 18 /* adtc [31:0] data addr >> R1 >> >> */ >> >> > +#define MMC_SEND_TUNING_BLOCK 19 /* adtc >> R1 >> >> */ >> >> > >> >> > /* class 3 */ >> >> > #define MMC_WRITE_DAT_UNTIL_STOP 20 /* adtc [31:0] data addr >> R1 >> >> */ >> >> > diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h >> >> > index 282d158..5203b97 100644 >> >> > --- a/include/linux/mmc/sdhci.h >> >> > +++ b/include/linux/mmc/sdhci.h >> >> > @@ -109,6 +109,7 @@ struct sdhci_host { >> >> > #define SDHCI_USE_ADMA (1<<1) /* Host is ADMA capable */ >> >> > #define SDHCI_REQ_USE_DMA (1<<2) /* Use DMA for this req. >> */ >> >> > #define SDHCI_DEVICE_DEAD (1<<3) /* Device unresponsive */ >> >> > +#define SDHCI_SDR50_NEEDS_TUNING (1<<4) /* SDR50 needs >> tuning >> >> */ >> >> > >> >> > unsigned int version; /* SDHCI spec. version */ >> >> > >> >> > @@ -147,6 +148,9 @@ struct sdhci_host { >> >> > >> >> > struct mmc_command *saved_abort_cmd; /* Abort command >> >> saved for data error recovery */ >> >> > >> >> > + wait_queue_head_t buf_ready_int; /* Waitqueue for >> >> Buffer Read Ready interrupt */ >> >> > + unsigned int tuning_done; /* Condition flag >> set >> >> when CMD19 succeeds */ >> >> > + >> >> > unsigned long private[0] ____cacheline_aligned; >> >> > }; >> >> > #endif /* __SDHCI_H */ >> >> > -- >> >> > 1.7.1 >> >> > >> >> > >> > >> > >> > > > > -- 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