* [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support
@ 2026-05-07 7:02 Sascha Hauer
2026-05-07 7:02 ` [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock Sascha Hauer
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX; +Cc: Sascha Hauer, Claude Opus 4.7
At least on RK3588 the dwcmshc core doesn't have an internal clock
divider, we fully rely on the clock tree to configure the MMC clock.
By default the clock comes from the 24MHz oscillator. For higher MMC
clocks we have to reparent to a PLL clock, but if we do this once the
6bit divider iss not enough to scale down to the 400kHz MMC
initialization clock. This means we must dynamically reparent the clock.
This series adds support for finding the best divider/mux combination
for composite clocks.
This series also adds some fixes to the dwcmshc driver which used to
timeout on writing sometimes.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (6):
mci: sdhci: rockchip: set hidspd before re-enabling the clock
mci: sdhci: rockchip: disable clock while setting DLL
mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd
mci: sdhci: rockchip: Update pre-change delay for rockchip platform
clk: composite: pick best parent for round_rate / set_rate
mci: sdhci: rockchip: officially support HS200
drivers/clk/clk-composite.c | 110 ++++++++++++++++++++++++++++++-----
drivers/mci/rockchip-dwcmshc-sdhci.c | 51 ++++++++++------
2 files changed, 128 insertions(+), 33 deletions(-)
---
base-commit: 019d102038a64e6b6e8f445cbfd2d15e68d0ec3f
change-id: 20260507-rockchip-emmc-0e8c5097cf33
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 8:05 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL Sascha Hauer
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX
Linux changes the SDHCI_CTRL_HISPD bit while the clock is disabled. While
I haven't observed any failures because of this, do the same as Linux to be
on the safe side.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/rockchip-dwcmshc-sdhci.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 8e55190eb8..981f90af63 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -146,6 +146,15 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
clk_set_rate(host->clks[CLK_CORE].clk, clock);
+ extra = sdhci_read8(&host->sdhci, SDHCI_HOST_CONTROL);
+
+ if (clock > 26000000)
+ extra |= SDHCI_CTRL_HISPD;
+ else
+ extra &= ~SDHCI_CTRL_HISPD;
+
+ sdhci_write8(&host->sdhci, SDHCI_HOST_CONTROL, extra);
+
sdhci_set_clock(&host->sdhci, clock, clk_get_rate(host->clks[CLK_CORE].clk));
/* Disable cmd conflict check */
@@ -212,7 +221,6 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
{
struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
- u16 val;
/* stop clock */
sdhci_write16(&host->sdhci, SDHCI_CLOCK_CONTROL, 0);
@@ -221,15 +229,6 @@ static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
rk_sdhci_set_clock(host, ios->clock);
sdhci_set_bus_width(&host->sdhci, ios->bus_width);
-
- val = sdhci_read8(&host->sdhci, SDHCI_HOST_CONTROL);
-
- if (ios->clock > 26000000)
- val |= SDHCI_CTRL_HISPD;
- else
- val &= ~SDHCI_CTRL_HISPD;
-
- sdhci_write8(&host->sdhci, SDHCI_HOST_CONTROL, val);
}
static void print_error(struct rk_sdhci_host *host, int cmdidx)
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
2026-05-07 7:02 ` [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 8:10 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd Sascha Hauer
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX
sdhci_set_clock() enabled the clock. Linux explicitly disables the clock
after that is called and before the DLL is configured. Do likewise in barebox.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/rockchip-dwcmshc-sdhci.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 981f90af63..398c258fb9 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -162,6 +162,9 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
extra &= ~BIT(0);
sdhci_write32(&host->sdhci, DWCMSHC_HOST_CTRL3, extra);
+ /* Disable clock while config DLL */
+ sdhci_write16(&host->sdhci, SDHCI_CLOCK_CONTROL, 0);
+
if (clock <= 52000000) {
/*
* Disable DLL and reset both of sample and drive clock.
@@ -181,7 +184,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
DLL_STRBIN_DELAY_NUM_SEL |
DLL_STRBIN_DELAY_NUM_DEFAULT << DLL_STRBIN_DELAY_NUM_OFFSET;
sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_STRBIN, extra);
- return;
+ goto enable_clock;
}
/* Reset DLL */
@@ -199,7 +202,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
500 * USEC_PER_MSEC);
if (err) {
dev_err(host->mci.hw_dev, "DLL lock timeout!\n");
- return;
+ goto enable_clock;
}
extra = 0x1 << 16 | /* tune clock stop en */
@@ -216,6 +219,10 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
DLL_STRBIN_TAPNUM_DEFAULT |
DLL_STRBIN_TAPNUM_FROM_SW;
sdhci_write32(&host->sdhci, DWCMSHC_EMMC_DLL_STRBIN, extra);
+enable_clock:
+ sdhci_enable_clk(&host->sdhci, 0);
+
+ return;
}
static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
2026-05-07 7:02 ` [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock Sascha Hauer
2026-05-07 7:02 ` [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 8:11 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform Sascha Hauer
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX
The MMC_RSP_BUSY flag indicates that we have to wait for the transfer
completion interrupt. Other SDHCI drivers do this, but the rockchip driver
missed it.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/rockchip-dwcmshc-sdhci.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 398c258fb9..2605a1368f 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -251,7 +251,7 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
{
struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
struct mci_data *data = cmd->data;
- u32 command, xfer;
+ u32 mask, command, xfer;
int ret;
dma_addr_t dma;
@@ -261,6 +261,10 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
+ mask = SDHCI_INT_CMD_COMPLETE;
+ if (cmd->resp_type & MMC_RSP_BUSY)
+ mask |= SDHCI_INT_XFER_COMPLETE;
+
sdhci_write8(&host->sdhci, SDHCI_TIMEOUT_CONTROL, 0xe);
sdhci_setup_data_dma(&host->sdhci, data, &dma);
@@ -274,7 +278,7 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
sdhci_write32(&host->sdhci, SDHCI_ARGUMENT, cmd->cmdarg);
sdhci_write16(&host->sdhci, SDHCI_COMMAND, command);
- ret = sdhci_wait_for_done(&host->sdhci, SDHCI_INT_CMD_COMPLETE);
+ ret = sdhci_wait_for_done(&host->sdhci, mask);
if (ret) {
sdhci_teardown_data(&host->sdhci, data, dma);
goto error;
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
` (2 preceding siblings ...)
2026-05-07 7:02 ` [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 8:12 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 5/6] clk: composite: pick best parent for round_rate / set_rate Sascha Hauer
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX
This is an adoption from Linux commit b75a52b0dda35 ("mmc: sdhci-of-dwcmshc:
Update DLL and pre-change delay for rockchip platform"). We already have
most of it, this only adds the pre-change delay part.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/rockchip-dwcmshc-sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 2605a1368f..9ffd52e7e0 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -206,7 +206,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
}
extra = 0x1 << 16 | /* tune clock stop en */
- 0x2 << 17 | /* pre-change delay */
+ 0x3 << 17 | /* pre-change delay */
0x3 << 19; /* post-change delay */
sdhci_write32(&host->sdhci, DWCMSHC_EMMC_ATCTRL, extra);
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] clk: composite: pick best parent for round_rate / set_rate
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
` (3 preceding siblings ...)
2026-05-07 7:02 ` [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 7:02 ` [PATCH 6/6] mci: sdhci: rockchip: officially support HS200 Sascha Hauer
2026-05-07 7:47 ` [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Ahmad Fatoum
6 siblings, 0 replies; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX; +Cc: Sascha Hauer, Claude Opus 4.7
From: Sascha Hauer <sascha@saschahauer.de>
Currently our clk composite driver uses the set_rate hook to set the rate
when available, otherwise it uses the mux to set the rate, but it never
tries to find the best combination of both. This patch fills the gap.
Doing this is necessary for example on Rockchip RK3588 where the clock
is adjustable only external to the SDHC controller. The knobs are a
6bit divider and a mux. The divider alone is not sufficient to scale a
high frequency down to the necessary 400kHz init clock while the 24MHz
clock is not high enough for faster speeds.
Walk all parents and pick the one whose rate-clock gets closest to
the requested rate, then reparent if needed before applying the
divider.
For mux-only, rate-only and clocks with CLK_SET_RATE_NO_REPARENT the
previous behaviour is preserved.
Assisted-by: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/clk/clk-composite.c | 110 ++++++++++++++++++++++++++++++++++++++------
1 file changed, 95 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index e8f1fa7a72..a23e828f58 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -39,16 +39,91 @@ static unsigned long clk_composite_recalc_rate(struct clk_hw *hw,
return parent_rate;
}
+/*
+ * Walk all parents of a (rate_hw + mux_hw) composite and pick the one
+ * whose rate-clock can get closest to the requested rate. Returns the
+ * chosen parent's index in *out_idx (or -1 to mean "stay on current parent"),
+ * the parent's rate in *parent_rate, and the achievable output rate as the
+ * function value.
+ * Falls back to round_rate on the current parent when reparenting
+ * isn't possible (no mux, or CLK_SET_RATE_NO_REPARENT).
+ */
+static long clk_composite_pick_parent(struct clk_hw *hw, unsigned long rate,
+ int *out_idx, unsigned long *parent_rate)
+{
+ struct clk_composite *composite = to_clk_composite(hw);
+ struct clk_hw *rate_hw = composite->rate_hw;
+ struct clk_hw *mux_hw = composite->mux_hw;
+ unsigned long best_rate = 0, best_prate = 0, best_diff = ULONG_MAX;
+ int best_idx = -1;
+ int i;
+
+ if (!rate_hw || !rate_hw->clk.ops->round_rate)
+ return -ENOSYS;
+
+ if (!mux_hw || (hw->clk.flags & CLK_SET_RATE_NO_REPARENT)) {
+ unsigned long prate = *parent_rate;
+ long achievable;
+
+ achievable = rate_hw->clk.ops->round_rate(rate_hw, rate, &prate);
+ if (achievable < 0)
+ return achievable;
+
+ *out_idx = -1;
+ *parent_rate = prate;
+ return achievable;
+ }
+
+ for (i = 0; i < hw->clk.num_parents; i++) {
+ struct clk_hw *p_hw = clk_hw_get_parent_by_index(hw, i);
+ unsigned long prate, diff;
+ long achievable;
+
+ if (!p_hw)
+ continue;
+
+ prate = clk_hw_get_rate(p_hw);
+ achievable = rate_hw->clk.ops->round_rate(rate_hw, rate, &prate);
+ if (achievable < 0)
+ continue;
+
+ diff = (achievable >= rate) ? achievable - rate
+ : rate - achievable;
+
+ if (diff < best_diff) {
+ best_idx = i;
+ best_prate = prate;
+ best_rate = achievable;
+ best_diff = diff;
+ if (!diff)
+ break;
+ }
+ }
+
+ if (best_idx < 0)
+ return -EINVAL;
+
+ *out_idx = best_idx;
+ *parent_rate = best_prate;
+ return best_rate;
+}
+
static long clk_composite_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
struct clk_composite *composite = to_clk_composite(hw);
- struct clk_hw *rate_hw = composite->rate_hw;
struct clk_hw *mux_hw = composite->mux_hw;
+ int idx;
+ long achievable;
- if (rate_hw)
- return rate_hw->clk.ops->round_rate(rate_hw, rate, prate);
+ achievable = clk_composite_pick_parent(hw, rate, &idx, prate);
+ if (achievable >= 0)
+ return achievable;
+
+ if (achievable != -ENOSYS)
+ return achievable;
+ /* No rate_hw — fall back to mux's round_rate if available. */
if (!(hw->clk.flags & CLK_SET_RATE_NO_REPARENT) &&
mux_hw &&
mux_hw->clk.ops->round_rate)
@@ -63,24 +138,29 @@ static int clk_composite_set_rate(struct clk_hw *hw, unsigned long rate,
struct clk_composite *composite = to_clk_composite(hw);
struct clk_hw *rate_hw = composite->rate_hw;
struct clk_hw *mux_hw = composite->mux_hw;
+ int idx = -1;
+ long achievable;
+
+ achievable = clk_composite_pick_parent(hw, rate, &idx, &parent_rate);
+ if (achievable >= 0) {
+ if (idx >= 0 && mux_hw && mux_hw->clk.ops->set_parent) {
+ int ret = mux_hw->clk.ops->set_parent(mux_hw, idx);
+ if (ret)
+ return ret;
+ }
+ return rate_hw->clk.ops->set_rate(rate_hw, rate, parent_rate);
+ }
+
+ if (achievable != -ENOSYS)
+ return achievable;
/*
- * When the rate clock is present use that to set the rate,
- * otherwise try the mux clock. We currently do not support
- * to find the best rate using a combination of both.
+ * No rate_hw. Fall back to letting the mux clk reparent itself,
+ * preserving the existing enable-count handoff.
*/
- if (rate_hw)
- return rate_hw->clk.ops->set_rate(rate_hw, rate, parent_rate);
-
if (!(hw->clk.flags & CLK_SET_RATE_NO_REPARENT) &&
mux_hw &&
mux_hw->clk.ops->set_rate) {
- /*
- * We'll call set_rate on the mux clk which in turn results
- * in reparenting the mux clk. Make sure the enable count
- * (which is stored in the composite clk, not the mux clk)
- * is transferred correctly.
- */
mux_hw->clk.enable_count = hw->clk.enable_count;
return mux_hw->clk.ops->set_rate(mux_hw, rate, parent_rate);
}
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/6] mci: sdhci: rockchip: officially support HS200
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
` (4 preceding siblings ...)
2026-05-07 7:02 ` [PATCH 5/6] clk: composite: pick best parent for round_rate / set_rate Sascha Hauer
@ 2026-05-07 7:02 ` Sascha Hauer
2026-05-07 8:15 ` Ahmad Fatoum
2026-05-07 7:47 ` [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Ahmad Fatoum
6 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 7:02 UTC (permalink / raw)
To: BAREBOX
"officially" is meant in the way that even with the SDHCI_QUIRK2_BROKEN_HS200
flag set the driver already does HS200 when the device tree mandates it.
Just remove the ineffective flag and wire up the execute_tuning hook.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/rockchip-dwcmshc-sdhci.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
index 9ffd52e7e0..04ee528f07 100644
--- a/drivers/mci/rockchip-dwcmshc-sdhci.c
+++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
@@ -300,11 +300,19 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
return ret;
}
+static int rk_sdhci_execute_tuning(struct mci_host *mci, u32 opcode)
+{
+ struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
+
+ return sdhci_execute_tuning(&host->sdhci, opcode);
+}
+
static const struct mci_ops rk_sdhci_ops = {
.send_cmd = rk_sdhci_send_cmd,
.set_ios = rk_sdhci_set_ios,
.init = rk_sdhci_init,
.card_present = rk_sdhci_card_present,
+ .execute_tuning = rk_sdhci_execute_tuning,
};
static int rk_sdhci_probe(struct device *dev)
@@ -351,9 +359,6 @@ static int rk_sdhci_probe(struct device *dev)
mci_of_parse(&host->mci);
- /* HS200 not supported by this driver at the moment */
- host->sdhci.quirks2 = SDHCI_QUIRK2_BROKEN_HS200;
-
sdhci_setup_host(&host->sdhci);
dev->priv = host;
--
2.47.3
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
` (5 preceding siblings ...)
2026-05-07 7:02 ` [PATCH 6/6] mci: sdhci: rockchip: officially support HS200 Sascha Hauer
@ 2026-05-07 7:47 ` Ahmad Fatoum
6 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 7:47 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX; +Cc: Sascha Hauer, Claude Opus 4.7
Hi,
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> At least on RK3588 the dwcmshc core doesn't have an internal clock
> divider, we fully rely on the clock tree to configure the MMC clock.
> By default the clock comes from the 24MHz oscillator. For higher MMC
> clocks we have to reparent to a PLL clock, but if we do this once the
> 6bit divider iss not enough to scale down to the 400kHz MMC
> initialization clock. This means we must dynamically reparent the clock.
> This series adds support for finding the best divider/mux combination
> for composite clocks.
>
> This series also adds some fixes to the dwcmshc driver which used to
> timeout on writing sometimes.
\o/
How much faster was the reading now in your testing?
Cheers,
Ahmad
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Sascha Hauer (6):
> mci: sdhci: rockchip: set hidspd before re-enabling the clock
> mci: sdhci: rockchip: disable clock while setting DLL
> mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd
> mci: sdhci: rockchip: Update pre-change delay for rockchip platform
> clk: composite: pick best parent for round_rate / set_rate
> mci: sdhci: rockchip: officially support HS200
>
> drivers/clk/clk-composite.c | 110 ++++++++++++++++++++++++++++++-----
> drivers/mci/rockchip-dwcmshc-sdhci.c | 51 ++++++++++------
> 2 files changed, 128 insertions(+), 33 deletions(-)
> ---
> base-commit: 019d102038a64e6b6e8f445cbfd2d15e68d0ec3f
> change-id: 20260507-rockchip-emmc-0e8c5097cf33
>
> Best regards,
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock
2026-05-07 7:02 ` [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock Sascha Hauer
@ 2026-05-07 8:05 ` Ahmad Fatoum
0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 8:05 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> Linux changes the SDHCI_CTRL_HISPD bit while the clock is disabled. While
> I haven't observed any failures because of this, do the same as Linux to be
> on the safe side.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/mci/rockchip-dwcmshc-sdhci.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 8e55190eb8..981f90af63 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -146,6 +146,15 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
>
> clk_set_rate(host->clks[CLK_CORE].clk, clock);
>
> + extra = sdhci_read8(&host->sdhci, SDHCI_HOST_CONTROL);
> +
> + if (clock > 26000000)
> + extra |= SDHCI_CTRL_HISPD;
> + else
> + extra &= ~SDHCI_CTRL_HISPD;
> +
> + sdhci_write8(&host->sdhci, SDHCI_HOST_CONTROL, extra);
> +
> sdhci_set_clock(&host->sdhci, clock, clk_get_rate(host->clks[CLK_CORE].clk));
>
> /* Disable cmd conflict check */
> @@ -212,7 +221,6 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
> static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
> {
> struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
> - u16 val;
>
> /* stop clock */
> sdhci_write16(&host->sdhci, SDHCI_CLOCK_CONTROL, 0);
> @@ -221,15 +229,6 @@ static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
> rk_sdhci_set_clock(host, ios->clock);
>
> sdhci_set_bus_width(&host->sdhci, ios->bus_width);
> -
> - val = sdhci_read8(&host->sdhci, SDHCI_HOST_CONTROL);
> -
> - if (ios->clock > 26000000)
> - val |= SDHCI_CTRL_HISPD;
> - else
> - val &= ~SDHCI_CTRL_HISPD;
> -
> - sdhci_write8(&host->sdhci, SDHCI_HOST_CONTROL, val);
> }
>
> static void print_error(struct rk_sdhci_host *host, int cmdidx)
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL
2026-05-07 7:02 ` [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL Sascha Hauer
@ 2026-05-07 8:10 ` Ahmad Fatoum
0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 8:10 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> sdhci_set_clock() enabled the clock. Linux explicitly disables the clock
> after that is called and before the DLL is configured. Do likewise in barebox.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> +enable_clock:
> + sdhci_enable_clk(&host->sdhci, 0);
> +
> + return;
Nit: Redundant return
> }
>
> static void rk_sdhci_set_ios(struct mci_host *mci, struct mci_ios *ios)
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd
2026-05-07 7:02 ` [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd Sascha Hauer
@ 2026-05-07 8:11 ` Ahmad Fatoum
0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 8:11 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> The MMC_RSP_BUSY flag indicates that we have to wait for the transfer
> completion interrupt. Other SDHCI drivers do this, but the rockchip driver
> missed it.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/mci/rockchip-dwcmshc-sdhci.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 398c258fb9..2605a1368f 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -251,7 +251,7 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
> {
> struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
> struct mci_data *data = cmd->data;
> - u32 command, xfer;
> + u32 mask, command, xfer;
> int ret;
> dma_addr_t dma;
>
> @@ -261,6 +261,10 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
>
> sdhci_write32(&host->sdhci, SDHCI_INT_STATUS, ~0);
>
> + mask = SDHCI_INT_CMD_COMPLETE;
> + if (cmd->resp_type & MMC_RSP_BUSY)
> + mask |= SDHCI_INT_XFER_COMPLETE;
> +
> sdhci_write8(&host->sdhci, SDHCI_TIMEOUT_CONTROL, 0xe);
>
> sdhci_setup_data_dma(&host->sdhci, data, &dma);
> @@ -274,7 +278,7 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
> sdhci_write32(&host->sdhci, SDHCI_ARGUMENT, cmd->cmdarg);
> sdhci_write16(&host->sdhci, SDHCI_COMMAND, command);
>
> - ret = sdhci_wait_for_done(&host->sdhci, SDHCI_INT_CMD_COMPLETE);
> + ret = sdhci_wait_for_done(&host->sdhci, mask);
> if (ret) {
> sdhci_teardown_data(&host->sdhci, data, dma);
> goto error;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform
2026-05-07 7:02 ` [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform Sascha Hauer
@ 2026-05-07 8:12 ` Ahmad Fatoum
0 siblings, 0 replies; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 8:12 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> This is an adoption from Linux commit b75a52b0dda35 ("mmc: sdhci-of-dwcmshc:
> Update DLL and pre-change delay for rockchip platform"). We already have
> most of it, this only adds the pre-change delay part.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/mci/rockchip-dwcmshc-sdhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 2605a1368f..9ffd52e7e0 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -206,7 +206,7 @@ static void rk_sdhci_set_clock(struct rk_sdhci_host *host, unsigned int clock)
> }
>
> extra = 0x1 << 16 | /* tune clock stop en */
> - 0x2 << 17 | /* pre-change delay */
> + 0x3 << 17 | /* pre-change delay */
> 0x3 << 19; /* post-change delay */
> sdhci_write32(&host->sdhci, DWCMSHC_EMMC_ATCTRL, extra);
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] mci: sdhci: rockchip: officially support HS200
2026-05-07 7:02 ` [PATCH 6/6] mci: sdhci: rockchip: officially support HS200 Sascha Hauer
@ 2026-05-07 8:15 ` Ahmad Fatoum
2026-05-07 8:36 ` Sascha Hauer
0 siblings, 1 reply; 15+ messages in thread
From: Ahmad Fatoum @ 2026-05-07 8:15 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
Hi,
On 5/7/26 9:02 AM, Sascha Hauer wrote:
> "officially" is meant in the way that even with the SDHCI_QUIRK2_BROKEN_HS200
> flag set the driver already does HS200 when the device tree mandates it.
Shouldn't we change mci-core.c to evaluate MMC_CAP2_HS200 then?
> Just remove the ineffective flag and wire up the execute_tuning hook.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cheers,
Ahmad
> ---
> drivers/mci/rockchip-dwcmshc-sdhci.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mci/rockchip-dwcmshc-sdhci.c b/drivers/mci/rockchip-dwcmshc-sdhci.c
> index 9ffd52e7e0..04ee528f07 100644
> --- a/drivers/mci/rockchip-dwcmshc-sdhci.c
> +++ b/drivers/mci/rockchip-dwcmshc-sdhci.c
> @@ -300,11 +300,19 @@ static int rk_sdhci_send_cmd(struct mci_host *mci, struct mci_cmd *cmd)
> return ret;
> }
>
> +static int rk_sdhci_execute_tuning(struct mci_host *mci, u32 opcode)
> +{
> + struct rk_sdhci_host *host = to_rk_sdhci_host(mci);
> +
> + return sdhci_execute_tuning(&host->sdhci, opcode);
> +}
> +
> static const struct mci_ops rk_sdhci_ops = {
> .send_cmd = rk_sdhci_send_cmd,
> .set_ios = rk_sdhci_set_ios,
> .init = rk_sdhci_init,
> .card_present = rk_sdhci_card_present,
> + .execute_tuning = rk_sdhci_execute_tuning,
> };
>
> static int rk_sdhci_probe(struct device *dev)
> @@ -351,9 +359,6 @@ static int rk_sdhci_probe(struct device *dev)
>
> mci_of_parse(&host->mci);
>
> - /* HS200 not supported by this driver at the moment */
> - host->sdhci.quirks2 = SDHCI_QUIRK2_BROKEN_HS200;
> -
> sdhci_setup_host(&host->sdhci);
>
> dev->priv = host;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 6/6] mci: sdhci: rockchip: officially support HS200
2026-05-07 8:15 ` Ahmad Fatoum
@ 2026-05-07 8:36 ` Sascha Hauer
0 siblings, 0 replies; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 8:36 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: BAREBOX
On 2026-05-07 10:15, Ahmad Fatoum wrote:
> Hi,
>
> On 5/7/26 9:02 AM, Sascha Hauer wrote:
> > "officially" is meant in the way that even with the SDHCI_QUIRK2_BROKEN_HS200
> > flag set the driver already does HS200 when the device tree mandates it.
>
> Shouldn't we change mci-core.c to evaluate MMC_CAP2_HS200 then?
It already does. The problem is that when the device tree has the
"mmc-hs200-1_8v" property than it's authorative. The SDHCI_QUIRK2_BROKEN_HS200
only prevents the MMC_CAP2_HS200 from being set based on other caps, but
it doesn't clear MMC_CAP2_HS200 when SDHCI_QUIRK2_BROKEN_HS200 is set.
The behaviour is the same as in Linux, so I hesitated from changing this.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support
@ 2026-05-07 8:03 Sascha Hauer
0 siblings, 0 replies; 15+ messages in thread
From: Sascha Hauer @ 2026-05-07 8:03 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: BAREBOX, Claude Opus 4.7, Sascha Hauer
On 2026-05-07 09:47, Ahmad Fatoum wrote:
> Hi,
>
> On 5/7/26 9:02 AM, Sascha Hauer wrote:
> > At least on RK3588 the dwcmshc core doesn't have an internal clock
> > divider, we fully rely on the clock tree to configure the MMC clock.
> > By default the clock comes from the 24MHz oscillator. For higher MMC
> > clocks we have to reparent to a PLL clock, but if we do this once the
> > 6bit divider iss not enough to scale down to the 400kHz MMC
> > initialization clock. This means we must dynamically reparent the clock.
> > This series adds support for finding the best divider/mux combination
> > for composite clocks.
> >
> > This series also adds some fixes to the dwcmshc driver which used to
> > timeout on writing sometimes.
>
> \o/
>
> How much faster was the reading now in your testing?
I now reach 120MB/s compared to 20MB/s before.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-05-07 9:21 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-07 7:02 [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Sascha Hauer
2026-05-07 7:02 ` [PATCH 1/6] mci: sdhci: rockchip: set hidspd before re-enabling the clock Sascha Hauer
2026-05-07 8:05 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 2/6] mci: sdhci: rockchip: disable clock while setting DLL Sascha Hauer
2026-05-07 8:10 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 3/6] mci: sdhci: rockchip: Wait for transfer complete interrupt with MMC_RSP_BUSY cmd Sascha Hauer
2026-05-07 8:11 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 4/6] mci: sdhci: rockchip: Update pre-change delay for rockchip platform Sascha Hauer
2026-05-07 8:12 ` Ahmad Fatoum
2026-05-07 7:02 ` [PATCH 5/6] clk: composite: pick best parent for round_rate / set_rate Sascha Hauer
2026-05-07 7:02 ` [PATCH 6/6] mci: sdhci: rockchip: officially support HS200 Sascha Hauer
2026-05-07 8:15 ` Ahmad Fatoum
2026-05-07 8:36 ` Sascha Hauer
2026-05-07 7:47 ` [PATCH 0/6] mci: rockchip-dwcmshc: add HS200 support Ahmad Fatoum
2026-05-07 8:03 Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox