[PATCH v2 00/17] rockchip: eMMC fixes for RK3568 and support for RK3588

This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2: - Add Signed-off-by tag and update commit message with a note from where reg-values originates from - Rename quirks to flags - Use driver data for hs200/hs400 txclk tapnum values - Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)

Set High Speed Enable bit for MMC High Speed (52MHz) mode.
Fixes: f12341a95295 ("mmc: sdhci: Fix HISPD bit handling") Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/sdhci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index c6b250b9a1b5..1389c18533fe 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -682,6 +682,7 @@ static int sdhci_set_ios(struct mmc *mmc) if (!no_hispd_bit) { if (mmc->selected_mode == MMC_HS || mmc->selected_mode == SD_HS || + mmc->selected_mode == MMC_HS_52 || mmc->selected_mode == MMC_DDR_52 || mmc->selected_mode == MMC_HS_200 || mmc->selected_mode == MMC_HS_400 ||

On 2023/4/19 00:46, Jonas Karlman wrote:
Set High Speed Enable bit for MMC High Speed (52MHz) mode.
Fixes: f12341a95295 ("mmc: sdhci: Fix HISPD bit handling") Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/sdhci.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index c6b250b9a1b5..1389c18533fe 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -682,6 +682,7 @@ static int sdhci_set_ios(struct mmc *mmc) if (!no_hispd_bit) { if (mmc->selected_mode == MMC_HS || mmc->selected_mode == SD_HS ||
mmc->selected_mode == MMC_DDR_52 || mmc->selected_mode == MMC_HS_200 || mmc->selected_mode == MMC_HS_400 ||mmc->selected_mode == MMC_HS_52 ||

Set correct UHS Mode Select field value for UHS SDR25 (50MHz) mode.
Fixes: d1c0a2200afb ("mmc: sdhci: Add support for HOST_CONTROL2 and setting UHS timings") Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/sdhci.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 1389c18533fe..86f81f5dfaf3 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -518,6 +518,10 @@ void sdhci_set_uhs_timing(struct sdhci_host *host) reg &= ~SDHCI_CTRL_UHS_MASK;
switch (mmc->selected_mode) { + case UHS_SDR25: + case MMC_HS: + reg |= SDHCI_CTRL_UHS_SDR25; + break; case UHS_SDR50: case MMC_HS_52: reg |= SDHCI_CTRL_UHS_SDR50;

On 2023/4/19 00:46, Jonas Karlman wrote:
Set correct UHS Mode Select field value for UHS SDR25 (50MHz) mode.
Fixes: d1c0a2200afb ("mmc: sdhci: Add support for HOST_CONTROL2 and setting UHS timings") Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/sdhci.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 1389c18533fe..86f81f5dfaf3 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -518,6 +518,10 @@ void sdhci_set_uhs_timing(struct sdhci_host *host) reg &= ~SDHCI_CTRL_UHS_MASK;
switch (mmc->selected_mode) {
- case UHS_SDR25:
- case MMC_HS:
reg |= SDHCI_CTRL_UHS_SDR25;
case UHS_SDR50: case MMC_HS_52: reg |= SDHCI_CTRL_UHS_SDR50;break;

The device private data is misused in rockchip_sdhci_of_to_plat and rockchip_sdhci_execute_tuning.
In these functions dev_get_priv is assigned to struct sdhci_host:
struct sdhci_host *host = dev_get_priv(dev);
Instead, the sdhci host should refer to host in struct rockchip_sdhc:
struct rockchip_sdhc *priv = dev_get_priv(dev); struct sdhci_host *host = &priv->host;
Because host is the first member in struct rockchip_sdhc this is not a real problem, lets fix it anyway and also use priv name consistently.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index e1409dd2c749..ae28840f6081 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -450,7 +450,8 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) { - struct sdhci_host *host = dev_get_priv(mmc->dev); + struct rockchip_sdhc *priv = dev_get_priv(mmc->dev); + struct sdhci_host *host = &priv->host; char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT; struct mmc_cmd cmd; u32 ctrl, blk_size; @@ -531,9 +532,9 @@ static int rockchip_sdhci_probe(struct udevice *dev) struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(dev); struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); struct rockchip_sdhc_plat *plat = dev_get_plat(dev); - struct rockchip_sdhc *prv = dev_get_priv(dev); + struct rockchip_sdhc *priv = dev_get_priv(dev); struct mmc_config *cfg = &plat->cfg; - struct sdhci_host *host = &prv->host; + struct sdhci_host *host = &priv->host; struct clk clk; int ret;
@@ -547,8 +548,8 @@ static int rockchip_sdhci_probe(struct udevice *dev) printf("%s fail to get clk\n", __func__); }
- prv->emmc_clk = clk; - prv->dev = dev; + priv->emmc_clk = clk; + priv->dev = dev;
if (data->get_phy) { ret = data->get_phy(dev); @@ -566,7 +567,7 @@ static int rockchip_sdhci_probe(struct udevice *dev) host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
host->mmc = &plat->mmc; - host->mmc->priv = &prv->host; + host->mmc->priv = &priv->host; host->mmc->dev = dev; upriv->mmc = host->mmc;
@@ -580,8 +581,9 @@ static int rockchip_sdhci_probe(struct udevice *dev) static int rockchip_sdhci_of_to_plat(struct udevice *dev) { struct rockchip_sdhc_plat *plat = dev_get_plat(dev); - struct sdhci_host *host = dev_get_priv(dev); + struct rockchip_sdhc *priv = dev_get_priv(dev); struct mmc_config *cfg = &plat->cfg; + struct sdhci_host *host = &priv->host; int ret;
host->name = dev->name;

On 2023/4/19 00:46, Jonas Karlman wrote:
The device private data is misused in rockchip_sdhci_of_to_plat and rockchip_sdhci_execute_tuning.
In these functions dev_get_priv is assigned to struct sdhci_host:
struct sdhci_host *host = dev_get_priv(dev);
Instead, the sdhci host should refer to host in struct rockchip_sdhc:
struct rockchip_sdhc *priv = dev_get_priv(dev); struct sdhci_host *host = &priv->host;
Because host is the first member in struct rockchip_sdhc this is not a real problem, lets fix it anyway and also use priv name consistently.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index e1409dd2c749..ae28840f6081 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -450,7 +450,8 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) {
- struct sdhci_host *host = dev_get_priv(mmc->dev);
- struct rockchip_sdhc *priv = dev_get_priv(mmc->dev);
- struct sdhci_host *host = &priv->host; char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT; struct mmc_cmd cmd; u32 ctrl, blk_size;
@@ -531,9 +532,9 @@ static int rockchip_sdhci_probe(struct udevice *dev) struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(dev); struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
- struct rockchip_sdhc *prv = dev_get_priv(dev);
- struct rockchip_sdhc *priv = dev_get_priv(dev); struct mmc_config *cfg = &plat->cfg;
- struct sdhci_host *host = &prv->host;
- struct sdhci_host *host = &priv->host; struct clk clk; int ret;
@@ -547,8 +548,8 @@ static int rockchip_sdhci_probe(struct udevice *dev) printf("%s fail to get clk\n", __func__); }
- prv->emmc_clk = clk;
- prv->dev = dev;
priv->emmc_clk = clk;
priv->dev = dev;
if (data->get_phy) { ret = data->get_phy(dev);
@@ -566,7 +567,7 @@ static int rockchip_sdhci_probe(struct udevice *dev) host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
host->mmc = &plat->mmc;
- host->mmc->priv = &prv->host;
- host->mmc->priv = &priv->host; host->mmc->dev = dev; upriv->mmc = host->mmc;
@@ -580,8 +581,9 @@ static int rockchip_sdhci_probe(struct udevice *dev) static int rockchip_sdhci_of_to_plat(struct udevice *dev) { struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
- struct sdhci_host *host = dev_get_priv(dev);
struct rockchip_sdhc *priv = dev_get_priv(dev); struct mmc_config *cfg = &plat->cfg;
struct sdhci_host *host = &priv->host; int ret;
host->name = dev->name;

Remove the unneeded emmc_phy_init now that the no-inverter flag is handled correctly after commit 2321a991bbb5 ("rockchip: sdhci: rk3568: bypass DLL when clk <= 52 MHz").
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 26 -------------------------- 1 file changed, 26 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index ae28840f6081..2a30974df501 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -112,7 +112,6 @@ struct rockchip_sdhc { };
struct sdhci_data { - int (*emmc_phy_init)(struct udevice *dev); int (*get_phy)(struct udevice *dev);
/** @@ -154,11 +153,6 @@ struct sdhci_data { int (*set_enhanced_strobe)(struct sdhci_host *host); };
-static int rk3399_emmc_phy_init(struct udevice *dev) -{ - return 0; -} - static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 clock) { u32 caldone, dllrdy, freqsel; @@ -294,18 +288,6 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_emmc_phy_init(struct udevice *dev) -{ - struct rockchip_sdhc *prv = dev_get_priv(dev); - struct sdhci_host *host = &prv->host; - u32 extra; - - extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL; - sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK); - - return 0; -} - static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); @@ -557,12 +539,6 @@ static int rockchip_sdhci_probe(struct udevice *dev) return ret; }
- if (data->emmc_phy_init) { - ret = data->emmc_phy_init(dev); - if (ret) - return ret; - } - host->ops = &rockchip_sdhci_ops; host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
@@ -605,7 +581,6 @@ static int rockchip_sdhci_bind(struct udevice *dev)
static const struct sdhci_data rk3399_data = { .get_phy = rk3399_emmc_get_phy, - .emmc_phy_init = rk3399_emmc_phy_init, .set_control_reg = rk3399_sdhci_set_control_reg, .set_ios_post = rk3399_sdhci_set_ios_post, .set_enhanced_strobe = rk3399_sdhci_set_enhanced_strobe, @@ -613,7 +588,6 @@ static const struct sdhci_data rk3399_data = {
static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, - .emmc_phy_init = rk3568_emmc_phy_init, .set_ios_post = rk3568_sdhci_set_ios_post, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

On 2023/4/19 00:46, Jonas Karlman wrote:
Remove the unneeded emmc_phy_init now that the no-inverter flag is handled correctly after commit 2321a991bbb5 ("rockchip: sdhci: rk3568: bypass DLL when clk <= 52 MHz").
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 26 -------------------------- 1 file changed, 26 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index ae28840f6081..2a30974df501 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -112,7 +112,6 @@ struct rockchip_sdhc { };
struct sdhci_data {
int (*emmc_phy_init)(struct udevice *dev); int (*get_phy)(struct udevice *dev);
/**
@@ -154,11 +153,6 @@ struct sdhci_data { int (*set_enhanced_strobe)(struct sdhci_host *host); };
-static int rk3399_emmc_phy_init(struct udevice *dev) -{
- return 0;
-}
- static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 clock) { u32 caldone, dllrdy, freqsel;
@@ -294,18 +288,6 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_emmc_phy_init(struct udevice *dev) -{
- struct rockchip_sdhc *prv = dev_get_priv(dev);
- struct sdhci_host *host = &prv->host;
- u32 extra;
- extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
- sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
- return 0;
-}
- static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
@@ -557,12 +539,6 @@ static int rockchip_sdhci_probe(struct udevice *dev) return ret; }
- if (data->emmc_phy_init) {
ret = data->emmc_phy_init(dev);
if (ret)
return ret;
- }
- host->ops = &rockchip_sdhci_ops; host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
@@ -605,7 +581,6 @@ static int rockchip_sdhci_bind(struct udevice *dev)
static const struct sdhci_data rk3399_data = { .get_phy = rk3399_emmc_get_phy,
- .emmc_phy_init = rk3399_emmc_phy_init, .set_control_reg = rk3399_sdhci_set_control_reg, .set_ios_post = rk3399_sdhci_set_ios_post, .set_enhanced_strobe = rk3399_sdhci_set_enhanced_strobe,
@@ -613,7 +588,6 @@ static const struct sdhci_data rk3399_data = {
static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy,
- .emmc_phy_init = rk3568_emmc_phy_init, .set_ios_post = rk3568_sdhci_set_ios_post, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

Add support for the set_clock and config_dll sdhci_ops. Use of these ops will allow configuration of DLL while the output clock is disabled.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 2a30974df501..bc9838ac7c45 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -139,6 +139,9 @@ struct sdhci_data { */ int (*set_ios_post)(struct sdhci_host *host);
+ void (*set_clock)(struct sdhci_host *host, u32 div); + int (*config_dll)(struct sdhci_host *host, u32 clock, bool enable); + /** * set_enhanced_strobe() - Set HS400 Enhanced Strobe config * @@ -430,6 +433,15 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
+static void rockchip_sdhci_set_clock(struct sdhci_host *host, u32 div) +{ + struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev); + + if (data->set_clock) + data->set_clock(host, div); +} + static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) { struct rockchip_sdhc *priv = dev_get_priv(mmc->dev); @@ -491,6 +503,17 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) return ret; }
+static int rockchip_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ + struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev); + + if (data->config_dll) + return data->config_dll(host, clock, enable); + + return 0; +} + static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); @@ -503,9 +526,11 @@ static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) }
static struct sdhci_ops rockchip_sdhci_ops = { - .set_ios_post = rockchip_sdhci_set_ios_post, - .platform_execute_tuning = &rockchip_sdhci_execute_tuning, .set_control_reg = rockchip_sdhci_set_control_reg, + .set_ios_post = rockchip_sdhci_set_ios_post, + .set_clock = rockchip_sdhci_set_clock, + .platform_execute_tuning = rockchip_sdhci_execute_tuning, + .config_dll = rockchip_sdhci_config_dll, .set_enhanced_strobe = rockchip_sdhci_set_enhanced_strobe, };

On 2023/4/19 00:46, Jonas Karlman wrote:
Add support for the set_clock and config_dll sdhci_ops. Use of these ops will allow configuration of DLL while the output clock is disabled.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 2a30974df501..bc9838ac7c45 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -139,6 +139,9 @@ struct sdhci_data { */ int (*set_ios_post)(struct sdhci_host *host);
- void (*set_clock)(struct sdhci_host *host, u32 div);
- int (*config_dll)(struct sdhci_host *host, u32 clock, bool enable);
- /**
- set_enhanced_strobe() - Set HS400 Enhanced Strobe config
@@ -430,6 +433,15 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
+static void rockchip_sdhci_set_clock(struct sdhci_host *host, u32 div) +{
- struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev);
- if (data->set_clock)
data->set_clock(host, div);
+}
- static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) { struct rockchip_sdhc *priv = dev_get_priv(mmc->dev);
@@ -491,6 +503,17 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) return ret; }
+static int rockchip_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{
- struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev);
- if (data->config_dll)
return data->config_dll(host, clock, enable);
- return 0;
+}
- static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
@@ -503,9 +526,11 @@ static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) }
static struct sdhci_ops rockchip_sdhci_ops = {
- .set_ios_post = rockchip_sdhci_set_ios_post,
- .platform_execute_tuning = &rockchip_sdhci_execute_tuning, .set_control_reg = rockchip_sdhci_set_control_reg,
- .set_ios_post = rockchip_sdhci_set_ios_post,
- .set_clock = rockchip_sdhci_set_clock,
- .platform_execute_tuning = rockchip_sdhci_execute_tuning,
- .config_dll = rockchip_sdhci_config_dll, .set_enhanced_strobe = rockchip_sdhci_set_enhanced_strobe, };

Change to configure clock and DLL in set_clock and config_dll ops instead of in the set_ios_post ops.
With this change the output clock is turned off while configuring DLL parameters, according to the design recommendations.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bc9838ac7c45..fdf48f4066c9 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -291,18 +291,24 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct mmc *mmc = host->mmc; + ulong rate; + + rate = clk_set_rate(&priv->emmc_clk, mmc->clock); + if (IS_ERR_VALUE(rate)) + printf("%s: Set clock rate failed: %ld\n", __func__, (long)rate); +} + +static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ int val, ret; u32 extra;
- if (clock > host->max_clk) - clock = host->max_clk; - if (clock) - clk_set_rate(&priv->emmc_clk, clock); - - sdhci_set_clock(host->mmc, clock); + if (!enable) + return 0;
if (clock >= 100 * MHz) { /* reset DLL */ @@ -386,14 +392,8 @@ static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc; - uint clock = mmc->tran_speed; u32 reg, vendor_reg;
- if (!clock) - clock = mmc->clock; - - rk3568_sdhci_emmc_set_clock(host, clock); - if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK; @@ -614,6 +614,8 @@ static const struct sdhci_data rk3399_data = { static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post, + .set_clock = rk3568_sdhci_set_clock, + .config_dll = rk3568_sdhci_config_dll, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

On 2023/4/19 00:46, Jonas Karlman wrote:
Change to configure clock and DLL in set_clock and config_dll ops instead of in the set_ios_post ops.
With this change the output clock is turned off while configuring DLL parameters, according to the design recommendations.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bc9838ac7c45..fdf48f4066c9 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -291,18 +291,24 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct mmc *mmc = host->mmc;
- ulong rate;
- rate = clk_set_rate(&priv->emmc_clk, mmc->clock);
- if (IS_ERR_VALUE(rate))
printf("%s: Set clock rate failed: %ld\n", __func__, (long)rate);
+}
+static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ int val, ret; u32 extra;
- if (clock > host->max_clk)
clock = host->max_clk;
- if (clock)
clk_set_rate(&priv->emmc_clk, clock);
- sdhci_set_clock(host->mmc, clock);
if (!enable)
return 0;
if (clock >= 100 * MHz) { /* reset DLL */
@@ -386,14 +392,8 @@ static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
uint clock = mmc->tran_speed; u32 reg, vendor_reg;
if (!clock)
clock = mmc->clock;
rk3568_sdhci_emmc_set_clock(host, clock);
if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK;
@@ -614,6 +614,8 @@ static const struct sdhci_data rk3399_data = { static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post,
- .set_clock = rk3568_sdhci_set_clock,
- .config_dll = rk3568_sdhci_config_dll, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

Hi Tom,
I got error report like below log when apply many of this patchset with "git-pw patch apply 1770392",
did you met this kind of issue and do you know why?
error: sha1 information is lacking or useless (drivers/mmc/rockchip_sdhci.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0006 mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops
Thanks,
- Kever
On 2023/4/19 00:46, Jonas Karlman wrote:
Change to configure clock and DLL in set_clock and config_dll ops instead of in the set_ios_post ops.
With this change the output clock is turned off while configuring DLL parameters, according to the design recommendations.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
v2:
No change
drivers/mmc/rockchip_sdhci.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bc9838ac7c45..fdf48f4066c9 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -291,18 +291,24 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct mmc *mmc = host->mmc;
- ulong rate;
- rate = clk_set_rate(&priv->emmc_clk, mmc->clock);
- if (IS_ERR_VALUE(rate))
printf("%s: Set clock rate failed: %ld\n", __func__, (long)rate);
+}
+static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ int val, ret; u32 extra;
- if (clock > host->max_clk)
clock = host->max_clk;
- if (clock)
clk_set_rate(&priv->emmc_clk, clock);
- sdhci_set_clock(host->mmc, clock);
if (!enable)
return 0;
if (clock >= 100 * MHz) { /* reset DLL */
@@ -386,14 +392,8 @@ static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
uint clock = mmc->tran_speed; u32 reg, vendor_reg;
if (!clock)
clock = mmc->clock;
rk3568_sdhci_emmc_set_clock(host, clock);
if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK;
@@ -614,6 +614,8 @@ static const struct sdhci_data rk3399_data = { static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post,
- .set_clock = rk3568_sdhci_set_clock,
- .config_dll = rk3568_sdhci_config_dll, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

Hi Kever, On 2023-04-20 12:19, Kever Yang wrote:
Hi Tom,
I got error report like below log when apply many of this patchset with "git-pw patch apply 1770392",
did you met this kind of issue and do you know why?
There are some interdependence in different incoming rockchip series, I have seen similar issue due to conflicts applying rk patches locally.
Regards, Jonas
error: sha1 information is lacking or useless (drivers/mmc/rockchip_sdhci.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0006 mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops
Thanks,
- Kever
On 2023/4/19 00:46, Jonas Karlman wrote:
Change to configure clock and DLL in set_clock and config_dll ops instead of in the set_ios_post ops.
With this change the output clock is turned off while configuring DLL parameters, according to the design recommendations.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
v2:
No change
drivers/mmc/rockchip_sdhci.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bc9838ac7c45..fdf48f4066c9 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -291,18 +291,24 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct mmc *mmc = host->mmc;
- ulong rate;
- rate = clk_set_rate(&priv->emmc_clk, mmc->clock);
- if (IS_ERR_VALUE(rate))
printf("%s: Set clock rate failed: %ld\n", __func__, (long)rate);
+}
+static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ int val, ret; u32 extra;
- if (clock > host->max_clk)
clock = host->max_clk;
- if (clock)
clk_set_rate(&priv->emmc_clk, clock);
- sdhci_set_clock(host->mmc, clock);
if (!enable)
return 0;
if (clock >= 100 * MHz) { /* reset DLL */
@@ -386,14 +392,8 @@ static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
uint clock = mmc->tran_speed; u32 reg, vendor_reg;
if (!clock)
clock = mmc->clock;
rk3568_sdhci_emmc_set_clock(host, clock);
if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK;
@@ -614,6 +614,8 @@ static const struct sdhci_data rk3399_data = { static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post,
- .set_clock = rk3568_sdhci_set_clock,
- .config_dll = rk3568_sdhci_config_dll, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

Hi Kever, On 2023-04-20 13:10, Jonas Karlman wrote:
Hi Kever, On 2023-04-20 12:19, Kever Yang wrote:
Hi Tom,
I got error report like below log when apply many of this patchset with "git-pw patch apply 1770392",
did you met this kind of issue and do you know why?
There are some interdependence in different incoming rockchip series, I have seen similar issue due to conflicts applying rk patches locally.
Look like this patch was conflicting with [1]. This patch also include a change how the mmc->clock is used. I have not been able to reproduce the issue described in [1] with this series.
Will send a rebased v3 of just this patch, remaining patches in this series apply cleanly on top of your master branch after the conflict in this patch is resolved.
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230307212646.56576-1-anar...
Regards, Jonas
Regards, Jonas
error: sha1 information is lacking or useless (drivers/mmc/rockchip_sdhci.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0006 mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops
Thanks,
- Kever
On 2023/4/19 00:46, Jonas Karlman wrote:
Change to configure clock and DLL in set_clock and config_dll ops instead of in the set_ios_post ops.
With this change the output clock is turned off while configuring DLL parameters, according to the design recommendations.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
v2:
No change
drivers/mmc/rockchip_sdhci.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bc9838ac7c45..fdf48f4066c9 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -291,18 +291,24 @@ static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) return 0; }
-static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct mmc *mmc = host->mmc;
- ulong rate;
- rate = clk_set_rate(&priv->emmc_clk, mmc->clock);
- if (IS_ERR_VALUE(rate))
printf("%s: Set clock rate failed: %ld\n", __func__, (long)rate);
+}
+static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) +{ int val, ret; u32 extra;
- if (clock > host->max_clk)
clock = host->max_clk;
- if (clock)
clk_set_rate(&priv->emmc_clk, clock);
- sdhci_set_clock(host->mmc, clock);
if (!enable)
return 0;
if (clock >= 100 * MHz) { /* reset DLL */
@@ -386,14 +392,8 @@ static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
uint clock = mmc->tran_speed; u32 reg, vendor_reg;
if (!clock)
clock = mmc->clock;
rk3568_sdhci_emmc_set_clock(host, clock);
if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK;
@@ -614,6 +614,8 @@ static const struct sdhci_data rk3399_data = { static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post,
- .set_clock = rk3568_sdhci_set_clock,
- .config_dll = rk3568_sdhci_config_dll, .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

On Thu, Apr 20, 2023 at 03:45:17PM +0000, Jonas Karlman wrote:
Hi Kever, On 2023-04-20 13:10, Jonas Karlman wrote:
Hi Kever, On 2023-04-20 12:19, Kever Yang wrote:
Hi Tom,
I got error report like below log when apply many of this patchset with "git-pw patch apply 1770392",
did you met this kind of issue and do you know why?
There are some interdependence in different incoming rockchip series, I have seen similar issue due to conflicts applying rk patches locally.
Look like this patch was conflicting with [1]. This patch also include a change how the mmc->clock is used. I have not been able to reproduce the issue described in [1] with this series.
Will send a rebased v3 of just this patch, remaining patches in this series apply cleanly on top of your master branch after the conflict in this patch is resolved.
In general, I've found using "b4 am" with "git am" to be better than patchwork directly, for the case where a full new series has been re-spun and re-posted, and I also don't want to just make a patchwork bundle for what I'm applying.

Check return value from mmc_send_cmd and clear HOST_CONTROL2 when there is an error. Also skip enable of interrupt signaling and remove a delay, a delay is already happening in sdhci_send_command.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index fdf48f4066c9..e56043fce88e 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -449,17 +449,16 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT; struct mmc_cmd cmd; u32 ctrl, blk_size; - int ret = 0; + int ret;
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); ctrl |= SDHCI_CTRL_EXEC_TUNING; sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE); - sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 64); - if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8) + if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && mmc->bus_width == 8) blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 128); sdhci_writew(host, blk_size, SDHCI_BLOCK_SIZE); sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE); @@ -469,36 +468,24 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) cmd.cmdarg = 0;
do { - if (tuning_loop_counter-- == 0) - break; - - mmc_send_cmd(mmc, &cmd, NULL); - - if (opcode == MMC_CMD_SEND_TUNING_BLOCK) - /* - * For tuning command, do not do busy loop. As tuning - * is happening (CLK-DATA latching for setup/hold time - * requirements), give time to complete - */ - udelay(1); - + ret = mmc_send_cmd(mmc, &cmd, NULL); ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); + if (ret || tuning_loop_counter-- == 0) + break; } while (ctrl & SDHCI_CTRL_EXEC_TUNING);
- if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) { - printf("%s:Tuning failed\n", __func__); - ret = -EIO; - } + if (ret || tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK)) { + if (!ret) + ret = -EIO; + printf("%s: Tuning failed: %d\n", __func__, ret);
- if (tuning_loop_counter < 0) { ctrl &= ~SDHCI_CTRL_TUNED_CLK; - sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL2); + ctrl &= ~SDHCI_CTRL_EXEC_TUNING; + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); }
/* Enable only interrupts served by the SD controller */ sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK, SDHCI_INT_ENABLE); - /* Mask all sdhci interrupt sources */ - sdhci_writel(host, 0x0, SDHCI_SIGNAL_ENABLE);
return ret; }

On 2023/4/19 00:46, Jonas Karlman wrote:
Check return value from mmc_send_cmd and clear HOST_CONTROL2 when there is an error. Also skip enable of interrupt signaling and remove a delay, a delay is already happening in sdhci_send_command.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index fdf48f4066c9..e56043fce88e 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -449,17 +449,16 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT; struct mmc_cmd cmd; u32 ctrl, blk_size;
- int ret = 0;
int ret;
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); ctrl |= SDHCI_CTRL_EXEC_TUNING; sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 64);
if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
- if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && mmc->bus_width == 8) blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 128); sdhci_writew(host, blk_size, SDHCI_BLOCK_SIZE); sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
@@ -469,36 +468,24 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) cmd.cmdarg = 0;
do {
if (tuning_loop_counter-- == 0)
break;
mmc_send_cmd(mmc, &cmd, NULL);
if (opcode == MMC_CMD_SEND_TUNING_BLOCK)
/*
* For tuning command, do not do busy loop. As tuning
* is happening (CLK-DATA latching for setup/hold time
* requirements), give time to complete
*/
udelay(1);
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);ret = mmc_send_cmd(mmc, &cmd, NULL);
if (ret || tuning_loop_counter-- == 0)
} while (ctrl & SDHCI_CTRL_EXEC_TUNING);break;
- if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
printf("%s:Tuning failed\n", __func__);
ret = -EIO;
- }
- if (ret || tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK)) {
if (!ret)
ret = -EIO;
printf("%s: Tuning failed: %d\n", __func__, ret);
- if (tuning_loop_counter < 0) { ctrl &= ~SDHCI_CTRL_TUNED_CLK;
sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL2);
ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
}
/* Enable only interrupts served by the SD controller */ sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK, SDHCI_INT_ENABLE);
/* Mask all sdhci interrupt sources */
sdhci_writel(host, 0x0, SDHCI_SIGNAL_ENABLE);
return ret; }

