2019-05-20 11:03:56

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 0/9] Add SD host controller support for SC9860 platform

This patch set adds optional clock support, HS400 enhanced strobe mode support,
PHY DLL configuration and other optimization to make the SD host controller
can work well on the Spreadtrum SC9860 platform.

Baolin Wang (9):
mmc: sdhci-sprd: Check the enable clock's return value correctly
dt-bindings: mmc: sprd: Add another optional clock documentation
mmc: sdhci-sprd: Add optional gate clock support
mmc: sdhci-sprd: Implement the get_max_timeout_count() interface
mmc: sdhci-sprd: Add HS400 enhanced strobe mode
mmc: sdhci-sprd: Enable PHY DLL to make clock stable
dt-bindings: mmc: sprd: Add PHY DLL delay documentation
mmc: sdhci-sprd: Add PHY DLL delay configuration
arm64: dts: sprd: Add Spreadtrum SD host controller support

.../devicetree/bindings/mmc/sdhci-sprd.txt | 19 +++
arch/arm64/boot/dts/sprd/whale2.dtsi | 35 ++++
drivers/mmc/host/sdhci-sprd.c | 171 +++++++++++++++++++-
3 files changed, 217 insertions(+), 8 deletions(-)

--
1.7.9.5



2019-05-20 11:09:20

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 4/9] mmc: sdhci-sprd: Implement the get_max_timeout_count() interface

Implement the get_max_timeout_count() interface to set the Spredtrum SD
host controller actual maximum timeout count.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index 31ba7d6..d91281d 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -285,6 +285,12 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
usleep_range(300, 500);
}

+static unsigned int sdhci_sprd_get_max_timeout_count(struct sdhci_host *host)
+{
+ /* The Spredtrum controller actual maximum timeout count is 1 << 31 */
+ return 1 << 31;
+}
+
static struct sdhci_ops sdhci_sprd_ops = {
.read_l = sdhci_sprd_readl,
.write_l = sdhci_sprd_writel,
@@ -296,6 +302,7 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
.reset = sdhci_reset,
.set_uhs_signaling = sdhci_sprd_set_uhs_signaling,
.hw_reset = sdhci_sprd_hw_reset,
+ .get_max_timeout_count = sdhci_sprd_get_max_timeout_count,
};

static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
--
1.7.9.5


2019-05-20 11:11:14

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 5/9] mmc: sdhci-sprd: Add HS400 enhanced strobe mode

Add HS400 enhanced strobe mode support for Spreadtrum SD host controller.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index d91281d..edec197 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -41,6 +41,7 @@
/* SDHCI_HOST_CONTROL2 */
#define SDHCI_SPRD_CTRL_HS200 0x0005
#define SDHCI_SPRD_CTRL_HS400 0x0006
+#define SDHCI_SPRD_CTRL_HS400ES 0x0007

/*
* According to the standard specification, BIT(3) of SDHCI_SOFTWARE_RESET is
@@ -132,6 +133,15 @@ static inline void sdhci_sprd_sd_clk_off(struct sdhci_host *host)
sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL);
}

+static inline void sdhci_sprd_sd_clk_on(struct sdhci_host *host)
+{
+ u16 ctrl;
+
+ ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
+ ctrl |= SDHCI_CLOCK_CARD_EN;
+ sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL);
+}
+
static inline void
sdhci_sprd_set_dll_invert(struct sdhci_host *host, u32 mask, bool en)
{
@@ -325,6 +335,26 @@ static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
sdhci_request(mmc, mrq);
}

+static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
+ struct mmc_ios *ios)
+{
+ struct sdhci_host *host = mmc_priv(mmc);
+ u16 ctrl_2;
+
+ if (!ios->enhanced_strobe)
+ return;
+
+ sdhci_sprd_sd_clk_off(host);
+
+ /* Set HS400 enhanced strobe mode */
+ ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
+ ctrl_2 |= SDHCI_SPRD_CTRL_HS400ES;
+ sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+ sdhci_sprd_sd_clk_on(host);
+}
+
static const struct sdhci_pltfm_data sdhci_sprd_pdata = {
.quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
.quirks2 = SDHCI_QUIRK2_BROKEN_HS200 |
@@ -346,6 +376,8 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
host->dma_mask = DMA_BIT_MASK(64);
pdev->dev.dma_mask = &host->dma_mask;
host->mmc_host_ops.request = sdhci_sprd_request;
+ host->mmc_host_ops.hs400_enhanced_strobe =
+ sdhci_sprd_hs400_enhanced_strobe;

host->mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
MMC_CAP_ERASE | MMC_CAP_CMD23;
--
1.7.9.5


2019-05-20 11:11:45

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 6/9] mmc: sdhci-sprd: Enable PHY DLL to make clock stable

For the Spreadtrum SD host controller, when we changed the clock to be
more than 52M, we should enable the PHY DLL which is used to track the
clock frequency to make the clock work more stable. Otherwise deviation
may occur of the higher clock.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 44 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index edec197..e6eda13 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -22,6 +22,13 @@
/* SDHCI_ARGUMENT2 register high 16bit */
#define SDHCI_SPRD_ARG2_STUFF GENMASK(31, 16)

+#define SDHCI_SPRD_REG_32_DLL_CFG 0x200
+#define SDHCI_SPRD_DLL_ALL_CPST_EN (BIT(18) | BIT(24) | BIT(25) | BIT(26) | BIT(27))
+#define SDHCI_SPRD_DLL_EN BIT(21)
+#define SDHCI_SPRD_DLL_SEARCH_MODE BIT(16)
+#define SDHCI_SPRD_DLL_INIT_COUNT 0xc00
+#define SDHCI_SPRD_DLL_PHASE_INTERNAL 0x3
+
#define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET 0x208
#define SDHCIBSPRD_IT_WR_DLY_INV BIT(5)
#define SDHCI_SPRD_BIT_CMD_DLY_INV BIT(13)
@@ -56,6 +63,7 @@
#define SDHCI_SPRD_CLK_MAX_DIV 1023

#define SDHCI_SPRD_CLK_DEF_RATE 26000000
+#define SDHCI_SPRD_PHY_DLL_CLK 52000000

struct sdhci_sprd_host {
u32 version;
@@ -200,9 +208,33 @@ static inline void _sdhci_sprd_set_clock(struct sdhci_host *host,
}
}

