Re: [PATCH v2 09/12] mmc: sd: add support for tuning during uhs initialization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux