[PATCH v2 0/3]

[PATCH v2 0/3]

[Yifeng Zhao]

RK3399 and RK3568 are use different sdhci controllers.
The drivers need to be updated to support these two platforms
and it's easy to support new platforms.


Changes in v2:
- Add mmc_of_parse to parse dts config.
- Used read_poll_timeout api to check dll lock status
- Add execute tuning api for hs200 tuning
- Used sdhci_set_clock api to set clock.
- Used read_poll_timeout api to check dll status.

Yifeng Zhao (3):
  mmc: rockchip_sdhci: add phy and clock config for rk3399
  mmc: rockchip_sdhci: Add support for RK3568
  rockchip: config: evb-rk3399: add hs200 and hs400 support

 configs/evb-rk3399_defconfig |   1 +
 drivers/mmc/rockchip_sdhci.c | 427 ++++++++++++++++++++++++++++++++---
 2 files changed, 392 insertions(+), 36 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

[Yifeng Zhao]

Add clock, phy and other configuration, it is convenient to support
new controller. Here a short summary of the changes:
- Add mmc_of_parse to parse dts config.
- Remove OF_PLATDATA related code.
- Reorder header inclusion.
- Add phy ops.
- add ops set_ios_post to modify the parameters of phy when the
  clock changes.
- Add execute tuning api for hs200 tuning

Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
---

Changes in v2:
- Add mmc_of_parse to parse dts config.
- Used read_poll_timeout api to check dll lock status
- Add execute tuning api for hs200 tuning

 drivers/mmc/rockchip_sdhci.c | 310 +++++++++++++++++++++++++++++++----
 1 file changed, 274 insertions(+), 36 deletions(-)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index d95f8b2a15..2973911446 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -6,90 +6,319 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
+#include <dm/ofnode.h>
 #include <dt-structs.h>
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/libfdt.h>
+#include <linux/iopoll.h>
 #include <malloc.h>
 #include <mapmem.h>
+#include "mmc_private.h"
 #include <sdhci.h>
-#include <clk.h>
+#include <syscon.h>
+#include <asm/arch-rockchip/clock.h>
+#include <asm/arch-rockchip/hardware.h>
 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define EMMC_MIN_FREQ	400000
+#define KHz	(1000)
+#define MHz	(1000 * KHz)
+#define SDHCI_TUNING_LOOP_COUNT		40
+
+#define PHYCTRL_CALDONE_MASK		0x1
+#define PHYCTRL_CALDONE_SHIFT		0x6
+#define PHYCTRL_CALDONE_DONE		0x1
+#define PHYCTRL_DLLRDY_MASK		0x1
+#define PHYCTRL_DLLRDY_SHIFT		0x5
+#define PHYCTRL_DLLRDY_DONE		0x1
+#define PHYCTRL_FREQSEL_200M		0x0
+#define PHYCTRL_FREQSEL_50M		0x1
+#define PHYCTRL_FREQSEL_100M		0x2
+#define PHYCTRL_FREQSEL_150M		0x3
+#define PHYCTRL_DLL_LOCK_WO_TMOUT(x)	\
+	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
+	PHYCTRL_DLLRDY_DONE)
 
 struct rockchip_sdhc_plat {
-#if CONFIG_IS_ENABLED(OF_PLATDATA)
-	struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
-#endif
 	struct mmc_config cfg;
 	struct mmc mmc;
 };
 
+struct rockchip_emmc_phy {
+	u32 emmcphy_con[7];
+	u32 reserved;
+	u32 emmcphy_status;
+};
+
 struct rockchip_sdhc {
 	struct sdhci_host host;
+	struct udevice *dev;
 	void *base;
+	struct rockchip_emmc_phy *phy;
+	struct clk emmc_clk;
+};
+
+struct sdhci_data {
+	int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
+	int (*emmc_phy_init)(struct udevice *dev);
+	int (*get_phy)(struct udevice *dev);
+};
+
+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;
+
+	writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
+	writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
+	writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
+
+	/*
+	 * According to the user manual, calpad calibration
+	 * cycle takes more than 2us without the minimal recommended
+	 * value, so we may need a little margin here
+	 */
+	udelay(3);
+	writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
+
+	/*
+	 * According to the user manual, it asks driver to
+	 * wait 5us for calpad busy trimming. But it seems that
+	 * 5us of caldone isn't enough for all cases.
+	 */
+	udelay(500);
+	caldone = readl(&phy->emmcphy_status);
+	caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
+	if (caldone != PHYCTRL_CALDONE_DONE) {
+		printf("%s: caldone timeout.\n", __func__);
+		return;
+	}
+
+	/* Set the frequency of the DLL operation */
+	if (clock < 75 * MHz)
+		freqsel = PHYCTRL_FREQSEL_50M;
+	else if (clock < 125 * MHz)
+		freqsel = PHYCTRL_FREQSEL_100M;
+	else if (clock < 175 * MHz)
+		freqsel = PHYCTRL_FREQSEL_150M;
+	else
+		freqsel = PHYCTRL_FREQSEL_200M;
+
+	/* Set the frequency of the DLL operation */
+	writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
+	writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
+
+	read_poll_timeout(readl, &phy->emmcphy_status, dllrdy,
+			  PHYCTRL_DLL_LOCK_WO_TMOUT(dllrdy), 1, 5000);
+}
+
+static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
+{
+	writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
+	writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
+}
+
+static int rk3399_emmc_get_phy(struct udevice *dev)
+{
+	struct rockchip_sdhc *priv = dev_get_priv(dev);
+
+	ofnode phy_node;
+	void *grf_base;
+	u32 grf_phy_offset, phandle;
+
+	phandle = dev_read_u32_default(dev, "phys", 0);
+	phy_node = ofnode_get_by_phandle(phandle);
+	if (!ofnode_valid(phy_node)) {
+		debug("Not found emmc phy device\n");
+		return -ENODEV;
+	}
+
+	grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
+	if (grf_base < 0)
+		printf("%s Get syscon grf failed", __func__);
+	grf_phy_offset = ofnode_read_u32_default(phy_node, "reg", 0);
+
+	priv->phy = (struct rockchip_emmc_phy *)(grf_base + grf_phy_offset);
+
+	return 0;
+}
+
+static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
+	int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;
+	int max_clk = host->max_clk;
+	unsigned long mmc_clock;
+
+	if (cycle_phy)
+		rk3399_emmc_phy_power_off(priv->phy);
+
+	if (clock) {
+		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);
+		if (IS_ERR_VALUE(mmc_clock))
+			host->max_clk = max_clk;
+		else
+			host->max_clk = mmc_clock;
+	}
+	sdhci_set_clock(host->mmc, clock);
+	if (!clock)
+		return 0;
+	host->max_clk = max_clk;
+
+	if (cycle_phy)
+		rk3399_emmc_phy_power_on(priv->phy, clock);
+
+	return 0;
+}
+
+static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
+{
+	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;
+	uint clock = mmc->tran_speed;
+	u32 reg;
+
+	if (!clock)
+		clock = mmc->clock;
+	data->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;
+		reg |= SDHCI_CTRL_HS400;
+		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
+	} else {
+		sdhci_set_uhs_timing(host);
+	}
+
+	return 0;
+}
+
+static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
+{
+	struct sdhci_host *host = dev_get_priv(mmc->dev);
+	char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;
+	struct mmc_cmd cmd;
+	u32 ctrl;
+	int ret = 0;
+
+	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);
+
+	do {
+		cmd.cmdidx = opcode;
+		cmd.resp_type = MMC_RSP_R1;
+		cmd.cmdarg = 0;
+
+		if (tuning_loop_counter-- == 0)
+			break;
+
+		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
+			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
+		else
+			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);
+
+		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
+
+		mmc_send_cmd(mmc, &cmd, NULL);
+
+		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
+
+	if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
+		printf("%s:Tuning failed\n", __func__);
+		ret = -1;
+	}
+
+	if (tuning_loop_counter < 0) {
+		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
+		sdhci_writel(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 0;
+}
+
+static struct sdhci_ops rockchip_sdhci_ops = {
+	.set_ios_post	= rockchip_sdhci_set_ios_post,
+	.platform_execute_tuning = &rockchip_sdhci_execute_tuning,
 };
 
-static int arasan_sdhci_probe(struct udevice *dev)
+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 mmc_config *cfg = &plat->cfg;
 	struct sdhci_host *host = &prv->host;
-	int max_frequency, ret;
 	struct clk clk;
+	int ret;
 
-#if CONFIG_IS_ENABLED(OF_PLATDATA)
-	struct dtd_rockchip_rk3399_sdhci_5_1 *dtplat = &plat->dtplat;
-
-	host->name = dev->name;
-	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
-	max_frequency = dtplat->max_frequency;
-	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
-#else
-	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
+	host->max_clk = cfg->f_max;
 	ret = clk_get_by_index(dev, 0, &clk);
-#endif
 	if (!ret) {
-		ret = clk_set_rate(&clk, max_frequency);
+		ret = clk_set_rate(&clk, host->max_clk);
 		if (IS_ERR_VALUE(ret))
 			printf("%s clk set rate fail!\n", __func__);
 	} else {
 		printf("%s fail to get clk\n", __func__);
 	}
 
+	prv->emmc_clk = clk;
+	prv->dev = dev;
+	ret = data->get_phy(dev);
+	if (ret)
+		return ret;
+
+	ret = data->emmc_phy_init(dev);
+	if (ret)
+		return ret;
+
+	host->ops = &rockchip_sdhci_ops;
+
 	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
-	host->max_clk = max_frequency;
-	/*
-	 * The sdhci-driver only supports 4bit and 8bit, as sdhci_setup_cfg
-	 * doesn't allow us to clear MMC_MODE_4BIT.  Consequently, we don't
-	 * check for other bus-width values.
-	 */
-	if (host->bus_width == 8)
-		host->host_caps |= MMC_MODE_8BIT;
 
 	host->mmc = &plat->mmc;
 	host->mmc->priv = &prv->host;
 	host->mmc->dev = dev;
 	upriv->mmc = host->mmc;
 
-	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);
+	ret = sdhci_setup_cfg(cfg, host, cfg->f_max, EMMC_MIN_FREQ);
 	if (ret)
 		return ret;
 
 	return sdhci_probe(dev);
 }
 
-static int arasan_sdhci_of_to_plat(struct udevice *dev)
+static int rockchip_sdhci_of_to_plat(struct udevice *dev)
 {
-#if !CONFIG_IS_ENABLED(OF_PLATDATA)
+	struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
 	struct sdhci_host *host = dev_get_priv(dev);
+	struct mmc_config *cfg = &plat->cfg;
+	int ret;
 
 	host->name = dev->name;
 	host->ioaddr = dev_read_addr_ptr(dev);
-	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
-#endif
+
+	ret = mmc_of_parse(dev, cfg);
+	if (ret)
+		return ret;
 
 	return 0;
 }
@@ -101,19 +330,28 @@ static int rockchip_sdhci_bind(struct udevice *dev)
 	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
 }
 
-static const struct udevice_id arasan_sdhci_ids[] = {
-	{ .compatible = "arasan,sdhci-5.1" },
+static const struct sdhci_data rk3399_data = {
+	.emmc_set_clock = rk3399_sdhci_emmc_set_clock,
+	.get_phy = rk3399_emmc_get_phy,
+	.emmc_phy_init = rk3399_emmc_phy_init,
+};
+
+static const struct udevice_id sdhci_ids[] = {
+	{
+		.compatible = "arasan,sdhci-5.1",
+		.data = (ulong)&rk3399_data,
+	},
 	{ }
 };
 
 U_BOOT_DRIVER(arasan_sdhci_drv) = {
-	.name		= "rockchip_rk3399_sdhci_5_1",
+	.name		= "rockchip_sdhci_5_1",
 	.id		= UCLASS_MMC,
-	.of_match	= arasan_sdhci_ids,
-	.of_to_plat = arasan_sdhci_of_to_plat,
+	.of_match	= sdhci_ids,
+	.of_to_plat	= rockchip_sdhci_of_to_plat,
 	.ops		= &sdhci_ops,
 	.bind		= rockchip_sdhci_bind,
-	.probe		= arasan_sdhci_probe,
+	.probe		= rockchip_sdhci_probe,
 	.priv_auto	= sizeof(struct rockchip_sdhc),
 	.plat_auto	= sizeof(struct rockchip_sdhc_plat),
 };
-- 
2.17.1

[PATCH v2 2/3] mmc: rockchip_sdhci: Add support for RK3568

[Yifeng Zhao]

This patch adds support for the RK3568 platform to this driver.

Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
---

Changes in v2:
- Used sdhci_set_clock api to set clock.
- Used read_poll_timeout api to check dll status.

 drivers/mmc/rockchip_sdhci.c | 117 +++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index 2973911446..5ac524c9c9 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -42,6 +42,34 @@
 	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
 	PHYCTRL_DLLRDY_DONE)
 
