D spec v5.1 adds discard support. The flows and commands matches those
in eMMC, Which leaves to set the appropriate discard arg in CMD38 if
DISCARD_SUPPORT (b313) is set in the SD_STATUS register.
We set this arg on card init: not in mmc_init_erase as one might expect
but arbitrarily once the card indicated its discard support. This is
because unlike erase, it doesn't really involve any logic, and we want
to avoid the unnecessary complication.
Avri Altman (3):
mmc: core: Calculate the discard arg only once
mmc: core: Indicate SD specs higher than 4.0
mmc: core: Add discard support to sd
drivers/mmc/core/block.c | 12 +++---------
drivers/mmc/core/core.c | 6 +++++-
drivers/mmc/core/mmc.c | 8 ++++++++
drivers/mmc/core/sd.c | 15 +++++++++++++++
include/linux/mmc/card.h | 4 ++++
include/linux/mmc/sd.h | 6 ++++++
6 files changed, 41 insertions(+), 10 deletions(-)
--
1.9.1
The discard arg is a read-only ext_csd parameter -
set it once on card init.
Signed-off-by: Avri Altman <[email protected]>
---
drivers/mmc/core/block.c | 12 +++---------
drivers/mmc/core/mmc.c | 8 ++++++++
include/linux/mmc/card.h | 2 ++
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index b08fb91..3502f2c 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -1124,7 +1124,7 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
{
struct mmc_blk_data *md = mq->blkdata;
struct mmc_card *card = md->queue.card;
- unsigned int from, nr, arg;
+ unsigned int from, nr;
int err = 0, type = MMC_BLK_DISCARD;
blk_status_t status = BLK_STS_OK;
@@ -1136,24 +1136,18 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
from = blk_rq_pos(req);
nr = blk_rq_sectors(req);
- if (mmc_can_discard(card))
- arg = MMC_DISCARD_ARG;
- else if (mmc_can_trim(card))
- arg = MMC_TRIM_ARG;
- else
- arg = MMC_ERASE_ARG;
do {
err = 0;
if (card->quirks & MMC_QUIRK_INAND_CMD38) {
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
INAND_CMD38_ARG_EXT_CSD,
- arg == MMC_TRIM_ARG ?
+ card->discard_arg == MMC_TRIM_ARG ?
INAND_CMD38_ARG_TRIM :
INAND_CMD38_ARG_ERASE,
0);
}
if (!err)
- err = mmc_erase(card, from, nr, arg);
+ err = mmc_erase(card, from, nr, card->discard_arg);
} while (err == -EIO && !mmc_blk_reset(md, card->host, type));
if (err)
status = BLK_STS_IOERR;
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index da892a5..120739c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1743,6 +1743,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
card->ext_csd.power_off_notification = EXT_CSD_POWER_ON;
}
+ /* set discard_arg */
+ if (mmc_can_discard(card))
+ card->discard_arg = MMC_DISCARD_ARG;
+ else if (mmc_can_trim(card))
+ card->discard_arg = MMC_TRIM_ARG;
+ else
+ card->discard_arg = MMC_ERASE_ARG;
+
/*
* Select timing interface
*/
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index de73778..447648b 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -308,6 +308,8 @@ struct mmc_card {
unsigned int nr_parts;
unsigned int bouncesz; /* Bounce buffer size */
+
+ unsigned int discard_arg; /* discard args */
};
static inline bool mmc_large_sector(struct mmc_card *card)
--
1.9.1
SD specs version 4.x and 5.x have a dedicated slices in the SCR register.
Higher versions will rely on a combination of the existing fields.
Signed-off-by: Avri Altman <[email protected]>
---
drivers/mmc/core/sd.c | 5 +++++
include/linux/mmc/card.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index d0d9f90..44f4cbc 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -209,6 +209,11 @@ static int mmc_decode_scr(struct mmc_card *card)
/* Check if Physical Layer Spec v3.0 is supported */
scr->sda_spec3 = UNSTUFF_BITS(resp, 47, 1);
+ if (scr->sda_spec3) {
+ scr->sda_spec4 = UNSTUFF_BITS(resp, 42, 1);
+ scr->sda_specx = UNSTUFF_BITS(resp, 38, 4);
+ }
+
if (UNSTUFF_BITS(resp, 55, 1))
card->erased_byte = 0xFF;
else
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 447648b..919e644 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -133,6 +133,8 @@ struct mmc_ext_csd {
struct sd_scr {
unsigned char sda_vsn;
unsigned char sda_spec3;
+ unsigned char sda_spec4;
+ unsigned char sda_specx;
unsigned char bus_widths;
#define SD_SCR_BUS_WIDTH_1 (1<<0)
#define SD_SCR_BUS_WIDTH_4 (1<<2)
--
1.9.1
SD spec v5.1 adds discard support. The flows and commands are similar to
mmc, so just set the discard arg in CMD38.
Actually, there is no need to check for the spec version like we are
doing, as it is assured that the reserved bits in earlier versions are
null. Do that anyway to document the spec version that introduce it.
Signed-off-by: Avri Altman <[email protected]>
---
drivers/mmc/core/core.c | 6 +++++-
drivers/mmc/core/sd.c | 10 ++++++++++
include/linux/mmc/sd.h | 6 ++++++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 5bd58b9..c495935 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2181,7 +2181,10 @@ int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
if (!card->erase_size)
return -EOPNOTSUPP;
- if (mmc_card_sd(card) && arg != MMC_ERASE_ARG)
+ if (mmc_card_sd(card) && arg == SD_DISCARD_ARG)
+ goto skip_arg_testing;
+
+ if (mmc_card_sd(card) && arg != SD_ERASE_ARG)
return -EOPNOTSUPP;
if ((arg & MMC_SECURE_ARGS) &&
@@ -2200,6 +2203,7 @@ int mmc_erase(struct mmc_card *card, unsigned int from, unsigned int nr,
if (arg == MMC_ERASE_ARG)
nr = mmc_align_erase_size(card, &from, &to, nr);
+skip_arg_testing:
if (nr == 0)
return 0;
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 44f4cbc..b554a17 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -231,6 +231,8 @@ static int mmc_read_ssr(struct mmc_card *card)
{
unsigned int au, es, et, eo;
__be32 *raw_ssr;
+ u32 resp[4] = {};
+ u8 discard_support;
int i;
if (!(card->csd.cmdclass & CCC_APP_SPEC)) {
@@ -276,6 +278,14 @@ static int mmc_read_ssr(struct mmc_card *card)
}
}
+ /*
+ * starting SD5.1 discard is supported if DISCARD_SUPPORT (b313) is set
+ */
+ resp[3] = card->raw_ssr[6];
+ discard_support = UNSTUFF_BITS(resp, 313 - 288, 1);
+ card->discard_arg = (card->scr.sda_specx && discard_support) ?
+ SD_DISCARD_ARG : SD_ERASE_ARG;
+
return 0;
}
diff --git a/include/linux/mmc/sd.h b/include/linux/mmc/sd.h
index 1ebcf9b..ec94a5a 100644
--- a/include/linux/mmc/sd.h
+++ b/include/linux/mmc/sd.h
@@ -91,4 +91,10 @@
#define SD_SWITCH_ACCESS_DEF 0
#define SD_SWITCH_ACCESS_HS 1
+/*
+ * Erase/discard
+ */
+#define SD_ERASE_ARG 0x00000000
+#define SD_DISCARD_ARG 0x00000001
+
#endif /* LINUX_MMC_SD_H */
--
1.9.1
On Sun, 3 Feb 2019 at 09:51, Avri Altman <[email protected]> wrote:
>
> The discard arg is a read-only ext_csd parameter -
> set it once on card init.
I like the idea here. There is really no point checking this for every
corresponding request, nice!
However, the "discard arg" isn't specific to eMMC, as it's also used
for SD cards. So, the change log is a bit miss-leading, I think. Can
you please clarify this.
>
> Signed-off-by: Avri Altman <[email protected]>
> ---
> drivers/mmc/core/block.c | 12 +++---------
> drivers/mmc/core/mmc.c | 8 ++++++++
> include/linux/mmc/card.h | 2 ++
> 3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index b08fb91..3502f2c 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -1124,7 +1124,7 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> {
> struct mmc_blk_data *md = mq->blkdata;
> struct mmc_card *card = md->queue.card;
> - unsigned int from, nr, arg;
> + unsigned int from, nr;
> int err = 0, type = MMC_BLK_DISCARD;
> blk_status_t status = BLK_STS_OK;
>
> @@ -1136,24 +1136,18 @@ static void mmc_blk_issue_discard_rq(struct mmc_queue *mq, struct request *req)
> from = blk_rq_pos(req);
> nr = blk_rq_sectors(req);
>
> - if (mmc_can_discard(card))
> - arg = MMC_DISCARD_ARG;
> - else if (mmc_can_trim(card))
> - arg = MMC_TRIM_ARG;
> - else
> - arg = MMC_ERASE_ARG;
> do {
> err = 0;
> if (card->quirks & MMC_QUIRK_INAND_CMD38) {
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> INAND_CMD38_ARG_EXT_CSD,
> - arg == MMC_TRIM_ARG ?
> + card->discard_arg == MMC_TRIM_ARG ?
> INAND_CMD38_ARG_TRIM :
> INAND_CMD38_ARG_ERASE,
> 0);
> }
> if (!err)
> - err = mmc_erase(card, from, nr, arg);
> + err = mmc_erase(card, from, nr, card->discard_arg);
> } while (err == -EIO && !mmc_blk_reset(md, card->host, type));
> if (err)
> status = BLK_STS_IOERR;
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index da892a5..120739c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1743,6 +1743,14 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> card->ext_csd.power_off_notification = EXT_CSD_POWER_ON;
> }
>
> + /* set discard_arg */
> + if (mmc_can_discard(card))
> + card->discard_arg = MMC_DISCARD_ARG;
> + else if (mmc_can_trim(card))
> + card->discard_arg = MMC_TRIM_ARG;
> + else
> + card->discard_arg = MMC_ERASE_ARG;
You are assigning card->discard_arg only for (e)MMC, while I think you
should do that also for SD.
I guess it practice it doesn't matter, because MMC_ERASE_ARG is zero.
Although, I would prefer to keep the code consistent, so I think you
should assign card->discard_arg for for SD cards as well.
> +
> /*
> * Select timing interface
> */
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index de73778..447648b 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -308,6 +308,8 @@ struct mmc_card {
> unsigned int nr_parts;
>
> unsigned int bouncesz; /* Bounce buffer size */
> +
> + unsigned int discard_arg; /* discard args */
Please move this a couple of lines above, close to the other "erase"
related variables.
You may even consider to rename the arg to "erase_arg", but I have no
strong opinion changing to that.
> };
>
> static inline bool mmc_large_sector(struct mmc_card *card)
> --
> 1.9.1
>
Kind regards
Uffe
> On Sun, 3 Feb 2019 at 09:51, Avri Altman <[email protected]> wrote:
> >
> > The discard arg is a read-only ext_csd parameter -
> > set it once on card init.
>
> I like the idea here. There is really no point checking this for every
> corresponding request, nice!
>
> However, the "discard arg" isn't specific to eMMC, as it's also used
> for SD cards. So, the change log is a bit miss-leading, I think. Can
> you please clarify this.
Done.
> >
> > + /* set discard_arg */
> > + if (mmc_can_discard(card))
> > + card->discard_arg = MMC_DISCARD_ARG;
> > + else if (mmc_can_trim(card))
> > + card->discard_arg = MMC_TRIM_ARG;
> > + else
> > + card->discard_arg = MMC_ERASE_ARG;
>
> You are assigning card->discard_arg only for (e)MMC, while I think you
> should do that also for SD.
>
> I guess it practice it doesn't matter, because MMC_ERASE_ARG is zero.
> Although, I would prefer to keep the code consistent, so I think you
> should assign card->discard_arg for for SD cards as well.
Done.
> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> > index de73778..447648b 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -308,6 +308,8 @@ struct mmc_card {
> > unsigned int nr_parts;
> >
> > unsigned int bouncesz; /* Bounce buffer size */
> > +
> > + unsigned int discard_arg; /* discard args */
>
> Please move this a couple of lines above, close to the other "erase"
> related variables.
Done.
>
> You may even consider to rename the arg to "erase_arg", but I have no
> strong opinion changing to that.
Done.
>
> > };
> >
> > static inline bool mmc_large_sector(struct mmc_card *card)
> > --
> > 1.9.1
> >
>
> Kind regards
> Uffe