+static void sdhci_sprd_enable_phy_dll(struct sdhci_host *host)
+{
+ u32 tmp;
+
+ tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
+ tmp &= ~(SDHCI_SPRD_DLL_EN | SDHCI_SPRD_DLL_ALL_CPST_EN);
+ sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
+ /* wait 1ms */
+ usleep_range(1000, 1250);
+
+ tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
+ tmp |= SDHCI_SPRD_DLL_ALL_CPST_EN | SDHCI_SPRD_DLL_SEARCH_MODE |
+ SDHCI_SPRD_DLL_INIT_COUNT | SDHCI_SPRD_DLL_PHASE_INTERNAL;
+ sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
+ /* wait 1ms */
+ usleep_range(1000, 1250);
+
+ tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
+ tmp |= SDHCI_SPRD_DLL_EN;
+ sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
+ /* wait 1ms */
+ usleep_range(1000, 1250);
+}
+
static void sdhci_sprd_set_clock(struct sdhci_host *host, unsigned int clock)
{
- bool en = false;
+ bool en = false, clk_changed = false;

if (clock == 0) {
sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
@@ -214,9 +246,19 @@ static void sdhci_sprd_set_clock(struct sdhci_host *host, unsigned int clock)
en = true;
sdhci_sprd_set_dll_invert(host, SDHCI_SPRD_BIT_CMD_DLY_INV |
SDHCI_SPRD_BIT_POSRD_DLY_INV, en);
+ clk_changed = true;
} else {
_sdhci_sprd_set_clock(host, clock);
}
+
+ /*
+ * According to the Spreadtrum SD host specification, when we changed
+ * the clock to be more than 52M, we should enable the PHY DLL which
+ * is used to track the clock frequency to make the clock work more
+ * stable. Otherwise deviation may occur of the higher clock.
+ */
+ if (clk_changed && clock > SDHCI_SPRD_PHY_DLL_CLK)
+ sdhci_sprd_enable_phy_dll(host);
}

static unsigned int sdhci_sprd_get_max_clock(struct sdhci_host *host)
--
1.7.9.5


2019-05-20 13:37:46

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 3/9] mmc: sdhci-sprd: Add optional gate clock support

For the Spreadtrum SC9860 platform, we should enable another gate clock
'2x_enable' to make the SD host controller work well.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index e741491..31ba7d6 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -60,6 +60,7 @@ struct sdhci_sprd_host {
u32 version;
struct clk *clk_sdio;
struct clk *clk_enable;
+ struct clk *clk_2x_enable;
u32 base_rate;
int flags; /* backup of host attribute */
};
@@ -364,6 +365,10 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
}
sprd_host->clk_enable = clk;

+ clk = devm_clk_get(&pdev->dev, "2x_enable");
+ if (!IS_ERR(clk))
+ sprd_host->clk_2x_enable = clk;
+
ret = clk_prepare_enable(sprd_host->clk_sdio);
if (ret)
goto pltfm_free;
@@ -372,6 +377,10 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
if (ret)
goto clk_disable;

+ ret = clk_prepare_enable(sprd_host->clk_2x_enable);
+ if (ret)
+ goto clk_disable2;
+
sdhci_sprd_init_config(host);
host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
@@ -408,6 +417,9 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
pm_runtime_set_suspended(&pdev->dev);

+ clk_disable_unprepare(sprd_host->clk_2x_enable);
+
+clk_disable2:
clk_disable_unprepare(sprd_host->clk_enable);

clk_disable:
@@ -427,6 +439,7 @@ static int sdhci_sprd_remove(struct platform_device *pdev)
mmc_remove_host(mmc);
clk_disable_unprepare(sprd_host->clk_sdio);
clk_disable_unprepare(sprd_host->clk_enable);
+ clk_disable_unprepare(sprd_host->clk_2x_enable);

mmc_free_host(mmc);

@@ -449,6 +462,7 @@ static int sdhci_sprd_runtime_suspend(struct device *dev)

clk_disable_unprepare(sprd_host->clk_sdio);
clk_disable_unprepare(sprd_host->clk_enable);
+ clk_disable_unprepare(sprd_host->clk_2x_enable);

return 0;
}
@@ -459,19 +473,28 @@ static int sdhci_sprd_runtime_resume(struct device *dev)
struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
int ret;

- ret = clk_prepare_enable(sprd_host->clk_enable);
+ ret = clk_prepare_enable(sprd_host->clk_2x_enable);
if (ret)
return ret;

+ ret = clk_prepare_enable(sprd_host->clk_enable);
+ if (ret)
+ goto clk_2x_disable;
+
ret = clk_prepare_enable(sprd_host->clk_sdio);
- if (ret) {
- clk_disable_unprepare(sprd_host->clk_enable);
- return ret;
- }
+ if (ret)
+ goto clk_disable;

sdhci_runtime_resume_host(host);
-
return 0;
+
+clk_disable:
+ clk_disable_unprepare(sprd_host->clk_enable);
+
+clk_2x_disable:
+ clk_disable_unprepare(sprd_host->clk_2x_enable);
+
+ return ret;
}
#endif

--
1.7.9.5


2019-05-20 13:37:59

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 7/9] dt-bindings: mmc: sprd: Add PHY DLL delay documentation

Introduce some PHY DLL delays properties to help to sample the PHY clock.

Signed-off-by: Baolin Wang <[email protected]>
---
.../devicetree/bindings/mmc/sdhci-sprd.txt | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
index a285c77..e675397 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
@@ -20,6 +20,23 @@ Optional properties:
- assigned-clocks: the same with "sdio" clock
- assigned-clock-parents: the default parent of "sdio" clock

+PHY DLL delays are used to delay the data valid window, and align the window
+to sampling clock. PHY DLL delays can be configured by following properties,
+and each property contains 4 cells which are used to configure the clock data
+write line delay value, clock read command line delay value, clock read data
+positive edge delay value and clock read data negative edge delay value.
+Each cell's delay value unit is cycle of the PHY clock.
+
+- sprd,phy-delay-legacy: Delay value for legacy timing.
+- sprd,phy-delay-sd-highspeed: Delay value for SD high-speed timing.
+- sprd,phy-delay-sd-uhs-sdr50: Delay value for SD UHS SDR50 timing.
+- sprd,phy-delay-sd-uhs-sdr104: Delay value for SD UHS SDR50 timing.
+- sprd,phy-delay-mmc-highspeed: Delay value for MMC high-speed timing.
+- sprd,phy-delay-mmc-ddr52: Delay value for MMC DDR52 timing.
+- sprd,phy-delay-mmc-hs200: Delay value for MMC HS200 timing.
+- sprd,phy-delay-mmc-hs400: Delay value for MMC HS400 timing.
+- sprd,phy-delay-mmc-hs400es: Delay value for MMC HS400 enhanced strobe timing.
+
Examples:

sdio0: sdio@20600000 {
@@ -33,6 +50,7 @@ sdio0: sdio@20600000 {
assigned-clocks = <&ap_clk CLK_EMMC_2X>;
assigned-clock-parents = <&rpll CLK_RPLL_390M>;

+ sprd,phy-delay-sd-uhs-sdr104 = <0x3f 0x7f 0x2e 0x2e>;
bus-width = <8>;
non-removable;
no-sdio;
--
1.7.9.5


2019-05-20 13:38:32

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 8/9] mmc: sdhci-sprd: Add PHY DLL delay configuration

Set the PHY DLL delay for each timing mode, which is used to sample the clock
accurately and make the clock more stable.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 51 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index e6eda13..911a09b 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -29,6 +29,8 @@
#define SDHCI_SPRD_DLL_INIT_COUNT 0xc00
#define SDHCI_SPRD_DLL_PHASE_INTERNAL 0x3

+#define SDHCI_SPRD_REG_32_DLL_DLY 0x204
+
#define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET 0x208
#define SDHCIBSPRD_IT_WR_DLY_INV BIT(5)
#define SDHCI_SPRD_BIT_CMD_DLY_INV BIT(13)
@@ -72,6 +74,24 @@ struct sdhci_sprd_host {
struct clk *clk_2x_enable;
u32 base_rate;
int flags; /* backup of host attribute */
+ u32 phy_delay[MMC_TIMING_MMC_HS400 + 2];
+};
+
+struct sdhci_sprd_phy_cfg {
+ const char *property;
+ u8 timing;
+};
+
+static const struct sdhci_sprd_phy_cfg sdhci_sprd_phy_cfgs[] = {
+ { "sprd,phy-delay-legacy", MMC_TIMING_LEGACY, },
+ { "sprd,phy-delay-sd-highspeed", MMC_TIMING_MMC_HS, },
+ { "sprd,phy-delay-sd-uhs-sdr50", MMC_TIMING_UHS_SDR50, },
+ { "sprd,phy-delay-sd-uhs-sdr104", MMC_TIMING_UHS_SDR104, },
+ { "sprd,phy-delay-mmc-highspeed", MMC_TIMING_MMC_HS, },
+ { "sprd,phy-delay-mmc-ddr52", MMC_TIMING_MMC_DDR52, },
+ { "sprd,phy-delay-mmc-hs200", MMC_TIMING_MMC_HS200, },
+ { "sprd,phy-delay-mmc-hs400", MMC_TIMING_MMC_HS400, },
+ { "sprd,phy-delay-mmc-hs400es", MMC_TIMING_MMC_HS400 + 1, },
};

#define TO_SPRD_HOST(host) sdhci_pltfm_priv(sdhci_priv(host))
@@ -276,6 +296,9 @@ static unsigned int sdhci_sprd_get_min_clock(struct sdhci_host *host)
static void sdhci_sprd_set_uhs_signaling(struct sdhci_host *host,
unsigned int timing)
{
+ struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+ struct mmc_host *mmc = host->mmc;
+ u32 *p = sprd_host->phy_delay;
u16 ctrl_2;

if (timing == host->timing)
@@ -314,6 +337,9 @@ static void sdhci_sprd_set_uhs_signaling(struct sdhci_host *host,
}

sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+
+ if (!mmc->ios.enhanced_strobe)
+ sdhci_writel(host, p[timing], SDHCI_SPRD_REG_32_DLL_DLY);
}

static void sdhci_sprd_hw_reset(struct sdhci_host *host)
@@ -381,6 +407,8 @@ static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
struct mmc_ios *ios)
{
struct sdhci_host *host = mmc_priv(mmc);
+ struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
+ u32 *p = sprd_host->phy_delay;
u16 ctrl_2;

if (!ios->enhanced_strobe)
@@ -395,6 +423,28 @@ static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);

sdhci_sprd_sd_clk_on(host);
+
+ /* Set the PHY DLL delay value for HS400 enhanced strobe mode */
+ sdhci_writel(host, p[MMC_TIMING_MMC_HS400 + 1],
+ SDHCI_SPRD_REG_32_DLL_DLY);
+}
+
+static void sdhci_sprd_phy_param_parse(struct sdhci_sprd_host *sprd_host,
+ struct device_node *np)
+{
+ u32 *p = sprd_host->phy_delay;
+ int ret, i, index;
+ u32 val[4];
+
+ for (i = 0; i < ARRAY_SIZE(sdhci_sprd_phy_cfgs); i++) {
+ ret = of_property_read_u32_array(np,
+ sdhci_sprd_phy_cfgs[i].property, val, 4);
+ if (ret)
+ continue;
+
+ index = sdhci_sprd_phy_cfgs[i].timing;
+ p[index] = val[0] | (val[1] << 8) | (val[2] << 16) | (val[3] << 24);
+ }
}

static const struct sdhci_pltfm_data sdhci_sprd_pdata = {
@@ -428,6 +478,7 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
goto pltfm_free;

sprd_host = TO_SPRD_HOST(host);
+ sdhci_sprd_phy_param_parse(sprd_host, pdev->dev.of_node);

clk = devm_clk_get(&pdev->dev, "sdio");
if (IS_ERR(clk)) {
--
1.7.9.5


2019-05-20 13:45:14

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 1/9] mmc: sdhci-sprd: Check the enable clock's return value correctly

Missed to check the enable clock's return value, fix it.

Signed-off-by: Baolin Wang <[email protected]>
---
drivers/mmc/host/sdhci-sprd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
index 9a822e2..e741491 100644
--- a/drivers/mmc/host/sdhci-sprd.c
+++ b/drivers/mmc/host/sdhci-sprd.c
@@ -368,7 +368,7 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
if (ret)
goto pltfm_free;

- clk_prepare_enable(sprd_host->clk_enable);
+ ret = clk_prepare_enable(sprd_host->clk_enable);
if (ret)
goto clk_disable;

--
1.7.9.5


2019-05-20 13:53:30

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 2/9] dt-bindings: mmc: sprd: Add another optional clock documentation

For some Spreadtrum platforms like SC9860 platform, we should enable another
gate clock '2x_enable' to make the SD host controller work well. Thus add
documentation for this optional clock.

Signed-off-by: Baolin Wang <[email protected]>
---
.../devicetree/bindings/mmc/sdhci-sprd.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
index 45c9978..a285c77 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
+++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
@@ -14,6 +14,7 @@ Required properties:
- clock-names: Should contain the following:
"sdio" - SDIO source clock (required)
"enable" - gate clock which used for enabling/disabling the device (required)
+ "2x_enable" - gate clock controlling the device for some special platforms (optional)

Optional properties:
- assigned-clocks: the same with "sdio" clock
--
1.7.9.5