+/* 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 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_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_DLYENA		BIT(27)
+#define DLL_TXCLK_TAPNUM_DEFAULT	0x10
+#define DLL_STRBIN_TAPNUM_DEFAULT	0x3
+#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 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))
+#define ROCKCHIP_MAX_CLKS		3
+
 struct rockchip_sdhc_plat {
 	struct mmc_config cfg;
 	struct mmc mmc;
@@ -178,6 +206,85 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clo
 	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);
+	int max_clk = host->max_clk;
+	unsigned long mmc_clock;
+	int val, ret;
+	u32 extra;
+
+	if (clock) {
+		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);
+		if (IS_ERR_VALUE(mmc_clock))
+			host->max_clk = max_clk;
+		else
+			host->max_clk = mmc_clock;
+	}
+
+	sdhci_set_clock(host->mmc, clock);
+	if (!clock)
+		return 0;
+	host->max_clk = max_clk;
+
+	if (clock >= 100 * MHz) {
+		/* reset DLL */
+		sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, DWCMSHC_EMMC_DLL_CTRL);
+		udelay(1);
+		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
+
+		/* Init DLL settings */
+		extra = DWCMSHC_EMMC_DLL_START_DEFAULT << DWCMSHC_EMMC_DLL_START_POINT |
+			DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |
+			DWCMSHC_EMMC_DLL_START;
+		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
+
+		ret = read_poll_timeout(readl, host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
+					val, DLL_LOCK_WO_TMOUT(val), 1, 500);
+		if (ret)
+			return ret;
+
+		extra = DWCMSHC_EMMC_DLL_DLYENA |
+			DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+
+		extra = DWCMSHC_EMMC_DLL_DLYENA |
+			DLL_TXCLK_TAPNUM_DEFAULT |
+			DLL_TXCLK_TAPNUM_FROM_SW;
+		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
+
+		extra = DWCMSHC_EMMC_DLL_DLYENA |
+			DLL_STRBIN_TAPNUM_DEFAULT;
+		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
+	} else {
+		/* reset the clock phase when the frequency is lower than 100MHz */
+		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
+		extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
+		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
+	}
+
+	return 0;
+}
+
+static int rk3568_emmc_get_phy(struct udevice *dev)
+{
+	return 0;
+}
+
 static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
 {
 	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
@@ -336,11 +443,21 @@ static const struct sdhci_data rk3399_data = {
 	.emmc_phy_init = rk3399_emmc_phy_init,
 };
 
+static const struct sdhci_data rk3568_data = {
+	.emmc_set_clock = rk3568_sdhci_emmc_set_clock,
+	.get_phy = rk3568_emmc_get_phy,
+	.emmc_phy_init = rk3568_emmc_phy_init,
+};
+
 static const struct udevice_id sdhci_ids[] = {
 	{
 		.compatible = "arasan,sdhci-5.1",
 		.data = (ulong)&rk3399_data,
 	},
+	{
+		.compatible = "rockchip,rk3568-dwcmshc",
+		.data = (ulong)&rk3568_data,
+	},
 	{ }
 };
 
-- 
2.17.1

[PATCH v2 3/3] rockchip: config: evb-rk3399: add hs200 and hs400 support

[Yifeng Zhao]

This enable hs200 and hs400 support for emmc on evb-rk3399.

Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
---

(no changes since v1)

 configs/evb-rk3399_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/evb-rk3399_defconfig b/configs/evb-rk3399_defconfig
index 909c68822c..687aeec26c 100644
--- a/configs/evb-rk3399_defconfig
+++ b/configs/evb-rk3399_defconfig
@@ -29,6 +29,7 @@ CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_ROCKCHIP_GPIO=y
 CONFIG_SYS_I2C_ROCKCHIP=y
 CONFIG_MISC=y
+CONFIG_MMC_HS400_SUPPORT=y
 CONFIG_MMC_DW=y
 CONFIG_MMC_DW_ROCKCHIP=y
 CONFIG_MMC_SDHCI=y
-- 
2.17.1

Re: [PATCH v2 3/3] rockchip: config: evb-rk3399: add hs200 and hs400 support

[Philipp Tomsich]

On Mon, 28 Jun 2021 at 11:20, Yifeng Zhao <yifeng.zhao@rock-chips.com> wrote:
>
> This enable hs200 and hs400 support for emmc on evb-rk3399.
>
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>

Re: [PATCH v2 0/3]

[Philipp Tomsich]

I had attempted to merge HDMI using this approach in 2017, but after
some discussions with Simon, we used a 'mini-drivers' approach.
I would like to see a similar approach taken here.

See
https://patchwork.ozlabs.org/project/uboot/patch/1493394792-20743-4-git-send-email-philipp.tomsich@theobroma-systems.com/#1650260
for the original discussion between Simon and me.

Thanks,
Philipp.


On Mon, 28 Jun 2021 at 11:20, Yifeng Zhao <yifeng.zhao@rock-chips.com>
wrote:

> RK3399 and RK3568 are use different sdhci controllers.
> The drivers need to be updated to support these two platforms
> and it's easy to support new platforms.
>
>
> Changes in v2:
> - Add mmc_of_parse to parse dts config.
> - Used read_poll_timeout api to check dll lock status
> - Add execute tuning api for hs200 tuning
> - Used sdhci_set_clock api to set clock.
> - Used read_poll_timeout api to check dll status.
>
> Yifeng Zhao (3):
>   mmc: rockchip_sdhci: add phy and clock config for rk3399
>   mmc: rockchip_sdhci: Add support for RK3568
>   rockchip: config: evb-rk3399: add hs200 and hs400 support
>
>  configs/evb-rk3399_defconfig |   1 +
>  drivers/mmc/rockchip_sdhci.c | 427 ++++++++++++++++++++++++++++++++---
>  2 files changed, 392 insertions(+), 36 deletions(-)
>
> --
> 2.17.1
>
>
>
>

Re: [PATCH v2 0/3]

[Peter Robinson]

Was the original mails of this series sent to the list? I don't see
them in the archives or patchwork.

On Mon, Jun 28, 2021 at 10:26 AM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> I had attempted to merge HDMI using this approach in 2017, but after
> some discussions with Simon, we used a 'mini-drivers' approach.
> I would like to see a similar approach taken here.
>
> See
> https://patchwork.ozlabs.org/project/uboot/patch/1493394792-20743-4-git-send-email-philipp.tomsich@theobroma-systems.com/#1650260
> for the original discussion between Simon and me.
>
> Thanks,
> Philipp.
>
>
> On Mon, 28 Jun 2021 at 11:20, Yifeng Zhao <yifeng.zhao@rock-chips.com>
> wrote:
>
> > RK3399 and RK3568 are use different sdhci controllers.
> > The drivers need to be updated to support these two platforms
> > and it's easy to support new platforms.
> >
> >
> > Changes in v2:
> > - Add mmc_of_parse to parse dts config.
> > - Used read_poll_timeout api to check dll lock status
> > - Add execute tuning api for hs200 tuning
> > - Used sdhci_set_clock api to set clock.
> > - Used read_poll_timeout api to check dll status.
> >
> > Yifeng Zhao (3):
> >   mmc: rockchip_sdhci: add phy and clock config for rk3399
> >   mmc: rockchip_sdhci: Add support for RK3568
> >   rockchip: config: evb-rk3399: add hs200 and hs400 support
> >
> >  configs/evb-rk3399_defconfig |   1 +
> >  drivers/mmc/rockchip_sdhci.c | 427 ++++++++++++++++++++++++++++++++---
> >  2 files changed, 392 insertions(+), 36 deletions(-)
> >
> > --
> > 2.17.1
> >
> >
> >
> >

Re: [PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

[Jaehoon Chung]

Hi Yifeng,

On 6/28/21 6:19 PM, Yifeng Zhao wrote:
> Add clock, phy and other configuration, it is convenient to support
> new controller. Here a short summary of the changes:
> - Add mmc_of_parse to parse dts config.
> - Remove OF_PLATDATA related code.
> - Reorder header inclusion.
> - Add phy ops.
> - add ops set_ios_post to modify the parameters of phy when the
>   clock changes.
> - Add execute tuning api for hs200 tuning
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> ---
> 
> Changes in v2:
> - Add mmc_of_parse to parse dts config.
> - Used read_poll_timeout api to check dll lock status
> - Add execute tuning api for hs200 tuning
> 
>  drivers/mmc/rockchip_sdhci.c | 310 +++++++++++++++++++++++++++++++----
>  1 file changed, 274 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index d95f8b2a15..2973911446 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -6,90 +6,319 @@
>   */
>  
>  #include <common.h>
> +#include <clk.h>
>  #include <dm.h>
> +#include <dm/ofnode.h>
>  #include <dt-structs.h>
> +#include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/libfdt.h>
> +#include <linux/iopoll.h>
>  #include <malloc.h>
>  #include <mapmem.h>
> +#include "mmc_private.h"
>  #include <sdhci.h>
> -#include <clk.h>
> +#include <syscon.h>
> +#include <asm/arch-rockchip/clock.h>
> +#include <asm/arch-rockchip/hardware.h>
>  
>  /* 400KHz is max freq for card ID etc. Use that as min */
>  #define EMMC_MIN_FREQ	400000
> +#define KHz	(1000)
> +#define MHz	(1000 * KHz)
> +#define SDHCI_TUNING_LOOP_COUNT		40
> +
> +#define PHYCTRL_CALDONE_MASK		0x1
> +#define PHYCTRL_CALDONE_SHIFT		0x6
> +#define PHYCTRL_CALDONE_DONE		0x1
> +#define PHYCTRL_DLLRDY_MASK		0x1
> +#define PHYCTRL_DLLRDY_SHIFT		0x5
> +#define PHYCTRL_DLLRDY_DONE		0x1
> +#define PHYCTRL_FREQSEL_200M		0x0
> +#define PHYCTRL_FREQSEL_50M		0x1
> +#define PHYCTRL_FREQSEL_100M		0x2
> +#define PHYCTRL_FREQSEL_150M		0x3
> +#define PHYCTRL_DLL_LOCK_WO_TMOUT(x)	\
> +	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
> +	PHYCTRL_DLLRDY_DONE)
>  
>  struct rockchip_sdhc_plat {
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
> -#endif
>  	struct mmc_config cfg;
>  	struct mmc mmc;
>  };
>  
> +struct rockchip_emmc_phy {
> +	u32 emmcphy_con[7];
> +	u32 reserved;
> +	u32 emmcphy_status;
> +};
> +
>  struct rockchip_sdhc {
>  	struct sdhci_host host;
> +	struct udevice *dev;
>  	void *base;
> +	struct rockchip_emmc_phy *phy;
> +	struct clk emmc_clk;
> +};
> +
> +struct sdhci_data {
> +	int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
> +	int (*emmc_phy_init)(struct udevice *dev);
> +	int (*get_phy)(struct udevice *dev);
> +};
> +
> +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;
> +
> +	writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);

RK_CLRSETBITS macro is (((clr) | (set))) << 16) | (set).
Which bit is cleared? When (7 << 4) is set, it means that cleared?


