RE: [PATCH v5] mmc: core: Fix the HPI execution sequence

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

 



Thanks Venkatraman. Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx>

Regards,
Subhash

> -----Original Message-----
> From: Venkatraman S [mailto:svenkatr@xxxxxx]
> Sent: Friday, June 22, 2012 11:43 AM
> To: cjb@xxxxxxxxxx
> Cc: linux-mmc@xxxxxxxxxxxxxxx; linkinjeon@xxxxxxxxx;
> jh80.chung@xxxxxxxxxxx; alex.lemberg@xxxxxxxxxxx;
> subhashj@xxxxxxxxxxxxxx; Venkatraman S; Kostya
> Subject: [PATCH v5] mmc: core: Fix the HPI execution sequence
> 
> mmc_execute_hpi should send the HPI command only once, and only if the
> card is in PRG state.
> 
> According to eMMC spec, the command's completion time is not dependent on
> OUT_OF_INTERRUPT_TIME. Only the transition out of PRG STATE is guarded by
> OUT_OF_INTERRUPT_TIME - which is defined to begin at the end of sending
> the command itself.
> 
> Specify the default timeout for the actual sending of HPI command, and
then
> use OUT_OF_INTERRUPT_TIME to wait for the transition out of PRG state.
> 
> Reported-by: Alex Lemberg <Alex.Lemberg@xxxxxxxxxxx>
> Signed-off-by: Venkatraman S <svenkatr@xxxxxx>
> Reviewed-by: Namjae Jeon <linkinjeon@xxxxxxxxx>
> Acked-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
> CC: Kostya <kdorfman@xxxxxxxxxxxxxx>
> ---
> v4->v5: Start time calculation after the HPI command has been sent
> 
> v3->v4: Fix coding style issues as suggested by Chris Ball.
>        Use time_after() for safe time diff calculations as
>         suggested by Subash Jadavani.
> 
>  drivers/mmc/core/core.c    |   55
++++++++++++++++++++++++++-----------------
> -
>  drivers/mmc/core/mmc_ops.c |    1 -
>  2 files changed, 32 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index
> 80ec427..f11de40 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -404,6 +404,7 @@ int mmc_interrupt_hpi(struct mmc_card *card)  {
>  	int err;
>  	u32 status;
> +	unsigned long prg_wait;
> 
>  	BUG_ON(!card);
> 
> @@ -419,30 +420,38 @@ int mmc_interrupt_hpi(struct mmc_card *card)
>  		goto out;
>  	}
> 
> -	/*
> -	 * If the card status is in PRG-state, we can send the HPI command.
> -	 */
> -	if (R1_CURRENT_STATE(status) == R1_STATE_PRG) {
> -		do {
> -			/*
> -			 * We don't know when the HPI command will finish
> -			 * processing, so we need to resend HPI until out
> -			 * of prg-state, and keep checking the card status
> -			 * with SEND_STATUS.  If a timeout error occurs when
> -			 * sending the HPI command, we are already out of
> -			 * prg-state.
> -			 */
> -			err = mmc_send_hpi_cmd(card, &status);
> -			if (err)
> -				pr_debug("%s: abort HPI (%d error)\n",
> -					 mmc_hostname(card->host), err);
> +	switch (R1_CURRENT_STATE(status)) {
> +	case R1_STATE_IDLE:
> +	case R1_STATE_READY:
> +	case R1_STATE_STBY:
> +		/*
> +		 * In idle states, HPI is not needed and the caller
> +		 * can issue the next intended command immediately
> +		 */
> +		goto out;
> +	case R1_STATE_PRG:
> +		break;
> +	default:
> +		/* In all other states, it's illegal to issue HPI */
> +		pr_debug("%s: HPI cannot be sent. Card state=%d\n",
> +			mmc_hostname(card->host),
> R1_CURRENT_STATE(status));
> +		err = -EINVAL;
> +		goto out;
> +	}
> 
> -			err = mmc_send_status(card, &status);
> -			if (err)
> -				break;
> -		} while (R1_CURRENT_STATE(status) == R1_STATE_PRG);
> -	} else
> -		pr_debug("%s: Left prg-state\n", mmc_hostname(card->host));
> +	err = mmc_send_hpi_cmd(card, &status);
> +	if (err)
> +		goto out;
> +
> +	prg_wait = jiffies +
msecs_to_jiffies(card->ext_csd.out_of_int_time);
> +	do {
> +		err = mmc_send_status(card, &status);
> +
> +		if (!err && R1_CURRENT_STATE(status) == R1_STATE_TRAN)
> +			break;
> +		if (time_after(jiffies, prg_wait))
> +			err = -ETIMEDOUT;
> +	} while (!err);
> 
>  out:
>  	mmc_release_host(card->host);
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 69370f4..0ed2cc5 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -569,7 +569,6 @@ int mmc_send_hpi_cmd(struct mmc_card *card, u32
> *status)
> 
>  	cmd.opcode = opcode;
>  	cmd.arg = card->rca << 16 | 1;
> -	cmd.cmd_timeout_ms = card->ext_csd.out_of_int_time;
> 
>  	err = mmc_wait_for_cmd(card->host, &cmd, 0);
>  	if (err) {
> --
> 1.7.10.rc2


--
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