2019-05-20 13:54:00

by Baolin Wang

[permalink] [raw]
Subject: [PATCH 9/9] arm64: dts: sprd: Add Spreadtrum SD host controller support

Add one Spreadtrum SD host controller to support eMMC card for Spreadtrum
SC9860 platform.

Signed-off-by: Baolin Wang <[email protected]>
---
arch/arm64/boot/dts/sprd/whale2.dtsi | 35 ++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/arch/arm64/boot/dts/sprd/whale2.dtsi b/arch/arm64/boot/dts/sprd/whale2.dtsi
index b5c5dce..86ec2b0 100644
--- a/arch/arm64/boot/dts/sprd/whale2.dtsi
+++ b/arch/arm64/boot/dts/sprd/whale2.dtsi
@@ -168,6 +168,34 @@
vdd-supply = <&vddusb33>;
sprd,vdd-voltage = <3300000>;
};
+
+ sdio3: sdio@50430000 {
+ compatible = "sprd,sdhci-r11";
+ reg = <0 0x50430000 0 0x1000>;
+ interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+
+ clock-names = "sdio", "enable", "2x_enable";
+ clocks = <&aon_prediv CLK_EMMC_2X>,
+ <&apahb_gate CLK_EMMC_EB>,
+ <&aon_gate CLK_EMMC_2X_EN>;
+ assigned-clocks = <&aon_prediv CLK_EMMC_2X>;
+ assigned-clock-parents = <&clk_l0_409m6>;
+
+ sprd,phy-delay-mmc-hs400 = <0x44 0x7f 0x2e 0x2e>;
+ sprd,phy-delay-mmc-hs200 = <0x0 0x8c 0x8c 0x8c>;
+ sprd,phy-delay-mmc-ddr52 = <0x3f 0x75 0x14 0x14>;
+ sprd,phy-delay-mmc-hs400es = <0x3f 0x3f 0x2e 0x2e>;
+ vmmc-supply = <&vddemmccore>;
+ bus-width = <8>;
+ non-removable;
+ no-sdio;
+ no-sd;
+ cap-mmc-hw-reset;
+ mmc-hs400-enhanced-strobe;
+ mmc-hs400-1_8v;
+ mmc-hs200-1_8v;
+ mmc-ddr-1_8v;
+ };
};

aon {
@@ -310,4 +338,11 @@
clock-frequency = <100000000>;
clock-output-names = "ext-rco-100m";
};
+
+ clk_l0_409m6: clk_l0_409m6 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <409600000>;
+ clock-output-names = "ext-409m6";
+ };
};
--
1.7.9.5


2019-06-03 08:43:43

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 0/9] Add SD host controller support for SC9860 platform

Hi Adrian & Ulf,

On Mon, 20 May 2019 at 18:12, Baolin Wang <[email protected]> wrote:
>
> This patch set adds optional clock support, HS400 enhanced strobe mode support,
> PHY DLL configuration and other optimization to make the SD host controller
> can work well on the Spreadtrum SC9860 platform.

Do you have any comments for this patch set? Thanks.

>
> Baolin Wang (9):
> mmc: sdhci-sprd: Check the enable clock's return value correctly
> dt-bindings: mmc: sprd: Add another optional clock documentation
> mmc: sdhci-sprd: Add optional gate clock support
> mmc: sdhci-sprd: Implement the get_max_timeout_count() interface
> mmc: sdhci-sprd: Add HS400 enhanced strobe mode
> mmc: sdhci-sprd: Enable PHY DLL to make clock stable
> dt-bindings: mmc: sprd: Add PHY DLL delay documentation
> mmc: sdhci-sprd: Add PHY DLL delay configuration
> arm64: dts: sprd: Add Spreadtrum SD host controller support
>
> .../devicetree/bindings/mmc/sdhci-sprd.txt | 19 +++
> arch/arm64/boot/dts/sprd/whale2.dtsi | 35 ++++
> drivers/mmc/host/sdhci-sprd.c | 171 +++++++++++++++++++-
> 3 files changed, 217 insertions(+), 8 deletions(-)
>
> --
> 1.7.9.5
>


--
Baolin Wang
Best Regards

2019-06-03 12:24:22

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 1/9] mmc: sdhci-sprd: Check the enable clock's return value correctly

On 20/05/19 1:11 PM, Baolin Wang wrote:
> Missed to check the enable clock's return value, fix it.
>
> Signed-off-by: Baolin Wang <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index 9a822e2..e741491 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -368,7 +368,7 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> if (ret)
> goto pltfm_free;
>
> - clk_prepare_enable(sprd_host->clk_enable);
> + ret = clk_prepare_enable(sprd_host->clk_enable);
> if (ret)
> goto clk_disable;
>
>

2019-06-03 12:38:35

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 4/9] mmc: sdhci-sprd: Implement the get_max_timeout_count() interface

On 20/05/19 1:11 PM, Baolin Wang wrote:
> Implement the get_max_timeout_count() interface to set the Spredtrum SD
> host controller actual maximum timeout count.
>
> Signed-off-by: Baolin Wang <[email protected]>

Seems surprising that there isn't a custom ->set_timeout() as well.
Nevertheless:

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index 31ba7d6..d91281d 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -285,6 +285,12 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
> usleep_range(300, 500);
> }
>
> +static unsigned int sdhci_sprd_get_max_timeout_count(struct sdhci_host *host)
> +{
> + /* The Spredtrum controller actual maximum timeout count is 1 << 31 */
> + return 1 << 31;
> +}
> +
> static struct sdhci_ops sdhci_sprd_ops = {
> .read_l = sdhci_sprd_readl,
> .write_l = sdhci_sprd_writel,
> @@ -296,6 +302,7 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
> .reset = sdhci_reset,
> .set_uhs_signaling = sdhci_sprd_set_uhs_signaling,
> .hw_reset = sdhci_sprd_hw_reset,
> + .get_max_timeout_count = sdhci_sprd_get_max_timeout_count,
> };
>
> static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
>

2019-06-03 12:42:02

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 5/9] mmc: sdhci-sprd: Add HS400 enhanced strobe mode

