Hi Simon-san,
> From: Behalf Of Simon Horman
> Sent: Tuesday, May 10, 2016 2:52 PM
>
> From: Ai Kyuse <ai.kyuse.uw@xxxxxxxxxxx>
>
> Add tuning support for use with SDR104 mode
>
> Signed-off-by: Ai Kyuse <ai.kyuse.uw@xxxxxxxxxxx>
> Signed-off-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>
I have some minor comments :)
< snip >
> @@ -753,6 +785,157 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
> return 0;
> }
>
> +#define TMIO_MMC_MAX_TUNING_LOOP 40
> +
> +static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct tmio_mmc_host *host = mmc_priv(mmc);
> + struct mmc_ios *ios = &mmc->ios;
> +
> + unsigned long timeout, val;
> + unsigned long *tap;
> + int tuning_loop_counter = TMIO_MMC_MAX_TUNING_LOOP;
< snip >
> + /*
> + * 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;
The tuning_loop_counter is 40 and timeout is 150.
However,
> + do {
> + if (host->prepare_tuning)
> + host->prepare_tuning(host, val % num);
> +
> + if (!tuning_loop_counter && !timeout)
> + break;
< snip >
> + tuning_loop_counter--;
> + timeout--;
> + mdelay(1);
> + } while ((val < (num * 2)) && (tuning_loop_counter || timeout));
They will be decreased by 1. So, I think we don't need either variable.
> + /*
> + * The Host Driver has exhausted the maximum number of loops allowed,
> + * so use fixed sampling frequency.
> + */
> + if (tuning_loop_counter || timeout) {
> + if (host->select_tuning) {
> + ret = host->select_tuning(host, tap);
> + if (ret < 0)
> + goto out;
> + }
> + host->done_tuning = true;
> + } else {
> + dev_warn(&host->pdev->dev, ": Tuning procedure failed\n");
The first 2 charactors ": " is strange to me.
> + ret = -EIO;
> + goto out;
> + }
> +
> +out:
> + kfree(data_buf);
> +err_data:
> + kfree(tap);
> +err_tap:
> + if (ret < 0 && host->hw_reset)
> + host->hw_reset(host);
We can use tmio_mmc_hw_reset() of this patch.
Then, we can remove the condition of "host->hw_reset".
Best regards,
Yoshihiro Shimoda
> + return ret;
> +
> +}
> +
> /* Process requests from the MMC layer */
> static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> @@ -778,6 +961,43 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
>
> spin_unlock_irqrestore(&host->lock, flags);
>
> + if (host->inquiry_tuning && host->inquiry_tuning(host) &&
> + !host->done_tuning) {
> + /* Start retuning */
> + ret = tmio_mmc_execute_tuning(mmc, MMC_SEND_TUNING_BLOCK);
> + if (ret)
> + goto fail;
> + /* Restore request */
> + host->mrq = mrq;
> + }
> +
> + if (mrq->sbc) {
> + init_completion(&host->completion);
> + ret = tmio_mmc_start_command(host, mrq->sbc);
> + if (ret)
> + goto fail;
> + ret = wait_for_completion_timeout(&host->completion,
> + msecs_to_jiffies(CMDREQ_TIMEOUT));
> + if (ret < 0)
> + goto fail;
> + if (!ret) {
> + ret = -ETIMEDOUT;
> + goto fail;
> + }
> + host->last_req_ts = jiffies;
> + host->mrq = mrq;
> + if (host->inquiry_tuning && host->inquiry_tuning(host) &&
> + !host->done_tuning) {
> + /* Start retuning */
> + ret = tmio_mmc_execute_tuning(mmc,
> + MMC_SEND_TUNING_BLOCK);
> + if (ret)
> + goto fail;
> + /* Restore request */
> + host->mrq = mrq;
> + }
> + }
> +
> if (mrq->data) {
> ret = tmio_mmc_start_data(host, mrq->data);
> if (ret)
> @@ -967,6 +1187,16 @@ static int tmio_mmc_card_busy(struct mmc_host *mmc)
> return !(sd_ctrl_read16_and_16_as_32(host, CTL_STATUS) & TMIO_STAT_DAT0);
> }
>
> +static void tmio_mmc_hw_reset(struct mmc_host *mmc)
> +{
> + struct tmio_mmc_host *host = mmc_priv(mmc);
> +
> + if (host->hw_reset)
> + host->hw_reset(host);
> +
> + host->done_tuning = false;
> +}
> +
> static struct mmc_host_ops tmio_mmc_ops = {
> .request = tmio_mmc_request,
> .set_ios = tmio_mmc_set_ios,
> @@ -975,6 +1205,8 @@ static struct mmc_host_ops tmio_mmc_ops = {
> .enable_sdio_irq = tmio_mmc_enable_sdio_irq,
> .card_busy = tmio_mmc_card_busy,
> .multi_io_quirk = tmio_multi_io_quirk,
> + .execute_tuning = tmio_mmc_execute_tuning,
> + .hw_reset = tmio_mmc_hw_reset,
> };
>
> static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
> @@ -1084,6 +1316,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
> mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
> mmc->max_seg_size = mmc->max_req_size;
>
> + _host->done_tuning = false;
> _host->native_hotplug = !(pdata->flags & TMIO_MMC_USE_GPIO_CD ||
> mmc->caps & MMC_CAP_NEEDS_POLL ||
> mmc->caps & MMC_CAP_NONREMOVABLE ||
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 7a26286db895..6c22b21f2d73 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -104,6 +104,9 @@
> */
> #define TMIO_MMC_CLK_ACTUAL (1 << 10)
>
> +/* Some controllers have UHS-I sampling clock controller */
> +#define TMIO_MMC_HAS_UHS_SCC (1 << 11)
> +
> int tmio_core_mmc_enable(void __iomem *cnf, int shift, unsigned long base);
> int tmio_core_mmc_resume(void __iomem *cnf, int shift, unsigned long base);
> void tmio_core_mmc_pwr(void __iomem *cnf, int shift, int state);
> --
> 2.1.4