> +	writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
> +
> +	/*
> +	 * According to the user manual, calpad calibration
> +	 * cycle takes more than 2us without the minimal recommended
> +	 * value, so we may need a little margin here
> +	 */
> +	udelay(3);
> +	writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
> +
> +	/*
> +	 * According to the user manual, it asks driver to
> +	 * wait 5us for calpad busy trimming. But it seems that
> +	 * 5us of caldone isn't enough for all cases.
> +	 */
> +	udelay(500);
> +	caldone = readl(&phy->emmcphy_status);
> +	caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> +	if (caldone != PHYCTRL_CALDONE_DONE) {
> +		printf("%s: caldone timeout.\n", __func__);
> +		return;
> +	}
> +
> +	/* Set the frequency of the DLL operation */
> +	if (clock < 75 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_50M;
> +	else if (clock < 125 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_100M;
> +	else if (clock < 175 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_150M;
> +	else
> +		freqsel = PHYCTRL_FREQSEL_200M;
> +
> +	/* Set the frequency of the DLL operation */
> +	writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
> +
> +	read_poll_timeout(readl, &phy->emmcphy_status, dllrdy,
> +			  PHYCTRL_DLL_LOCK_WO_TMOUT(dllrdy), 1, 5000);
> +}
> +
> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
> +{
> +	writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
> +	writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
> +}
> +
> +static int rk3399_emmc_get_phy(struct udevice *dev)
> +{
> +	struct rockchip_sdhc *priv = dev_get_priv(dev);
> +
> +	ofnode phy_node;
> +	void *grf_base;
> +	u32 grf_phy_offset, phandle;
> +
> +	phandle = dev_read_u32_default(dev, "phys", 0);
> +	phy_node = ofnode_get_by_phandle(phandle);
> +	if (!ofnode_valid(phy_node)) {
> +		debug("Not found emmc phy device\n");
> +		return -ENODEV;
> +	}
> +
> +	grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +	if (grf_base < 0)
> +		printf("%s Get syscon grf failed", __func__);
> +	grf_phy_offset = ofnode_read_u32_default(phy_node, "reg", 0);
> +
> +	priv->phy = (struct rockchip_emmc_phy *)(grf_base + grf_phy_offset);

If grf_base is failed to get, is (grf_base + grp_phy_offset) correct?

> +
> +	return 0;
> +}
> +
> +static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
> +	int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;
> +	int max_clk = host->max_clk;
> +	unsigned long mmc_clock;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_off(priv->phy);
> +
> +	if (clock) {
> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);
> +		if (IS_ERR_VALUE(mmc_clock))
> +			host->max_clk = max_clk;
> +		else
> +			host->max_clk = mmc_clock;
> +	}
> +	sdhci_set_clock(host->mmc, clock);
> +	if (!clock)
> +		return 0;
> +	host->max_clk = max_clk;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_on(priv->phy, clock);
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
> +{
> +	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;
> +	uint clock = mmc->tran_speed;
> +	u32 reg;
> +
> +	if (!clock)
> +		clock = mmc->clock;
> +	data->emmc_set_clock(host, clock);

I think that it needs to check whether callback function is null or not.
It's possible to exist the case that not to assign to callback function in future.

> +
> +	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_CTRL_HS400;
> +		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> +	} else {
> +		sdhci_set_uhs_timing(host);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
> +{
> +	struct sdhci_host *host = dev_get_priv(mmc->dev);
> +	char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;
> +	struct mmc_cmd cmd;
> +	u32 ctrl;
> +	int ret = 0;
> +
> +	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);
> +
> +	do {
> +		cmd.cmdidx = opcode;
> +		cmd.resp_type = MMC_RSP_R1;
> +		cmd.cmdarg = 0;
> +
> +		if (tuning_loop_counter-- == 0)
> +			break;
> +
> +		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
> +		else
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);

Why it needs to pass to 7? It seems that always (dma & 0x7) in SDHCI_MAKE_BLKSZ.
#define SDHCI_MAKE_BLKSZ(blksz) (1 << 12) | (blksz & 0xFFF)?


> +
> +		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
> +
> +		mmc_send_cmd(mmc, &cmd, NULL);
> +
> +		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
> +
> +	if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
> +		printf("%s:Tuning failed\n", __func__);
> +		ret = -1;
> +	}
> +
> +	if (tuning_loop_counter < 0) {
> +		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> +		sdhci_writel(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 0;

Doesn't need to return "ret" when it's failed?

> +}
> +
> +static struct sdhci_ops rockchip_sdhci_ops = {
> +	.set_ios_post	= rockchip_sdhci_set_ios_post,
> +	.platform_execute_tuning = &rockchip_sdhci_execute_tuning,
>  };
>  
> -static int arasan_sdhci_probe(struct udevice *dev)
> +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 mmc_config *cfg = &plat->cfg;
>  	struct sdhci_host *host = &prv->host;
> -	int max_frequency, ret;
>  	struct clk clk;
> +	int ret;
>  
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 *dtplat = &plat->dtplat;
> -
> -	host->name = dev->name;
> -	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
> -	max_frequency = dtplat->max_frequency;
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
> -#else
> -	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
> +	host->max_clk = cfg->f_max;
>  	ret = clk_get_by_index(dev, 0, &clk);
> -#endif
>  	if (!ret) {
> -		ret = clk_set_rate(&clk, max_frequency);
> +		ret = clk_set_rate(&clk, host->max_clk);
>  		if (IS_ERR_VALUE(ret))
>  			printf("%s clk set rate fail!\n", __func__);
>  	} else {
>  		printf("%s fail to get clk\n", __func__);
>  	}
>  
> +	prv->emmc_clk = clk;
> +	prv->dev = dev;
> +	ret = data->get_phy(dev);

Ditto. Check data->get_phy.

> +	if (ret)
> +		return ret;
> +
> +	ret = data->emmc_phy_init(dev);

Ditto.

Best Regards,
Jaehoon Chung

> +	if (ret)
> +		return ret;
> +
> +	host->ops = &rockchip_sdhci_ops;
> +
>  	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
> -	host->max_clk = max_frequency;
> -	/*
> -	 * The sdhci-driver only supports 4bit and 8bit, as sdhci_setup_cfg
> -	 * doesn't allow us to clear MMC_MODE_4BIT.  Consequently, we don't
> -	 * check for other bus-width values.
> -	 */
> -	if (host->bus_width == 8)
> -		host->host_caps |= MMC_MODE_8BIT;
>  
>  	host->mmc = &plat->mmc;
>  	host->mmc->priv = &prv->host;
>  	host->mmc->dev = dev;
>  	upriv->mmc = host->mmc;
>  
> -	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);
> +	ret = sdhci_setup_cfg(cfg, host, cfg->f_max, EMMC_MIN_FREQ);
>  	if (ret)
>  		return ret;
>  
>  	return sdhci_probe(dev);
>  }
>  
> -static int arasan_sdhci_of_to_plat(struct udevice *dev)
> +static int rockchip_sdhci_of_to_plat(struct udevice *dev)
>  {
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> +	struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
>  	struct sdhci_host *host = dev_get_priv(dev);
> +	struct mmc_config *cfg = &plat->cfg;
> +	int ret;
>  
>  	host->name = dev->name;
>  	host->ioaddr = dev_read_addr_ptr(dev);
> -	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
> -#endif
> +
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;
>  
>  	return 0;
>  }
> @@ -101,19 +330,28 @@ static int rockchip_sdhci_bind(struct udevice *dev)
>  	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
>  }
>  
> -static const struct udevice_id arasan_sdhci_ids[] = {
> -	{ .compatible = "arasan,sdhci-5.1" },
> +static const struct sdhci_data rk3399_data = {
> +	.emmc_set_clock = rk3399_sdhci_emmc_set_clock,
> +	.get_phy = rk3399_emmc_get_phy,
> +	.emmc_phy_init = rk3399_emmc_phy_init,
> +};
> +
> +static const struct udevice_id sdhci_ids[] = {
> +	{
> +		.compatible = "arasan,sdhci-5.1",
> +		.data = (ulong)&rk3399_data,
> +	},
>  	{ }
>  };
>  
>  U_BOOT_DRIVER(arasan_sdhci_drv) = {
> -	.name		= "rockchip_rk3399_sdhci_5_1",
> +	.name		= "rockchip_sdhci_5_1",
>  	.id		= UCLASS_MMC,
> -	.of_match	= arasan_sdhci_ids,
> -	.of_to_plat = arasan_sdhci_of_to_plat,
> +	.of_match	= sdhci_ids,
> +	.of_to_plat	= rockchip_sdhci_of_to_plat,
>  	.ops		= &sdhci_ops,
>  	.bind		= rockchip_sdhci_bind,
> -	.probe		= arasan_sdhci_probe,
> +	.probe		= rockchip_sdhci_probe,
>  	.priv_auto	= sizeof(struct rockchip_sdhc),
>  	.plat_auto	= sizeof(struct rockchip_sdhc_plat),
>  };
> 

Re: [PATCH v2 2/3] mmc: rockchip_sdhci: Add support for RK3568

[Jaehoon Chung]

On 6/28/21 6:19 PM, Yifeng Zhao wrote:
> This patch adds support for the RK3568 platform to this driver.
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> ---
> 
> Changes in v2:
> - Used sdhci_set_clock api to set clock.
> - Used read_poll_timeout api to check dll status.
> 
>  drivers/mmc/rockchip_sdhci.c | 117 +++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index 2973911446..5ac524c9c9 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -42,6 +42,34 @@
>  	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
>  	PHYCTRL_DLLRDY_DONE)
>  
> +/* 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 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_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_DLYENA		BIT(27)
> +#define DLL_TXCLK_TAPNUM_DEFAULT	0x10
> +#define DLL_STRBIN_TAPNUM_DEFAULT	0x3
> +#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 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))
> +#define ROCKCHIP_MAX_CLKS		3
> +
>  struct rockchip_sdhc_plat {
>  	struct mmc_config cfg;
>  	struct mmc mmc;
> @@ -178,6 +206,85 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clo
>  	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);
> +	int max_clk = host->max_clk;
> +	unsigned long mmc_clock;
> +	int val, ret;
> +	u32 extra;
> +
> +	if (clock) {
> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);

Not need to use mmc_clock. It's possible to assign to host->max_clk directly.

host->max_clk = clk_set_rate();
if (IS_ERR_VALUE(host->max_clk))
	host->max_clk = max_clk;

> +		if (IS_ERR_VALUE(mmc_clock))
> +			host->max_clk = max_clk;
> +		else
> +			host->max_clk = mmc_clock;
> +	}
> +
> +	sdhci_set_clock(host->mmc, clock);
> +	if (!clock)
> +		return 0;
> +	host->max_clk = max_clk;

Why reassign with max_clk?

> +
> +	if (clock >= 100 * MHz) {
> +		/* reset DLL */
> +		sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, DWCMSHC_EMMC_DLL_CTRL);
> +		udelay(1);
> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +
> +		/* Init DLL settings */
> +		extra = DWCMSHC_EMMC_DLL_START_DEFAULT << DWCMSHC_EMMC_DLL_START_POINT |
> +			DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |
> +			DWCMSHC_EMMC_DLL_START;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
> +
> +		ret = read_poll_timeout(readl, host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,
> +					val, DLL_LOCK_WO_TMOUT(val), 1, 500);
> +		if (ret)
> +			return ret;
> +
> +		extra = DWCMSHC_EMMC_DLL_DLYENA |
> +			DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> +		extra = DWCMSHC_EMMC_DLL_DLYENA |
> +			DLL_TXCLK_TAPNUM_DEFAULT |
> +			DLL_TXCLK_TAPNUM_FROM_SW;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
> +
> +		extra = DWCMSHC_EMMC_DLL_DLYENA |
> +			DLL_STRBIN_TAPNUM_DEFAULT;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> +	} else {
> +		/* reset the clock phase when the frequency is lower than 100MHz */
> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +		extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rk3568_emmc_get_phy(struct udevice *dev)
> +{
> +	return 0;
> +}
> +
>  static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
>  {
>  	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
> @@ -336,11 +443,21 @@ static const struct sdhci_data rk3399_data = {
>  	.emmc_phy_init = rk3399_emmc_phy_init,
>  };
>  
> +static const struct sdhci_data rk3568_data = {
> +	.emmc_set_clock = rk3568_sdhci_emmc_set_clock,
> +	.get_phy = rk3568_emmc_get_phy,
> +	.emmc_phy_init = rk3568_emmc_phy_init,
> +};
> +
>  static const struct udevice_id sdhci_ids[] = {
>  	{
>  		.compatible = "arasan,sdhci-5.1",
>  		.data = (ulong)&rk3399_data,
>  	},
> +	{
> +		.compatible = "rockchip,rk3568-dwcmshc",
> +		.data = (ulong)&rk3568_data,
> +	},
>  	{ }
>  };
>  
> 

Re: Re: [PATCH v2 2/3] mmc: rockchip_sdhci: Add support for RK3568

[赵仪峰]

Hi Jeahoon,


>On 6/28/21 6:19 PM, Yifeng Zhao wrote:



>> This patch adds support for the RK3568 platform to this driver.



>> 



>> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>



>> ---



>> 



>> Changes in v2:



>> - Used sdhci_set_clock api to set clock.



>> - Used read_poll_timeout api to check dll status.



>> 



>>  drivers/mmc/rockchip_sdhci.c | 117 +++++++++++++++++++++++++++++++++++



>>  1 file changed, 117 insertions(+)



>> 



>> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c



>> index 2973911446..5ac524c9c9 100644



>> --- a/drivers/mmc/rockchip_sdhci.c



>> +++ b/drivers/mmc/rockchip_sdhci.c



>> @@ -42,6 +42,34 @@



>>  	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\



>>  	PHYCTRL_DLLRDY_DONE)



>>  



>> +/* 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 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_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_DLYENA		BIT(27)



>> +#define DLL_TXCLK_TAPNUM_DEFAULT	0x10



>> +#define DLL_STRBIN_TAPNUM_DEFAULT	0x3



>> +#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 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))



>> +#define ROCKCHIP_MAX_CLKS		3



>> +



>>  struct rockchip_sdhc_plat {



>>  	struct mmc_config cfg;



>>  	struct mmc mmc;



>> @@ -178,6 +206,85 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clo



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



>> +	int max_clk = host->max_clk;



>> +	unsigned long mmc_clock;



>> +	int val, ret;



>> +	u32 extra;



>> +



>> +	if (clock) {



>> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);



>



>Not need to use mmc_clock. It's possible to assign to host->max_clk directly.



>



>host->max_clk = clk_set_rate();



>if (IS_ERR_VALUE(host->max_clk))



>	host->max_clk = max_clk;




The variable types are different, IS_ERR_VALUE(host->max_clk) will not got the right results.

unsigned int max_clk;

#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)


>> +		if (IS_ERR_VALUE(mmc_clock))



>> +			host->max_clk = max_clk;



>> +		else



>> +			host->max_clk = mmc_clock;



>> +	}



>> +



>> +	sdhci_set_clock(host->mmc, clock);



>> +	if (!clock)



>> +		return 0;



>> +	host->max_clk = max_clk;



>



>Why reassign with max_clk?


The host->max_clk is config by dts and need to reassign.
I will recheck  the div config in sdhci_set_clock， it maybe work and no need to call clk_set_rate() to chang the source clock??


>



>> +



>> +	if (clock >= 100 * MHz) {



>> +		/* reset DLL */



>> +		sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, DWCMSHC_EMMC_DLL_CTRL);