Refactor set_ios_post ops to correctly set UHS Speed Select field values according to TRM. Also set or unset Enhanced Strobe Enable bit and eMMC Card present bit in set_ios_post, the Enhanced Strobe Enable bit was never unset after switching to HS400ES mode.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 68 ++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index e56043fce88e..8e29430a483e 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -372,20 +372,6 @@ static int rk3568_emmc_get_phy(struct udevice *dev)
static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) { - struct mmc *mmc = host->mmc; - u32 vendor; - int reg; - - reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK) - + DWCMSHC_EMMC_CONTROL; - - vendor = sdhci_readl(host, reg); - if (mmc->selected_mode == MMC_HS_400_ES) - vendor |= DWCMSHC_ENHANCED_STROBE; - else - vendor &= ~DWCMSHC_ENHANCED_STROBE; - sdhci_writel(host, vendor, reg); - return 0; }
@@ -394,21 +380,51 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) struct mmc *mmc = host->mmc; u32 reg, vendor_reg;
- if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { - reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); - reg &= ~SDHCI_CTRL_UHS_MASK; + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); + reg &= ~SDHCI_CTRL_UHS_MASK; + + switch (mmc->selected_mode) { + case UHS_SDR25: + case MMC_HS: + case MMC_HS_52: + reg |= SDHCI_CTRL_UHS_SDR25; + break; + case UHS_SDR50: + reg |= SDHCI_CTRL_UHS_SDR50; + break; + case UHS_DDR50: + case MMC_DDR_52: + reg |= SDHCI_CTRL_UHS_DDR50; + break; + case UHS_SDR104: + case MMC_HS_200: + reg |= SDHCI_CTRL_UHS_SDR104; + break; + case MMC_HS_400: + case MMC_HS_400_ES: reg |= DWCMSHC_CTRL_HS400; - sdhci_writew(host, reg, SDHCI_HOST_CONTROL2); + break; + default: + reg |= SDHCI_CTRL_UHS_SDR12; + } + + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
- vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK) - + DWCMSHC_EMMC_CONTROL; - /* set CARD_IS_EMMC bit to enable Data Strobe for HS400 */ - reg = sdhci_readw(host, vendor_reg); + vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK) + + DWCMSHC_EMMC_CONTROL; + reg = sdhci_readw(host, vendor_reg); + + if (IS_MMC(mmc)) reg |= DWCMSHC_CARD_IS_EMMC; - sdhci_writew(host, reg, vendor_reg); - } else { - sdhci_set_uhs_timing(host); - } + else + reg &= ~DWCMSHC_CARD_IS_EMMC; + + if (mmc->selected_mode == MMC_HS_400_ES) + reg |= DWCMSHC_ENHANCED_STROBE; + else + reg &= ~DWCMSHC_ENHANCED_STROBE; + + sdhci_writew(host, reg, vendor_reg);
return 0; }

On 2023/4/19 00:46, Jonas Karlman wrote:
Refactor set_ios_post ops to correctly set UHS Speed Select field values according to TRM. Also set or unset Enhanced Strobe Enable bit and eMMC Card present bit in set_ios_post, the Enhanced Strobe Enable bit was never unset after switching to HS400ES mode.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 68 ++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index e56043fce88e..8e29430a483e 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -372,20 +372,6 @@ static int rk3568_emmc_get_phy(struct udevice *dev)
static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) {
- struct mmc *mmc = host->mmc;
- u32 vendor;
- int reg;
- reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
+ DWCMSHC_EMMC_CONTROL;
- vendor = sdhci_readl(host, reg);
- if (mmc->selected_mode == MMC_HS_400_ES)
vendor |= DWCMSHC_ENHANCED_STROBE;
- else
vendor &= ~DWCMSHC_ENHANCED_STROBE;
- sdhci_writel(host, vendor, reg);
- return 0; }
@@ -394,21 +380,51 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) struct mmc *mmc = host->mmc; u32 reg, vendor_reg;
- if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) {
reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
reg &= ~SDHCI_CTRL_UHS_MASK;
- reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
- reg &= ~SDHCI_CTRL_UHS_MASK;
- switch (mmc->selected_mode) {
- case UHS_SDR25:
- case MMC_HS:
- case MMC_HS_52:
reg |= SDHCI_CTRL_UHS_SDR25;
break;
- case UHS_SDR50:
reg |= SDHCI_CTRL_UHS_SDR50;
break;
- case UHS_DDR50:
- case MMC_DDR_52:
reg |= SDHCI_CTRL_UHS_DDR50;
break;
- case UHS_SDR104:
- case MMC_HS_200:
reg |= SDHCI_CTRL_UHS_SDR104;
break;
- case MMC_HS_400:
- case MMC_HS_400_ES: reg |= DWCMSHC_CTRL_HS400;
sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
break;
- default:
reg |= SDHCI_CTRL_UHS_SDR12;
- }
- sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
+ DWCMSHC_EMMC_CONTROL;
/* set CARD_IS_EMMC bit to enable Data Strobe for HS400 */
reg = sdhci_readw(host, vendor_reg);
- vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
+ DWCMSHC_EMMC_CONTROL;
- reg = sdhci_readw(host, vendor_reg);
- if (IS_MMC(mmc)) reg |= DWCMSHC_CARD_IS_EMMC;
sdhci_writew(host, reg, vendor_reg);
- } else {
sdhci_set_uhs_timing(host);
- }
else
reg &= ~DWCMSHC_CARD_IS_EMMC;
if (mmc->selected_mode == MMC_HS_400_ES)
reg |= DWCMSHC_ENHANCED_STROBE;
else
reg &= ~DWCMSHC_ENHANCED_STROBE;
sdhci_writew(host, reg, vendor_reg);
return 0; }

