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(-)
--
2.24.0
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.
Signed-off-by: Eddie James <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
---
drivers/clk/clk-ast2600.c | 49 ++++++++++++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index 99afc949925f..177368cac6dd 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -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
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.
Signed-off-by: Eddie James <[email protected]>
---
drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 56912e30c47e..a1bcc0f4ba9e 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -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
On Fri, 10 Jul 2020, at 05:27, Eddie James wrote:
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
On Thu, 9 Jul 2020 at 19:57, Eddie James <[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
> Reviewed-by: Andrew Jeffery <[email protected]>
Acked-by: Joel Stanley <[email protected]>
Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
Stephen, I think this should go to stable too.
Cheers,
Joel
> ---
> drivers/clk/clk-ast2600.c | 49 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index 99afc949925f..177368cac6dd 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -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
>
On Fri, 10 Jul 2020 at 01:14, Andrew Jeffery <[email protected]> wrote:
>
>
>
> On Fri, 10 Jul 2020, at 05:27, Eddie James wrote:
> > 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.
> >
> > Signed-off-by: Eddie James <[email protected]>
>
> Reviewed-by: Andrew Jeffery <[email protected]>
Acked-by: Joel Stanley <[email protected]>
Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
Stephen, I think this should go to stable too along with 1/2.
Cheers,
Joel
On 9/07/20 10:57 pm, Eddie James wrote:
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
Acked-by: Adrian Hunter <[email protected]>
> ---
> drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index 56912e30c47e..a1bcc0f4ba9e 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -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;
> }
>
On Thu, 9 Jul 2020 at 21:57, Eddie James <[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
Looks like I can pick this for fixes, as a standalone fix without patch1? No?
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index 56912e30c47e..a1bcc0f4ba9e 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -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
>
On Fri, 10 Jul 2020, at 17:09, Ulf Hansson wrote:
> On Thu, 9 Jul 2020 at 21:57, Eddie James <[email protected]> wrote:
> >
> > 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.
> >
> > Signed-off-by: Eddie James <[email protected]>
>
> Looks like I can pick this for fixes, as a standalone fix without patch1? No?
Yes, please do.
Thanks,
Andrew
On Thu, 9 Jul 2020 at 21:57, Eddie James <[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
Applied for fixes and by adding a stable tag for v5.4+, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-of-aspeed.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> index 56912e30c47e..a1bcc0f4ba9e 100644
> --- a/drivers/mmc/host/sdhci-of-aspeed.c
> +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> @@ -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
>
Quoting Eddie James (2020-07-09 12:57:05)
> 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.
>
> Signed-off-by: Eddie James <[email protected]>
> Reviewed-by: Andrew Jeffery <[email protected]>
> ---
Applied to clk-fixes