Skip to contentSkip to conversation

clk: Aspeed: Fix eMMC clock speeds

PATCH Applied Eddie James 6 participants 11 messages Jul 9, 2020 – Jul 11, 2020

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover clk: Aspeed: Fix eMMC clock speeds

There were two problems affecting clock speeds to the eMMC chip. Firstly, the AST2600 clock was not muxed correctly to be derived from the MPLL. Secondly, the SDHCI clock control divider was not calculated correctly. This series addresses these problems.

Eddie James (2):
clk: AST2600: Add mux for EMMC clock
mmc: sdhci-of-aspeed: Fix clock divider calculation

 drivers/clk/clk-ast2600.c          | 49 +++++++++++++++++++++++++-----
 drivers/mmc/host/sdhci-of-aspeed.c |  2 +-
 2 files changed, 42 insertions(+), 9 deletions(-)
Signature · 1 line
2.24.0
#1 lore raw

PATCH 1/2 clk: AST2600: Add mux for EMMC clock

The EMMC clock can be derived from either the HPLL or the MPLL. Register a clock mux so that the rate is calculated correctly based upon the parent.

---
 drivers/clk/clk-ast2600.c | 49 ++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)
drivers/clk/clk-ast2600.c +41 -8
index 99afc949925f..177368cac6dd 100644
@@ -131,6 +131,18 @@ static const struct clk_div_table ast2600_eclk_div_table[] = {
 	{ 0 }
 };
 
