diff mbox series

[1/2] mmc: core: use mrq->sbc when sending CMD23 for RPMB

Message ID 20181126133814.1185-2-wsa+renesas@sang-engineering.com (mailing list archive)
State New, archived
Headers show
Series mmc: refactor RPMB block count handling | expand

Commit Message

Wolfram Sang Nov. 26, 2018, 1:38 p.m. UTC 
When sending out CMD23 in the blk preparation, the comment there
rightfully says:

	 * However, it is not sufficient to just send CMD23,
	 * and avoid the final CMD12, as on an error condition
	 * CMD12 (stop) needs to be sent anyway. This, coupled
	 * with Auto-CMD23 enhancements provided by some
	 * hosts, means that the complexity of dealing
	 * with this is best left to the host. If CMD23 is
	 * supported by card and host, we'll fill sbc in and let
	 * the host deal with handling it correctly.

Let's do this behaviour for RPMB as well, and not send CMD23
independently. Otherwise IP cores (like Renesas SDHI) may timeout
because of automatic CMD23/CMD12 handling.

Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Tested-by: Clément Péron <peron.clem@gmail.com>
---
 drivers/mmc/core/block.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Avri Altman Nov. 26, 2018, 3:52 p.m. UTC | #1 
> 
> When sending out CMD23 in the blk preparation, the comment there
> rightfully says:
> 
> 	 * However, it is not sufficient to just send CMD23,
> 	 * and avoid the final CMD12, as on an error condition
> 	 * CMD12 (stop) needs to be sent anyway. This, coupled
> 	 * with Auto-CMD23 enhancements provided by some
> 	 * hosts, means that the complexity of dealing
> 	 * with this is best left to the host. If CMD23 is
> 	 * supported by card and host, we'll fill sbc in and let
> 	 * the host deal with handling it correctly.
> 
> Let's do this behaviour for RPMB as well, and not send CMD23
> independently. Otherwise IP cores (like Renesas SDHI) may timeout
> because of automatic CMD23/CMD12 handling.
> 
> Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Clément Péron <peron.clem@gmail.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
Niklas Söderlund Nov. 26, 2018, 3:52 p.m. UTC | #2 
Hi Wolfram,

Thanks for your patch.

On 2018-11-26 14:38:13 +0100, Wolfram Sang wrote:
> When sending out CMD23 in the blk preparation, the comment there
> rightfully says:
> 
> 	 * However, it is not sufficient to just send CMD23,
> 	 * and avoid the final CMD12, as on an error condition
> 	 * CMD12 (stop) needs to be sent anyway. This, coupled
> 	 * with Auto-CMD23 enhancements provided by some
> 	 * hosts, means that the complexity of dealing
> 	 * with this is best left to the host. If CMD23 is
> 	 * supported by card and host, we'll fill sbc in and let
> 	 * the host deal with handling it correctly.
> 
> Let's do this behaviour for RPMB as well, and not send CMD23
> independently. Otherwise IP cores (like Renesas SDHI) may timeout
> because of automatic CMD23/CMD12 handling.
> 
> Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Clément Péron <peron.clem@gmail.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/mmc/core/block.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index c35b5b08bb33..111934838da2 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -472,7 +472,7 @@ static int ioctl_do_sanitize(struct mmc_card *card)
>  static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>  			       struct mmc_blk_ioc_data *idata)
>  {
> -	struct mmc_command cmd = {};
> +	struct mmc_command cmd = {}, sbc = {};
>  	struct mmc_data data = {};
>  	struct mmc_request mrq = {};
>  	struct scatterlist sg;
> @@ -550,10 +550,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>  	}
>  
>  	if (idata->rpmb) {
> -		err = mmc_set_blockcount(card, data.blocks,
> -			idata->ic.write_flag & (1 << 31));
> -		if (err)
> -			return err;
> +		sbc.opcode = MMC_SET_BLOCK_COUNT;
> +		/*
> +		 * We don't do any blockcount validation because the max size
> +		 * may be increased by a future standard. We just copy the
> +		 * 'Reliable Write' bit here.
> +		 */
> +		sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
> +		sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +		mrq.sbc = &sbc;
>  	}
>  
>  	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&
> -- 
> 2.11.0
>
Ulf Hansson Dec. 5, 2018, 2:23 p.m. UTC | #3 
On Mon, 26 Nov 2018 at 14:38, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> When sending out CMD23 in the blk preparation, the comment there
> rightfully says:
>
>          * However, it is not sufficient to just send CMD23,
>          * and avoid the final CMD12, as on an error condition
>          * CMD12 (stop) needs to be sent anyway. This, coupled
>          * with Auto-CMD23 enhancements provided by some
>          * hosts, means that the complexity of dealing
>          * with this is best left to the host. If CMD23 is
>          * supported by card and host, we'll fill sbc in and let
>          * the host deal with handling it correctly.
>
> Let's do this behaviour for RPMB as well, and not send CMD23
> independently. Otherwise IP cores (like Renesas SDHI) may timeout
> because of automatic CMD23/CMD12 handling.
>
> Reported-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Clément Péron <peron.clem@gmail.com>

Applied for next and by adding a stable tag, thanks!

Kind regards
Uffe

> ---
>  drivers/mmc/core/block.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index c35b5b08bb33..111934838da2 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -472,7 +472,7 @@ static int ioctl_do_sanitize(struct mmc_card *card)
>  static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>                                struct mmc_blk_ioc_data *idata)
>  {
> -       struct mmc_command cmd = {};
> +       struct mmc_command cmd = {}, sbc = {};
>         struct mmc_data data = {};
>         struct mmc_request mrq = {};
>         struct scatterlist sg;
> @@ -550,10 +550,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>         }
>
>         if (idata->rpmb) {
> -               err = mmc_set_blockcount(card, data.blocks,
> -                       idata->ic.write_flag & (1 << 31));
> -               if (err)
> -                       return err;
> +               sbc.opcode = MMC_SET_BLOCK_COUNT;
> +               /*
> +                * We don't do any blockcount validation because the max size
> +                * may be increased by a future standard. We just copy the
> +                * 'Reliable Write' bit here.
> +                */
> +               sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
> +               sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +               mrq.sbc = &sbc;
>         }
>
>         if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&
> --
> 2.11.0
>
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index c35b5b08bb33..111934838da2 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -472,7 +472,7 @@  static int ioctl_do_sanitize(struct mmc_card *card)
 static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 			       struct mmc_blk_ioc_data *idata)
 {
-	struct mmc_command cmd = {};
+	struct mmc_command cmd = {}, sbc = {};
 	struct mmc_data data = {};
 	struct mmc_request mrq = {};
 	struct scatterlist sg;
@@ -550,10 +550,15 @@  static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	}
 
 	if (idata->rpmb) {
-		err = mmc_set_blockcount(card, data.blocks,
-			idata->ic.write_flag & (1 << 31));
-		if (err)
-			return err;
+		sbc.opcode = MMC_SET_BLOCK_COUNT;
+		/*
+		 * We don't do any blockcount validation because the max size
+		 * may be increased by a future standard. We just copy the
+		 * 'Reliable Write' bit here.
+		 */
+		sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
+		sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+		mrq.sbc = &sbc;
 	}
 
 	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&