RE: [PATCH/RFC 1/3] mmc: tmio: Add tuning support

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

 



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




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux