* [PATCH RFT 0/3] mmc: tmio: honor busy timeouts properly
@ 2020-11-20 14:26 Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout Wolfram Sang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Wolfram Sang @ 2020-11-20 14:26 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Yoshihiro Shimoda, Masahiro Yamada, Wolfram Sang
TMIO has a HW busy timeout detection which was always running but the
driver did not handle it yet in a proper way. This series fixes that.
Patch 1 sets max_busy_timeout, so the MMC core will not use R1B when it
is beyond the maximum HW timeout. Patch 2 adds proper handling of the
timeout interrupt. After ensuring both, we can finally enable the
WAIT_WHILE_BUSY capability to save the polling.
This was tested using my Renesas Salvator-XS (R-Car M3-N) and Lager
(R-Car H2). However, more advanced testing from the BSP team and/or
Shimoda-san would be much appreciated.
The branch is based on mmc/next plus one patch "[PATCH] mmc: tmio: Fix
command error processing". A branch is available here:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/busy_timeout
Yamada-san: I enabled setting max_busy_timeout for Uniphier as well
because I am quite sure this HW supports it and it will fix timeout
issues with long erase timeouts. I did not enable WAIT_WHILE_BUSY
because this is "only" an improvement and not a fix. I think opt-in
after some testing is good for that case.
I think all patches are 5.11 material to get enough testing. Patch 1
is IMHO stable material, nonetheless. I am open for discussions, of
course.
Looking forward to comments, reviews, tags...
Thanks and happy hacking,
Wolfram
Masaharu Hayakawa (1):
mmc: tmio: Add data timeout error detection
Wolfram Sang (2):
mmc: tmio: set max_busy_timeout
mmc: renesas_sdhi: enable WAIT_WHILE_BUSY
drivers/mmc/host/renesas_sdhi_core.c | 3 ++
drivers/mmc/host/renesas_sdhi_internal_dmac.c | 4 +--
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 7 ++--
drivers/mmc/host/tmio_mmc.h | 8 +++--
drivers/mmc/host/tmio_mmc_core.c | 32 +++++++++++++++----
drivers/mmc/host/uniphier-sd.c | 1 +
include/linux/mfd/tmio.h | 7 +++-
7 files changed, 48 insertions(+), 14 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout
2020-11-20 14:26 [PATCH RFT 0/3] mmc: tmio: honor busy timeouts properly Wolfram Sang
@ 2020-11-20 14:26 ` Wolfram Sang
2020-11-20 14:51 ` Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 2/3] mmc: tmio: Add data timeout error detection Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 3/3] mmc: renesas_sdhi: enable WAIT_WHILE_BUSY Wolfram Sang
2 siblings, 1 reply; 5+ messages in thread
From: Wolfram Sang @ 2020-11-20 14:26 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Yoshihiro Shimoda, Masahiro Yamada, Wolfram Sang
Set max_busy_timeouts for variants known to support the TOPxx bits in
the SD_OPTION register. The timeout mechanism was running in the
background but not yet properly handled in the driver. So, let the MMC
core know when to not use R1B to avoid unhandled timeouts.
My datasheets for older variants (tmio_mmc.c) suggest that they support
it, too. However, actual bit descriptions are lacking, so I chose an
opt-in approach.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/renesas_sdhi_core.c | 3 +++
drivers/mmc/host/tmio_mmc.h | 2 ++
drivers/mmc/host/tmio_mmc_core.c | 15 +++++++++++++++
drivers/mmc/host/uniphier-sd.c | 1 +
include/linux/mfd/tmio.h | 7 ++++++-
5 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index bb937411c2ec..153767054c05 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -1041,6 +1041,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
/* All SDHI have SDIO status bits which must be 1 */
mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
+ /* All SDHI support HW busy detection */
+ mmc_data->flags |= TMIO_MMC_USE_BUSY_TIMEOUT;
+
dev_pm_domain_start(&pdev->dev);
ret = renesas_sdhi_clk_enable(host);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 7ff41185896a..819198af17f4 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -80,6 +80,8 @@
#define CLK_CTL_SCLKEN BIT(8)
/* Definitions for values the CTL_SD_MEM_CARD_OPT register can take */
+#define CARD_OPT_TOP_MASK 0xf0
+#define CARD_OPT_TOP_SHIFT 4
#define CARD_OPT_WIDTH8 BIT(13)
#define CARD_OPT_WIDTH BIT(15)
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index cb4149fd12e0..2518fa9f9fc6 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -885,6 +885,18 @@ static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, reg);
}
+static void tmio_mmc_max_busy_timeout(struct tmio_mmc_host *host)
+{
+ u16 val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
+ unsigned int clk_rate = host->mmc->actual_clock ?: host->mmc->f_max;
+ unsigned int cycles;
+
+ val = (val & CARD_OPT_TOP_MASK) >> CARD_OPT_TOP_SHIFT;
+ cycles = 1 << (13 + val);
+
+ host->mmc->max_busy_timeout = cycles / (clk_rate * MSEC_PER_SEC);
+}
+
/* Set MMC clock / power.
* Note: This controller uses a simple divider scheme therefore it cannot
* run a MMC card at full speed (20MHz). The max clock is 24MHz on SD, but as
@@ -943,6 +955,9 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
break;
}
+ if (host->pdata->flags & TMIO_MMC_USE_BUSY_TIMEOUT)
+ tmio_mmc_max_busy_timeout(host);
+
/* Let things settle. delay taken from winCE driver */
usleep_range(140, 200);
if (PTR_ERR(host->mrq) == -EINTR)
diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c
index 3092466a99ab..a6cd16771d4e 100644
--- a/drivers/mmc/host/uniphier-sd.c
+++ b/drivers/mmc/host/uniphier-sd.c
@@ -586,6 +586,7 @@ static int uniphier_sd_probe(struct platform_device *pdev)
tmio_data = &priv->tmio_data;
tmio_data->flags |= TMIO_MMC_32BIT_DATA_PORT;
+ tmio_data->flags |= TMIO_MMC_USE_BUSY_TIMEOUT;
host = tmio_mmc_host_alloc(pdev, tmio_data);
if (IS_ERR(host))
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 8ba042430d8e..27264fe4b3b9 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -55,7 +55,12 @@
*/
#define TMIO_MMC_HAS_IDLE_WAIT BIT(4)
-/* BIT(5) is unused */
+/*
+ * Use the busy timeout feature. Probably all TMIO versions support it. Yet,
+ * we don't have documentation for old variants, so we enable only known good
+ * variants with this flag. Can be removed once all variants are known good.
+ */
+#define TMIO_MMC_USE_BUSY_TIMEOUT BIT(5)
/*
* Some controllers have CMD12 automatically
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH RFT 2/3] mmc: tmio: Add data timeout error detection
2020-11-20 14:26 [PATCH RFT 0/3] mmc: tmio: honor busy timeouts properly Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout Wolfram Sang
@ 2020-11-20 14:26 ` Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 3/3] mmc: renesas_sdhi: enable WAIT_WHILE_BUSY Wolfram Sang
2 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2020-11-20 14:26 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Yoshihiro Shimoda, Masahiro Yamada,
Masaharu Hayakawa, Takeshi Saito, Wolfram Sang
From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
Currently, busy timeout is not checked for data transfer command. But,
if the temperature condition changes, the data cannot be acquired
correctly and timeout may occur. Also, we could reproduce an issue by
using mmc_test driver (e.g. "Correct xfer_size at write (start
failure)"). Therefore, this adds timeout error check.
Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com>
[saito: rework commit message.]
Signed-off-by: Takeshi Saito <takeshi.saito.xv@renesas.com>
[shimoda: rebase, add commit description]
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/tmio_mmc.h | 6 ++++--
drivers/mmc/host/tmio_mmc_core.c | 17 +++++++++++------
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 819198af17f4..15cf850e75d3 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -100,8 +100,10 @@
/* This is the mask used at reset by the chip */
#define TMIO_MASK_INIT_RCAR2 0x8b7f031d /* Initial value for R-Car Gen2+ */
#define TMIO_MASK_ALL 0x837f031d
-#define TMIO_MASK_READOP (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND)
-#define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND)
+#define TMIO_MASK_READOP (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND | \
+ TMIO_STAT_DATATIMEOUT)
+#define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND | \
+ TMIO_STAT_DATATIMEOUT)
#define TMIO_MASK_CMD (TMIO_STAT_CMDRESPEND | TMIO_STAT_CMDTIMEOUT | \
TMIO_STAT_CARD_REMOVE | TMIO_STAT_CARD_INSERT)
#define TMIO_MASK_IRQ (TMIO_MASK_READOP | TMIO_MASK_WRITEOP | TMIO_MASK_CMD)
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 2518fa9f9fc6..9c76bc5ac46c 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -477,8 +477,10 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
if (!data)
goto out;
- if (stat & TMIO_STAT_CRCFAIL || stat & TMIO_STAT_STOPBIT_ERR ||
- stat & TMIO_STAT_TXUNDERRUN)
+ if (stat & TMIO_STAT_DATATIMEOUT)
+ data->error = -ETIMEDOUT;
+ else if (stat & TMIO_STAT_CRCFAIL || stat & TMIO_STAT_STOPBIT_ERR ||
+ stat & TMIO_STAT_TXUNDERRUN)
data->error = -EILSEQ;
if (host->dma_on && (data->flags & MMC_DATA_WRITE)) {
u32 status = sd_ctrl_read16_and_16_as_32(host, CTL_STATUS);
@@ -501,11 +503,13 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
}
if (done) {
- tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND);
+ tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND |
+ TMIO_STAT_DATATIMEOUT);
tmio_mmc_dataend_dma(host);
}
} else if (host->dma_on && (data->flags & MMC_DATA_READ)) {
- tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND);
+ tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND |
+ TMIO_STAT_DATATIMEOUT);
tmio_mmc_dataend_dma(host);
} else {
tmio_mmc_do_data_irq(host);
@@ -619,8 +623,9 @@ static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host, int ireg,
}
/* Data transfer completion */
- if (ireg & TMIO_STAT_DATAEND) {
- tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND);
+ if (ireg & (TMIO_STAT_DATAEND | TMIO_STAT_DATATIMEOUT)) {
+ tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_DATAEND |
+ TMIO_STAT_DATATIMEOUT);
tmio_mmc_data_irq(host, status);
return true;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH RFT 3/3] mmc: renesas_sdhi: enable WAIT_WHILE_BUSY
2020-11-20 14:26 [PATCH RFT 0/3] mmc: tmio: honor busy timeouts properly Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 2/3] mmc: tmio: Add data timeout error detection Wolfram Sang
@ 2020-11-20 14:26 ` Wolfram Sang
2 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2020-11-20 14:26 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Yoshihiro Shimoda, Masahiro Yamada, Wolfram Sang
Now that we got the timeout handling in the driver correct, we can use
this capability to avoid polling via the MMC core.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/renesas_sdhi_internal_dmac.c | 4 ++--
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 7 ++++---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index fe13e1ea22dc..d8b811c46628 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -88,7 +88,7 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
static const struct renesas_sdhi_of_data of_rza2_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY,
+ TMIO_MMC_HAVE_CBSY | MMC_CAP_WAIT_WHILE_BUSY,
.tmio_ocr_mask = MMC_VDD_32_33,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
MMC_CAP_CMD23,
@@ -105,7 +105,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
+ MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY,
.capabilities2 = MMC_CAP2_NO_WRITE_PROTECT | MMC_CAP2_MERGE_CAPABLE,
.bus_shift = 2,
.scc_offset = 0x1000,
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index c5f789675302..0a3494fcc5e8 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -31,13 +31,14 @@ static const struct renesas_sdhi_of_data of_default_cfg = {
static const struct renesas_sdhi_of_data of_rz_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_32BIT_DATA_PORT |
- TMIO_MMC_HAVE_CBSY,
+ TMIO_MMC_HAVE_CBSY | MMC_CAP_WAIT_WHILE_BUSY,
.tmio_ocr_mask = MMC_VDD_32_33,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
};
static const struct renesas_sdhi_of_data of_rcar_gen1_compatible = {
- .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL,
+ .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
+ MMC_CAP_WAIT_WHILE_BUSY,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ,
.capabilities2 = MMC_CAP2_NO_WRITE_PROTECT,
};
@@ -58,7 +59,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen2_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
+ MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY,
.capabilities2 = MMC_CAP2_NO_WRITE_PROTECT,
.dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES,
.dma_rx_offset = 0x2000,
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout
2020-11-20 14:26 ` [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout Wolfram Sang
@ 2020-11-20 14:51 ` Wolfram Sang
0 siblings, 0 replies; 5+ messages in thread
From: Wolfram Sang @ 2020-11-20 14:51 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Masahiro Yamada
[-- Attachment #1: Type: text/plain, Size: 155 bytes --]
> + host->mmc->max_busy_timeout = cycles / (clk_rate * MSEC_PER_SEC);
This should be "/ MSEC_PER_SEC"! Sorry, it was a bad merge conflict
resolution :(
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-20 14:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 14:26 [PATCH RFT 0/3] mmc: tmio: honor busy timeouts properly Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 1/3] mmc: tmio: set max_busy_timeout Wolfram Sang
2020-11-20 14:51 ` Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 2/3] mmc: tmio: Add data timeout error detection Wolfram Sang
2020-11-20 14:26 ` [PATCH RFT 3/3] mmc: renesas_sdhi: enable WAIT_WHILE_BUSY Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).