Remove empty implementations of get_phy and set_enhanced_strobe ops. Change driver set_enhanced_strobe to return 0 in order to allow missing implementation of the ops.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 8e29430a483e..9716fbb54dd4 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -365,16 +365,6 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab return 0; }
-static int rk3568_emmc_get_phy(struct udevice *dev) -{ - return 0; -} - -static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) -{ - return 0; -} - static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc; @@ -525,7 +515,7 @@ static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) if (data->set_enhanced_strobe) return data->set_enhanced_strobe(host);
- return -ENOTSUPP; + return 0; }
static struct sdhci_ops rockchip_sdhci_ops = { @@ -615,11 +605,9 @@ static const struct sdhci_data rk3399_data = { };
static const struct sdhci_data rk3568_data = { - .get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post, .set_clock = rk3568_sdhci_set_clock, .config_dll = rk3568_sdhci_config_dll, - .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };
static const struct udevice_id sdhci_ids[] = {

On 2023/4/19 00:46, Jonas Karlman wrote:
Remove empty implementations of get_phy and set_enhanced_strobe ops. Change driver set_enhanced_strobe to return 0 in order to allow missing implementation of the ops.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 8e29430a483e..9716fbb54dd4 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -365,16 +365,6 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab return 0; }
-static int rk3568_emmc_get_phy(struct udevice *dev) -{
- return 0;
-}
-static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) -{
- return 0;
-}
- static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
@@ -525,7 +515,7 @@ static int rockchip_sdhci_set_enhanced_strobe(struct sdhci_host *host) if (data->set_enhanced_strobe) return data->set_enhanced_strobe(host);
- return -ENOTSUPP;
return 0; }
static struct sdhci_ops rockchip_sdhci_ops = {
@@ -615,11 +605,9 @@ static const struct sdhci_data rk3399_data = { };
static const struct sdhci_data rk3568_data = {
.get_phy = rk3568_emmc_get_phy, .set_ios_post = rk3568_sdhci_set_ios_post, .set_clock = rk3568_sdhci_set_clock, .config_dll = rk3568_sdhci_config_dll,
.set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };
static const struct udevice_id sdhci_ids[] = {

This rearrange and remove duplicate defines to make the code cleaner.
There is no need to read vendor area1 and use an offset each time, it is easier and clearer to just use the reg offset defined in TRM, same as the other vendor regs.
This also removes use of the misspelled const for the RK3588 CMDOUT reg, it will be re-added when support for RK3588 is introduced.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/mmc/rockchip_sdhci.c | 40 ++++++++++++------------------------ 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 9716fbb54dd4..bcf65e091741 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -47,46 +47,36 @@ #define ARASAN_VENDOR_REGISTER 0x78 #define ARASAN_VENDOR_ENHANCED_STROBE BIT(0)
-/* DWC IP vendor area 1 pointer */ -#define DWCMSHC_P_VENDOR_AREA1 0xe8 -#define DWCMSHC_AREA1_MASK GENMASK(11, 0) -/* Offset inside the vendor area 1 */ -#define DWCMSHC_EMMC_CONTROL 0x2c +/* Rockchip specific Registers */ +#define DWCMSHC_EMMC_EMMC_CTRL 0x52c #define DWCMSHC_CARD_IS_EMMC BIT(0) #define DWCMSHC_ENHANCED_STROBE BIT(8) - -/* Rockchip specific Registers */ #define DWCMSHC_EMMC_DLL_CTRL 0x800 #define DWCMSHC_EMMC_DLL_CTRL_RESET BIT(1) #define DWCMSHC_EMMC_DLL_RXCLK 0x804 #define DWCMSHC_EMMC_DLL_TXCLK 0x808 #define DWCMSHC_EMMC_DLL_STRBIN 0x80c -#define DECMSHC_EMMC_DLL_CMDOUT 0x810 #define DWCMSHC_EMMC_DLL_STATUS0 0x840 #define DWCMSHC_EMMC_DLL_STATUS1 0x844 #define DWCMSHC_EMMC_DLL_START BIT(0) -#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL 29 +#define DWCMSHC_EMMC_DLL_LOCKED BIT(8) +#define DWCMSHC_EMMC_DLL_TIMEOUT BIT(9) #define DWCMSHC_EMMC_DLL_START_POINT 16 #define DWCMSHC_EMMC_DLL_START_DEFAULT 5 #define DWCMSHC_EMMC_DLL_INC_VALUE 2 #define DWCMSHC_EMMC_DLL_INC 8 #define DWCMSHC_EMMC_DLL_BYPASS BIT(24) #define DWCMSHC_EMMC_DLL_DLYENA BIT(27) +#define DLL_RXCLK_NO_INVERTER BIT(29) +#define DLL_RXCLK_ORI_GATE BIT(31) #define DLL_TXCLK_TAPNUM_DEFAULT 0xA - +#define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_TAPNUM_DEFAULT 0x8 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 #define DLL_STRBIN_DELAY_NUM_DEFAULT 0x16
-#define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) -#define DWCMSHC_EMMC_DLL_LOCKED BIT(8) -#define DWCMSHC_EMMC_DLL_TIMEOUT BIT(9) -#define DLL_RXCLK_NO_INVERTER 1 -#define DLL_RXCLK_INVERTER 0 -#define DLL_RXCLK_ORI_GATE BIT(31) -#define DWCMSHC_ENHANCED_STROBE BIT(8) #define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) @@ -328,8 +318,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab if (ret) return ret;
- extra = DWCMSHC_EMMC_DLL_DLYENA | - DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL; + extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_NO_INVERTER; sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
extra = DWCMSHC_EMMC_DLL_DLYENA | @@ -346,10 +335,9 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab * Disable DLL and reset both of sample and drive clock. * The bypass bit and start bit need to be set if DLL is not locked. */ - sdhci_writel(host, DWCMSHC_EMMC_DLL_BYPASS | DWCMSHC_EMMC_DLL_START, - DWCMSHC_EMMC_DLL_CTRL); + extra = DWCMSHC_EMMC_DLL_BYPASS | DWCMSHC_EMMC_DLL_START; + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL); sdhci_writel(host, DLL_RXCLK_ORI_GATE, DWCMSHC_EMMC_DLL_RXCLK); - sdhci_writel(host, 0, DECMSHC_EMMC_DLL_CMDOUT); sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK); /* * Before switching to hs400es mode, the driver will enable @@ -368,7 +356,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc; - u32 reg, vendor_reg; + u32 reg;
reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK; @@ -400,9 +388,7 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host)
sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
- vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK) - + DWCMSHC_EMMC_CONTROL; - reg = sdhci_readw(host, vendor_reg); + reg = sdhci_readw(host, DWCMSHC_EMMC_EMMC_CTRL);
if (IS_MMC(mmc)) reg |= DWCMSHC_CARD_IS_EMMC; @@ -414,7 +400,7 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) else reg &= ~DWCMSHC_ENHANCED_STROBE;
- sdhci_writew(host, reg, vendor_reg); + sdhci_writew(host, reg, DWCMSHC_EMMC_EMMC_CTRL);
return 0; }

On 2023/4/19 00:46, Jonas Karlman wrote:
This rearrange and remove duplicate defines to make the code cleaner.
There is no need to read vendor area1 and use an offset each time, it is easier and clearer to just use the reg offset defined in TRM, same as the other vendor regs.
This also removes use of the misspelled const for the RK3588 CMDOUT reg, it will be re-added when support for RK3588 is introduced.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/mmc/rockchip_sdhci.c | 40 ++++++++++++------------------------ 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 9716fbb54dd4..bcf65e091741 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -47,46 +47,36 @@ #define ARASAN_VENDOR_REGISTER 0x78 #define ARASAN_VENDOR_ENHANCED_STROBE BIT(0)
-/* DWC IP vendor area 1 pointer */ -#define DWCMSHC_P_VENDOR_AREA1 0xe8 -#define DWCMSHC_AREA1_MASK GENMASK(11, 0) -/* Offset inside the vendor area 1 */ -#define DWCMSHC_EMMC_CONTROL 0x2c +/* Rockchip specific Registers */ +#define DWCMSHC_EMMC_EMMC_CTRL 0x52c #define DWCMSHC_CARD_IS_EMMC BIT(0) #define DWCMSHC_ENHANCED_STROBE BIT(8)
-/* Rockchip specific Registers */ #define DWCMSHC_EMMC_DLL_CTRL 0x800 #define DWCMSHC_EMMC_DLL_CTRL_RESET BIT(1) #define DWCMSHC_EMMC_DLL_RXCLK 0x804 #define DWCMSHC_EMMC_DLL_TXCLK 0x808 #define DWCMSHC_EMMC_DLL_STRBIN 0x80c -#define DECMSHC_EMMC_DLL_CMDOUT 0x810 #define DWCMSHC_EMMC_DLL_STATUS0 0x840 #define DWCMSHC_EMMC_DLL_STATUS1 0x844 #define DWCMSHC_EMMC_DLL_START BIT(0) -#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL 29 +#define DWCMSHC_EMMC_DLL_LOCKED BIT(8) +#define DWCMSHC_EMMC_DLL_TIMEOUT BIT(9) #define DWCMSHC_EMMC_DLL_START_POINT 16 #define DWCMSHC_EMMC_DLL_START_DEFAULT 5 #define DWCMSHC_EMMC_DLL_INC_VALUE 2 #define DWCMSHC_EMMC_DLL_INC 8 #define DWCMSHC_EMMC_DLL_BYPASS BIT(24) #define DWCMSHC_EMMC_DLL_DLYENA BIT(27) +#define DLL_RXCLK_NO_INVERTER BIT(29) +#define DLL_RXCLK_ORI_GATE BIT(31) #define DLL_TXCLK_TAPNUM_DEFAULT 0xA
+#define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_TAPNUM_DEFAULT 0x8 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 #define DLL_STRBIN_DELAY_NUM_DEFAULT 0x16
-#define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) -#define DWCMSHC_EMMC_DLL_LOCKED BIT(8) -#define DWCMSHC_EMMC_DLL_TIMEOUT BIT(9) -#define DLL_RXCLK_NO_INVERTER 1 -#define DLL_RXCLK_INVERTER 0 -#define DLL_RXCLK_ORI_GATE BIT(31) -#define DWCMSHC_ENHANCED_STROBE BIT(8) #define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) @@ -328,8 +318,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab if (ret) return ret;
extra = DWCMSHC_EMMC_DLL_DLYENA |
DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_NO_INVERTER;
sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
extra = DWCMSHC_EMMC_DLL_DLYENA |
@@ -346,10 +335,9 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab * Disable DLL and reset both of sample and drive clock. * The bypass bit and start bit need to be set if DLL is not locked. */
sdhci_writel(host, DWCMSHC_EMMC_DLL_BYPASS | DWCMSHC_EMMC_DLL_START,
DWCMSHC_EMMC_DLL_CTRL);
extra = DWCMSHC_EMMC_DLL_BYPASS | DWCMSHC_EMMC_DLL_START;
sdhci_writel(host, DLL_RXCLK_ORI_GATE, DWCMSHC_EMMC_DLL_RXCLK);sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK); /*sdhci_writel(host, 0, DECMSHC_EMMC_DLL_CMDOUT);
- Before switching to hs400es mode, the driver will enable
@@ -368,7 +356,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc;
- u32 reg, vendor_reg;
u32 reg;
reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); reg &= ~SDHCI_CTRL_UHS_MASK;
@@ -400,9 +388,7 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host)
sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
- vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
+ DWCMSHC_EMMC_CONTROL;
- reg = sdhci_readw(host, vendor_reg);
reg = sdhci_readw(host, DWCMSHC_EMMC_EMMC_CTRL);
if (IS_MMC(mmc)) reg |= DWCMSHC_CARD_IS_EMMC;
@@ -414,7 +400,7 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) else reg &= ~DWCMSHC_ENHANCED_STROBE;
- sdhci_writew(host, reg, vendor_reg);
sdhci_writew(host, reg, DWCMSHC_EMMC_EMMC_CTRL);
return 0; }

Adjust tap number for transmit clock, tap number and delay number for strobe input to fix HS400 modes on RK3568.
New values have been picked from vendor kernel and u-boot and have successfully been tested with rock-3a-rk3568_defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
Signed-off-by: Yifeng Zhao yifeng.zhao@rock-chips.com Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - Add Signed-off-by tag
drivers/mmc/rockchip_sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bcf65e091741..12a616d3dfb8 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -69,13 +69,13 @@ #define DWCMSHC_EMMC_DLL_DLYENA BIT(27) #define DLL_RXCLK_NO_INVERTER BIT(29) #define DLL_RXCLK_ORI_GATE BIT(31) -#define DLL_TXCLK_TAPNUM_DEFAULT 0xA +#define DLL_TXCLK_TAPNUM_DEFAULT 0x10 #define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) -#define DLL_STRBIN_TAPNUM_DEFAULT 0x8 +#define DLL_STRBIN_TAPNUM_DEFAULT 0x4 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 -#define DLL_STRBIN_DELAY_NUM_DEFAULT 0x16 +#define DLL_STRBIN_DELAY_NUM_DEFAULT 0x10
#define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \

On 2023/4/19 00:46, Jonas Karlman wrote:
Adjust tap number for transmit clock, tap number and delay number for strobe input to fix HS400 modes on RK3568.
New values have been picked from vendor kernel and u-boot and have successfully been tested with rock-3a-rk3568_defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
Signed-off-by: Yifeng Zhao yifeng.zhao@rock-chips.com Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
Add Signed-off-by tag
drivers/mmc/rockchip_sdhci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index bcf65e091741..12a616d3dfb8 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -69,13 +69,13 @@ #define DWCMSHC_EMMC_DLL_DLYENA BIT(27) #define DLL_RXCLK_NO_INVERTER BIT(29) #define DLL_RXCLK_ORI_GATE BIT(31) -#define DLL_TXCLK_TAPNUM_DEFAULT 0xA +#define DLL_TXCLK_TAPNUM_DEFAULT 0x10 #define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) -#define DLL_STRBIN_TAPNUM_DEFAULT 0x8 +#define DLL_STRBIN_TAPNUM_DEFAULT 0x4 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 -#define DLL_STRBIN_DELAY_NUM_DEFAULT 0x16 +#define DLL_STRBIN_DELAY_NUM_DEFAULT 0x10
#define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \

Add supported mmc modes to rk3568-rock-3a device tree.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index 9ef1e8477067..10226b029ba3 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -13,6 +13,14 @@ }; };
+&sdhci { + cap-mmc-highspeed; + mmc-ddr-1_8v; + mmc-hs200-1_8v; + mmc-hs400-1_8v; + mmc-hs400-enhanced-strobe; +}; + &sdmmc2 { status = "disabled"; };

On 2023/4/19 00:46, Jonas Karlman wrote:
Add supported mmc modes to rk3568-rock-3a device tree.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index 9ef1e8477067..10226b029ba3 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -13,6 +13,14 @@ }; };
+&sdhci {
- cap-mmc-highspeed;
- mmc-ddr-1_8v;
- mmc-hs200-1_8v;
- mmc-hs400-1_8v;
- mmc-hs400-enhanced-strobe;
+};
- &sdmmc2 { status = "disabled"; };

Add support for RK3588 to the rockchip sdhci driver.
Use driver data to handle differences between RK3568 and RK3588:
- Set "Receive original clock source is auto gating" for RK3588. - Set "Receive clock source is no-inverted" only on RK3568 and "Transmit clock source is invertion of original clock input" for RK3588. - Use different txclk_tapnum for HS400 modes on RK3588. - Configure the CMDOUT reg for HS400 modes for RK3588.
This is based on the mainline linux and vendor kernel driver and have successfully been tested with rock5b-rk3588_defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
Signed-off-by: Yifeng Zhao yifeng.zhao@rock-chips.com Signed-off-by: Jonas Karlman jonas@kwiboo.se Reviewed-by: Shawn Lin shawn.lin@rock-chips.com --- v2: - Add Signed-off-by tag - Update commit message - Rename quirks to flags - Save HS200/HS400 txclk_tapnum as driver data - Remove use of rockchip,txclk-tapnum prop - Collect r-b tag
drivers/mmc/rockchip_sdhci.c | 57 +++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 12a616d3dfb8..2857dcc9ec4f 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -56,6 +56,7 @@ #define DWCMSHC_EMMC_DLL_RXCLK 0x804 #define DWCMSHC_EMMC_DLL_TXCLK 0x808 #define DWCMSHC_EMMC_DLL_STRBIN 0x80c +#define DWCMSHC_EMMC_DLL_CMDOUT 0x810 #define DWCMSHC_EMMC_DLL_STATUS0 0x840 #define DWCMSHC_EMMC_DLL_STATUS1 0x844 #define DWCMSHC_EMMC_DLL_START BIT(0) @@ -70,18 +71,27 @@ #define DLL_RXCLK_NO_INVERTER BIT(29) #define DLL_RXCLK_ORI_GATE BIT(31) #define DLL_TXCLK_TAPNUM_DEFAULT 0x10 +#define DLL_TXCLK_TAPNUM_90_DEGREES 0x9 #define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) +#define DLL_TXCLK_NO_INVERTER BIT(29) #define DLL_STRBIN_TAPNUM_DEFAULT 0x4 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 #define DLL_STRBIN_DELAY_NUM_DEFAULT 0x10 +#define DLL_CMDOUT_TAPNUM_90_DEGREES 0x8 +#define DLL_CMDOUT_TAPNUM_FROM_SW BIT(24) +#define DLL_CMDOUT_SRC_CLK_NEG BIT(28) +#define DLL_CMDOUT_EN_SRC_CLK_NEG BIT(29) +#define DLL_CMDOUT_BOTH_CLK_EDGE BIT(30)
#define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) #define ROCKCHIP_MAX_CLKS 3
+#define FLAG_INVERTER_FLAG_IN_RXCLK BIT(0) + struct rockchip_sdhc_plat { struct mmc_config cfg; struct mmc mmc; @@ -144,6 +154,10 @@ struct sdhci_data { * Return: 0 if successful, -ve on error */ int (*set_enhanced_strobe)(struct sdhci_host *host); + + u32 flags; + u8 hs200_txclk_tapnum; + u8 hs400_txclk_tapnum; };
static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 clock) @@ -294,8 +308,11 @@ static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div)
static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) { + struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev); + struct mmc *mmc = host->mmc; int val, ret; - u32 extra; + u32 extra, txclk_tapnum;
if (!enable) return 0; @@ -318,12 +335,28 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab if (ret) return ret;
- extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_NO_INVERTER; + extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_ORI_GATE; + if (data->flags & FLAG_INVERTER_FLAG_IN_RXCLK) + extra |= DLL_RXCLK_NO_INVERTER; sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+ txclk_tapnum = data->hs200_txclk_tapnum; + if (mmc->selected_mode == MMC_HS_400 || + mmc->selected_mode == MMC_HS_400_ES) { + txclk_tapnum = data->hs400_txclk_tapnum; + + extra = DLL_CMDOUT_SRC_CLK_NEG | + DLL_CMDOUT_BOTH_CLK_EDGE | + DWCMSHC_EMMC_DLL_DLYENA | + DLL_CMDOUT_TAPNUM_90_DEGREES | + DLL_CMDOUT_TAPNUM_FROM_SW; + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CMDOUT); + } + extra = DWCMSHC_EMMC_DLL_DLYENA | - DLL_TXCLK_TAPNUM_DEFAULT | - DLL_TXCLK_TAPNUM_FROM_SW; + DLL_TXCLK_TAPNUM_FROM_SW | + DLL_TXCLK_NO_INVERTER | + txclk_tapnum; sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
extra = DWCMSHC_EMMC_DLL_DLYENA | @@ -339,6 +372,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL); sdhci_writel(host, DLL_RXCLK_ORI_GATE, DWCMSHC_EMMC_DLL_RXCLK); sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK); + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CMDOUT); /* * Before switching to hs400es mode, the driver will enable * enhanced strobe first. PHY needs to configure the parameters @@ -594,6 +628,17 @@ static const struct sdhci_data rk3568_data = { .set_ios_post = rk3568_sdhci_set_ios_post, .set_clock = rk3568_sdhci_set_clock, .config_dll = rk3568_sdhci_config_dll, + .flags = FLAG_INVERTER_FLAG_IN_RXCLK, + .hs200_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT, + .hs400_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT, +}; + +static const struct sdhci_data rk3588_data = { + .set_ios_post = rk3568_sdhci_set_ios_post, + .set_clock = rk3568_sdhci_set_clock, + .config_dll = rk3568_sdhci_config_dll, + .hs200_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT, + .hs400_txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES, };
static const struct udevice_id sdhci_ids[] = { @@ -605,6 +650,10 @@ static const struct udevice_id sdhci_ids[] = { .compatible = "rockchip,rk3568-dwcmshc", .data = (ulong)&rk3568_data, }, + { + .compatible = "rockchip,rk3588-dwcmshc", + .data = (ulong)&rk3588_data, + }, { } };

On 2023/4/19 00:46, Jonas Karlman wrote:
Add support for RK3588 to the rockchip sdhci driver.
Use driver data to handle differences between RK3568 and RK3588:
- Set "Receive original clock source is auto gating" for RK3588.
- Set "Receive clock source is no-inverted" only on RK3568 and "Transmit clock source is invertion of original clock input" for RK3588.
- Use different txclk_tapnum for HS400 modes on RK3588.
- Configure the CMDOUT reg for HS400 modes for RK3588.
This is based on the mainline linux and vendor kernel driver and have successfully been tested with rock5b-rk3588_defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
Signed-off-by: Yifeng Zhao yifeng.zhao@rock-chips.com Signed-off-by: Jonas Karlman jonas@kwiboo.se Reviewed-by: Shawn Lin shawn.lin@rock-chips.com
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
Add Signed-off-by tag
Update commit message
Rename quirks to flags
Save HS200/HS400 txclk_tapnum as driver data
Remove use of rockchip,txclk-tapnum prop
Collect r-b tag
drivers/mmc/rockchip_sdhci.c | 57 +++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 12a616d3dfb8..2857dcc9ec4f 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -56,6 +56,7 @@ #define DWCMSHC_EMMC_DLL_RXCLK 0x804 #define DWCMSHC_EMMC_DLL_TXCLK 0x808 #define DWCMSHC_EMMC_DLL_STRBIN 0x80c +#define DWCMSHC_EMMC_DLL_CMDOUT 0x810 #define DWCMSHC_EMMC_DLL_STATUS0 0x840 #define DWCMSHC_EMMC_DLL_STATUS1 0x844 #define DWCMSHC_EMMC_DLL_START BIT(0) @@ -70,18 +71,27 @@ #define DLL_RXCLK_NO_INVERTER BIT(29) #define DLL_RXCLK_ORI_GATE BIT(31) #define DLL_TXCLK_TAPNUM_DEFAULT 0x10 +#define DLL_TXCLK_TAPNUM_90_DEGREES 0x9 #define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) +#define DLL_TXCLK_NO_INVERTER BIT(29) #define DLL_STRBIN_TAPNUM_DEFAULT 0x4 #define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) #define DLL_STRBIN_DELAY_NUM_SEL BIT(26) #define DLL_STRBIN_DELAY_NUM_OFFSET 16 #define DLL_STRBIN_DELAY_NUM_DEFAULT 0x10 +#define DLL_CMDOUT_TAPNUM_90_DEGREES 0x8 +#define DLL_CMDOUT_TAPNUM_FROM_SW BIT(24) +#define DLL_CMDOUT_SRC_CLK_NEG BIT(28) +#define DLL_CMDOUT_EN_SRC_CLK_NEG BIT(29) +#define DLL_CMDOUT_BOTH_CLK_EDGE BIT(30)
#define DLL_LOCK_WO_TMOUT(x) \ ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) #define ROCKCHIP_MAX_CLKS 3
+#define FLAG_INVERTER_FLAG_IN_RXCLK BIT(0)
- struct rockchip_sdhc_plat { struct mmc_config cfg; struct mmc mmc;
@@ -144,6 +154,10 @@ struct sdhci_data { * Return: 0 if successful, -ve on error */ int (*set_enhanced_strobe)(struct sdhci_host *host);
u32 flags;
u8 hs200_txclk_tapnum;
u8 hs400_txclk_tapnum; };
static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 clock)
@@ -294,8 +308,11 @@ static void rk3568_sdhci_set_clock(struct sdhci_host *host, u32 div)
static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enable) {
- struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
- struct sdhci_data *data = (struct sdhci_data *)dev_get_driver_data(priv->dev);
- struct mmc *mmc = host->mmc; int val, ret;
- u32 extra;
u32 extra, txclk_tapnum;
if (!enable) return 0;
@@ -318,12 +335,28 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab if (ret) return ret;
extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_NO_INVERTER;
extra = DWCMSHC_EMMC_DLL_DLYENA | DLL_RXCLK_ORI_GATE;
if (data->flags & FLAG_INVERTER_FLAG_IN_RXCLK)
extra |= DLL_RXCLK_NO_INVERTER;
sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
txclk_tapnum = data->hs200_txclk_tapnum;
if (mmc->selected_mode == MMC_HS_400 ||
mmc->selected_mode == MMC_HS_400_ES) {
txclk_tapnum = data->hs400_txclk_tapnum;
extra = DLL_CMDOUT_SRC_CLK_NEG |
DLL_CMDOUT_BOTH_CLK_EDGE |
DWCMSHC_EMMC_DLL_DLYENA |
DLL_CMDOUT_TAPNUM_90_DEGREES |
DLL_CMDOUT_TAPNUM_FROM_SW;
sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CMDOUT);
}
extra = DWCMSHC_EMMC_DLL_DLYENA |
DLL_TXCLK_TAPNUM_DEFAULT |
DLL_TXCLK_TAPNUM_FROM_SW;
DLL_TXCLK_TAPNUM_FROM_SW |
DLL_TXCLK_NO_INVERTER |
txclk_tapnum;
sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
extra = DWCMSHC_EMMC_DLL_DLYENA |
@@ -339,6 +372,7 @@ static int rk3568_sdhci_config_dll(struct sdhci_host *host, u32 clock, bool enab sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL); sdhci_writel(host, DLL_RXCLK_ORI_GATE, DWCMSHC_EMMC_DLL_RXCLK); sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
/*sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CMDOUT);
- Before switching to hs400es mode, the driver will enable
- enhanced strobe first. PHY needs to configure the parameters
@@ -594,6 +628,17 @@ static const struct sdhci_data rk3568_data = { .set_ios_post = rk3568_sdhci_set_ios_post, .set_clock = rk3568_sdhci_set_clock, .config_dll = rk3568_sdhci_config_dll,
- .flags = FLAG_INVERTER_FLAG_IN_RXCLK,
- .hs200_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT,
- .hs400_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT,
+};
+static const struct sdhci_data rk3588_data = {
.set_ios_post = rk3568_sdhci_set_ios_post,
.set_clock = rk3568_sdhci_set_clock,
.config_dll = rk3568_sdhci_config_dll,
.hs200_txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT,
.hs400_txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES, };
static const struct udevice_id sdhci_ids[] = {
@@ -605,6 +650,10 @@ static const struct udevice_id sdhci_ids[] = { .compatible = "rockchip,rk3568-dwcmshc", .data = (ulong)&rk3568_data, },
- {
.compatible = "rockchip,rk3588-dwcmshc",
.data = (ulong)&rk3588_data,
- }, { } };

Add sdhci node to SPL and u-boot,spl-boot-order. Also add more supported mmc modes and pinctrl.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - Change u-boot,dm-spl to bootph-pre-ram
arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 ++++++++++-- arch/arm/dts/rk3588s-u-boot.dtsi | 4 ++++ 2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi index 4c6f0311d6a1..85075bf435f9 100644 --- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi +++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi @@ -7,11 +7,11 @@
/ { aliases { - mmc0 = &sdmmc; + mmc1 = &sdmmc; };
chosen { - u-boot,spl-boot-order = "same-as-spl", &sdmmc; + u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci; }; };
@@ -19,3 +19,11 @@ bus-width = <4>; status = "okay"; }; + +&sdhci { + cap-mmc-highspeed; + mmc-ddr-1_8v; + mmc-hs200-1_8v; + pinctrl-names = "default"; + pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_data_strobe &emmc_rstnout>; +}; diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi b/arch/arm/dts/rk3588s-u-boot.dtsi index 3cb22f35420e..48fa94ab4039 100644 --- a/arch/arm/dts/rk3588s-u-boot.dtsi +++ b/arch/arm/dts/rk3588s-u-boot.dtsi @@ -59,6 +59,10 @@ u-boot,spl-fifo-mode; };
+&sdhci { + bootph-pre-ram; +}; + &uart2 { clock-frequency = <24000000>; bootph-pre-ram;

On 2023/4/19 00:46, Jonas Karlman wrote:
Add sdhci node to SPL and u-boot,spl-boot-order. Also add more supported mmc modes and pinctrl.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
Change u-boot,dm-spl to bootph-pre-ram
arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 ++++++++++-- arch/arm/dts/rk3588s-u-boot.dtsi | 4 ++++ 2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi index 4c6f0311d6a1..85075bf435f9 100644 --- a/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi +++ b/arch/arm/dts/rk3588-rock-5b-u-boot.dtsi @@ -7,11 +7,11 @@
/ { aliases {
mmc0 = &sdmmc;
mmc1 = &sdmmc;
};
chosen {
u-boot,spl-boot-order = "same-as-spl", &sdmmc;
}; };u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci;
@@ -19,3 +19,11 @@ bus-width = <4>; status = "okay"; };
+&sdhci {
- cap-mmc-highspeed;
- mmc-ddr-1_8v;
- mmc-hs200-1_8v;
- pinctrl-names = "default";
- pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_data_strobe &emmc_rstnout>;
+}; diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi b/arch/arm/dts/rk3588s-u-boot.dtsi index 3cb22f35420e..48fa94ab4039 100644 --- a/arch/arm/dts/rk3588s-u-boot.dtsi +++ b/arch/arm/dts/rk3588s-u-boot.dtsi @@ -59,6 +59,10 @@ u-boot,spl-fifo-mode; };
+&sdhci {
- bootph-pre-ram;
+};
- &uart2 { clock-frequency = <24000000>; bootph-pre-ram;

The device tree sdhci node reference the TMCLK_EMMC clock, add limited support this clock to rk3588 cru driver. Fixes probe of sdhci driver.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
drivers/clk/rockchip/clk_rk3588.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c index 41e31b61a55b..d0cc19b47883 100644 --- a/drivers/clk/rockchip/clk_rk3588.c +++ b/drivers/clk/rockchip/clk_rk3588.c @@ -1553,6 +1553,7 @@ static ulong rk3588_clk_get_rate(struct clk *clk) case DCLK_DECOM: rate = rk3588_mmc_get_clk(priv, clk->id); break; + case TMCLK_EMMC: case TCLK_WDT0: rate = OSC_HZ; break; @@ -1702,6 +1703,7 @@ static ulong rk3588_clk_set_rate(struct clk *clk, ulong rate) case DCLK_DECOM: ret = rk3588_mmc_set_clk(priv, clk->id, rate); break; + case TMCLK_EMMC: case TCLK_WDT0: ret = OSC_HZ; break;

On 2023/4/19 00:46, Jonas Karlman wrote:
The device tree sdhci node reference the TMCLK_EMMC clock, add limited support this clock to rk3588 cru driver. Fixes probe of sdhci driver.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
drivers/clk/rockchip/clk_rk3588.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/clk/rockchip/clk_rk3588.c b/drivers/clk/rockchip/clk_rk3588.c index 41e31b61a55b..d0cc19b47883 100644 --- a/drivers/clk/rockchip/clk_rk3588.c +++ b/drivers/clk/rockchip/clk_rk3588.c @@ -1553,6 +1553,7 @@ static ulong rk3588_clk_get_rate(struct clk *clk) case DCLK_DECOM: rate = rk3588_mmc_get_clk(priv, clk->id); break;
- case TMCLK_EMMC: case TCLK_WDT0: rate = OSC_HZ; break;
@@ -1702,6 +1703,7 @@ static ulong rk3588_clk_set_rate(struct clk *clk, ulong rate) case DCLK_DECOM: ret = rk3588_mmc_set_clk(priv, clk->id, rate); break;
- case TMCLK_EMMC: case TCLK_WDT0: ret = OSC_HZ; break;

From: Peter Geis pgwipeout@gmail.com
Rockchip emmc devices have a similar issue to Rockchip dwmmc devices, where performing DMA to SRAM later causes issues with suspend/resume.
Allow us to toggle SDMA in SPL for sdhci similar to ADMA support, so we can ensure DMA is not used when loading the SRAM code.
Signed-off-by: Peter Geis pgwipeout@gmail.com Reviewed-by: Jaehoon Chung jh80.chung@samsung.com Reviewed-by: Kever Yang kever.yang@rock-chips.com [jonas@kwiboo.se: add Kconfig default value and fix ADMA typo] Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - Remove link tag
v1: - Patch picked from [1]
[1] https://patchwork.ozlabs.org/project/uboot/patch/20220222013131.3114990-3-pg...
drivers/mmc/Kconfig | 8 ++++++++ drivers/mmc/sdhci.c | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 59fb0fb50fb9..de01b9687bad 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -476,6 +476,14 @@ config MMC_SDHCI_SDMA This enables support for the SDMA (Single Operation DMA) defined in the SD Host Controller Standard Specification Version 1.00 .
+config SPL_MMC_SDHCI_SDMA + bool "Support SDHCI SDMA in SPL" + depends on SPL_MMC && MMC_SDHCI + default y if MMC_SDHCI_SDMA + help + This enables support for the SDMA (Single Operation DMA) defined + in the SD Host Controller Standard Specification Version 1.00 in SPL. + config MMC_SDHCI_ADMA bool "Support SDHCI ADMA2" depends on MMC_SDHCI diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 86f81f5dfaf3..9cbe126106c5 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -70,7 +70,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host, struct mmc_data *data) } }
-#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)) +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)) static void sdhci_prepare_dma(struct sdhci_host *host, struct mmc_data *data, int *is_aligned, int trans_bytes) { @@ -177,7 +177,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data) } } while (!(stat & SDHCI_INT_DATA_END));
-#if (defined(CONFIG_MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)) +#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)) dma_unmap_single(host->start_addr, data->blocks * data->blocksize, mmc_get_dma_dir(data)); #endif @@ -877,7 +877,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, #endif debug("%s, caps: 0x%x\n", __func__, caps);
-#ifdef CONFIG_MMC_SDHCI_SDMA +#if CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) if ((caps & SDHCI_CAN_DO_SDMA)) { host->flags |= USE_SDMA; } else { @@ -887,7 +887,7 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, #endif #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA) if (!(caps & SDHCI_CAN_DO_ADMA2)) { - printf("%s: Your controller doesn't support SDMA!!\n", + printf("%s: Your controller doesn't support ADMA!!\n", __func__); return -EINVAL; }

Using DMA to load TF-A into SRAM fails when booting from eMMC on RK3588.
## Checking hash(es) for Image atf-3 ... sha256 error! Bad hash value for 'hash' hash node in 'atf-3' image node spl_load_simple_fit: can't load image loadables index 2 (ret = -1) mmc_load_image_raw_sector: mmc block read error
Fix this by using PIO mode in SPL and limit the number of blocks used in a single read command to avoid triggering Data End Bit Error interrupt.
Signed-off-by: Jonas Karlman jonas@kwiboo.se --- v2: - No change
configs/rock5b-rk3588_defconfig | 1 + drivers/mmc/rockchip_sdhci.c | 8 ++++++++ 2 files changed, 9 insertions(+)
diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig index 3fcc6a26bb51..d3136ac850fe 100644 --- a/configs/rock5b-rk3588_defconfig +++ b/configs/rock5b-rk3588_defconfig @@ -58,6 +58,7 @@ CONFIG_MMC_DW=y CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_SDMA=y +# CONFIG_SPL_MMC_SDHCI_SDMA is not set CONFIG_MMC_SDHCI_ROCKCHIP=y CONFIG_ETH_DESIGNWARE=y CONFIG_GMAC_ROCKCHIP=y diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 2857dcc9ec4f..4f110976f4e8 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -589,6 +589,14 @@ static int rockchip_sdhci_probe(struct udevice *dev) if (ret) return ret;
+ /* + * Reading more than 4 blocks with a single CMD18 command in PIO mode + * triggers Data End Bit Error on RK3568 and RK3588. Limit to reading + * max 4 blocks in one command when using PIO mode. + */ + if (!(host->flags & USE_DMA)) + cfg->b_max = 4; + return sdhci_probe(dev); }