>> +		udelay(1);



>> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);



>> +



>> +		/* Init DLL settings */



>> +		extra = DWCMSHC_EMMC_DLL_START_DEFAULT << DWCMSHC_EMMC_DLL_START_POINT |



>> +			DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |



>> +			DWCMSHC_EMMC_DLL_START;



>> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);



>> +



>> +		ret = read_poll_timeout(readl, host->ioaddr + DWCMSHC_EMMC_DLL_STATUS0,



>> +					val, DLL_LOCK_WO_TMOUT(val), 1, 500);



>> +		if (ret)



>> +			return ret;



>> +



>> +		extra = DWCMSHC_EMMC_DLL_DLYENA |



>> +			DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;



>> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);



>> +



>> +		extra = DWCMSHC_EMMC_DLL_DLYENA |



>> +			DLL_TXCLK_TAPNUM_DEFAULT |



>> +			DLL_TXCLK_TAPNUM_FROM_SW;



>> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);



>> +



>> +		extra = DWCMSHC_EMMC_DLL_DLYENA |



>> +			DLL_STRBIN_TAPNUM_DEFAULT;



>> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);



>> +	} else {



>> +		/* reset the clock phase when the frequency is lower than 100MHz */



>> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);



>> +		extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;



>> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);



>> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);



>> +		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);



>> +	}



>> +



>> +	return 0;



>> +}



>> +



>> +static int rk3568_emmc_get_phy(struct udevice *dev)



>> +{



>> +	return 0;



>> +}



>> +



>>  static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)



>>  {



>>  	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);



>> @@ -336,11 +443,21 @@ static const struct sdhci_data rk3399_data = {



>>  	.emmc_phy_init = rk3399_emmc_phy_init,



>>  };



>>  



>> +static const struct sdhci_data rk3568_data = {



>> +	.emmc_set_clock = rk3568_sdhci_emmc_set_clock,



>> +	.get_phy = rk3568_emmc_get_phy,



>> +	.emmc_phy_init = rk3568_emmc_phy_init,



>> +};



>> +



>>  static const struct udevice_id sdhci_ids[] = {



>>  	{



>>  		.compatible = "arasan,sdhci-5.1",



>>  		.data = (ulong)&rk3399_data,



>>  	},



>> +	{



>> +		.compatible = "rockchip,rk3568-dwcmshc",



>> +		.data = (ulong)&rk3568_data,



>> +	},



>>  	{ }



>>  };



>>  



>> 



>



>



>



>

Re: [PATCH v2 2/3] mmc: rockchip_sdhci: Add support for RK3568

[Jaehoon Chung]

Hi,

On 6/29/21 11:07 AM, 赵仪峰 wrote:
> 
> 
> The variable types are different, IS_ERR_VALUE(host->max_clk) will not got the right results.
> 
> unsigned int max_clk;
> 
> #define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)

Thanks for kindly explanation. 
I didn't check its type. 

Best Regards,
Jaehoon Chung

> 

Re: Re: [PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

[赵仪峰]

Hi Jaehoon,


>RK_CLRSETBITS macro is (((clr) | (set))) << 16) | (set).

>Which bit is cleared? When (7 << 4) is set, it means that cleared?

Yes, this ops clear the bit4-bit6.


>> +

>> +		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)

>> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);

>> +		else

>> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);

>

>Why it needs to pass to 7? It seems that always (dma & 0x7) in SDHCI_MAKE_BLKSZ.

>#define SDHCI_MAKE_BLKSZ(blksz) (1 << 12) | (blksz & 0xFFF)?

In sdhci.h defined ： 
#define  SDHCI_MAKE_BLKSZ(dma, blksz) (((dma & 0x7) << 12) | (blksz & 0xFFF))
....
#define SDHCI_DEFAULT_BOUNDARY_ARG	(7)

I will modify the code using SDHCI_DEFAULT_BOUNDARY_ARG， refer to iproc_sdhci.c

blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 64);
if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
	blk_size = SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG, 128);
sdhci_writew(host, blk_size, SDHCI_BLOCK_SIZE);




>Hi Yifeng,



>



>On 6/28/21 6:19 PM, Yifeng Zhao wrote:



>> Add clock, phy and other configuration, it is convenient to support



>> new controller. Here a short summary of the changes:



>> - Add mmc_of_parse to parse dts config.



>> - Remove OF_PLATDATA related code.



>> - Reorder header inclusion.



>> - Add phy ops.



>> - add ops set_ios_post to modify the parameters of phy when the



>>   clock changes.



>> - Add execute tuning api for hs200 tuning



>> 



>> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>



>> ---



>> 



>> Changes in v2:



>> - Add mmc_of_parse to parse dts config.



>> - Used read_poll_timeout api to check dll lock status



>> - Add execute tuning api for hs200 tuning



>> 



>>  drivers/mmc/rockchip_sdhci.c | 310 +++++++++++++++++++++++++++++++----



>>  1 file changed, 274 insertions(+), 36 deletions(-)



>> 



>> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c



>> index d95f8b2a15..2973911446 100644



>> --- a/drivers/mmc/rockchip_sdhci.c



>> +++ b/drivers/mmc/rockchip_sdhci.c



>> @@ -6,90 +6,319 @@



>>   */



>>  



>>  #include <common.h>



>> +#include <clk.h>



>>  #include <dm.h>



>> +#include <dm/ofnode.h>



>>  #include <dt-structs.h>



>> +#include <linux/delay.h>



>>  #include <linux/err.h>



>>  #include <linux/libfdt.h>



>> +#include <linux/iopoll.h>



>>  #include <malloc.h>



>>  #include <mapmem.h>



>> +#include "mmc_private.h"



>>  #include <sdhci.h>



>> -#include <clk.h>



>> +#include <syscon.h>



>> +#include <asm/arch-rockchip/clock.h>



>> +#include <asm/arch-rockchip/hardware.h>



>>  



>>  /* 400KHz is max freq for card ID etc. Use that as min */



>>  #define EMMC_MIN_FREQ	400000



>> +#define KHz	(1000)



>> +#define MHz	(1000 * KHz)



>> +#define SDHCI_TUNING_LOOP_COUNT		40



>> +



>> +#define PHYCTRL_CALDONE_MASK		0x1



>> +#define PHYCTRL_CALDONE_SHIFT		0x6



>> +#define PHYCTRL_CALDONE_DONE		0x1



>> +#define PHYCTRL_DLLRDY_MASK		0x1



>> +#define PHYCTRL_DLLRDY_SHIFT		0x5



>> +#define PHYCTRL_DLLRDY_DONE		0x1



>> +#define PHYCTRL_FREQSEL_200M		0x0



>> +#define PHYCTRL_FREQSEL_50M		0x1



>> +#define PHYCTRL_FREQSEL_100M		0x2



>> +#define PHYCTRL_FREQSEL_150M		0x3



>> +#define PHYCTRL_DLL_LOCK_WO_TMOUT(x)	\



>> +	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\



>> +	PHYCTRL_DLLRDY_DONE)



>>  



>>  struct rockchip_sdhc_plat {



>> -#if CONFIG_IS_ENABLED(OF_PLATDATA)



>> -	struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;



>> -#endif



>>  	struct mmc_config cfg;



>>  	struct mmc mmc;



>>  };



>>  



>> +struct rockchip_emmc_phy {



>> +	u32 emmcphy_con[7];



>> +	u32 reserved;



>> +	u32 emmcphy_status;



>> +};



>> +



>>  struct rockchip_sdhc {



>>  	struct sdhci_host host;



>> +	struct udevice *dev;



>>  	void *base;



>> +	struct rockchip_emmc_phy *phy;



>> +	struct clk emmc_clk;



>> +};



>> +



>> +struct sdhci_data {



>> +	int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);



>> +	int (*emmc_phy_init)(struct udevice *dev);



>> +	int (*get_phy)(struct udevice *dev);



>> +};



>> +



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



>> +



>> +	writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);



>



>RK_CLRSETBITS macro is (((clr) | (set))) << 16) | (set).



>Which bit is cleared? When (7 << 4) is set, it means that cleared?



>



>



>> +	writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);



>> +	writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);



>> +



>> +	/*



>> +	 * According to the user manual, calpad calibration



>> +	 * cycle takes more than 2us without the minimal recommended



>> +	 * value, so we may need a little margin here



>> +	 */



>> +	udelay(3);



>> +	writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);



>> +



>> +	/*



>> +	 * According to the user manual, it asks driver to



>> +	 * wait 5us for calpad busy trimming. But it seems that



>> +	 * 5us of caldone isn't enough for all cases.



>> +	 */



>> +	udelay(500);



>> +	caldone = readl(&phy->emmcphy_status);



>> +	caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;



>> +	if (caldone != PHYCTRL_CALDONE_DONE) {



>> +		printf("%s: caldone timeout.\n", __func__);



>> +		return;



>> +	}



>> +



>> +	/* Set the frequency of the DLL operation */



>> +	if (clock < 75 * MHz)



>> +		freqsel = PHYCTRL_FREQSEL_50M;



>> +	else if (clock < 125 * MHz)



>> +		freqsel = PHYCTRL_FREQSEL_100M;



>> +	else if (clock < 175 * MHz)



>> +		freqsel = PHYCTRL_FREQSEL_150M;



>> +	else



>> +		freqsel = PHYCTRL_FREQSEL_200M;



>> +



>> +	/* Set the frequency of the DLL operation */



>> +	writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);



