| Message ID | 20231105102944.1974512-1-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: core: Remove obsolete quirk for Sandisk cards | expand |
Gentle ping. Thanks, Avri > -----Original Message----- > From: Avri Altman <avri.altman@wdc.com> > Sent: Sunday, November 5, 2023 12:30 PM > To: Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc@vger.kernel.org > Cc: Dominique Martinet <asmadeus@codewreck.org>; Avri Altman > <Avri.Altman@wdc.com> > Subject: [PATCH] mmc: core: Remove obsolete quirk for Sandisk cards > > This quirk applies to an old set of Sandisk cards that are no longer being sold > for many years now, and it is vanishingly unlikely that will ever meet a V6.7 > kernel. > > I came across those old cards when I tried to decide what to do with Sandisk > cards that their quirk is designated by OEM id. For many years we read > wrong oemid, reading 16 bits instead of 8. Fixing it however, turned out to be > impractical, and it's better that each eMMC vendor will handle its own > devices. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/mmc/core/block.c | 32 +------------------------------- > drivers/mmc/core/quirks.h | 12 ------------ include/linux/mmc/card.h | 1 - > 3 files changed, 1 insertion(+), 44 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index > 152dfe593c43..6aa68ac1129e 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -1122,17 +1122,7 @@ static void mmc_blk_issue_erase_rq(struct > mmc_queue *mq, struct request *req, > nr = blk_rq_sectors(req); > > do { > - err = 0; > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, > EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - erase_arg == MMC_TRIM_ARG ? > - INAND_CMD38_ARG_TRIM : > - INAND_CMD38_ARG_ERASE, > - card->ext_csd.generic_cmd6_time); > - } > - if (!err) > - err = mmc_erase(card, from, nr, erase_arg); > + err = mmc_erase(card, from, nr, erase_arg); > } while (err == -EIO && !mmc_blk_reset(md, card->host, type)); > if (err) > status = BLK_STS_IOERR; > @@ -1182,17 +1172,6 @@ static void mmc_blk_issue_secdiscard_rq(struct > mmc_queue *mq, > arg = MMC_SECURE_ERASE_ARG; > > retry: > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - arg == MMC_SECURE_TRIM1_ARG ? > - INAND_CMD38_ARG_SECTRIM1 : > - INAND_CMD38_ARG_SECERASE, > - card->ext_csd.generic_cmd6_time); > - if (err) > - goto out_retry; > - } > - > err = mmc_erase(card, from, nr, arg); > if (err == -EIO) > goto out_retry; > @@ -1202,15 +1181,6 @@ static void mmc_blk_issue_secdiscard_rq(struct > mmc_queue *mq, > } > > if (arg == MMC_SECURE_TRIM1_ARG) { > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, > EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - INAND_CMD38_ARG_SECTRIM2, > - card->ext_csd.generic_cmd6_time); > - if (err) > - goto out_retry; > - } > - > err = mmc_erase(card, from, nr, > MMC_SECURE_TRIM2_ARG); > if (err == -EIO) > goto out_retry; > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index > cca71867bc4a..028f55f07ba3 100644 > --- a/drivers/mmc/core/quirks.h > +++ b/drivers/mmc/core/quirks.h > @@ -22,18 +22,6 @@ static const struct mmc_fixup __maybe_unused > mmc_blk_fixups[] = { #define INAND_CMD38_ARG_SECERASE 0x80 #define > INAND_CMD38_ARG_SECTRIM1 0x81 #define > INAND_CMD38_ARG_SECTRIM2 0x88 > - /* CMD38 argument is passed through EXT_CSD[113] */ > - MMC_FIXUP("SEM02G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM04G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM08G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM16G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM32G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - > /* > * Some MMC cards experience performance degradation with > CMD23 > * instead of CMD12-bounded multiblock transfers. For now we'll diff > --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index > 7b12eebc5586..edb27f3c6489 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -284,7 +284,6 @@ struct mmc_card { > /* (missing CIA registers) */ > #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4) /* SDIO card > has nonstd function interfaces */ > #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect > CD/DAT[3] resistor */ > -#define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices > have broken CMD38 */ > #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid > CMD23 for regular multiblock */ > #define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid > sending 512 bytes in */ > /* byte mode */ > -- > 2.42.0
On Sun, 5 Nov 2023 at 11:29, Avri Altman <avri.altman@wdc.com> wrote: > > This quirk applies to an old set of Sandisk cards that are no longer > being sold for many years now, and it is vanishingly unlikely that will > ever meet a V6.7 kernel. Are you really sure about that? The postmarket-os products are being used in all kinds of different deployments, for example. > > I came across those old cards when I tried to decide what to do with > Sandisk cards that their quirk is designated by OEM id. For many years > we read wrong oemid, reading 16 bits instead of 8. Fixing it however, > turned out to be impractical, and it's better that each eMMC vendor will > handle its own devices. Right, but I assume the quirk is working as expected for these old Sandisk eMMCs, no? > > Signed-off-by: Avri Altman <avri.altman@wdc.com> Kind regards Uffe > --- > drivers/mmc/core/block.c | 32 +------------------------------- > drivers/mmc/core/quirks.h | 12 ------------ > include/linux/mmc/card.h | 1 - > 3 files changed, 1 insertion(+), 44 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index 152dfe593c43..6aa68ac1129e 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -1122,17 +1122,7 @@ static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req, > nr = blk_rq_sectors(req); > > do { > - err = 0; > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - erase_arg == MMC_TRIM_ARG ? > - INAND_CMD38_ARG_TRIM : > - INAND_CMD38_ARG_ERASE, > - card->ext_csd.generic_cmd6_time); > - } > - if (!err) > - err = mmc_erase(card, from, nr, erase_arg); > + err = mmc_erase(card, from, nr, erase_arg); > } while (err == -EIO && !mmc_blk_reset(md, card->host, type)); > if (err) > status = BLK_STS_IOERR; > @@ -1182,17 +1172,6 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq, > arg = MMC_SECURE_ERASE_ARG; > > retry: > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - arg == MMC_SECURE_TRIM1_ARG ? > - INAND_CMD38_ARG_SECTRIM1 : > - INAND_CMD38_ARG_SECERASE, > - card->ext_csd.generic_cmd6_time); > - if (err) > - goto out_retry; > - } > - > err = mmc_erase(card, from, nr, arg); > if (err == -EIO) > goto out_retry; > @@ -1202,15 +1181,6 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq, > } > > if (arg == MMC_SECURE_TRIM1_ARG) { > - if (card->quirks & MMC_QUIRK_INAND_CMD38) { > - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, > - INAND_CMD38_ARG_EXT_CSD, > - INAND_CMD38_ARG_SECTRIM2, > - card->ext_csd.generic_cmd6_time); > - if (err) > - goto out_retry; > - } > - > err = mmc_erase(card, from, nr, MMC_SECURE_TRIM2_ARG); > if (err == -EIO) > goto out_retry; > diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h > index cca71867bc4a..028f55f07ba3 100644 > --- a/drivers/mmc/core/quirks.h > +++ b/drivers/mmc/core/quirks.h > @@ -22,18 +22,6 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = { > #define INAND_CMD38_ARG_SECERASE 0x80 > #define INAND_CMD38_ARG_SECTRIM1 0x81 > #define INAND_CMD38_ARG_SECTRIM2 0x88 > - /* CMD38 argument is passed through EXT_CSD[113] */ > - MMC_FIXUP("SEM02G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM04G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM08G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM16G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - MMC_FIXUP("SEM32G", CID_MANFID_SANDISK, 0x100, add_quirk, > - MMC_QUIRK_INAND_CMD38), > - > /* > * Some MMC cards experience performance degradation with CMD23 > * instead of CMD12-bounded multiblock transfers. For now we'll > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index 7b12eebc5586..edb27f3c6489 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -284,7 +284,6 @@ struct mmc_card { > /* (missing CIA registers) */ > #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4) /* SDIO card has nonstd function interfaces */ > #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect CD/DAT[3] resistor */ > -#define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices have broken CMD38 */ > #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */ > #define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid sending 512 bytes in */ > /* byte mode */ > -- > 2.42.0 >
On Sun, Nov 5, 2023 at 11:30 AM Avri Altman <avri.altman@wdc.com> wrote: > This quirk applies to an old set of Sandisk cards that are no longer > being sold for many years now, and it is vanishingly unlikely that will > ever meet a V6.7 kernel. > > I came across those old cards when I tried to decide what to do with > Sandisk cards that their quirk is designated by OEM id. For many years > we read wrong oemid, reading 16 bits instead of 8. Fixing it however, > turned out to be impractical, and it's better that each eMMC vendor will > handle its own devices. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> NAK why are you assuming these are not in use with v6.7? One of these eMMC cards is in use on Nokia n900 which has an active user community: https://wiki.postmarketos.org/wiki/Nokia_N900_(nokia-n900) I am using several of them on different ST-Ericsson U8500-based systems, including the HREF reference design that even Ulf is using now and then. Yours, Linus Walleij
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 152dfe593c43..6aa68ac1129e 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1122,17 +1122,7 @@ static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req, nr = blk_rq_sectors(req); do { - err = 0; - if (card->quirks & MMC_QUIRK_INAND_CMD38) { - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, - INAND_CMD38_ARG_EXT_CSD, - erase_arg == MMC_TRIM_ARG ? - INAND_CMD38_ARG_TRIM : - INAND_CMD38_ARG_ERASE, - card->ext_csd.generic_cmd6_time); - } - if (!err) - err = mmc_erase(card, from, nr, erase_arg); + err = mmc_erase(card, from, nr, erase_arg); } while (err == -EIO && !mmc_blk_reset(md, card->host, type)); if (err) status = BLK_STS_IOERR; @@ -1182,17 +1172,6 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq, arg = MMC_SECURE_ERASE_ARG; retry: - if (card->quirks & MMC_QUIRK_INAND_CMD38) { - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, - INAND_CMD38_ARG_EXT_CSD, - arg == MMC_SECURE_TRIM1_ARG ? - INAND_CMD38_ARG_SECTRIM1 : - INAND_CMD38_ARG_SECERASE, - card->ext_csd.generic_cmd6_time); - if (err) - goto out_retry; - } - err = mmc_erase(card, from, nr, arg); if (err == -EIO) goto out_retry; @@ -1202,15 +1181,6 @@ static void mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq, } if (arg == MMC_SECURE_TRIM1_ARG) { - if (card->quirks & MMC_QUIRK_INAND_CMD38) { - err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, - INAND_CMD38_ARG_EXT_CSD, - INAND_CMD38_ARG_SECTRIM2, - card->ext_csd.generic_cmd6_time); - if (err) - goto out_retry; - } - err = mmc_erase(card, from, nr, MMC_SECURE_TRIM2_ARG); if (err == -EIO) goto out_retry; diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index cca71867bc4a..028f55f07ba3 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -22,18 +22,6 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = { #define INAND_CMD38_ARG_SECERASE 0x80 #define INAND_CMD38_ARG_SECTRIM1 0x81 #define INAND_CMD38_ARG_SECTRIM2 0x88 - /* CMD38 argument is passed through EXT_CSD[113] */ - MMC_FIXUP("SEM02G", CID_MANFID_SANDISK, 0x100, add_quirk, - MMC_QUIRK_INAND_CMD38), - MMC_FIXUP("SEM04G", CID_MANFID_SANDISK, 0x100, add_quirk, - MMC_QUIRK_INAND_CMD38), - MMC_FIXUP("SEM08G", CID_MANFID_SANDISK, 0x100, add_quirk, - MMC_QUIRK_INAND_CMD38), - MMC_FIXUP("SEM16G", CID_MANFID_SANDISK, 0x100, add_quirk, - MMC_QUIRK_INAND_CMD38), - MMC_FIXUP("SEM32G", CID_MANFID_SANDISK, 0x100, add_quirk, - MMC_QUIRK_INAND_CMD38), - /* * Some MMC cards experience performance degradation with CMD23 * instead of CMD12-bounded multiblock transfers. For now we'll diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index 7b12eebc5586..edb27f3c6489 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -284,7 +284,6 @@ struct mmc_card { /* (missing CIA registers) */ #define MMC_QUIRK_NONSTD_FUNC_IF (1<<4) /* SDIO card has nonstd function interfaces */ #define MMC_QUIRK_DISABLE_CD (1<<5) /* disconnect CD/DAT[3] resistor */ -#define MMC_QUIRK_INAND_CMD38 (1<<6) /* iNAND devices have broken CMD38 */ #define MMC_QUIRK_BLK_NO_CMD23 (1<<7) /* Avoid CMD23 for regular multiblock */ #define MMC_QUIRK_BROKEN_BYTE_MODE_512 (1<<8) /* Avoid sending 512 bytes in */ /* byte mode */
This quirk applies to an old set of Sandisk cards that are no longer being sold for many years now, and it is vanishingly unlikely that will ever meet a V6.7 kernel. I came across those old cards when I tried to decide what to do with Sandisk cards that their quirk is designated by OEM id. For many years we read wrong oemid, reading 16 bits instead of 8. Fixing it however, turned out to be impractical, and it's better that each eMMC vendor will handle its own devices. Signed-off-by: Avri Altman <avri.altman@wdc.com> --- drivers/mmc/core/block.c | 32 +------------------------------- drivers/mmc/core/quirks.h | 12 ------------ include/linux/mmc/card.h | 1 - 3 files changed, 1 insertion(+), 44 deletions(-)