diff mbox

[1/4] mmc: sdhci-esdhc-imx: Convert to mmc_send_tuning()

Message ID 20141205111100.GC21173@shlinux1.ap.freescale.net (mailing list archive)
State New, archived
Headers show

Commit Message

Aisheng Dong Dec. 5, 2014, 11:11 a.m. UTC
On Mon, Dec 01, 2014 at 05:02:07PM +0100, Ulf Hansson wrote:
> Instead of having a local function taking care of sending the tuning
> command, let's use the common mmc_send_tuning() API provided by the mmc
> core. In this way the request will be handled as any other request by
> sdhci core.
> 
> As an effect of this change, the pm_runtime_get_sync() call at
> esdhc_prepare_tuning() isn't needed any more.
> 
> This patch will also introduce another change in behavior, since before
> the response pattern to the tuning command wasn't verified by
> sdhci-esdhc-imx. The mmc_send_tuning() does that.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

After applying your mmc_send_tuning() fix patch and it's usage update,
i tested it worked well on a imx6dl sabreauto board.

So you can add my tag when update the patch.
Tested-by: Dong Aisheng <b29396@freescale.com>
Acked-by: Dong Aisheng <b29396@freescale.com>

Regards
Dong Aisheng


> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 68 ++------------------------------------
>  1 file changed, 3 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 0135f00..a33d64c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -65,8 +65,6 @@
>  /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
>  #define ESDHC_TUNING_START_TAP		0x1
>  
> -#define ESDHC_TUNING_BLOCK_PATTERN_LEN	64
> -
>  /* pinctrl state */
>  #define ESDHC_PINCTRL_STATE_100MHZ	"state_100mhz"
>  #define ESDHC_PINCTRL_STATE_200MHZ	"state_200mhz"
> @@ -692,8 +690,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
>  	/* FIXME: delay a bit for card to be ready for next tuning due to errors */
>  	mdelay(1);
>  
> -	/* This is balanced by the runtime put in sdhci_tasklet_finish */
> -	pm_runtime_get_sync(host->mmc->parent);
>  	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
>  	reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL |
>  			ESDHC_MIX_CTRL_FBCLK_SEL;
> @@ -704,54 +700,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
>  			val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
>  }
>  
> -static void esdhc_request_done(struct mmc_request *mrq)
> -{
> -	complete(&mrq->completion);
> -}
> -
> -static int esdhc_send_tuning_cmd(struct sdhci_host *host, u32 opcode,
> -				 struct scatterlist *sg)
> -{
> -	struct mmc_command cmd = {0};
> -	struct mmc_request mrq = {NULL};
> -	struct mmc_data data = {0};
> -
> -	cmd.opcode = opcode;
> -	cmd.arg = 0;
> -	cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> -
> -	data.blksz = ESDHC_TUNING_BLOCK_PATTERN_LEN;
> -	data.blocks = 1;
> -	data.flags = MMC_DATA_READ;
> -	data.sg = sg;
> -	data.sg_len = 1;
> -
> -	mrq.cmd = &cmd;
> -	mrq.cmd->mrq = &mrq;
> -	mrq.data = &data;
> -	mrq.data->mrq = &mrq;
> -	mrq.cmd->data = mrq.data;
> -
> -	mrq.done = esdhc_request_done;
> -	init_completion(&(mrq.completion));
> -
> -	spin_lock_irq(&host->lock);
> -	host->mrq = &mrq;
> -
> -	sdhci_send_command(host, mrq.cmd);
> -
> -	spin_unlock_irq(&host->lock);
> -
> -	wait_for_completion(&mrq.completion);
> -
> -	if (cmd.error)
> -		return cmd.error;
> -	if (data.error)
> -		return data.error;
> -
> -	return 0;
> -}
> -
>  static void esdhc_post_tuning(struct sdhci_host *host)
>  {
>  	u32 reg;
> @@ -763,21 +711,13 @@ static void esdhc_post_tuning(struct sdhci_host *host)
>  
>  static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>  {
> -	struct scatterlist sg;
> -	char *tuning_pattern;
>  	int min, max, avg, ret;
>  
> -	tuning_pattern = kmalloc(ESDHC_TUNING_BLOCK_PATTERN_LEN, GFP_KERNEL);
> -	if (!tuning_pattern)
> -		return -ENOMEM;
> -
> -	sg_init_one(&sg, tuning_pattern, ESDHC_TUNING_BLOCK_PATTERN_LEN);
> -
>  	/* find the mininum delay first which can pass tuning */
>  	min = ESDHC_TUNE_CTRL_MIN;
>  	while (min < ESDHC_TUNE_CTRL_MAX) {
>  		esdhc_prepare_tuning(host, min);
> -		if (!esdhc_send_tuning_cmd(host, opcode, &sg))
> +		if (!mmc_send_tuning(host->mmc->card))
>  			break;
>  		min += ESDHC_TUNE_CTRL_STEP;
>  	}
> @@ -786,7 +726,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>  	max = min + ESDHC_TUNE_CTRL_STEP;
>  	while (max < ESDHC_TUNE_CTRL_MAX) {
>  		esdhc_prepare_tuning(host, max);
> -		if (esdhc_send_tuning_cmd(host, opcode, &sg)) {
> +		if (mmc_send_tuning(host->mmc->card)) {
>  			max -= ESDHC_TUNE_CTRL_STEP;
>  			break;
>  		}
> @@ -796,11 +736,9 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>  	/* use average delay to get the best timing */
>  	avg = (min + max) / 2;
>  	esdhc_prepare_tuning(host, avg);
> -	ret = esdhc_send_tuning_cmd(host, opcode, &sg);
> +	ret = mmc_send_tuning(host->mmc->card);
>  	esdhc_post_tuning(host);
>  
> -	kfree(tuning_pattern);
> -
>  	dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
>  		ret ? "failed" : "passed", avg, ret);
>  
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ulf Hansson Dec. 5, 2014, 12:02 p.m. UTC | #1
On 5 December 2014 at 12:11, Dong Aisheng <b29396@freescale.com> wrote:
> On Mon, Dec 01, 2014 at 05:02:07PM +0100, Ulf Hansson wrote:
>> Instead of having a local function taking care of sending the tuning
>> command, let's use the common mmc_send_tuning() API provided by the mmc
>> core. In this way the request will be handled as any other request by
>> sdhci core.
>>
>> As an effect of this change, the pm_runtime_get_sync() call at
>> esdhc_prepare_tuning() isn't needed any more.
>>
>> This patch will also introduce another change in behavior, since before
>> the response pattern to the tuning command wasn't verified by
>> sdhci-esdhc-imx. The mmc_send_tuning() does that.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> After applying your mmc_send_tuning() fix patch and it's usage update,
> i tested it worked well on a imx6dl sabreauto board.
>
> So you can add my tag when update the patch.
> Tested-by: Dong Aisheng <b29396@freescale.com>
> Acked-by: Dong Aisheng <b29396@freescale.com>

Thanks Dong for helping out! Much appreciated!

Kind regards
Uffe

>
> Regards
> Dong Aisheng
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a33d64c..12711ab 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -717,7 +717,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>         min = ESDHC_TUNE_CTRL_MIN;
>         while (min < ESDHC_TUNE_CTRL_MAX) {
>                 esdhc_prepare_tuning(host, min);
> -               if (!mmc_send_tuning(host->mmc->card))
> +               if (!mmc_send_tuning(host->mmc))
>                         break;
>                 min += ESDHC_TUNE_CTRL_STEP;
>         }
> @@ -726,7 +726,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>         max = min + ESDHC_TUNE_CTRL_STEP;
>         while (max < ESDHC_TUNE_CTRL_MAX) {
>                 esdhc_prepare_tuning(host, max);
> -               if (mmc_send_tuning(host->mmc->card)) {
> +               if (mmc_send_tuning(host->mmc)) {
>                         max -= ESDHC_TUNE_CTRL_STEP;
>                         break;
>                 }
> @@ -736,7 +736,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>         /* use average delay to get the best timing */
>         avg = (min + max) / 2;
>         esdhc_prepare_tuning(host, avg);
> -       ret = mmc_send_tuning(host->mmc->card);
> +       ret = mmc_send_tuning(host->mmc);
>         esdhc_post_tuning(host);
>
>         dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
>
>> ---
>>  drivers/mmc/host/sdhci-esdhc-imx.c | 68 ++------------------------------------
>>  1 file changed, 3 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
>> index 0135f00..a33d64c 100644
>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
>> @@ -65,8 +65,6 @@
>>  /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
>>  #define ESDHC_TUNING_START_TAP               0x1
>>
>> -#define ESDHC_TUNING_BLOCK_PATTERN_LEN       64
>> -
>>  /* pinctrl state */
>>  #define ESDHC_PINCTRL_STATE_100MHZ   "state_100mhz"
>>  #define ESDHC_PINCTRL_STATE_200MHZ   "state_200mhz"
>> @@ -692,8 +690,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
>>       /* FIXME: delay a bit for card to be ready for next tuning due to errors */
>>       mdelay(1);
>>
>> -     /* This is balanced by the runtime put in sdhci_tasklet_finish */
>> -     pm_runtime_get_sync(host->mmc->parent);
>>       reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
>>       reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL |
>>                       ESDHC_MIX_CTRL_FBCLK_SEL;
>> @@ -704,54 +700,6 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
>>                       val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
>>  }
>>
>> -static void esdhc_request_done(struct mmc_request *mrq)
>> -{
>> -     complete(&mrq->completion);
>> -}
>> -
>> -static int esdhc_send_tuning_cmd(struct sdhci_host *host, u32 opcode,
>> -                              struct scatterlist *sg)
>> -{
>> -     struct mmc_command cmd = {0};
>> -     struct mmc_request mrq = {NULL};
>> -     struct mmc_data data = {0};
>> -
>> -     cmd.opcode = opcode;
>> -     cmd.arg = 0;
>> -     cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
>> -
>> -     data.blksz = ESDHC_TUNING_BLOCK_PATTERN_LEN;
>> -     data.blocks = 1;
>> -     data.flags = MMC_DATA_READ;
>> -     data.sg = sg;
>> -     data.sg_len = 1;
>> -
>> -     mrq.cmd = &cmd;
>> -     mrq.cmd->mrq = &mrq;
>> -     mrq.data = &data;
>> -     mrq.data->mrq = &mrq;
>> -     mrq.cmd->data = mrq.data;
>> -
>> -     mrq.done = esdhc_request_done;
>> -     init_completion(&(mrq.completion));
>> -
>> -     spin_lock_irq(&host->lock);
>> -     host->mrq = &mrq;
>> -
>> -     sdhci_send_command(host, mrq.cmd);
>> -
>> -     spin_unlock_irq(&host->lock);
>> -
>> -     wait_for_completion(&mrq.completion);
>> -
>> -     if (cmd.error)
>> -             return cmd.error;
>> -     if (data.error)
>> -             return data.error;
>> -
>> -     return 0;
>> -}
>> -
>>  static void esdhc_post_tuning(struct sdhci_host *host)
>>  {
>>       u32 reg;
>> @@ -763,21 +711,13 @@ static void esdhc_post_tuning(struct sdhci_host *host)
>>
>>  static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>>  {
>> -     struct scatterlist sg;
>> -     char *tuning_pattern;
>>       int min, max, avg, ret;
>>
>> -     tuning_pattern = kmalloc(ESDHC_TUNING_BLOCK_PATTERN_LEN, GFP_KERNEL);
>> -     if (!tuning_pattern)
>> -             return -ENOMEM;
>> -
>> -     sg_init_one(&sg, tuning_pattern, ESDHC_TUNING_BLOCK_PATTERN_LEN);
>> -
>>       /* find the mininum delay first which can pass tuning */
>>       min = ESDHC_TUNE_CTRL_MIN;
>>       while (min < ESDHC_TUNE_CTRL_MAX) {
>>               esdhc_prepare_tuning(host, min);
>> -             if (!esdhc_send_tuning_cmd(host, opcode, &sg))
>> +             if (!mmc_send_tuning(host->mmc->card))
>>                       break;
>>               min += ESDHC_TUNE_CTRL_STEP;
>>       }
>> @@ -786,7 +726,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>>       max = min + ESDHC_TUNE_CTRL_STEP;
>>       while (max < ESDHC_TUNE_CTRL_MAX) {
>>               esdhc_prepare_tuning(host, max);
>> -             if (esdhc_send_tuning_cmd(host, opcode, &sg)) {
>> +             if (mmc_send_tuning(host->mmc->card)) {
>>                       max -= ESDHC_TUNE_CTRL_STEP;
>>                       break;
>>               }
>> @@ -796,11 +736,9 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
>>       /* use average delay to get the best timing */
>>       avg = (min + max) / 2;
>>       esdhc_prepare_tuning(host, avg);
>> -     ret = esdhc_send_tuning_cmd(host, opcode, &sg);
>> +     ret = mmc_send_tuning(host->mmc->card);
>>       esdhc_post_tuning(host);
>>
>> -     kfree(tuning_pattern);
>> -
>>       dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",
>>               ret ? "failed" : "passed", avg, ret);
>>
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a33d64c..12711ab 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -717,7 +717,7 @@  static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
        min = ESDHC_TUNE_CTRL_MIN;
        while (min < ESDHC_TUNE_CTRL_MAX) {
                esdhc_prepare_tuning(host, min);
-               if (!mmc_send_tuning(host->mmc->card))
+               if (!mmc_send_tuning(host->mmc))
                        break;
                min += ESDHC_TUNE_CTRL_STEP;
        }
@@ -726,7 +726,7 @@  static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
        max = min + ESDHC_TUNE_CTRL_STEP;
        while (max < ESDHC_TUNE_CTRL_MAX) {
                esdhc_prepare_tuning(host, max);
-               if (mmc_send_tuning(host->mmc->card)) {
+               if (mmc_send_tuning(host->mmc)) {
                        max -= ESDHC_TUNE_CTRL_STEP;
                        break;
                }
@@ -736,7 +736,7 @@  static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
        /* use average delay to get the best timing */
        avg = (min + max) / 2;
        esdhc_prepare_tuning(host, avg);
-       ret = mmc_send_tuning(host->mmc->card);
+       ret = mmc_send_tuning(host->mmc);
        esdhc_post_tuning(host);

        dev_dbg(mmc_dev(host->mmc), "tunning %s at 0x%x ret %d\n",