On 2023/4/19 00:46, Jonas Karlman wrote:
Using DMA to load TF-A into SRAM fails when booting from eMMC on RK3588.
## Checking hash(es) for Image atf-3 ... sha256 error! Bad hash value for 'hash' hash node in 'atf-3' image node spl_load_simple_fit: can't load image loadables index 2 (ret = -1) mmc_load_image_raw_sector: mmc block read error
Fix this by using PIO mode in SPL and limit the number of blocks used in a single read command to avoid triggering Data End Bit Error interrupt.
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
v2:
No change
configs/rock5b-rk3588_defconfig | 1 + drivers/mmc/rockchip_sdhci.c | 8 ++++++++ 2 files changed, 9 insertions(+)
diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig index 3fcc6a26bb51..d3136ac850fe 100644 --- a/configs/rock5b-rk3588_defconfig +++ b/configs/rock5b-rk3588_defconfig @@ -58,6 +58,7 @@ CONFIG_MMC_DW=y CONFIG_MMC_DW_ROCKCHIP=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_SDMA=y +# CONFIG_SPL_MMC_SDHCI_SDMA is not set CONFIG_MMC_SDHCI_ROCKCHIP=y CONFIG_ETH_DESIGNWARE=y CONFIG_GMAC_ROCKCHIP=y diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 2857dcc9ec4f..4f110976f4e8 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -589,6 +589,14 @@ static int rockchip_sdhci_probe(struct udevice *dev) if (ret) return ret;
- /*
* Reading more than 4 blocks with a single CMD18 command in PIO mode
* triggers Data End Bit Error on RK3568 and RK3588. Limit to reading
* max 4 blocks in one command when using PIO mode.
*/
- if (!(host->flags & USE_DMA))
cfg->b_max = 4;
- return sdhci_probe(dev); }