>> +	writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);



>> +



>> +	read_poll_timeout(readl, &phy->emmcphy_status, dllrdy,



>> +			  PHYCTRL_DLL_LOCK_WO_TMOUT(dllrdy), 1, 5000);



>> +}



>> +



>> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)



>> +{



>> +	writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);



>> +	writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);



>> +}



>> +



>> +static int rk3399_emmc_get_phy(struct udevice *dev)



>> +{



>> +	struct rockchip_sdhc *priv = dev_get_priv(dev);



>> +



>> +	ofnode phy_node;



>> +	void *grf_base;



>> +	u32 grf_phy_offset, phandle;



>> +



>> +	phandle = dev_read_u32_default(dev, "phys", 0);



>> +	phy_node = ofnode_get_by_phandle(phandle);



>> +	if (!ofnode_valid(phy_node)) {



>> +		debug("Not found emmc phy device\n");



>> +		return -ENODEV;



>> +	}



>> +



>> +	grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);



>> +	if (grf_base < 0)



>> +		printf("%s Get syscon grf failed", __func__);



>> +	grf_phy_offset = ofnode_read_u32_default(phy_node, "reg", 0);



>> +



>> +	priv->phy = (struct rockchip_emmc_phy *)(grf_base + grf_phy_offset);



>



>If grf_base is failed to get, is (grf_base + grp_phy_offset) correct?



>



>> +



>> +	return 0;



>> +}



>> +



>> +static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock)



>> +{



>> +	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);



>> +	int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;



>> +	int max_clk = host->max_clk;



>> +	unsigned long mmc_clock;



>> +



>> +	if (cycle_phy)



>> +		rk3399_emmc_phy_power_off(priv->phy);



>> +



>> +	if (clock) {



>> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);



>> +		if (IS_ERR_VALUE(mmc_clock))



>> +			host->max_clk = max_clk;



>> +		else



>> +			host->max_clk = mmc_clock;



>> +	}



>> +	sdhci_set_clock(host->mmc, clock);



>> +	if (!clock)



>> +		return 0;



>> +	host->max_clk = max_clk;



>> +



>> +	if (cycle_phy)



>> +		rk3399_emmc_phy_power_on(priv->phy, clock);



>> +



>> +	return 0;



>> +}



>> +



>> +static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)



>> +{



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



>> +	uint clock = mmc->tran_speed;



>> +	u32 reg;



>> +



>> +	if (!clock)



>> +		clock = mmc->clock;



>> +	data->emmc_set_clock(host, clock);



>



>I think that it needs to check whether callback function is null or not.



>It's possible to exist the case that not to assign to callback function in future.



>



>> +



>> +	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_CTRL_HS400;



>> +		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);



>> +	} else {



>> +		sdhci_set_uhs_timing(host);



>> +	}



>> +



>> +	return 0;



>> +}



>> +



>> +static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)



>> +{



>> +	struct sdhci_host *host = dev_get_priv(mmc->dev);



>> +	char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;



>> +	struct mmc_cmd cmd;



>> +	u32 ctrl;



>> +	int ret = 0;



>> +



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



>> +



>> +	do {



>> +		cmd.cmdidx = opcode;



>> +		cmd.resp_type = MMC_RSP_R1;



>> +		cmd.cmdarg = 0;



>> +



>> +		if (tuning_loop_counter-- == 0)



>> +			break;



>> +



>> +		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)



>> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);



>> +		else



>> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);



>



>Why it needs to pass to 7? It seems that always (dma & 0x7) in SDHCI_MAKE_BLKSZ.



>#define SDHCI_MAKE_BLKSZ(blksz) (1 << 12) | (blksz & 0xFFF)?



>



>



>> +



>> +		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);



>> +



>> +		mmc_send_cmd(mmc, &cmd, NULL);



>> +



>> +		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);



>> +	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);



>> +



>> +	if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {



>> +		printf("%s:Tuning failed\n", __func__);



>> +		ret = -1;



>> +	}



>> +



>> +	if (tuning_loop_counter < 0) {



>> +		ctrl &= ~SDHCI_CTRL_TUNED_CLK;



>> +		sdhci_writel(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 0;



>



>Doesn't need to return "ret" when it's failed?



>



>> +}



>> +



>> +static struct sdhci_ops rockchip_sdhci_ops = {



>> +	.set_ios_post	= rockchip_sdhci_set_ios_post,



>> +	.platform_execute_tuning = &rockchip_sdhci_execute_tuning,



>>  };



>>  



>> -static int arasan_sdhci_probe(struct udevice *dev)



>> +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 mmc_config *cfg = &plat->cfg;



>>  	struct sdhci_host *host = &prv->host;



>> -	int max_frequency, ret;



>>  	struct clk clk;



>> +	int ret;



>>  



>> -#if CONFIG_IS_ENABLED(OF_PLATDATA)



>> -	struct dtd_rockchip_rk3399_sdhci_5_1 *dtplat = &plat->dtplat;



>> -



>> -	host->name = dev->name;



>> -	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);



>> -	max_frequency = dtplat->max_frequency;



>> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);



>> -#else



>> -	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);



>> +	host->max_clk = cfg->f_max;



>>  	ret = clk_get_by_index(dev, 0, &clk);



>> -#endif



>>  	if (!ret) {



>> -		ret = clk_set_rate(&clk, max_frequency);



>> +		ret = clk_set_rate(&clk, host->max_clk);



>>  		if (IS_ERR_VALUE(ret))



>>  			printf("%s clk set rate fail!\n", __func__);



>>  	} else {



>>  		printf("%s fail to get clk\n", __func__);



>>  	}



>>  



>> +	prv->emmc_clk = clk;



>> +	prv->dev = dev;



>> +	ret = data->get_phy(dev);



>



>Ditto. Check data->get_phy.



>



>> +	if (ret)



>> +		return ret;



>> +



>> +	ret = data->emmc_phy_init(dev);



>



>Ditto.



>



>Best Regards,



>Jaehoon Chung



>



>> +	if (ret)



>> +		return ret;



>> +



>> +	host->ops = &rockchip_sdhci_ops;



>> +



>>  	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;



>> -	host->max_clk = max_frequency;



>> -	/*



>> -	 * The sdhci-driver only supports 4bit and 8bit, as sdhci_setup_cfg



>> -	 * doesn't allow us to clear MMC_MODE_4BIT.  Consequently, we don't



>> -	 * check for other bus-width values.



>> -	 */



>> -	if (host->bus_width == 8)



>> -		host->host_caps |= MMC_MODE_8BIT;



>>  



>>  	host->mmc = &plat->mmc;



>>  	host->mmc->priv = &prv->host;



>>  	host->mmc->dev = dev;



>>  	upriv->mmc = host->mmc;



>>  



>> -	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);



>> +	ret = sdhci_setup_cfg(cfg, host, cfg->f_max, EMMC_MIN_FREQ);



>>  	if (ret)



>>  		return ret;



>>  



>>  	return sdhci_probe(dev);



>>  }



>>  



>> -static int arasan_sdhci_of_to_plat(struct udevice *dev)



>> +static int rockchip_sdhci_of_to_plat(struct udevice *dev)



>>  {



>> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)



>> +	struct rockchip_sdhc_plat *plat = dev_get_plat(dev);



>>  	struct sdhci_host *host = dev_get_priv(dev);



>> +	struct mmc_config *cfg = &plat->cfg;



>> +	int ret;



>>  



>>  	host->name = dev->name;



>>  	host->ioaddr = dev_read_addr_ptr(dev);



>> -	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);



>> -#endif



>> +



>> +	ret = mmc_of_parse(dev, cfg);



>> +	if (ret)



>> +		return ret;



>>  



>>  	return 0;



>>  }



>> @@ -101,19 +330,28 @@ static int rockchip_sdhci_bind(struct udevice *dev)



>>  	return sdhci_bind(dev, &plat->mmc, &plat->cfg);



>>  }



>>  



>> -static const struct udevice_id arasan_sdhci_ids[] = {



>> -	{ .compatible = "arasan,sdhci-5.1" },



>> +static const struct sdhci_data rk3399_data = {



>> +	.emmc_set_clock = rk3399_sdhci_emmc_set_clock,



>> +	.get_phy = rk3399_emmc_get_phy,



>> +	.emmc_phy_init = rk3399_emmc_phy_init,



>> +};



>> +



>> +static const struct udevice_id sdhci_ids[] = {



>> +	{



>> +		.compatible = "arasan,sdhci-5.1",



>> +		.data = (ulong)&rk3399_data,



>> +	},



>>  	{ }



>>  };



>>  



>>  U_BOOT_DRIVER(arasan_sdhci_drv) = {



>> -	.name		= "rockchip_rk3399_sdhci_5_1",



>> +	.name		= "rockchip_sdhci_5_1",



>>  	.id		= UCLASS_MMC,



>> -	.of_match	= arasan_sdhci_ids,



>> -	.of_to_plat = arasan_sdhci_of_to_plat,



>> +	.of_match	= sdhci_ids,



>> +	.of_to_plat	= rockchip_sdhci_of_to_plat,



>>  	.ops		= &sdhci_ops,



>>  	.bind		= rockchip_sdhci_bind,



>> -	.probe		= arasan_sdhci_probe,



>> +	.probe		= rockchip_sdhci_probe,



>>  	.priv_auto	= sizeof(struct rockchip_sdhc),



>>  	.plat_auto	= sizeof(struct rockchip_sdhc_plat),



>>  };



>> 



>



>



>



>

Re: [PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

[Peng Fan (OSS)]

On 2021/6/28 17:19, Yifeng Zhao wrote:
> Add clock, phy and other configuration, it is convenient to support
> new controller. Here a short summary of the changes:
> - Add mmc_of_parse to parse dts config.
> - Remove OF_PLATDATA related code.
> - Reorder header inclusion.
> - Add phy ops.
> - add ops set_ios_post to modify the parameters of phy when the
>    clock changes.
> - Add execute tuning api for hs200 tuning
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> ---
> 
> Changes in v2:
> - Add mmc_of_parse to parse dts config.
> - Used read_poll_timeout api to check dll lock status
> - Add execute tuning api for hs200 tuning
> 
>   drivers/mmc/rockchip_sdhci.c | 310 +++++++++++++++++++++++++++++++----
>   1 file changed, 274 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index d95f8b2a15..2973911446 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -6,90 +6,319 @@
>    */
>   
>   #include <common.h>
> +#include <clk.h>
>   #include <dm.h>
> +#include <dm/ofnode.h>
>   #include <dt-structs.h>
> +#include <linux/delay.h>
>   #include <linux/err.h>
>   #include <linux/libfdt.h>
> +#include <linux/iopoll.h>
>   #include <malloc.h>
>   #include <mapmem.h>
> +#include "mmc_private.h"
>   #include <sdhci.h>
> -#include <clk.h>
> +#include <syscon.h>
> +#include <asm/arch-rockchip/clock.h>
> +#include <asm/arch-rockchip/hardware.h>
>   
>   /* 400KHz is max freq for card ID etc. Use that as min */
>   #define EMMC_MIN_FREQ	400000
> +#define KHz	(1000)
> +#define MHz	(1000 * KHz)
> +#define SDHCI_TUNING_LOOP_COUNT		40
> +
> +#define PHYCTRL_CALDONE_MASK		0x1
> +#define PHYCTRL_CALDONE_SHIFT		0x6
> +#define PHYCTRL_CALDONE_DONE		0x1
> +#define PHYCTRL_DLLRDY_MASK		0x1
> +#define PHYCTRL_DLLRDY_SHIFT		0x5
> +#define PHYCTRL_DLLRDY_DONE		0x1
> +#define PHYCTRL_FREQSEL_200M		0x0
> +#define PHYCTRL_FREQSEL_50M		0x1
> +#define PHYCTRL_FREQSEL_100M		0x2
> +#define PHYCTRL_FREQSEL_150M		0x3
> +#define PHYCTRL_DLL_LOCK_WO_TMOUT(x)	\
> +	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
> +	PHYCTRL_DLLRDY_DONE)
>   
>   struct rockchip_sdhc_plat {
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
> -#endif

Please explain why remove this?

>   	struct mmc_config cfg;
>   	struct mmc mmc;
>   };
>   
> +struct rockchip_emmc_phy {
> +	u32 emmcphy_con[7];
> +	u32 reserved;
> +	u32 emmcphy_status;
> +};
> +
>   struct rockchip_sdhc {
>   	struct sdhci_host host;
> +	struct udevice *dev;
>   	void *base;
> +	struct rockchip_emmc_phy *phy;
> +	struct clk emmc_clk;
> +};
> +
> +struct sdhci_data {
> +	int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
> +	int (*emmc_phy_init)(struct udevice *dev);
> +	int (*get_phy)(struct udevice *dev);
> +};
> +
> +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;
> +
> +	writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
> +	writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);