+static const struct clk_div_table ast2600_emmc_extclk_div_table[] = {
+	{ 0x0, 2 },
+	{ 0x1, 4 },
+	{ 0x2, 6 },
+	{ 0x3, 8 },
+	{ 0x4, 10 },
+	{ 0x5, 12 },
+	{ 0x6, 14 },
+	{ 0x7, 16 },
+	{ 0 }
+};
+
 static const struct clk_div_table ast2600_mac_div_table[] = {
 	{ 0x0, 4 },
 	{ 0x1, 4 },
@@ -390,6 +402,11 @@ static struct clk_hw *aspeed_g6_clk_hw_register_gate(struct device *dev,
 	return hw;
 }
 
+static const char *const emmc_extclk_parent_names[] = {
+	"emmc_extclk_hpll_in",
+	"mpll",
+};
+
 static const char * const vclk_parent_names[] = {
 	"dpll",
 	"d1pll",
@@ -459,16 +476,32 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_UARTX] = hw;
 
-	/* EMMC ext clock divider */
-	hw = clk_hw_register_gate(dev, "emmc_extclk_gate", "hpll", 0,
-			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 15, 0,
-			&aspeed_g6_clk_lock);
+	/* EMMC ext clock */
+	hw = clk_hw_register_fixed_factor(dev, "emmc_extclk_hpll_in", "hpll",
+					  0, 1, 2);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
-	hw = clk_hw_register_divider_table(dev, "emmc_extclk", "emmc_extclk_gate", 0,
-			scu_g6_base + ASPEED_G6_CLK_SELECTION1, 12, 3, 0,
-			ast2600_div_table,
-			&aspeed_g6_clk_lock);
+
+	hw = clk_hw_register_mux(dev, "emmc_extclk_mux",
+				 emmc_extclk_parent_names,
+				 ARRAY_SIZE(emmc_extclk_parent_names), 0,
+				 scu_g6_base + ASPEED_G6_CLK_SELECTION1, 11, 1,
+				 0, &aspeed_g6_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	hw = clk_hw_register_gate(dev, "emmc_extclk_gate", "emmc_extclk_mux",
+				  0, scu_g6_base + ASPEED_G6_CLK_SELECTION1,
+				  15, 0, &aspeed_g6_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+
+	hw = clk_hw_register_divider_table(dev, "emmc_extclk",
+					   "emmc_extclk_gate", 0,
+					   scu_g6_base +
+						ASPEED_G6_CLK_SELECTION1, 12,
+					   3, 0, ast2600_emmc_extclk_div_table,
+					   &aspeed_g6_clk_lock);
 	if (IS_ERR(hw))
 		return PTR_ERR(hw);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
Signature · 1 line
2.24.0
maintainer Acked-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] clk: AST2600: Add mux for EMMC clock

Quoted 7 lines · Eddie James

The EMMC clock can be derived from either the HPLL or the MPLL. Register a clock mux so that the rate is calculated correctly based upon the parent.

Fixes d3d04f6c330a ("clk: Add support for AST2600 SoC")

Stephen, I think this should go to stable too.

Cheers,

Joel

Quoted 83 lines
---
 drivers/clk/clk-ast2600.c | 49 ++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)
drivers/clk/clk-ast2600.c +41 -9
index 99afc949925f..177368cac6dd 100644
@@ -131,6 +131,18 @@ static const struct clk_div_table ast2600_eclk_div_table[] = {
        { 0 }
 };
 
+static const struct clk_div_table ast2600_emmc_extclk_div_table[] = {
+       { 0x0, 2 },
+       { 0x1, 4 },
+       { 0x2, 6 },
+       { 0x3, 8 },
+       { 0x4, 10 },
+       { 0x5, 12 },
+       { 0x6, 14 },
+       { 0x7, 16 },
+       { 0 }
+};
+
 static const struct clk_div_table ast2600_mac_div_table[] = {
        { 0x0, 4 },
        { 0x1, 4 },
@@ -390,6 +402,11 @@ static struct clk_hw *aspeed_g6_clk_hw_register_gate(struct device *dev,
        return hw;
 }
 
+static const char *const emmc_extclk_parent_names[] = {
+       "emmc_extclk_hpll_in",
+       "mpll",
+};
+
 static const char * const vclk_parent_names[] = {
        "dpll",
        "d1pll",
@@ -459,16 +476,32 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
                return PTR_ERR(hw);
        aspeed_g6_clk_data->hws[ASPEED_CLK_UARTX] = hw;
 
-       /* EMMC ext clock divider */
-       hw = clk_hw_register_gate(dev, "emmc_extclk_gate", "hpll", 0,
-                       scu_g6_base + ASPEED_G6_CLK_SELECTION1, 15, 0,
-                       &aspeed_g6_clk_lock);
+       /* EMMC ext clock */
+       hw = clk_hw_register_fixed_factor(dev, "emmc_extclk_hpll_in", "hpll",
+                                         0, 1, 2);
        if (IS_ERR(hw))
                return PTR_ERR(hw);
-       hw = clk_hw_register_divider_table(dev, "emmc_extclk", "emmc_extclk_gate", 0,
-                       scu_g6_base + ASPEED_G6_CLK_SELECTION1, 12, 3, 0,
-                       ast2600_div_table,
-                       &aspeed_g6_clk_lock);
+
+       hw = clk_hw_register_mux(dev, "emmc_extclk_mux",
+                                emmc_extclk_parent_names,
+                                ARRAY_SIZE(emmc_extclk_parent_names), 0,
+                                scu_g6_base + ASPEED_G6_CLK_SELECTION1, 11, 1,
+                                0, &aspeed_g6_clk_lock);
+       if (IS_ERR(hw))
+               return PTR_ERR(hw);
+
+       hw = clk_hw_register_gate(dev, "emmc_extclk_gate", "emmc_extclk_mux",
+                                 0, scu_g6_base + ASPEED_G6_CLK_SELECTION1,
+                                 15, 0, &aspeed_g6_clk_lock);
+       if (IS_ERR(hw))
+               return PTR_ERR(hw);
+
+       hw = clk_hw_register_divider_table(dev, "emmc_extclk",
+                                          "emmc_extclk_gate", 0,
+                                          scu_g6_base +
+                                               ASPEED_G6_CLK_SELECTION1, 12,
+                                          3, 0, ast2600_emmc_extclk_div_table,
+                                          &aspeed_g6_clk_lock);
        if (IS_ERR(hw))
                return PTR_ERR(hw);
        aspeed_g6_clk_data->hws[ASPEED_CLK_EMMC] = hw;
--

2.24.0

#4 lore raw

PATCH 2/2 mmc: sdhci-of-aspeed: Fix clock divider calculation

In reply to Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

---
 drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/sdhci-of-aspeed.c +1 -1
index 56912e30c47e..a1bcc0f4ba9e 100644
@@ -68,7 +68,7 @@ static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (WARN_ON(clock > host->max_clk))
 		clock = host->max_clk;
 
-	for (div = 1; div < 256; div *= 2) {
+	for (div = 2; div < 256; div *= 2) {
 		if ((parent / div) <= clock)
 			break;
 	}
Signature · 1 line
2.24.0
Reviewed-by on v1 2/2
#5 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

Quoted 5 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

maintainer Acked-by
#6 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

Quoted 11 lines · Andrew Jeffery
Quoted 5 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

Fixes d3d04f6c330a ("clk: Add support for AST2600 SoC")

Stephen, I think this should go to stable too along with 1/2.

Cheers,

Joel

maintainer Acked-by on v1 2/2
#7 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

In reply to Eddie James
Quoted 5 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

Quoted 18 lines
---
 drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/sdhci-of-aspeed.c +1 -1
index 56912e30c47e..a1bcc0f4ba9e 100644
@@ -68,7 +68,7 @@ static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	if (WARN_ON(clock > host->max_clk))
 		clock = host->max_clk;
 
-	for (div = 1; div < 256; div *= 2) {
+	for (div = 2; div < 256; div *= 2) {
 		if ((parent / div) <= clock)
 			break;
 	}
#8 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

In reply to Eddie James
Quoted 6 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

Looks like I can pick this for fixes, as a standalone fix without patch1? No?

Kind regards
Uffe

Quoted 20 lines
---
 drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/sdhci-of-aspeed.c +1 -2
index 56912e30c47e..a1bcc0f4ba9e 100644
@@ -68,7 +68,7 @@ static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
        if (WARN_ON(clock > host->max_clk))
                clock = host->max_clk;
 
-       for (div = 1; div < 256; div *= 2) {
+       for (div = 2; div < 256; div *= 2) {
                if ((parent / div) <= clock)
                        break;
        }
--

2.24.0

#9 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

Quoted 9 lines · Ulf Hansson
Quoted 6 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

Looks like I can pick this for fixes, as a standalone fix without patch1? No?

Yes, please do.

Thanks,

Andrew

#10 lore raw

Re: [PATCH 2/2] mmc: sdhci-of-aspeed: Fix clock divider calculation

In reply to Eddie James
Quoted 6 lines · Eddie James

When calculating the clock divider, start dividing at 2 instead of 1. The divider is divided by two at the end of the calculation, so starting at 1 may result in a divider of 0, which shouldn't happen.

Applied for fixes and by adding a stable tag for v5.4+, thanks!

Kind regards
Uffe

Quoted 20 lines
---
 drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/host/sdhci-of-aspeed.c +1 -2
index 56912e30c47e..a1bcc0f4ba9e 100644
@@ -68,7 +68,7 @@ static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
        if (WARN_ON(clock > host->max_clk))
                clock = host->max_clk;
 
-       for (div = 1; div < 256; div *= 2) {
+       for (div = 2; div < 256; div *= 2) {
                if ((parent / div) <= clock)
                        break;
        }
--

2.24.0