Re: [PATCH 2/3] mmc: core: Mark close-ended ffu in progress

From: Adrian Hunter <adrian.hunter@intel.com>
To: Avri Altman <avri.altman@wdc.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	linux-mmc@vger.kernel.org
Cc: Daniil Lunev <dlunev@google.com>,
	Asutosh Das <quic_asutoshd@quicinc.com>
Subject: Re: [PATCH 2/3] mmc: core: Mark close-ended ffu in progress
Date: Fri, 20 Oct 2023 08:48:34 +0300	[thread overview]
Message-ID: <d1505d2f-90f9-4fd6-a928-a590b72fce37@intel.com> (raw)
In-Reply-To: <20231012184041.1837455-3-avri.altman@wdc.com>

On 12/10/23 21:40, Avri Altman wrote:
> The SDHCI_QUIRK2_FFU_ACMD12 quirk was invented to prevent from those
> bogus sdhci to use auto-cmd12 after cmd25 in a close-ended ffu process.
> Capture the applicable mrq and mark it so it won't use auto-cmd12 post
> that cmd25.
> 
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> ---
>  drivers/mmc/core/block.c | 25 +++++++++++++++++++++++++
>  include/linux/mmc/mmc.h  |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 3a8f27c3e310..d92e7322c6da 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -114,6 +114,8 @@ struct mmc_blk_data {
>  	unsigned int	flags;
>  #define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
>  #define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
> +#define MMC_BLK_FFU	(1 << 2)	/* FFU in progress */
> +#define MMC_BLK_CE	(1 << 3)	/* close-ended FFU in progress */
>  
>  	struct kref	kref;
>  	unsigned int	read_only;
> @@ -548,6 +550,29 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>  	    (cmd.opcode == MMC_SWITCH))
>  		return mmc_sanitize(card, idata->ic.cmd_timeout_ms);
>  
> +	if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_MODE_CONFIG) &&
> +	    (cmd.opcode == MMC_SWITCH)) {
> +		u8 value = MMC_EXTRACT_VALUE_FROM_ARG(cmd.arg);
> +
> +		if (value == 1) {
> +			md->flags |= MMC_BLK_FFU;
> +		} else if (value == 0) {
> +			/* switch back to normal mode is always happening */
> +			md->flags &= ~MMC_BLK_FFU;
> +			md->flags &= ~MMC_BLK_CE;
> +		}
> +	}
> +
> +	if ((md->flags & MMC_BLK_FFU) && cmd.opcode == MMC_SET_BLOCK_COUNT) {
> +		md->flags &= ~MMC_BLK_FFU;
> +		md->flags |= MMC_BLK_CE;
> +	}
> +
> +	if ((md->flags & MMC_BLK_CE) && mmc_op_multi(cmd.opcode)) {
> +		mrq.ffu = true;
> +		md->flags &= ~MMC_BLK_CE;
> +	}

Could it be a separate helper function so it ends up like

	mrq.ffu = mmc_is_ffu_cmd(&cmd)

But perhaps then mrq.ffu is not needed and sdhci-msm.c
could call mmc_is_ffu_cmd() directly?

> +
>  	/* If it's an R1B response we need some more preparations. */
>  	busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
>  	r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 6f7993803ee7..d4d10cabaa57 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -254,6 +254,7 @@ static inline bool mmc_ready_for_data(u32 status)
>   */
>  
>  #define EXT_CSD_CMDQ_MODE_EN		15	/* R/W */
> +#define EXT_CSD_MODE_CONFIG		30	/* R/W */
>  #define EXT_CSD_FLUSH_CACHE		32      /* W */
>  #define EXT_CSD_CACHE_CTRL		33      /* R/W */
>  #define EXT_CSD_POWER_OFF_NOTIFICATION	34	/* R/W */