On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
Thanks ! Eugen

Hi Eugen,
On 2023-04-27 16:45, Eugen Hristev wrote:
On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
I know that there was some issue reading more then 4 sectors using CMD18. But here it looks like the initial 1 sector read using CMD17 fails. Try with CONFIG_MMC_VERBOSE=y, CONFIG_MMC_TRACE=y and CONFIG_LOGLEVEL=8 to get a little more detailed log.
Workaround to force using PIO read-mode and limit to 4 sectors: https://source.denx.de/u-boot/u-boot/-/commit/2cc6cde647e2cf61a29f389e8d263b...
Did a quick runtime test booting from my eMMC module using latest plain master branch, commit 2356053a945899687e894a3e3b3de09dd9814bb8, yesterday using a Radxa 32GB eMMC 5.1 module: https://gist.github.com/Kwiboo/3ff6be7cca04098b4fe3d555637ded37
It could also be worth testing with pinctrl included in SPL, I add them in my defconfig and SPI NOR series (was not included in my runtime test): https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
Regards, Jonas
Thanks ! Eugen

On 4/27/23 18:25, Jonas Karlman wrote:
Hi Eugen,
On 2023-04-27 16:45, Eugen Hristev wrote:
On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
I know that there was some issue reading more then 4 sectors using CMD18. But here it looks like the initial 1 sector read using CMD17 fails. Try with CONFIG_MMC_VERBOSE=y, CONFIG_MMC_TRACE=y and CONFIG_LOGLEVEL=8 to get a little more detailed log.
Yes I saw that from debug, it's the initial sector where the FIT should reside
Workaround to force using PIO read-mode and limit to 4 sectors: https://source.denx.de/u-boot/u-boot/-/commit/2cc6cde647e2cf61a29f389e8d263b...
Did a quick runtime test booting from my eMMC module using latest plain master branch, commit 2356053a945899687e894a3e3b3de09dd9814bb8, yesterday using a Radxa 32GB eMMC 5.1 module: https://gist.github.com/Kwiboo/3ff6be7cca04098b4fe3d555637ded37
It could also be worth testing with pinctrl included in SPL, I add them in my defconfig and SPI NOR series (was not included in my runtime test): https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
I added them, otherwise I was getting another error, card not inserted, etc, so I figured the pins are missing from SPL.
The error I am getting indicates to me some problems during transfer, I had a look on the patch to disable SDMA, but I guess that doesn't apply to SPL
I also have to mention that in U-boot proper, the eMMC works fine, if I boot it from another media
Regards, Jonas
Thanks ! Eugen