On 20/05/19 1:11 PM, Baolin Wang wrote:
> Add HS400 enhanced strobe mode support for Spreadtrum SD host controller.
>
> Signed-off-by: Baolin Wang <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index d91281d..edec197 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -41,6 +41,7 @@
> /* SDHCI_HOST_CONTROL2 */
> #define SDHCI_SPRD_CTRL_HS200 0x0005
> #define SDHCI_SPRD_CTRL_HS400 0x0006
> +#define SDHCI_SPRD_CTRL_HS400ES 0x0007
>
> /*
> * According to the standard specification, BIT(3) of SDHCI_SOFTWARE_RESET is
> @@ -132,6 +133,15 @@ static inline void sdhci_sprd_sd_clk_off(struct sdhci_host *host)
> sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL);
> }
>
> +static inline void sdhci_sprd_sd_clk_on(struct sdhci_host *host)
> +{
> + u16 ctrl;
> +
> + ctrl = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + ctrl |= SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, ctrl, SDHCI_CLOCK_CONTROL);
> +}
> +
> static inline void
> sdhci_sprd_set_dll_invert(struct sdhci_host *host, u32 mask, bool en)
> {
> @@ -325,6 +335,26 @@ static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
> sdhci_request(mmc, mrq);
> }
>
> +static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u16 ctrl_2;
> +
> + if (!ios->enhanced_strobe)
> + return;
> +
> + sdhci_sprd_sd_clk_off(host);
> +
> + /* Set HS400 enhanced strobe mode */
> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + ctrl_2 |= SDHCI_SPRD_CTRL_HS400ES;
> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> + sdhci_sprd_sd_clk_on(host);
> +}
> +
> static const struct sdhci_pltfm_data sdhci_sprd_pdata = {
> .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK,
> .quirks2 = SDHCI_QUIRK2_BROKEN_HS200 |
> @@ -346,6 +376,8 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> host->dma_mask = DMA_BIT_MASK(64);
> pdev->dev.dma_mask = &host->dma_mask;
> host->mmc_host_ops.request = sdhci_sprd_request;
> + host->mmc_host_ops.hs400_enhanced_strobe =
> + sdhci_sprd_hs400_enhanced_strobe;
>
> host->mmc->caps = MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED |
> MMC_CAP_ERASE | MMC_CAP_CMD23;
>

2019-06-03 12:45:20

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 6/9] mmc: sdhci-sprd: Enable PHY DLL to make clock stable

On 20/05/19 1:11 PM, Baolin Wang wrote:
> For the Spreadtrum SD host controller, when we changed the clock to be
> more than 52M, we should enable the PHY DLL which is used to track the
> clock frequency to make the clock work more stable. Otherwise deviation
> may occur of the higher clock.
>
> Signed-off-by: Baolin Wang <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 44 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index edec197..e6eda13 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -22,6 +22,13 @@
> /* SDHCI_ARGUMENT2 register high 16bit */
> #define SDHCI_SPRD_ARG2_STUFF GENMASK(31, 16)
>
> +#define SDHCI_SPRD_REG_32_DLL_CFG 0x200
> +#define SDHCI_SPRD_DLL_ALL_CPST_EN (BIT(18) | BIT(24) | BIT(25) | BIT(26) | BIT(27))
> +#define SDHCI_SPRD_DLL_EN BIT(21)
> +#define SDHCI_SPRD_DLL_SEARCH_MODE BIT(16)
> +#define SDHCI_SPRD_DLL_INIT_COUNT 0xc00
> +#define SDHCI_SPRD_DLL_PHASE_INTERNAL 0x3
> +
> #define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET 0x208
> #define SDHCIBSPRD_IT_WR_DLY_INV BIT(5)
> #define SDHCI_SPRD_BIT_CMD_DLY_INV BIT(13)
> @@ -56,6 +63,7 @@
> #define SDHCI_SPRD_CLK_MAX_DIV 1023
>
> #define SDHCI_SPRD_CLK_DEF_RATE 26000000
> +#define SDHCI_SPRD_PHY_DLL_CLK 52000000
>
> struct sdhci_sprd_host {
> u32 version;
> @@ -200,9 +208,33 @@ static inline void _sdhci_sprd_set_clock(struct sdhci_host *host,
> }
> }
>
> +static void sdhci_sprd_enable_phy_dll(struct sdhci_host *host)
> +{
> + u32 tmp;
> +
> + tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
> + tmp &= ~(SDHCI_SPRD_DLL_EN | SDHCI_SPRD_DLL_ALL_CPST_EN);
> + sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
> + /* wait 1ms */
> + usleep_range(1000, 1250);
> +
> + tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
> + tmp |= SDHCI_SPRD_DLL_ALL_CPST_EN | SDHCI_SPRD_DLL_SEARCH_MODE |
> + SDHCI_SPRD_DLL_INIT_COUNT | SDHCI_SPRD_DLL_PHASE_INTERNAL;
> + sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
> + /* wait 1ms */
> + usleep_range(1000, 1250);
> +
> + tmp = sdhci_readl(host, SDHCI_SPRD_REG_32_DLL_CFG);
> + tmp |= SDHCI_SPRD_DLL_EN;
> + sdhci_writel(host, tmp, SDHCI_SPRD_REG_32_DLL_CFG);
> + /* wait 1ms */
> + usleep_range(1000, 1250);
> +}
> +
> static void sdhci_sprd_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> - bool en = false;
> + bool en = false, clk_changed = false;
>
> if (clock == 0) {
> sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> @@ -214,9 +246,19 @@ static void sdhci_sprd_set_clock(struct sdhci_host *host, unsigned int clock)
> en = true;
> sdhci_sprd_set_dll_invert(host, SDHCI_SPRD_BIT_CMD_DLY_INV |
> SDHCI_SPRD_BIT_POSRD_DLY_INV, en);
> + clk_changed = true;
> } else {
> _sdhci_sprd_set_clock(host, clock);
> }
> +
> + /*
> + * According to the Spreadtrum SD host specification, when we changed
> + * the clock to be more than 52M, we should enable the PHY DLL which
> + * is used to track the clock frequency to make the clock work more
> + * stable. Otherwise deviation may occur of the higher clock.
> + */
> + if (clk_changed && clock > SDHCI_SPRD_PHY_DLL_CLK)
> + sdhci_sprd_enable_phy_dll(host);
> }
>
> static unsigned int sdhci_sprd_get_max_clock(struct sdhci_host *host)
>

2019-06-03 13:05:44

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 8/9] mmc: sdhci-sprd: Add PHY DLL delay configuration

On 20/05/19 1:12 PM, Baolin Wang wrote:
> Set the PHY DLL delay for each timing mode, which is used to sample the clock
> accurately and make the clock more stable.
>
> Signed-off-by: Baolin Wang <[email protected]>