Would clrsetbits_le32 work?

> +
> +	/*
> +	 * According to the user manual, calpad calibration
> +	 * cycle takes more than 2us without the minimal recommended
> +	 * value, so we may need a little margin here
> +	 */
> +	udelay(3);
> +	writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);

Ditto.

Regards,
Peng.
> +
> +	/*
> +	 * According to the user manual, it asks driver to
> +	 * wait 5us for calpad busy trimming. But it seems that
> +	 * 5us of caldone isn't enough for all cases.
> +	 */
> +	udelay(500);
> +	caldone = readl(&phy->emmcphy_status);
> +	caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> +	if (caldone != PHYCTRL_CALDONE_DONE) {
> +		printf("%s: caldone timeout.\n", __func__);
> +		return;
> +	}
> +
> +	/* Set the frequency of the DLL operation */
> +	if (clock < 75 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_50M;
> +	else if (clock < 125 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_100M;
> +	else if (clock < 175 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_150M;
> +	else
> +		freqsel = PHYCTRL_FREQSEL_200M;
> +
> +	/* Set the frequency of the DLL operation */
> +	writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);

Ditto.

> +
> +	read_poll_timeout(readl, &phy->emmcphy_status, dllrdy,
> +			  PHYCTRL_DLL_LOCK_WO_TMOUT(dllrdy), 1, 5000);
> +}
> +
> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
> +{
> +	writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
> +	writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
> +}
> +
> +static int rk3399_emmc_get_phy(struct udevice *dev)
> +{
> +	struct rockchip_sdhc *priv = dev_get_priv(dev);
> +
> +	ofnode phy_node;
> +	void *grf_base;
> +	u32 grf_phy_offset, phandle;
> +
> +	phandle = dev_read_u32_default(dev, "phys", 0);
> +	phy_node = ofnode_get_by_phandle(phandle);
> +	if (!ofnode_valid(phy_node)) {
> +		debug("Not found emmc phy device\n");
> +		return -ENODEV;
> +	}
> +
> +	grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +	if (grf_base < 0)
> +		printf("%s Get syscon grf failed", __func__);
> +	grf_phy_offset = ofnode_read_u32_default(phy_node, "reg", 0);
> +
> +	priv->phy = (struct rockchip_emmc_phy *)(grf_base + grf_phy_offset);
> +
> +	return 0;
> +}
> +
> +static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
> +	int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;
> +	int max_clk = host->max_clk;
> +	unsigned long mmc_clock;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_off(priv->phy);
> +
> +	if (clock) {
> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);
> +		if (IS_ERR_VALUE(mmc_clock))
> +			host->max_clk = max_clk;
> +		else
> +			host->max_clk = mmc_clock;
> +	}
> +	sdhci_set_clock(host->mmc, clock);
> +	if (!clock)
> +		return 0;
> +	host->max_clk = max_clk;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_on(priv->phy, clock);
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
> +{
> +	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;
> +	uint clock = mmc->tran_speed;
> +	u32 reg;
> +
> +	if (!clock)
> +		clock = mmc->clock;
> +	data->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;
> +		reg |= SDHCI_CTRL_HS400;
> +		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> +	} else {
> +		sdhci_set_uhs_timing(host);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
> +{
> +	struct sdhci_host *host = dev_get_priv(mmc->dev);
> +	char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;
> +	struct mmc_cmd cmd;
> +	u32 ctrl;
> +	int ret = 0;
> +
> +	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);
> +
> +	do {
> +		cmd.cmdidx = opcode;
> +		cmd.resp_type = MMC_RSP_R1;
> +		cmd.cmdarg = 0;
> +
> +		if (tuning_loop_counter-- == 0)
> +			break;
> +
> +		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
> +		else
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);
> +
> +		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
> +
> +		mmc_send_cmd(mmc, &cmd, NULL);
> +
> +		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
> +
> +	if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
> +		printf("%s:Tuning failed\n", __func__);
> +		ret = -1;
> +	}
> +
> +	if (tuning_loop_counter < 0) {
> +		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> +		sdhci_writel(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 0;
> +}
> +
> +static struct sdhci_ops rockchip_sdhci_ops = {
> +	.set_ios_post	= rockchip_sdhci_set_ios_post,
> +	.platform_execute_tuning = &rockchip_sdhci_execute_tuning,
>   };
>   
> -static int arasan_sdhci_probe(struct udevice *dev)
> +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 mmc_config *cfg = &plat->cfg;
>   	struct sdhci_host *host = &prv->host;
> -	int max_frequency, ret;
>   	struct clk clk;
> +	int ret;
>   
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 *dtplat = &plat->dtplat;
> -
> -	host->name = dev->name;
> -	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
> -	max_frequency = dtplat->max_frequency;
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
> -#else
> -	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
> +	host->max_clk = cfg->f_max;
>   	ret = clk_get_by_index(dev, 0, &clk);
> -#endif
>   	if (!ret) {
> -		ret = clk_set_rate(&clk, max_frequency);
> +		ret = clk_set_rate(&clk, host->max_clk);
>   		if (IS_ERR_VALUE(ret))
>   			printf("%s clk set rate fail!\n", __func__);
>   	} else {
>   		printf("%s fail to get clk\n", __func__);
>   	}
>   
> +	prv->emmc_clk = clk;
> +	prv->dev = dev;
> +	ret = data->get_phy(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = data->emmc_phy_init(dev);
> +	if (ret)
> +		return ret;
> +
> +	host->ops = &rockchip_sdhci_ops;
> +
>   	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
> -	host->max_clk = max_frequency;
> -	/*
> -	 * The sdhci-driver only supports 4bit and 8bit, as sdhci_setup_cfg
> -	 * doesn't allow us to clear MMC_MODE_4BIT.  Consequently, we don't
> -	 * check for other bus-width values.
> -	 */
> -	if (host->bus_width == 8)
> -		host->host_caps |= MMC_MODE_8BIT;
>   
>   	host->mmc = &plat->mmc;
>   	host->mmc->priv = &prv->host;
>   	host->mmc->dev = dev;
>   	upriv->mmc = host->mmc;
>   
> -	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);
> +	ret = sdhci_setup_cfg(cfg, host, cfg->f_max, EMMC_MIN_FREQ);
>   	if (ret)
>   		return ret;
>   
>   	return sdhci_probe(dev);
>   }
>   
> -static int arasan_sdhci_of_to_plat(struct udevice *dev)
> +static int rockchip_sdhci_of_to_plat(struct udevice *dev)
>   {
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> +	struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
>   	struct sdhci_host *host = dev_get_priv(dev);
> +	struct mmc_config *cfg = &plat->cfg;
> +	int ret;
>   
>   	host->name = dev->name;
>   	host->ioaddr = dev_read_addr_ptr(dev);
> -	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
> -#endif
> +
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;
>   
>   	return 0;
>   }
> @@ -101,19 +330,28 @@ static int rockchip_sdhci_bind(struct udevice *dev)
>   	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
>   }
>   
> -static const struct udevice_id arasan_sdhci_ids[] = {
> -	{ .compatible = "arasan,sdhci-5.1" },
> +static const struct sdhci_data rk3399_data = {
> +	.emmc_set_clock = rk3399_sdhci_emmc_set_clock,
> +	.get_phy = rk3399_emmc_get_phy,
> +	.emmc_phy_init = rk3399_emmc_phy_init,
> +};
> +
> +static const struct udevice_id sdhci_ids[] = {
> +	{
> +		.compatible = "arasan,sdhci-5.1",
> +		.data = (ulong)&rk3399_data,
> +	},
>   	{ }
>   };
>   
>   U_BOOT_DRIVER(arasan_sdhci_drv) = {
> -	.name		= "rockchip_rk3399_sdhci_5_1",
> +	.name		= "rockchip_sdhci_5_1",
>   	.id		= UCLASS_MMC,
> -	.of_match	= arasan_sdhci_ids,
> -	.of_to_plat = arasan_sdhci_of_to_plat,
> +	.of_match	= sdhci_ids,
> +	.of_to_plat	= rockchip_sdhci_of_to_plat,
>   	.ops		= &sdhci_ops,
>   	.bind		= rockchip_sdhci_bind,
> -	.probe		= arasan_sdhci_probe,
> +	.probe		= rockchip_sdhci_probe,
>   	.priv_auto	= sizeof(struct rockchip_sdhc),
>   	.plat_auto	= sizeof(struct rockchip_sdhc_plat),
>   };
> 

Re: [PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

[Peng Fan (OSS)]

On 2021/6/28 17:19, Yifeng Zhao wrote:
> Add clock, phy and other configuration, it is convenient to support
> new controller. Here a short summary of the changes:
> - Add mmc_of_parse to parse dts config.
> - Remove OF_PLATDATA related code.
> - Reorder header inclusion.
> - Add phy ops.

One more generic question,
could phy part be moved to a phy driver?

Regards,
Peng.

> - add ops set_ios_post to modify the parameters of phy when the
>    clock changes.
> - Add execute tuning api for hs200 tuning
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> ---
> 
> Changes in v2:
> - Add mmc_of_parse to parse dts config.
> - Used read_poll_timeout api to check dll lock status
> - Add execute tuning api for hs200 tuning
> 
>   drivers/mmc/rockchip_sdhci.c | 310 +++++++++++++++++++++++++++++++----
>   1 file changed, 274 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index d95f8b2a15..2973911446 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -6,90 +6,319 @@
>    */
>   
>   #include <common.h>
> +#include <clk.h>
>   #include <dm.h>
> +#include <dm/ofnode.h>
>   #include <dt-structs.h>
> +#include <linux/delay.h>
>   #include <linux/err.h>
>   #include <linux/libfdt.h>
> +#include <linux/iopoll.h>
>   #include <malloc.h>
>   #include <mapmem.h>
> +#include "mmc_private.h"
>   #include <sdhci.h>
> -#include <clk.h>
> +#include <syscon.h>
> +#include <asm/arch-rockchip/clock.h>
> +#include <asm/arch-rockchip/hardware.h>
>   
>   /* 400KHz is max freq for card ID etc. Use that as min */
>   #define EMMC_MIN_FREQ	400000
> +#define KHz	(1000)
> +#define MHz	(1000 * KHz)
> +#define SDHCI_TUNING_LOOP_COUNT		40
> +
> +#define PHYCTRL_CALDONE_MASK		0x1
> +#define PHYCTRL_CALDONE_SHIFT		0x6
> +#define PHYCTRL_CALDONE_DONE		0x1
> +#define PHYCTRL_DLLRDY_MASK		0x1
> +#define PHYCTRL_DLLRDY_SHIFT		0x5
> +#define PHYCTRL_DLLRDY_DONE		0x1
> +#define PHYCTRL_FREQSEL_200M		0x0
> +#define PHYCTRL_FREQSEL_50M		0x1
> +#define PHYCTRL_FREQSEL_100M		0x2
> +#define PHYCTRL_FREQSEL_150M		0x3
> +#define PHYCTRL_DLL_LOCK_WO_TMOUT(x)	\
> +	((((x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
> +	PHYCTRL_DLLRDY_DONE)
>   
>   struct rockchip_sdhc_plat {
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
> -#endif
>   	struct mmc_config cfg;
>   	struct mmc mmc;
>   };
>   
> +struct rockchip_emmc_phy {
> +	u32 emmcphy_con[7];
> +	u32 reserved;
> +	u32 emmcphy_status;
> +};
> +
>   struct rockchip_sdhc {
>   	struct sdhci_host host;
> +	struct udevice *dev;
>   	void *base;
> +	struct rockchip_emmc_phy *phy;
> +	struct clk emmc_clk;
> +};
> +
> +struct sdhci_data {
> +	int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
> +	int (*emmc_phy_init)(struct udevice *dev);
> +	int (*get_phy)(struct udevice *dev);
> +};
> +
> +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;
> +
> +	writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
> +	writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
> +
> +	/*
> +	 * According to the user manual, calpad calibration
> +	 * cycle takes more than 2us without the minimal recommended
> +	 * value, so we may need a little margin here
> +	 */
> +	udelay(3);
> +	writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
> +
> +	/*
> +	 * According to the user manual, it asks driver to
> +	 * wait 5us for calpad busy trimming. But it seems that
> +	 * 5us of caldone isn't enough for all cases.
> +	 */
> +	udelay(500);
> +	caldone = readl(&phy->emmcphy_status);
> +	caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> +	if (caldone != PHYCTRL_CALDONE_DONE) {
> +		printf("%s: caldone timeout.\n", __func__);
> +		return;
> +	}
> +
> +	/* Set the frequency of the DLL operation */
> +	if (clock < 75 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_50M;
> +	else if (clock < 125 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_100M;
> +	else if (clock < 175 * MHz)
> +		freqsel = PHYCTRL_FREQSEL_150M;
> +	else
> +		freqsel = PHYCTRL_FREQSEL_200M;
> +
> +	/* Set the frequency of the DLL operation */
> +	writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
> +	writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
> +
> +	read_poll_timeout(readl, &phy->emmcphy_status, dllrdy,
> +			  PHYCTRL_DLL_LOCK_WO_TMOUT(dllrdy), 1, 5000);
> +}
> +
> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
> +{
> +	writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
> +	writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
> +}
> +
> +static int rk3399_emmc_get_phy(struct udevice *dev)
> +{
> +	struct rockchip_sdhc *priv = dev_get_priv(dev);
> +
> +	ofnode phy_node;
> +	void *grf_base;
> +	u32 grf_phy_offset, phandle;
> +
> +	phandle = dev_read_u32_default(dev, "phys", 0);
> +	phy_node = ofnode_get_by_phandle(phandle);
> +	if (!ofnode_valid(phy_node)) {
> +		debug("Not found emmc phy device\n");
> +		return -ENODEV;
> +	}
> +
> +	grf_base = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +	if (grf_base < 0)
> +		printf("%s Get syscon grf failed", __func__);
> +	grf_phy_offset = ofnode_read_u32_default(phy_node, "reg", 0);
> +
> +	priv->phy = (struct rockchip_emmc_phy *)(grf_base + grf_phy_offset);
> +
> +	return 0;
> +}
> +
> +static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> +	struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host);
> +	int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;
> +	int max_clk = host->max_clk;
> +	unsigned long mmc_clock;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_off(priv->phy);
> +
> +	if (clock) {
> +		mmc_clock = clk_set_rate(&priv->emmc_clk, clock);
> +		if (IS_ERR_VALUE(mmc_clock))
> +			host->max_clk = max_clk;
> +		else
> +			host->max_clk = mmc_clock;
> +	}
> +	sdhci_set_clock(host->mmc, clock);
> +	if (!clock)
> +		return 0;
> +	host->max_clk = max_clk;
> +
> +	if (cycle_phy)
> +		rk3399_emmc_phy_power_on(priv->phy, clock);
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
> +{
> +	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;
> +	uint clock = mmc->tran_speed;
> +	u32 reg;
> +
> +	if (!clock)
> +		clock = mmc->clock;
> +	data->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;
> +		reg |= SDHCI_CTRL_HS400;
> +		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> +	} else {
> +		sdhci_set_uhs_timing(host);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
> +{
> +	struct sdhci_host *host = dev_get_priv(mmc->dev);
> +	char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;
> +	struct mmc_cmd cmd;
> +	u32 ctrl;
> +	int ret = 0;
> +
> +	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);
> +
> +	do {
> +		cmd.cmdidx = opcode;
> +		cmd.resp_type = MMC_RSP_R1;
> +		cmd.cmdarg = 0;
> +
> +		if (tuning_loop_counter-- == 0)
> +			break;
> +
> +		if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200 && host->mmc->bus_width == 8)
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
> +		else
> +			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);
> +
> +		sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
> +
> +		mmc_send_cmd(mmc, &cmd, NULL);
> +
> +		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +	} while (ctrl & SDHCI_CTRL_EXEC_TUNING);
> +
> +	if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
> +		printf("%s:Tuning failed\n", __func__);
> +		ret = -1;
> +	}
> +
> +	if (tuning_loop_counter < 0) {
> +		ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> +		sdhci_writel(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 0;
> +}
> +
> +static struct sdhci_ops rockchip_sdhci_ops = {
> +	.set_ios_post	= rockchip_sdhci_set_ios_post,
> +	.platform_execute_tuning = &rockchip_sdhci_execute_tuning,
>   };
>   
> -static int arasan_sdhci_probe(struct udevice *dev)
> +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 mmc_config *cfg = &plat->cfg;
>   	struct sdhci_host *host = &prv->host;
> -	int max_frequency, ret;
>   	struct clk clk;
> +	int ret;
>   
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct dtd_rockchip_rk3399_sdhci_5_1 *dtplat = &plat->dtplat;
> -
> -	host->name = dev->name;
> -	host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);
> -	max_frequency = dtplat->max_frequency;
> -	ret = clk_get_by_driver_info(dev, dtplat->clocks, &clk);
> -#else
> -	max_frequency = dev_read_u32_default(dev, "max-frequency", 0);
> +	host->max_clk = cfg->f_max;
>   	ret = clk_get_by_index(dev, 0, &clk);
> -#endif
>   	if (!ret) {
> -		ret = clk_set_rate(&clk, max_frequency);
> +		ret = clk_set_rate(&clk, host->max_clk);
>   		if (IS_ERR_VALUE(ret))
>   			printf("%s clk set rate fail!\n", __func__);
>   	} else {
>   		printf("%s fail to get clk\n", __func__);
>   	}
>   
> +	prv->emmc_clk = clk;
> +	prv->dev = dev;
> +	ret = data->get_phy(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = data->emmc_phy_init(dev);
> +	if (ret)
> +		return ret;
> +
> +	host->ops = &rockchip_sdhci_ops;
> +
>   	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
> -	host->max_clk = max_frequency;
> -	/*
> -	 * The sdhci-driver only supports 4bit and 8bit, as sdhci_setup_cfg
> -	 * doesn't allow us to clear MMC_MODE_4BIT.  Consequently, we don't
> -	 * check for other bus-width values.
> -	 */
> -	if (host->bus_width == 8)
> -		host->host_caps |= MMC_MODE_8BIT;
>   
>   	host->mmc = &plat->mmc;
>   	host->mmc->priv = &prv->host;
>   	host->mmc->dev = dev;
>   	upriv->mmc = host->mmc;
>   
> -	ret = sdhci_setup_cfg(&plat->cfg, host, 0, EMMC_MIN_FREQ);
> +	ret = sdhci_setup_cfg(cfg, host, cfg->f_max, EMMC_MIN_FREQ);
>   	if (ret)
>   		return ret;
>   
>   	return sdhci_probe(dev);
>   }
>   
> -static int arasan_sdhci_of_to_plat(struct udevice *dev)
> +static int rockchip_sdhci_of_to_plat(struct udevice *dev)
>   {
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> +	struct rockchip_sdhc_plat *plat = dev_get_plat(dev);
>   	struct sdhci_host *host = dev_get_priv(dev);
> +	struct mmc_config *cfg = &plat->cfg;
> +	int ret;
>   
>   	host->name = dev->name;
>   	host->ioaddr = dev_read_addr_ptr(dev);
> -	host->bus_width = dev_read_u32_default(dev, "bus-width", 4);
> -#endif
> +
> +	ret = mmc_of_parse(dev, cfg);
> +	if (ret)
> +		return ret;
>   
>   	return 0;
>   }
> @@ -101,19 +330,28 @@ static int rockchip_sdhci_bind(struct udevice *dev)
>   	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
>   }
>   
> -static const struct udevice_id arasan_sdhci_ids[] = {
> -	{ .compatible = "arasan,sdhci-5.1" },
> +static const struct sdhci_data rk3399_data = {
> +	.emmc_set_clock = rk3399_sdhci_emmc_set_clock,
> +	.get_phy = rk3399_emmc_get_phy,
> +	.emmc_phy_init = rk3399_emmc_phy_init,
> +};
> +
> +static const struct udevice_id sdhci_ids[] = {
> +	{
> +		.compatible = "arasan,sdhci-5.1",
> +		.data = (ulong)&rk3399_data,
> +	},
>   	{ }
>   };
>   
>   U_BOOT_DRIVER(arasan_sdhci_drv) = {
> -	.name		= "rockchip_rk3399_sdhci_5_1",
> +	.name		= "rockchip_sdhci_5_1",
>   	.id		= UCLASS_MMC,
> -	.of_match	= arasan_sdhci_ids,
> -	.of_to_plat = arasan_sdhci_of_to_plat,
> +	.of_match	= sdhci_ids,
> +	.of_to_plat	= rockchip_sdhci_of_to_plat,
>   	.ops		= &sdhci_ops,
>   	.bind		= rockchip_sdhci_bind,
> -	.probe		= arasan_sdhci_probe,
> +	.probe		= rockchip_sdhci_probe,
>   	.priv_auto	= sizeof(struct rockchip_sdhc),
>   	.plat_auto	= sizeof(struct rockchip_sdhc_plat),
>   };
> 

Re: [PATCH v2 0/3]

[Florian Fainelli]

On 7/31/2020 10:10 PM, Rakesh Pillai wrote:
> 
> 
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Saturday, August 1, 2020 12:17 AM
>> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
>> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/3]
>>
>> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
>>> The history recording will be compiled only if
>>> ATH10K_DEBUG is enabled, and also enabled via
>>> the module parameter. Once the history recording
>>> is enabled via module parameter, it can be enabled
>>> or disabled runtime via debugfs.
>>
>> Why not use trace prints and retrieving them via the function tracer?
>> This seems very ad-hoc.
> 
> Tracing needs to be enabled to capture the events.
> But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
> It wont even allocate memory if not enabled via module parameter.

I would suggest researching what other drivers do and also considering
the benefits, for someone doing system analysis of plugging into the
kernel's general tracing mechanism to have all information in the same
place and just do filtering on the record/report side.
-- 
Florian

Re: [PATCH v2 0/3]

[Florian Fainelli]

On 7/31/2020 10:10 PM, Rakesh Pillai wrote:
> 
> 
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Saturday, August 1, 2020 12:17 AM
>> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
>> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
>> netdev@vger.kernel.org
>> Subject: Re: [PATCH v2 0/3]
>>
>> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
>>> The history recording will be compiled only if
>>> ATH10K_DEBUG is enabled, and also enabled via
>>> the module parameter. Once the history recording
>>> is enabled via module parameter, it can be enabled
>>> or disabled runtime via debugfs.
>>
>> Why not use trace prints and retrieving them via the function tracer?
>> This seems very ad-hoc.
> 
> Tracing needs to be enabled to capture the events.
> But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
> It wont even allocate memory if not enabled via module parameter.

I would suggest researching what other drivers do and also considering
the benefits, for someone doing system analysis of plugging into the
kernel's general tracing mechanism to have all information in the same
place and just do filtering on the record/report side.
-- 
Florian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

RE: [PATCH v2 0/3]

[Rakesh Pillai]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, August 1, 2020 12:17 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/3]
> 
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > The history recording will be compiled only if
> > ATH10K_DEBUG is enabled, and also enabled via
> > the module parameter. Once the history recording
> > is enabled via module parameter, it can be enabled
> > or disabled runtime via debugfs.
> 
> Why not use trace prints and retrieving them via the function tracer?
> This seems very ad-hoc.

Tracing needs to be enabled to capture the events.
But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
It wont even allocate memory if not enabled via module parameter.

> 
> >
> > ---
> > Changes from v1:
> > - Add module param and debugfs to enable/disable history recording.
> >
> > Rakesh Pillai (3):
> >   ath10k: Add history for tracking certain events
> >   ath10k: Add module param to enable history
> >   ath10k: Add debugfs support to enable event history
> >
> >  drivers/net/wireless/ath/ath10k/ce.c      |   1 +
> >  drivers/net/wireless/ath/ath10k/core.c    |   3 +
> >  drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
> >  drivers/net/wireless/ath/ath10k/debug.c   | 207
> ++++++++++++++++++++++++++++++
> >  drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
> >  drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
> >  drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
> >  drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
> >  8 files changed, 393 insertions(+), 1 deletion(-)
> >
> 
> 
> --
> Florian

RE: [PATCH v2 0/3]

[Rakesh Pillai]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, August 1, 2020 12:17 AM
> To: Rakesh Pillai <pillair@codeaurora.org>; ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> kvalo@codeaurora.org; davem@davemloft.net; kuba@kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2 0/3]
> 
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > The history recording will be compiled only if
> > ATH10K_DEBUG is enabled, and also enabled via
> > the module parameter. Once the history recording
> > is enabled via module parameter, it can be enabled
> > or disabled runtime via debugfs.
> 
> Why not use trace prints and retrieving them via the function tracer?
> This seems very ad-hoc.

