On 30 May 2018 at 04:11, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
> R1_STATUS(x) now is only used by ioctl_rpmb_card_status_poll(),
> which checks all bits as possible. But according to the spec,
> bit 17 and bit 18 should be ignored, as well bit 14 which is
> reserved(must be set to 0) quoting from the spec and these rule
> apply to all places checking the device status. So change
> its checking from 0xFFFFE000 to 0xFFF9A000.
>
> As a bonus, we reuse it for mmc_do_erase() as well as
> mmc_switch_status_error().
> (1) Currently mmc_switch_status_error() doesn't check bit 25, but
> it means device is locked but not unlocked by CMD42 prior to any
> operations which need check busy, which is also not allowed.
> (2) mmc_do_erase() also forgot to to check bit 15, WP_ERASE_SKIP.
> The spec says "Only partial address space was erased due to existing
> write protected blocks.", which obviously means we should fail this I/O.
> Otherwise, the partial erased data stored in nonvalatile flash violates
> the data integrity from the view of I/O owner, which probably confuse
> it when further used.
>
> So reusing R1_STATUS for them not only improve the readability but also
> slove real problems.
>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
This looks like a relevant change even applicable outside the series.
I have picked this for next, thanks!
Kind regards
Uffe
>
> ---
>
> Changes in v3: None
> Changes in v2:
> - improve the patch sequence and fold in all related changes to make
> the patch look better.
>
> drivers/mmc/core/core.c | 2 +-
> drivers/mmc/core/mmc_ops.c | 2 +-
> include/linux/mmc/mmc.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 281826d..6780c2b 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2078,7 +2078,7 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from,
> cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> /* Do not retry else we can't see errors */
> err = mmc_wait_for_cmd(card->host, &cmd, 0);
> - if (err || (cmd.resp[0] & 0xFDF92000)) {
> + if (err || R1_STATUS(cmd.resp[0])) {
> pr_err("error %d requesting status %#x\n",
> err, cmd.resp[0]);
> err = -EIO;
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 126fa65..ee5f5ea 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -417,7 +417,7 @@ static int mmc_switch_status_error(struct mmc_host *host, u32 status)
> if (status & R1_SPI_ILLEGAL_COMMAND)
> return -EBADMSG;
> } else {
> - if (status & 0xFDFFA000)
> + if (R1_STATUS(status))
> pr_warn("%s: unexpected status %#x after switch\n",
> mmc_hostname(host), status);
> if (status & R1_SWITCH_ERROR)
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index 3ffc27a..897a87c 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -144,7 +144,7 @@ static inline bool mmc_op_multi(u32 opcode)
> #define R1_WP_ERASE_SKIP (1 << 15) /* sx, c */
> #define R1_CARD_ECC_DISABLED (1 << 14) /* sx, a */
> #define R1_ERASE_RESET (1 << 13) /* sr, c */
> -#define R1_STATUS(x) (x & 0xFFFFE000)
> +#define R1_STATUS(x) (x & 0xFFF9A000)
> #define R1_CURRENT_STATE(x) ((x & 0x00001E00) >> 9) /* sx, b (4 bits) */
> #define R1_READY_FOR_DATA (1 << 8) /* sx, a */
> #define R1_SWITCH_ERROR (1 << 7) /* sx, c */
> --
> 1.9.1
>
>
> --
> 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
--
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