One comment, nevertheless:

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 51 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index e6eda13..911a09b 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -29,6 +29,8 @@
> #define SDHCI_SPRD_DLL_INIT_COUNT 0xc00
> #define SDHCI_SPRD_DLL_PHASE_INTERNAL 0x3
>
> +#define SDHCI_SPRD_REG_32_DLL_DLY 0x204
> +
> #define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET 0x208
> #define SDHCIBSPRD_IT_WR_DLY_INV BIT(5)
> #define SDHCI_SPRD_BIT_CMD_DLY_INV BIT(13)
> @@ -72,6 +74,24 @@ struct sdhci_sprd_host {
> struct clk *clk_2x_enable;
> u32 base_rate;
> int flags; /* backup of host attribute */
> + u32 phy_delay[MMC_TIMING_MMC_HS400 + 2];
> +};
> +
> +struct sdhci_sprd_phy_cfg {
> + const char *property;
> + u8 timing;
> +};
> +
> +static const struct sdhci_sprd_phy_cfg sdhci_sprd_phy_cfgs[] = {
> + { "sprd,phy-delay-legacy", MMC_TIMING_LEGACY, },
> + { "sprd,phy-delay-sd-highspeed", MMC_TIMING_MMC_HS, },

Did you mean MMC_TIMING_SD_HS

> + { "sprd,phy-delay-sd-uhs-sdr50", MMC_TIMING_UHS_SDR50, },
> + { "sprd,phy-delay-sd-uhs-sdr104", MMC_TIMING_UHS_SDR104, },
> + { "sprd,phy-delay-mmc-highspeed", MMC_TIMING_MMC_HS, },
> + { "sprd,phy-delay-mmc-ddr52", MMC_TIMING_MMC_DDR52, },
> + { "sprd,phy-delay-mmc-hs200", MMC_TIMING_MMC_HS200, },
> + { "sprd,phy-delay-mmc-hs400", MMC_TIMING_MMC_HS400, },
> + { "sprd,phy-delay-mmc-hs400es", MMC_TIMING_MMC_HS400 + 1, },
> };
>
> #define TO_SPRD_HOST(host) sdhci_pltfm_priv(sdhci_priv(host))
> @@ -276,6 +296,9 @@ static unsigned int sdhci_sprd_get_min_clock(struct sdhci_host *host)
> static void sdhci_sprd_set_uhs_signaling(struct sdhci_host *host,
> unsigned int timing)
> {
> + struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
> + struct mmc_host *mmc = host->mmc;
> + u32 *p = sprd_host->phy_delay;
> u16 ctrl_2;
>
> if (timing == host->timing)
> @@ -314,6 +337,9 @@ static void sdhci_sprd_set_uhs_signaling(struct sdhci_host *host,
> }
>
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> + if (!mmc->ios.enhanced_strobe)
> + sdhci_writel(host, p[timing], SDHCI_SPRD_REG_32_DLL_DLY);
> }
>
> static void sdhci_sprd_hw_reset(struct sdhci_host *host)
> @@ -381,6 +407,8 @@ static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
> struct mmc_ios *ios)
> {
> struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
> + u32 *p = sprd_host->phy_delay;
> u16 ctrl_2;
>
> if (!ios->enhanced_strobe)
> @@ -395,6 +423,28 @@ static void sdhci_sprd_hs400_enhanced_strobe(struct mmc_host *mmc,
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>
> sdhci_sprd_sd_clk_on(host);
> +
> + /* Set the PHY DLL delay value for HS400 enhanced strobe mode */
> + sdhci_writel(host, p[MMC_TIMING_MMC_HS400 + 1],
> + SDHCI_SPRD_REG_32_DLL_DLY);
> +}
> +
> +static void sdhci_sprd_phy_param_parse(struct sdhci_sprd_host *sprd_host,
> + struct device_node *np)
> +{
> + u32 *p = sprd_host->phy_delay;
> + int ret, i, index;
> + u32 val[4];
> +
> + for (i = 0; i < ARRAY_SIZE(sdhci_sprd_phy_cfgs); i++) {
> + ret = of_property_read_u32_array(np,
> + sdhci_sprd_phy_cfgs[i].property, val, 4);
> + if (ret)
> + continue;
> +
> + index = sdhci_sprd_phy_cfgs[i].timing;
> + p[index] = val[0] | (val[1] << 8) | (val[2] << 16) | (val[3] << 24);
> + }
> }
>
> static const struct sdhci_pltfm_data sdhci_sprd_pdata = {
> @@ -428,6 +478,7 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> goto pltfm_free;
>
> sprd_host = TO_SPRD_HOST(host);
> + sdhci_sprd_phy_param_parse(sprd_host, pdev->dev.of_node);
>
> clk = devm_clk_get(&pdev->dev, "sdio");
> if (IS_ERR(clk)) {
>

2019-06-03 13:36:46

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 2/9] dt-bindings: mmc: sprd: Add another optional clock documentation

On Mon, 20 May 2019 at 12:12, Baolin Wang <[email protected]> wrote:
>
> For some Spreadtrum platforms like SC9860 platform, we should enable another
> gate clock '2x_enable' to make the SD host controller work well. Thus add
> documentation for this optional clock.
>
> Signed-off-by: Baolin Wang <[email protected]>
> ---
> .../devicetree/bindings/mmc/sdhci-sprd.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> index 45c9978..a285c77 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> @@ -14,6 +14,7 @@ Required properties:
> - clock-names: Should contain the following:
> "sdio" - SDIO source clock (required)
> "enable" - gate clock which used for enabling/disabling the device (required)
> + "2x_enable" - gate clock controlling the device for some special platforms (optional)

This is a bit vague, could you please elaborate (and fold in that
information to the doc) on what kind of clock this is?

[...]

Kind regards
Uffe

2019-06-03 16:58:40

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH 3/9] mmc: sdhci-sprd: Add optional gate clock support

On 20/05/19 1:11 PM, Baolin Wang wrote:
> For the Spreadtrum SC9860 platform, we should enable another gate clock
> '2x_enable' to make the SD host controller work well.
>
> Signed-off-by: Baolin Wang <[email protected]>

Acked-by: Adrian Hunter <[email protected]>

> ---
> drivers/mmc/host/sdhci-sprd.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> index e741491..31ba7d6 100644
> --- a/drivers/mmc/host/sdhci-sprd.c
> +++ b/drivers/mmc/host/sdhci-sprd.c
> @@ -60,6 +60,7 @@ struct sdhci_sprd_host {
> u32 version;
> struct clk *clk_sdio;
> struct clk *clk_enable;
> + struct clk *clk_2x_enable;
> u32 base_rate;
> int flags; /* backup of host attribute */
> };
> @@ -364,6 +365,10 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> }
> sprd_host->clk_enable = clk;
>
> + clk = devm_clk_get(&pdev->dev, "2x_enable");
> + if (!IS_ERR(clk))
> + sprd_host->clk_2x_enable = clk;
> +
> ret = clk_prepare_enable(sprd_host->clk_sdio);
> if (ret)
> goto pltfm_free;
> @@ -372,6 +377,10 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> if (ret)
> goto clk_disable;
>
> + ret = clk_prepare_enable(sprd_host->clk_2x_enable);
> + if (ret)
> + goto clk_disable2;
> +
> sdhci_sprd_init_config(host);
> host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> sprd_host->version = ((host->version & SDHCI_VENDOR_VER_MASK) >>
> @@ -408,6 +417,9 @@ static int sdhci_sprd_probe(struct platform_device *pdev)
> pm_runtime_disable(&pdev->dev);
> pm_runtime_set_suspended(&pdev->dev);
>
> + clk_disable_unprepare(sprd_host->clk_2x_enable);
> +
> +clk_disable2:
> clk_disable_unprepare(sprd_host->clk_enable);
>
> clk_disable:
> @@ -427,6 +439,7 @@ static int sdhci_sprd_remove(struct platform_device *pdev)
> mmc_remove_host(mmc);
> clk_disable_unprepare(sprd_host->clk_sdio);
> clk_disable_unprepare(sprd_host->clk_enable);
> + clk_disable_unprepare(sprd_host->clk_2x_enable);
>
> mmc_free_host(mmc);
>
> @@ -449,6 +462,7 @@ static int sdhci_sprd_runtime_suspend(struct device *dev)
>
> clk_disable_unprepare(sprd_host->clk_sdio);
> clk_disable_unprepare(sprd_host->clk_enable);
> + clk_disable_unprepare(sprd_host->clk_2x_enable);
>
> return 0;
> }
> @@ -459,19 +473,28 @@ static int sdhci_sprd_runtime_resume(struct device *dev)
> struct sdhci_sprd_host *sprd_host = TO_SPRD_HOST(host);
> int ret;
>
> - ret = clk_prepare_enable(sprd_host->clk_enable);
> + ret = clk_prepare_enable(sprd_host->clk_2x_enable);
> if (ret)
> return ret;
>
> + ret = clk_prepare_enable(sprd_host->clk_enable);
> + if (ret)
> + goto clk_2x_disable;
> +
> ret = clk_prepare_enable(sprd_host->clk_sdio);
> - if (ret) {
> - clk_disable_unprepare(sprd_host->clk_enable);
> - return ret;
> - }
> + if (ret)
> + goto clk_disable;
>
> sdhci_runtime_resume_host(host);
> -
> return 0;
> +
> +clk_disable:
> + clk_disable_unprepare(sprd_host->clk_enable);
> +
> +clk_2x_disable:
> + clk_disable_unprepare(sprd_host->clk_2x_enable);
> +
> + return ret;
> }
> #endif
>
>

