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

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

[Jonas Karlman]

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-eugen.hristev@collabora.com/
[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(-)

-- 
2.40.0

[PATCH v2 01/17] mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode

[Jonas Karlman]

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 ||
-- 
2.40.0

[PATCH v2 02/17] mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode

[Jonas Karlman]

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;
-- 
2.40.0

[PATCH v2 03/17] mmc: rockchip_sdhci: Fix use of device private data

[Jonas Karlman]

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;
-- 
2.40.0

[PATCH v2 04/17] mmc: rockchip_sdhci: Remove unneeded emmc_phy_init

[Jonas Karlman]

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,
 };
-- 
2.40.0

[PATCH v2 05/17] mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops

[Jonas Karlman]

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,
 };
 
-- 
2.40.0

[PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Jonas Karlman]

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,
 };
 
-- 
2.40.0

[PATCH v2 07/17] mmc: rockchip_sdhci: Refactor execute tuning error handling

[Jonas Karlman]

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;
 }
-- 
2.40.0

[PATCH v2 08/17] mmc: rockchip_sdhci: Update speed mode controls in set_ios_post

[Jonas Karlman]

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;
 }
-- 
2.40.0

[PATCH v2 09/17] mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops

[Jonas Karlman]

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[] = {
-- 
2.40.0

[PATCH v2 10/17] mmc: rockchip_sdhci: Rearrange and simplify used regs and flags

[Jonas Karlman]

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;
 }
-- 
2.40.0

[PATCH v2 11/17] mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568

[Jonas Karlman]

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) && \
-- 
2.40.0

[PATCH v2 12/17] rockchip: rk3568-rock-3a: Enable support for more eMMC modes

[Jonas Karlman]

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";
 };
-- 
2.40.0

[PATCH v2 13/17] mmc: rockchip_sdhci: Add support for RK3588

[Jonas Karlman]

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,
+	},
 	{ }
 };
 
-- 
2.40.0

[PATCH v2 14/17] rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb

[Jonas Karlman]

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;
-- 
2.40.0

[PATCH v2 15/17] clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support

[Jonas Karlman]

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;
-- 
2.40.0

[PATCH v2 16/17] mmc: sdhci: Allow disabling of SDMA in SPL

[Jonas Karlman]

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-pgwipeout@gmail.com/

 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;
 	}
-- 
2.40.0

[PATCH v2 17/17] mmc: rockchip_sdhci: Limit number of blocks read in a single command

[Jonas Karlman]

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);
 }
 
-- 
2.40.0

Re: [PATCH v2 01/17] mmc: sdhci: Fix HISPD bit handling for MMC HS 52MHz mode

[Kever Yang]

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_HS_52 ||
>   		    mmc->selected_mode == MMC_DDR_52 ||
>   		    mmc->selected_mode == MMC_HS_200 ||
>   		    mmc->selected_mode == MMC_HS_400 ||

Re: [PATCH v2 02/17] mmc: sdhci: Set UHS Mode Select field for UHS SDR25 mode

[Kever Yang]

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;
> +		break;
>   	case UHS_SDR50:
>   	case MMC_HS_52:
>   		reg |= SDHCI_CTRL_UHS_SDR50;

Re: [PATCH v2 03/17] mmc: rockchip_sdhci: Fix use of device private data

[Kever Yang]

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;

Re: [PATCH v2 04/17] mmc: rockchip_sdhci: Remove unneeded emmc_phy_init

[Kever Yang]

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,
>   };

Re: [PATCH v2 05/17] mmc: rockchip_sdhci: Add set_clock and config_dll sdhci_ops

[Kever Yang]

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,
>   };
>   

Re: [PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Kever Yang]

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,
>   };
>   

Re: [PATCH v2 07/17] mmc: rockchip_sdhci: Refactor execute tuning error handling

[Kever Yang]

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);
> -
> +		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;
>   }

Re: [PATCH v2 08/17] mmc: rockchip_sdhci: Update speed mode controls in set_ios_post

[Kever Yang]

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;
>   }

Re: [PATCH v2 09/17] mmc: rockchip_sdhci: Remove empty get_phy and set_enhanced_strobe ops

[Kever Yang]

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[] = {

Re: [PATCH v2 10/17] mmc: rockchip_sdhci: Rearrange and simplify used regs and flags

[Kever Yang]

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, 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;
>   }

Re: [PATCH v2 11/17] mmc: rockchip_sdhci: Fix HS400 and HS400ES mode on RK3568

[Kever Yang]

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) && \

Re: [PATCH v2 12/17] rockchip: rk3568-rock-3a: Enable support for more eMMC modes

[Kever Yang]

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";
>   };

Re: [PATCH v2 13/17] mmc: rockchip_sdhci: Add support for RK3588

[Kever Yang]

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,
> +	},
>   	{ }
>   };
>   

Re: [PATCH v2 14/17] rockchip: rk3588-rock-5b: Include eMMC node in SPL dtb

[Kever Yang]

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;

Re: [PATCH v2 15/17] clk: rockchip: rk3588: Add limited TMCLK_EMMC clock support

[Kever Yang]

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;

Re: [PATCH v2 17/17] mmc: rockchip_sdhci: Limit number of blocks read in a single command

[Kever Yang]

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);
>   }
>   

Re: [PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Kever Yang]

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,
>   };
>   

Re: [PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Jonas Karlman]

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,
>>   };
>>   

Re: [PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Jonas Karlman]

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-anarsoul@gmail.com/

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,
>>>   };
>>>   
> 

Re: [PATCH v2 06/17] mmc: rockchip_sdhci: Use set_clock and config_dll sdhci_ops

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Eugen Hristev]

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-eugen.hristev@collabora.com/
> [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

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

[Jonas Karlman]

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-eugen.hristev@collabora.com/
>> [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/2cc6cde647e2cf61a29f389e8d263bf19672f0f5

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-jonas@kwiboo.se/

Regards,
Jonas

> 
> Thanks !
> Eugen

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

[Eugen Hristev]

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-eugen.hristev@collabora.com/
>>> [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/2cc6cde647e2cf61a29f389e8d263bf19672f0f5
> 
> 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-jonas@kwiboo.se/

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
> 

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

[Jonas Karlman]

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-eugen.hristev@collabora.com/
>>>> [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/2cc6cde647e2cf61a29f389e8d263bf19672f0f5
>>
>> 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-jonas@kwiboo.se/
>> 
> 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
>>
> 

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

[Eugen Hristev]

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-eugen.hristev@collabora.com/
>>>>> [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/2cc6cde647e2cf61a29f389e8d263bf19672f0f5
>>>
>>> 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-jonas@kwiboo.se/
>>>
>> 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
>>>
>>
> 

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

[Jonas Karlman]

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-eugen.hristev@collabora.com/
>>>>>> [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-jonas@kwiboo.se/

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/2cc6cde647e2cf61a29f389e8d263bf19672f0f5
>>>>
>>>> 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-jonas@kwiboo.se/
>>>>
>>> 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
>>>>
>>>
>>
>