Re: [PATCH] mmc: dw_mmc: Avoid hung state if GEN_CMD transfer fails

[Vincent Whitchurch <vincent.whitchurch@axis.com>]

On Wed, Nov 03, 2021 at 07:27:16PM +0100, Mårten Lindahl wrote:
> If we get a data error during a block transfer command, a stop command
> (CMD12) is normally initiated. But this does not work for the general
> command (CMD56), but instead the action is ignored and an uninitialized
> command struct is used for the stop action, with unexpected result.
> 
> Fix this by adding a check for GEN_CMD when preparing stop transmission.
> 
> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 6578cc64ae9e..988c32e93e03 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -335,7 +335,8 @@ static u32 dw_mci_prep_stop_abort(struct dw_mci *host, struct mmc_command *cmd)
>  	    cmdr == MMC_WRITE_BLOCK ||
>  	    cmdr == MMC_WRITE_MULTIPLE_BLOCK ||
>  	    cmdr == MMC_SEND_TUNING_BLOCK ||
> -	    cmdr == MMC_SEND_TUNING_BLOCK_HS200) {
> +	    cmdr == MMC_SEND_TUNING_BLOCK_HS200 ||
> +	    cmdr == MMC_GEN_CMD) {
>  		stop->opcode = MMC_STOP_TRANSMISSION;
>  		stop->arg = 0;
>  		stop->flags = MMC_RSP_R1B | MMC_CMD_AC;

While this fix looks correct for CMD56, the "Data transfer mode"
sections of the eMMC and SD specifications list several more data
commands, all of which can be aborted by CMD12, but which aren't handled
in the if above.

If I'm not mistaken, those will also result in an uninitialized stop
command being sent in the case of an error, since the driver calls
send_stop_abort() on any data error.

Is there a reason why those other commands should not be in the list
above, or should we fix this list so that CMD12 is initialized for all
data commands except SD_IO_RW_EXTENDED?