Tracing needs to be enabled to capture the events.
But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash.
It wont even allocate memory if not enabled via module parameter.

> 
> >
> > ---
> > Changes from v1:
> > - Add module param and debugfs to enable/disable history recording.
> >
> > Rakesh Pillai (3):
> >   ath10k: Add history for tracking certain events
> >   ath10k: Add module param to enable history
> >   ath10k: Add debugfs support to enable event history
> >
> >  drivers/net/wireless/ath/ath10k/ce.c      |   1 +
> >  drivers/net/wireless/ath/ath10k/core.c    |   3 +
> >  drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
> >  drivers/net/wireless/ath/ath10k/debug.c   | 207
> ++++++++++++++++++++++++++++++
> >  drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
> >  drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
> >  drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
> >  drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
> >  8 files changed, 393 insertions(+), 1 deletion(-)
> >
> 
> 
> --
> Florian


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 0/3]

[Jakub Kicinski]

On Fri, 31 Jul 2020 23:57:19 +0530 Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.

Have you seen the trace_devlink_hwmsg() interface?
Could it be used here?

Re: [PATCH v2 0/3]

[Jakub Kicinski]

On Fri, 31 Jul 2020 23:57:19 +0530 Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.

Have you seen the trace_devlink_hwmsg() interface?
Could it be used here?

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH v2 0/3]