2019-06-04 02:20:14

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 8/9] mmc: sdhci-sprd: Add PHY DLL delay configuration

Hi Adrian,

On Mon, 3 Jun 2019 at 21:03, Adrian Hunter <[email protected]> wrote:
>
> On 20/05/19 1:12 PM, Baolin Wang wrote:
> > Set the PHY DLL delay for each timing mode, which is used to sample the clock
> > accurately and make the clock more stable.
> >
> > Signed-off-by: Baolin Wang <[email protected]>
>
> One comment, nevertheless:
>
> Acked-by: Adrian Hunter <[email protected]>
>
> > ---
> > drivers/mmc/host/sdhci-sprd.c | 51 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> > index e6eda13..911a09b 100644
> > --- a/drivers/mmc/host/sdhci-sprd.c
> > +++ b/drivers/mmc/host/sdhci-sprd.c
> > @@ -29,6 +29,8 @@
> > #define SDHCI_SPRD_DLL_INIT_COUNT 0xc00
> > #define SDHCI_SPRD_DLL_PHASE_INTERNAL 0x3
> >
> > +#define SDHCI_SPRD_REG_32_DLL_DLY 0x204
> > +
> > #define SDHCI_SPRD_REG_32_DLL_DLY_OFFSET 0x208
> > #define SDHCIBSPRD_IT_WR_DLY_INV BIT(5)
> > #define SDHCI_SPRD_BIT_CMD_DLY_INV BIT(13)
> > @@ -72,6 +74,24 @@ struct sdhci_sprd_host {
> > struct clk *clk_2x_enable;
> > u32 base_rate;
> > int flags; /* backup of host attribute */
> > + u32 phy_delay[MMC_TIMING_MMC_HS400 + 2];
> > +};
> > +
> > +struct sdhci_sprd_phy_cfg {
> > + const char *property;
> > + u8 timing;
> > +};
> > +
> > +static const struct sdhci_sprd_phy_cfg sdhci_sprd_phy_cfgs[] = {
> > + { "sprd,phy-delay-legacy", MMC_TIMING_LEGACY, },
> > + { "sprd,phy-delay-sd-highspeed", MMC_TIMING_MMC_HS, },
>
> Did you mean MMC_TIMING_SD_HS

Ah, yes, my copy mistake and will fix it in next version.
Thanks for your reviewing and comments.

--
Baolin Wang
Best Regards

2019-06-04 02:34:48

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 2/9] dt-bindings: mmc: sprd: Add another optional clock documentation

Hi Ulf,

On Mon, 3 Jun 2019 at 21:34, Ulf Hansson <[email protected]> wrote:
>
> On Mon, 20 May 2019 at 12:12, Baolin Wang <[email protected]> wrote:
> >
> > For some Spreadtrum platforms like SC9860 platform, we should enable another
> > gate clock '2x_enable' to make the SD host controller work well. Thus add
> > documentation for this optional clock.
> >
> > Signed-off-by: Baolin Wang <[email protected]>
> > ---
> > .../devicetree/bindings/mmc/sdhci-sprd.txt | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > index 45c9978..a285c77 100644
> > --- a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > @@ -14,6 +14,7 @@ Required properties:
> > - clock-names: Should contain the following:
> > "sdio" - SDIO source clock (required)
> > "enable" - gate clock which used for enabling/disabling the device (required)
> > + "2x_enable" - gate clock controlling the device for some special platforms (optional)
>
> This is a bit vague, could you please elaborate (and fold in that
> information to the doc) on what kind of clock this is?

Sorry for confusing. For some Spreadtrum platfroms like SC9860
platform, we should enable 2 gate clocks to enable SD host controller,
that means we have 2 serialized clock gates. I know that's a little
weird, but that's our clock's design.

--
Baolin Wang
Best Regards

2019-06-04 06:55:15

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 2/9] dt-bindings: mmc: sprd: Add another optional clock documentation

