Delegate setting the Enhanced Strobe configuration to individual drivers if they set a function for it. Return -ENOTSUPP if they do not, like what the MMC uclass does.

Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com Reviewed-by: Jaehoon Chung jh80.chung@samsung.com ---

Changes in v2: - Add tag: "Reviewed-by: Jaehoon Chung jh80.chung@samsung.com"

drivers/mmc/sdhci.c | 18 ++++++++++++++++++ include/sdhci.h | 1 + 2 files changed, 19 insertions(+)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 766e4a6b0c5e..bf989a594f7e 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -513,6 +513,7 @@ void sdhci_set_uhs_timing(struct sdhci_host *host) reg |= SDHCI_CTRL_UHS_SDR104; break; case MMC_HS_400: + case MMC_HS_400_ES: reg |= SDHCI_CTRL_HS400; break; default: @@ -666,6 +667,7 @@ static int sdhci_set_ios(struct mmc *mmc) mmc->selected_mode == MMC_DDR_52 || mmc->selected_mode == MMC_HS_200 || mmc->selected_mode == MMC_HS_400 || + mmc->selected_mode == MMC_HS_400_ES || mmc->selected_mode == UHS_SDR25 || mmc->selected_mode == UHS_SDR50 || mmc->selected_mode == UHS_SDR104 || @@ -799,6 +801,19 @@ static int sdhci_wait_dat0(struct udevice *dev, int state, return -ETIMEDOUT; }

+#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT) +static int sdhci_set_enhanced_strobe(struct udevice *dev) +{ + struct mmc *mmc = mmc_get_mmc_dev(dev); + struct sdhci_host *host = mmc->priv; + + if (host->ops && host->ops->set_enhanced_strobe) + return host->ops->set_enhanced_strobe(host); + + return -ENOTSUPP; +} +#endif + const struct dm_mmc_ops sdhci_ops = { .send_cmd = sdhci_send_command, .set_ios = sdhci_set_ios, @@ -808,6 +823,9 @@ const struct dm_mmc_ops sdhci_ops = { .execute_tuning = sdhci_execute_tuning, #endif .wait_dat0 = sdhci_wait_dat0, +#if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT) + .set_enhanced_strobe = sdhci_set_enhanced_strobe, +#endif }; #else static const struct mmc_ops sdhci_ops = { diff --git a/include/sdhci.h b/include/sdhci.h index c718dd7206c1..7a65fdf95d30 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -272,6 +272,7 @@ struct sdhci_ops { int (*platform_execute_tuning)(struct mmc *host, u8 opcode); int (*set_delay)(struct sdhci_host *host); int (*deferred_probe)(struct sdhci_host *host); + int (*set_enhanced_strobe)(struct sdhci_host *host); };

#define ADMA_MAX_LEN 65532

The Rockchip RK3399 eMMC PHY has to be power-cycled while changing its clock speed to some higher speeds. This is dependent on the desired SDHCI clock speed, and it looks like the PHY should be powered off while setting the SDHCI clock in these cases.

Commit ac804143cfd1 ("mmc: rockchip_sdhci: add phy and clock config for rk3399") attempts to do this in the set_ios_post() hook by setting the SDHCI clock once more while the PHY is turned off/on as necessary, as the SDHCI framework does not provide a way to override how it sets its clock. However, the commit breaks reinitializing the eMMC on a few boards including chromebook_kevin and reportedly ROCKPro64.

This patch reworks the power cycling to utilize the SDHCI framework slightly better (using the set_control_reg() hook to power off the PHY and set_ios_post() hook to power it back on) which happens to fix the issue, at least on a chromebook_kevin.

Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com --- RK3568 parts only build-tested as I don't have a RK3568 board.

Changes in v2: - Add this patch

drivers/mmc/rockchip_sdhci.c | 53 +++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index 278473899c7c..f0d7ba4774d6 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -90,9 +90,10 @@ struct rockchip_sdhc { };

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); + void (*set_control_reg)(struct sdhci_host *host); + int (*set_ios_post)(struct sdhci_host *host); };

static int rk3399_emmc_phy_init(struct udevice *dev) @@ -182,15 +183,28 @@ static int rk3399_emmc_get_phy(struct udevice *dev) return 0; }

-static int rk3399_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clock) +static void rk3399_sdhci_set_control_reg(struct sdhci_host *host) { struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct mmc *mmc = host->mmc; + uint clock = mmc->tran_speed; int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ;

if (cycle_phy) rk3399_emmc_phy_power_off(priv->phy);

- sdhci_set_clock(host->mmc, clock); + sdhci_set_control_reg(host); +}; + +static int rk3399_sdhci_set_ios_post(struct sdhci_host *host) +{ + struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, host); + struct mmc *mmc = host->mmc; + uint clock = mmc->tran_speed; + int cycle_phy = host->clock != clock && clock > EMMC_MIN_FREQ; + + if (!clock) + clock = mmc->clock;