On 2023-04-27 17:33, Eugen Hristev wrote:
On 4/27/23 18:25, Jonas Karlman wrote:
Hi Eugen,
On 2023-04-27 16:45, Eugen Hristev wrote:
On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
I know that there was some issue reading more then 4 sectors using CMD18. But here it looks like the initial 1 sector read using CMD17 fails. Try with CONFIG_MMC_VERBOSE=y, CONFIG_MMC_TRACE=y and CONFIG_LOGLEVEL=8 to get a little more detailed log.
Yes I saw that from debug, it's the initial sector where the FIT should reside
Please enable above configs and compare what CMD_SEND fails, here is current master branch with above 3 extra CONFIG enabled: https://gist.github.com/Kwiboo/56d82e2d7799331ff21999847a178657
CMD_SEND:17 ARG 0x00004000 MMC_RSP_R1,5,6,7 0x00000900
That should be the CMD17 that tries to read FIT image header from sector 0x4000. How far does yours gets?
Workaround to force using PIO read-mode and limit to 4 sectors: https://source.denx.de/u-boot/u-boot/-/commit/2cc6cde647e2cf61a29f389e8d263b...
Did a quick runtime test booting from my eMMC module using latest plain master branch, commit 2356053a945899687e894a3e3b3de09dd9814bb8, yesterday using a Radxa 32GB eMMC 5.1 module: https://gist.github.com/Kwiboo/3ff6be7cca04098b4fe3d555637ded37
It could also be worth testing with pinctrl included in SPL, I add them in my defconfig and SPI NOR series (was not included in my runtime test): https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
I added them, otherwise I was getting another error, card not inserted, etc, so I figured the pins are missing from SPL.
The error I am getting indicates to me some problems during transfer, I had a look on the patch to disable SDMA, but I guess that doesn't apply to SPL
SDMA should be disabled in SPL using:
# CONFIG_SPL_MMC_SDHCI_SDMA is not set
But that should only be needed for loading atf into sram. Reading first sector to detect FIT image should still work with or without SDMA.
I also have to mention that in U-boot proper, the eMMC works fine, if I boot it from another media
Strange, are you sure you have FIT image written at sector 0x4000?
Did a re-test booting from different eMMC modules without any issue. My modules originate from Pine64, HardKernel or Radxa, 8, 16 or 32 GB in size. My board is a ROCK 5B V1.42 2022.08.29 revision if that could make any difference.
I write rockchip-u-boot.bin to eMMC and SD-card in the same way:
dd if=rockchip-u-boot.bin of=/dev/mmcblk0 seek=64
Regards, Jonas
Regards, Jonas
Thanks ! Eugen