On Tue, 4 Jun 2019 at 04:33, Baolin Wang <[email protected]> wrote:
>
> Hi Ulf,
>
> On Mon, 3 Jun 2019 at 21:34, Ulf Hansson <[email protected]> wrote:
> >
> > On Mon, 20 May 2019 at 12:12, Baolin Wang <[email protected]> wrote:
> > >
> > > For some Spreadtrum platforms like SC9860 platform, we should enable another
> > > gate clock '2x_enable' to make the SD host controller work well. Thus add
> > > documentation for this optional clock.
> > >
> > > Signed-off-by: Baolin Wang <[email protected]>
> > > ---
> > > .../devicetree/bindings/mmc/sdhci-sprd.txt | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > > index 45c9978..a285c77 100644
> > > --- a/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > > +++ b/Documentation/devicetree/bindings/mmc/sdhci-sprd.txt
> > > @@ -14,6 +14,7 @@ Required properties:
> > > - clock-names: Should contain the following:
> > > "sdio" - SDIO source clock (required)
> > > "enable" - gate clock which used for enabling/disabling the device (required)
> > > + "2x_enable" - gate clock controlling the device for some special platforms (optional)
> >
> > This is a bit vague, could you please elaborate (and fold in that
> > information to the doc) on what kind of clock this is?
>
> Sorry for confusing. For some Spreadtrum platfroms like SC9860
> platform, we should enable 2 gate clocks to enable SD host controller,
> that means we have 2 serialized clock gates. I know that's a little
> weird, but that's our clock's design.

Okay, just wanted to make sure this new clock isn't something that
should be modeled through the clock tree.

Thanks for explaining, then I am happy with the patch as is.

Kind regards
Uffe

2019-06-04 07:17:07

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 0/9] Add SD host controller support for SC9860 platform

On Mon, 3 Jun 2019 at 10:42, Baolin Wang <[email protected]> wrote:
>
> Hi Adrian & Ulf,
>
> On Mon, 20 May 2019 at 18:12, Baolin Wang <[email protected]> wrote:
> >
> > This patch set adds optional clock support, HS400 enhanced strobe mode support,
> > PHY DLL configuration and other optimization to make the SD host controller
> > can work well on the Spreadtrum SC9860 platform.
>
> Do you have any comments for this patch set? Thanks.
>

Seems like the series is almost ready to go. However, due to a few the
minor comments/questions from Adrian, I am expecting a new version
from you before applying.

Kind regards
Uffe

> >
> > Baolin Wang (9):
> > mmc: sdhci-sprd: Check the enable clock's return value correctly
> > dt-bindings: mmc: sprd: Add another optional clock documentation
> > mmc: sdhci-sprd: Add optional gate clock support
> > mmc: sdhci-sprd: Implement the get_max_timeout_count() interface
> > mmc: sdhci-sprd: Add HS400 enhanced strobe mode
> > mmc: sdhci-sprd: Enable PHY DLL to make clock stable
> > dt-bindings: mmc: sprd: Add PHY DLL delay documentation
> > mmc: sdhci-sprd: Add PHY DLL delay configuration
> > arm64: dts: sprd: Add Spreadtrum SD host controller support
> >
> > .../devicetree/bindings/mmc/sdhci-sprd.txt | 19 +++
> > arch/arm64/boot/dts/sprd/whale2.dtsi | 35 ++++
> > drivers/mmc/host/sdhci-sprd.c | 171 +++++++++++++++++++-
> > 3 files changed, 217 insertions(+), 8 deletions(-)
> >
> > --
> > 1.7.9.5
> >
>
>
> --
> Baolin Wang
> Best Regards

2019-06-04 07:22:52

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 0/9] Add SD host controller support for SC9860 platform

On Tue, 4 Jun 2019 at 15:14, Ulf Hansson <[email protected]> wrote:
>
> On Mon, 3 Jun 2019 at 10:42, Baolin Wang <[email protected]> wrote:
> >
> > Hi Adrian & Ulf,
> >
> > On Mon, 20 May 2019 at 18:12, Baolin Wang <[email protected]> wrote:
> > >
> > > This patch set adds optional clock support, HS400 enhanced strobe mode support,
> > > PHY DLL configuration and other optimization to make the SD host controller
> > > can work well on the Spreadtrum SC9860 platform.
> >
> > Do you have any comments for this patch set? Thanks.
> >
>
> Seems like the series is almost ready to go. However, due to a few the
> minor comments/questions from Adrian, I am expecting a new version
> from you before applying.

Okay, I am preparing the V2 with addressing the minor comment. Thanks.

--
Baolin Wang
Best Regards

2019-06-04 08:05:03

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH 4/9] mmc: sdhci-sprd: Implement the get_max_timeout_count() interface

On Mon, 3 Jun 2019 at 20:35, Adrian Hunter <[email protected]> wrote:
>
> On 20/05/19 1:11 PM, Baolin Wang wrote:
> > Implement the get_max_timeout_count() interface to set the Spredtrum SD
> > host controller actual maximum timeout count.
> >
> > Signed-off-by: Baolin Wang <[email protected]>
>
> Seems surprising that there isn't a custom ->set_timeout() as well.

Until now we did not find issues when using sdhci_calc_timeout().
Thanks for your reviewing.

> Nevertheless:
>
> Acked-by: Adrian Hunter <[email protected]>
>
> > ---
> > drivers/mmc/host/sdhci-sprd.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-sprd.c b/drivers/mmc/host/sdhci-sprd.c
> > index 31ba7d6..d91281d 100644
> > --- a/drivers/mmc/host/sdhci-sprd.c
> > +++ b/drivers/mmc/host/sdhci-sprd.c
> > @@ -285,6 +285,12 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
> > usleep_range(300, 500);
> > }
> >
> > +static unsigned int sdhci_sprd_get_max_timeout_count(struct sdhci_host *host)
> > +{
> > + /* The Spredtrum controller actual maximum timeout count is 1 << 31 */
> > + return 1 << 31;
> > +}
> > +
> > static struct sdhci_ops sdhci_sprd_ops = {
> > .read_l = sdhci_sprd_readl,
> > .write_l = sdhci_sprd_writel,
> > @@ -296,6 +302,7 @@ static void sdhci_sprd_hw_reset(struct sdhci_host *host)
> > .reset = sdhci_reset,
> > .set_uhs_signaling = sdhci_sprd_set_uhs_signaling,
> > .hw_reset = sdhci_sprd_hw_reset,
> > + .get_max_timeout_count = sdhci_sprd_get_max_timeout_count,
> > };
> >
> > static void sdhci_sprd_request(struct mmc_host *mmc, struct mmc_request *mrq)
> >
>


--
Baolin Wang
Best Regards