if (cycle_phy) rk3399_emmc_phy_power_on(priv->phy, clock); @@ -269,10 +283,8 @@ static int rk3568_emmc_get_phy(struct udevice *dev) return 0; }

-static int rockchip_sdhci_set_ios_post(struct sdhci_host *host) +static int rk3568_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; @@ -280,8 +292,7 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host) if (!clock) clock = mmc->clock;

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

if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) { reg = sdhci_readw(host, SDHCI_HOST_CONTROL2); @@ -295,6 +306,26 @@ static int rockchip_sdhci_set_ios_post(struct sdhci_host *host) return 0; }

+static void rockchip_sdhci_set_control_reg(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); + + if (data->set_control_reg) + data->set_control_reg(host); +} + +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); + + if (data->set_ios_post) + return data->set_ios_post(host); + + return 0; +} + static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) { struct sdhci_host *host = dev_get_priv(mmc->dev); @@ -358,6 +389,7 @@ static int rockchip_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) static struct sdhci_ops rockchip_sdhci_ops = { .set_ios_post = rockchip_sdhci_set_ios_post, .platform_execute_tuning = &rockchip_sdhci_execute_tuning, + .set_control_reg = rockchip_sdhci_set_control_reg, };

static int rockchip_sdhci_probe(struct udevice *dev) @@ -436,15 +468,16 @@ static int rockchip_sdhci_bind(struct udevice *dev) }

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, + .set_control_reg = rk3399_sdhci_set_control_reg, + .set_ios_post = rk3399_sdhci_set_ios_post, };

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, + .set_ios_post = rk3568_sdhci_set_ios_post, };

static const struct udevice_id sdhci_ids[] = {

On RK3568, a register bit must be set to enable Enhanced Strobe. However, it appears that the address of this register may differ from vendor to vendor and should be read from the underlying MMC IP. Let the Rockchip SDHCI driver read this address and set the relevant bit when Enhanced Strobe configuration is requested.

This is mostly ported from Linux's Synopsys DWC MSHC driver which happens to be the underlying IP. (drivers/mmc/host/sdhci-of-dwcmshc.c in Linux tree).

Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com --- Didn't add Reviewed-by tag due to changes. Only build-tested as I don't have a RK3568 board.

Changes in v2: - Rename rk3568_set_enhanced_strobe -> rk3568_sdhci_set_enhanced_strobe - Let set_enhanced_strobe() unset the ES bit if mode is not HS400_ES

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

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index f920c5141142..3030d746f890 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -45,6 +45,13 @@ #define ARASAN_VENDOR_REGISTER 0x78 #define ARASAN_VENDOR_ENHANCED_STROBE BIT(0)

+/* DWC IP vendor area 1 pointer */ +#define DWCMSHC_P_VENDOR_AREA1 0xe8 +#define DWCMSHC_AREA1_MASK GENMASK(11, 0) +/* Offset inside the vendor area 1 */ +#define DWCMSHC_EMMC_CONTROL 0x2c +#define DWCMSHC_ENHANCED_STROBE BIT(8) + /* Rockchip specific Registers */ #define DWCMSHC_EMMC_DLL_CTRL 0x800 #define DWCMSHC_EMMC_DLL_CTRL_RESET BIT(1) @@ -311,6 +318,25 @@ static int rk3568_emmc_get_phy(struct udevice *dev) return 0; }

+static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host) +{ + struct mmc *mmc = host->mmc; + u32 vendor; + int reg; + + reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK) + + DWCMSHC_EMMC_CONTROL; + + vendor = sdhci_readl(host, reg); + if (mmc->selected_mode == MMC_HS_400_ES) + vendor |= DWCMSHC_ENHANCED_STROBE; + else + vendor &= ~DWCMSHC_ENHANCED_STROBE; + sdhci_writel(host, vendor, reg); + + return 0; +} + static int rk3568_sdhci_set_ios_post(struct sdhci_host *host) { struct mmc *mmc = host->mmc; @@ -519,6 +545,7 @@ static const struct sdhci_data rk3568_data = { .get_phy = rk3568_emmc_get_phy, .emmc_phy_init = rk3568_emmc_phy_init, .set_ios_post = rk3568_sdhci_set_ios_post, + .set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe, };

static const struct udevice_id sdhci_ids[] = {