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

29 Jun 2021

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: Re: [PATCH v2 1/3] mmc: rockchip_sdhci: add phy and clock config for rk3399

赵仪峰