From b890b17324dfff1bca05885bd859cb7c1b4c368f Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Tue, 15 Oct 2024 11:22:53 +0100 Subject: [PATCH 1/6] mmc: don't reference requests after finishing them Posted write tracking introduced in the commit below raced with re-use of the requests between completion and submission, potentially causing underflow of the pending write count. Fixes: e6c1e862b2b8 ("mmc: restrict posted write counts for SD cards in CQ mode") Signed-off-by: Jonathan Bell --- drivers/mmc/core/block.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 3d08b20759aff9..bfdf2c4f98cba7 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1524,6 +1524,7 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req) struct request_queue *q = req->q; struct mmc_host *host = mq->card->host; enum mmc_issue_type issue_type = mmc_issue_type(mq, req); + bool write = req_op(req) == REQ_OP_WRITE; unsigned long flags; bool put_card; int err; @@ -1555,7 +1556,7 @@ static void mmc_blk_cqe_complete_rq(struct mmc_queue *mq, struct request *req) spin_lock_irqsave(&mq->lock, flags); - if (req_op(req) == REQ_OP_WRITE) + if (write) mq->pending_writes--; mq->in_flight[issue_type] -= 1; @@ -2170,15 +2171,16 @@ static void mmc_blk_mq_poll_completion(struct mmc_queue *mq, } static void mmc_blk_mq_dec_in_flight(struct mmc_queue *mq, enum mmc_issue_type issue_type, - struct request *req) + bool write) { unsigned long flags; bool put_card; spin_lock_irqsave(&mq->lock, flags); - if (req_op(req) == REQ_OP_WRITE) + if (write) mq->pending_writes--; + mq->in_flight[issue_type] -= 1; put_card = (mmc_tot_in_flight(mq) == 0); @@ -2193,6 +2195,7 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req, bool can_sleep) { enum mmc_issue_type issue_type = mmc_issue_type(mq, req); + bool write = req_op(req) == REQ_OP_WRITE; struct mmc_queue_req *mqrq = req_to_mmc_queue_req(req); struct mmc_request *mrq = &mqrq->brq.mrq; struct mmc_host *host = mq->card->host; @@ -2212,7 +2215,7 @@ static void mmc_blk_mq_post_req(struct mmc_queue *mq, struct request *req, blk_mq_complete_request(req); } - mmc_blk_mq_dec_in_flight(mq, issue_type, req); + mmc_blk_mq_dec_in_flight(mq, issue_type, write); } void mmc_blk_mq_recovery(struct mmc_queue *mq) From cf1b612522e23e0352fb56160d6dfd1c3bf93e58 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Tue, 15 Oct 2024 12:24:36 +0100 Subject: [PATCH 2/6] Revert "mmc: sd: halt CQHCI before issuing a cache flush command" __mmc_start_request turns CQE off before issuing the actual command so this additional call is redundant. This reverts commit 8e40644b272a4ddc9d3b58b4373dffcef02d1b63. --- drivers/mmc/core/sd.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index d445dd3abfec33..b1ea31d98442c1 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1273,14 +1273,6 @@ static int sd_flush_cache(struct mmc_host *host) reg_buf = card->ext_reg_buf; - /* - * Flushing requires sending CMD49 (adtc), which can't be done as a DCMD - * and conflicts with CQHCI - temporarily turn CQE off to use the SDHCI - * command/argument registers. - */ - if (host->cqe_on) - host->cqe_ops->cqe_off(host); - /* * Set Flush Cache at bit 0 in the performance enhancement register at * 261 bytes offset. From 0232b80e7683ba827e893ff7c788312b776eec31 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Tue, 15 Oct 2024 14:35:42 +0100 Subject: [PATCH 3/6] mmc: quirks: disable cache on more known-bad Sandisk card date ranges Cards with manufacture dates in 2019 and 2020 have been seen in the wild that hang indefinitely if issued a cache flush command in CQ mode. Signed-off-by: Jonathan Bell --- drivers/mmc/core/quirks.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index ddefedc9746e65..8607998175c65b 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -33,6 +33,18 @@ static const struct mmc_fixup __maybe_unused mmc_sd_fixups[] = { 0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd, MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY), + /* + * Early Sandisk Extreme and Extreme Pro A2 cards never finish SD cache + * flush in CQ mode. Latest card date this was seen on is 10/2020. + */ + _FIXUP_EXT(CID_NAME_ANY, CID_MANFID_SANDISK_SD, 0x5344, 2019, CID_MONTH_ANY, + 0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd, + MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY), + + _FIXUP_EXT(CID_NAME_ANY, CID_MANFID_SANDISK_SD, 0x5344, 2020, CID_MONTH_ANY, + 0, -1ull, SDIO_ANY_ID, SDIO_ANY_ID, add_quirk_sd, + MMC_QUIRK_BROKEN_SD_CACHE, EXT_CSD_REV_ANY), + END_FIXUP }; From 8ea0d8340e2a19053880e933cdc3188cc337b26c Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Fri, 18 Oct 2024 13:11:11 +0100 Subject: [PATCH 4/6] mmc: block: disable CQ on SD cards when doing non-Discard erase Only CMD38 with Arg=0x1 (Discard) is supported when in CQ mode, so turn it off before issuing a non-discard erase op. Signed-off-by: Jonathan Bell --- drivers/mmc/core/block.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index bfdf2c4f98cba7..d5aca7b6dac773 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1177,12 +1177,26 @@ static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req, unsigned int from, nr; int err = 0; blk_status_t status = BLK_STS_OK; + bool restart_cmdq = false; if (!mmc_can_erase(card)) { status = BLK_STS_NOTSUPP; goto fail; } + /* + * Only Discard ops are supported with SD cards in CQ mode + * (SD Physical Spec v9.00 4.19.2) + */ + if (mmc_card_sd(card) && card->ext_csd.cmdq_en && erase_arg != SD_DISCARD_ARG) { + restart_cmdq = true; + err = mmc_sd_cmdq_disable(card); + if (err) { + status = BLK_STS_IOERR; + goto fail; + } + } + from = blk_rq_pos(req); nr = blk_rq_sectors(req); @@ -1203,6 +1217,11 @@ static void mmc_blk_issue_erase_rq(struct mmc_queue *mq, struct request *req, status = BLK_STS_IOERR; else mmc_blk_reset_success(md, type); + + if (restart_cmdq) + err = mmc_sd_cmdq_enable(card); + if (err) + status = BLK_STS_IOERR; fail: blk_mq_end_request(req, status); } From 67f4346f1031d9482a1ab972ad39201815aebb06 Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Fri, 18 Oct 2024 14:06:01 +0100 Subject: [PATCH 5/6] DTS: bcm2712; re-enable SD slot CQE by default on Pi 5 This reverts commit 1b92c9369569137d8f2b8bf82884a05999e1f73b. Signed-off-by: Jonathan Bell --- arch/arm/boot/dts/overlays/README | 6 +++--- arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/overlays/README b/arch/arm/boot/dts/overlays/README index 47cea04fea5f88..e1df8b1ba3c77e 100644 --- a/arch/arm/boot/dts/overlays/README +++ b/arch/arm/boot/dts/overlays/README @@ -378,9 +378,9 @@ Params: non-lite SKU of CM4). (default "on") - sd_cqe Use to enable Command Queueing on the SD - interface for faster Class A2 card performance - (Pi 5 only, default "off") + sd_cqe Set to "off" to disable Command Queueing if you + have an incompatible Class A2 SD card + (Pi 5 only, default "on") sd_overclock Clock (in MHz) to use when the MMC framework requests 50MHz diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts index 25b489e41a5d0a..b35c6ac787af89 100644 --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts @@ -365,6 +365,7 @@ dpi_16bit_gpio2: &rp1_dpi_16bit_gpio2 { }; sd-uhs-sdr50; sd-uhs-ddr50; sd-uhs-sdr104; + supports-cqe; cd-gpios = <&gio_aon 5 GPIO_ACTIVE_LOW>; //no-1-8-v; status = "okay"; From d0abbb2931048f0512fae335b55ba81773b5519a Mon Sep 17 00:00:00 2001 From: Jonathan Bell Date: Fri, 18 Oct 2024 16:01:28 +0100 Subject: [PATCH 6/6] mmc: quirks: add MMC_QUIRK_BROKEN_ERASE for Phison/Integral cards Recent Integral cards end up with corrupt sectors after a flash erase. This covers sizes for the A2 range, which can't be differentiated from the A1 range which might not have the same issue. Signed-off-by: Jonathan Bell --- drivers/mmc/core/quirks.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/mmc/core/quirks.h b/drivers/mmc/core/quirks.h index 8607998175c65b..2c6921e6f6dd4a 100644 --- a/drivers/mmc/core/quirks.h +++ b/drivers/mmc/core/quirks.h @@ -162,6 +162,15 @@ static const struct mmc_fixup __maybe_unused mmc_blk_fixups[] = { MMC_FIXUP("SD32G", 0x41, 0x3432, add_quirk, MMC_QUIRK_ERASE_BROKEN), MMC_FIXUP("SD64G", 0x41, 0x3432, add_quirk, MMC_QUIRK_ERASE_BROKEN), + /* + * Larger Integral SD cards using rebranded Phison controllers trash + * nearby flash blocks after erases. + */ + MMC_FIXUP("SD64G", 0x27, 0x5048, add_quirk, MMC_QUIRK_ERASE_BROKEN), + MMC_FIXUP("SD128", 0x27, 0x5048, add_quirk, MMC_QUIRK_ERASE_BROKEN), + MMC_FIXUP("SD256", 0x27, 0x5048, add_quirk, MMC_QUIRK_ERASE_BROKEN), + MMC_FIXUP("SD512", 0x27, 0x5048, add_quirk, MMC_QUIRK_ERASE_BROKEN), + END_FIXUP };