[Florian Fainelli]

On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.

Why not use trace prints and retrieving them via the function tracer?
This seems very ad-hoc.

> 
> ---
> Changes from v1:
> - Add module param and debugfs to enable/disable history recording.
> 
> Rakesh Pillai (3):
>   ath10k: Add history for tracking certain events
>   ath10k: Add module param to enable history
>   ath10k: Add debugfs support to enable event history
> 
>  drivers/net/wireless/ath/ath10k/ce.c      |   1 +
>  drivers/net/wireless/ath/ath10k/core.c    |   3 +
>  drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
>  drivers/net/wireless/ath/ath10k/debug.c   | 207 ++++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
>  drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
>  drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
>  drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
>  8 files changed, 393 insertions(+), 1 deletion(-)
> 


-- 
Florian

Re: [PATCH v2 0/3]

[Florian Fainelli]

On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> The history recording will be compiled only if
> ATH10K_DEBUG is enabled, and also enabled via
> the module parameter. Once the history recording
> is enabled via module parameter, it can be enabled
> or disabled runtime via debugfs.

Why not use trace prints and retrieving them via the function tracer?
This seems very ad-hoc.

> 
> ---
> Changes from v1:
> - Add module param and debugfs to enable/disable history recording.
> 
> Rakesh Pillai (3):
>   ath10k: Add history for tracking certain events
>   ath10k: Add module param to enable history
>   ath10k: Add debugfs support to enable event history
> 
>  drivers/net/wireless/ath/ath10k/ce.c      |   1 +
>  drivers/net/wireless/ath/ath10k/core.c    |   3 +
>  drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
>  drivers/net/wireless/ath/ath10k/debug.c   | 207 ++++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
>  drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
>  drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
>  drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
>  8 files changed, 393 insertions(+), 1 deletion(-)
> 


-- 
Florian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 0/3]

[Rakesh Pillai]

The history recording will be compiled only if
ATH10K_DEBUG is enabled, and also enabled via
the module parameter. Once the history recording
is enabled via module parameter, it can be enabled
or disabled runtime via debugfs.

---
Changes from v1:
- Add module param and debugfs to enable/disable history recording.

Rakesh Pillai (3):
  ath10k: Add history for tracking certain events
  ath10k: Add module param to enable history
  ath10k: Add debugfs support to enable event history

 drivers/net/wireless/ath/ath10k/ce.c      |   1 +
 drivers/net/wireless/ath/ath10k/core.c    |   3 +
 drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
 drivers/net/wireless/ath/ath10k/debug.c   | 207 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
 drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
 8 files changed, 393 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 0/3]

[Rakesh Pillai]

The history recording will be compiled only if
ATH10K_DEBUG is enabled, and also enabled via
the module parameter. Once the history recording
is enabled via module parameter, it can be enabled
or disabled runtime via debugfs.

---
Changes from v1:
- Add module param and debugfs to enable/disable history recording.

Rakesh Pillai (3):
  ath10k: Add history for tracking certain events
  ath10k: Add module param to enable history
  ath10k: Add debugfs support to enable event history

 drivers/net/wireless/ath/ath10k/ce.c      |   1 +
 drivers/net/wireless/ath/ath10k/core.c    |   3 +
 drivers/net/wireless/ath/ath10k/core.h    |  82 ++++++++++++
 drivers/net/wireless/ath/ath10k/debug.c   | 207 ++++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/debug.h   |  75 +++++++++++
 drivers/net/wireless/ath/ath10k/snoc.c    |  15 ++-
 drivers/net/wireless/ath/ath10k/wmi-tlv.c |   1 +
 drivers/net/wireless/ath/ath10k/wmi.c     |  10 ++
 8 files changed, 393 insertions(+), 1 deletion(-)

-- 
2.7.4


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH v2 0/3]

[Gregory CLEMENT]

Hello,

A few month ago this series was sent and has not been merged while it
didn't have anything against it. I've ported the series onto v5.7-rc1,
added the acked by and reviewed by received on the first series and
took into account the comment from Robin Murphy for the last patch.

For the record this the original cover letter explaining the purpose
of this series:

The Atmel USB device controller is the only one having the description
of the End Point configuration in the device tree.

This configuration depend on the version of the controller used in the
SoC. This variation is already documented by the compatible string of
the controller.

In this series the configuration is associated to the compatible
string which allows to remove the EP child node from the device
tree. The main benefit of it, beyond the simplification of the device
tree, is the reduction of the size of the dtb which was too big to be
managed by the bootloader.

The first two patches should be merged through the USB subsystem while
the last one should be take by the a91 subsystem. Moreover this last
patch should be merged only once the change in the driver is merged.

Gregory

Gregory CLEMENT (3):
  usb: gadget: udc: atmel: Don't use DT to configure end point
  dt-bindings: usb: atmel: Mark EP child node as deprecated
  ARM: dts: at91: Remove the USB EP child node

 .../devicetree/bindings/usb/atmel-usb.txt     |  56 +-------
 arch/arm/boot/dts/at91sam9g45.dtsi            |  54 --------
 arch/arm/boot/dts/at91sam9rl.dtsi             |  54 --------
 arch/arm/boot/dts/at91sam9x5.dtsi             |  54 --------
 arch/arm/boot/dts/sama5d2.dtsi                | 120 ------------------
 arch/arm/boot/dts/sama5d3.dtsi                | 107 ----------------
 arch/arm/boot/dts/sama5d4.dtsi                | 120 ------------------
 drivers/usb/gadget/udc/atmel_usba_udc.c       | 112 ++++++++++------
 drivers/usb/gadget/udc/atmel_usba_udc.h       |  12 ++
 9 files changed, 87 insertions(+), 602 deletions(-)

-- 
2.26.2

[PATCH v2 0/3]

[Gregory CLEMENT]

Hello,

A few month ago this series was sent and has not been merged while it
didn't have anything against it. I've ported the series onto v5.7-rc1,
added the acked by and reviewed by received on the first series and
took into account the comment from Robin Murphy for the last patch.

For the record this the original cover letter explaining the purpose
of this series:

The Atmel USB device controller is the only one having the description
of the End Point configuration in the device tree.

This configuration depend on the version of the controller used in the
SoC. This variation is already documented by the compatible string of
the controller.

In this series the configuration is associated to the compatible
string which allows to remove the EP child node from the device
tree. The main benefit of it, beyond the simplification of the device
tree, is the reduction of the size of the dtb which was too big to be
managed by the bootloader.

The first two patches should be merged through the USB subsystem while
the last one should be take by the a91 subsystem. Moreover this last
patch should be merged only once the change in the driver is merged.

Gregory

Gregory CLEMENT (3):
  usb: gadget: udc: atmel: Don't use DT to configure end point
  dt-bindings: usb: atmel: Mark EP child node as deprecated
  ARM: dts: at91: Remove the USB EP child node

 .../devicetree/bindings/usb/atmel-usb.txt     |  56 +-------
 arch/arm/boot/dts/at91sam9g45.dtsi            |  54 --------
 arch/arm/boot/dts/at91sam9rl.dtsi             |  54 --------
 arch/arm/boot/dts/at91sam9x5.dtsi             |  54 --------
 arch/arm/boot/dts/sama5d2.dtsi                | 120 ------------------
 arch/arm/boot/dts/sama5d3.dtsi                | 107 ----------------
 arch/arm/boot/dts/sama5d4.dtsi                | 120 ------------------
 drivers/usb/gadget/udc/atmel_usba_udc.c       | 112 ++++++++++------
 drivers/usb/gadget/udc/atmel_usba_udc.h       |  12 ++
 9 files changed, 87 insertions(+), 602 deletions(-)

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/3]

[Simon Horman]

Hi,

this short series enables the on-board ethernet
of the r8a7790 SoC for on lager board.

It is based on the renesas-next-20130513 tag of my renesas tree.

Dependencies noted in individual patches.

Simon Horman (3):
  ARM: shmobile: r8a7790: add Ether support
  ARM: shmobile: lager: enable Ether
  ARM: shmobile: lager: enable nfsroot in DTS

 arch/arm/boot/dts/r8a7790-lager.dts           |  2 +-
 arch/arm/mach-shmobile/board-lager.c          | 25 +++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-r8a7790.c        |  4 ++++
 arch/arm/mach-shmobile/include/mach/r8a7790.h |  3 +++
 arch/arm/mach-shmobile/setup-r8a7790.c        | 15 +++++++++++++++
 5 files changed, 48 insertions(+), 1 deletion(-)

-- 
1.8.2.1

[PATCH v2 0/3]

[Simon Horman]

Hi,

this short series enables the on-board ethernet
of the r8a7790 SoC for on lager board.

It is based on the renesas-next-20130513 tag of my renesas tree.

Dependencies noted in individual patches.

Simon Horman (3):
  ARM: shmobile: r8a7790: add Ether support
  ARM: shmobile: lager: enable Ether
  ARM: shmobile: lager: enable nfsroot in DTS

 arch/arm/boot/dts/r8a7790-lager.dts           |  2 +-
 arch/arm/mach-shmobile/board-lager.c          | 25 +++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-r8a7790.c        |  4 ++++
 arch/arm/mach-shmobile/include/mach/r8a7790.h |  3 +++
 arch/arm/mach-shmobile/setup-r8a7790.c        | 15 +++++++++++++++
 5 files changed, 48 insertions(+), 1 deletion(-)

-- 
1.8.2.1

[PATCH v2 0/3]

[Larry D'Anna]

I've fixed his series according to Jeff' and Junio's comments.

Larry D'Anna (3):
  fix an error message in git-push so it goes to stderr
  clean up some of the output from git push --porcelain
  make git push --dry-run --porcelain exit with status 0 even if
    updates will be rejected

 builtin-push.c      |    8 ++++----
 builtin-send-pack.c |    4 ++++
 send-pack.h         |    1 +
 transport.c         |   11 +++++++----
 4 files changed, 16 insertions(+), 8 deletions(-)