On 4/27/23 21:27, Jonas Karlman wrote:
On 2023-04-27 17:33, Eugen Hristev wrote:
On 4/27/23 18:25, Jonas Karlman wrote:
Hi Eugen,
On 2023-04-27 16:45, Eugen Hristev wrote:
On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
I know that there was some issue reading more then 4 sectors using CMD18. But here it looks like the initial 1 sector read using CMD17 fails. Try with CONFIG_MMC_VERBOSE=y, CONFIG_MMC_TRACE=y and CONFIG_LOGLEVEL=8 to get a little more detailed log.
Yes I saw that from debug, it's the initial sector where the FIT should reside
Please enable above configs and compare what CMD_SEND fails, here is current master branch with above 3 extra CONFIG enabled: https://gist.github.com/Kwiboo/56d82e2d7799331ff21999847a178657
CMD_SEND:17 ARG 0x00004000 MMC_RSP_R1,5,6,7 0x00000900
That should be the CMD17 that tries to read FIT image header from sector 0x4000. How far does yours gets?
I did some debugging, then I got to understand that somehow I probably wasn't writing the u-boot.itb right as you said. Maybe rkdeveloptool has been playing tricks on me. Even if I tried to write the file with Linux as well and didn't work Oh well , now it works. So thanks a lot for your suggestions ! Do you wish to send a patch to add the pinctrl to SPL for the eMMC or I should do it ? Those are required for SPL operations if the eMMC is not initialized by the bootrom , e.g. I am testing loading the SPL with rockusb protocol.
Thanks again and sorry for all the noise , my bad ! Eugen
Workaround to force using PIO read-mode and limit to 4 sectors: https://source.denx.de/u-boot/u-boot/-/commit/2cc6cde647e2cf61a29f389e8d263b...
Did a quick runtime test booting from my eMMC module using latest plain master branch, commit 2356053a945899687e894a3e3b3de09dd9814bb8, yesterday using a Radxa 32GB eMMC 5.1 module: https://gist.github.com/Kwiboo/3ff6be7cca04098b4fe3d555637ded37
It could also be worth testing with pinctrl included in SPL, I add them in my defconfig and SPI NOR series (was not included in my runtime test): https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
I added them, otherwise I was getting another error, card not inserted, etc, so I figured the pins are missing from SPL.
The error I am getting indicates to me some problems during transfer, I had a look on the patch to disable SDMA, but I guess that doesn't apply to SPL
SDMA should be disabled in SPL using:
# CONFIG_SPL_MMC_SDHCI_SDMA is not set
But that should only be needed for loading atf into sram. Reading first sector to detect FIT image should still work with or without SDMA.
I also have to mention that in U-boot proper, the eMMC works fine, if I boot it from another media
Strange, are you sure you have FIT image written at sector 0x4000?
Did a re-test booting from different eMMC modules without any issue. My modules originate from Pine64, HardKernel or Radxa, 8, 16 or 32 GB in size. My board is a ROCK 5B V1.42 2022.08.29 revision if that could make any difference.
I write rockchip-u-boot.bin to eMMC and SD-card in the same way:
dd if=rockchip-u-boot.bin of=/dev/mmcblk0 seek=64
Regards, Jonas
Regards, Jonas
Thanks ! Eugen

On 2023-04-28 12:58, Eugen Hristev wrote:
On 4/27/23 21:27, Jonas Karlman wrote:
On 2023-04-27 17:33, Eugen Hristev wrote:
On 4/27/23 18:25, Jonas Karlman wrote:
Hi Eugen,
On 2023-04-27 16:45, Eugen Hristev wrote:
On 4/18/23 19:46, Jonas Karlman wrote:
This series fixes eMMC HS400 modes on RK3568 and add support for RK3588.
It has been tested with rock-3a-rk3568/rock5b-rk3588 defconfig and
CONFIG_MMC_HS200_SUPPORT=y CONFIG_MMC_HS400_SUPPORT=y CONFIG_MMC_HS400_ES_SUPPORT=y CONFIG_MMC_SPEED_MODE_SET=y
using the following command to switch mode and then read 512 MiB of data from eMMC into memory,
=> mmc dev 0 0 <mode> && mmc info && mmc read 10000000 2000 10000
for each of the modes below.
0 = MMC legacy 1 = MMC High Speed (26MHz) 3 = MMC High Speed (52MHz) 4 = MMC DDR52 (52MHz) 10 = HS200 (200MHz) 11 = HS400 (200MHz) 12 = HS400ES (200MHz)
All reads have reported OK, prior to this some of these modes worked and others failed.
Patch 1-2 fixes an issue with high-speed bit and uhs speed mode field. Patch 3-6 refactors the rk3568 driver to use set_clock and config_dll ops, so that clocks and regs are changed when output clock is disabled. Patch 7-10 continues refactoring and simplification of the driver. Patch 11-12 updates tap and delay values to fix HS400 modes on RK3568. Patch 13-15 adds support for RK3588 to driver and device tree. Patch 16-17 adds workarounds needed to use PIO mode in SPL to successfully load TF-A into SRAM when booting from eMMC on RK3588.
Note that this series does not include any change to defconfigs to enable HS200/HS400/HS400ES modes.
Changes in v2:
- Add Signed-off-by tag and update commit message with a note from where reg-values originates from
- Rename quirks to flags
- Use driver data for hs200/hs400 txclk tapnum values
- Change u-boot,dm-spl to bootph-pre-ram
This series require working pinctrl, see [1]. A copy of this series and its dependencies can be found at [2].
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230315153215.389809-1-eug... [2] https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-emmc-v2
Jonas Karlman (16): mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode mmc: rockchip_sdhci: Fix use of device private data mmc: rockchip_sdhci: Remove unneeded emmc_phy_init ops mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops mmc: rockchip_sdhci: Refactor execute tuning error handling mmc: rockchip_sdhci: Update speed mode controls in set_ios_post mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops mmc: rockchip_sdhci: Rearrange and simplify used regs and flags mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568 rockchip: rk3568-rock-3a: Enable support for more eMMC modes mmc: rockchip_sdhci: Add support for RK3588 rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support mmc: rockchip_sdhci: Limit number of blocks read in a single command
Peter Geis (1): mmc: sdhci: Allow disabling of SDMA in SPL
arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 8 + arch/arm/dts/rk3588-rock-5b-u-boot.dtsi | 12 +- arch/arm/dts/rk3588s-u-boot.dtsi | 4 + configs/rock5b-rk3588_defconfig | 1 + drivers/clk/rockchip/clk_rk3588.c | 2 + drivers/mmc/Kconfig | 8 + drivers/mmc/rockchip_sdhci.c | 309 +++++++++++++----------- drivers/mmc/sdhci.c | 13 +- 8 files changed, 215 insertions(+), 142 deletions(-)
Hi Jonas,
I saw in the logs you provided in the series that the SPL should also work in eMMC in rk3588, however, I am getting this error on my rock5b :
Trying to boot from MMC2 spl: mmc boot mode: raw hdr read sector 4000, count=1 mmc_load_image_raw_sector: mmc block read error
Do you have any idea of what's going wrong ?
I know that there was some issue reading more then 4 sectors using CMD18. But here it looks like the initial 1 sector read using CMD17 fails. Try with CONFIG_MMC_VERBOSE=y, CONFIG_MMC_TRACE=y and CONFIG_LOGLEVEL=8 to get a little more detailed log.
Yes I saw that from debug, it's the initial sector where the FIT should reside
Please enable above configs and compare what CMD_SEND fails, here is current master branch with above 3 extra CONFIG enabled: https://gist.github.com/Kwiboo/56d82e2d7799331ff21999847a178657
CMD_SEND:17 ARG 0x00004000 MMC_RSP_R1,5,6,7 0x00000900
That should be the CMD17 that tries to read FIT image header from sector 0x4000. How far does yours gets?
I did some debugging, then I got to understand that somehow I probably wasn't writing the u-boot.itb right as you said. Maybe rkdeveloptool has been playing tricks on me. Even if I tried to write the file with Linux as well and didn't work Oh well , now it works. So thanks a lot for your suggestions !
Great that it works and that you found the issue :-)
Do you wish to send a patch to add the pinctrl to SPL for the eMMC or I should do it ?
I have already sent a series that includes a commit with such pinctrl change, can see now that I forgot to cc you, sorry about that.
Please see the following patch from my series "rockchip: rk35xx: Update defconfigs and enable boot from SPI NOR flash".
https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
Regards, Jonas
Those are required for SPL operations if the eMMC is not initialized by the bootrom , e.g. I am testing loading the SPL with rockusb protocol.
Thanks again and sorry for all the noise , my bad ! Eugen
Workaround to force using PIO read-mode and limit to 4 sectors: https://source.denx.de/u-boot/u-boot/-/commit/2cc6cde647e2cf61a29f389e8d263b...
Did a quick runtime test booting from my eMMC module using latest plain master branch, commit 2356053a945899687e894a3e3b3de09dd9814bb8, yesterday using a Radxa 32GB eMMC 5.1 module: https://gist.github.com/Kwiboo/3ff6be7cca04098b4fe3d555637ded37
It could also be worth testing with pinctrl included in SPL, I add them in my defconfig and SPI NOR series (was not included in my runtime test): https://patchwork.ozlabs.org/project/uboot/patch/20230422012309.402799-13-jo...
I added them, otherwise I was getting another error, card not inserted, etc, so I figured the pins are missing from SPL.
The error I am getting indicates to me some problems during transfer, I had a look on the patch to disable SDMA, but I guess that doesn't apply to SPL
SDMA should be disabled in SPL using:
# CONFIG_SPL_MMC_SDHCI_SDMA is not set
But that should only be needed for loading atf into sram. Reading first sector to detect FIT image should still work with or without SDMA.
I also have to mention that in U-boot proper, the eMMC works fine, if I boot it from another media
Strange, are you sure you have FIT image written at sector 0x4000?
Did a re-test booting from different eMMC modules without any issue. My modules originate from Pine64, HardKernel or Radxa, 8, 16 or 32 GB in size. My board is a ROCK 5B V1.42 2022.08.29 revision if that could make any difference.
I write rockchip-u-boot.bin to eMMC and SD-card in the same way:
dd if=rockchip-u-boot.bin of=/dev/mmcblk0 seek=64
Regards, Jonas
Regards, Jonas
Thanks ! Eugen
participants (4)
-
Eugen Hristev
-
Jonas Karlman
-
Kever Yang
-
Tom Rini