Skip to contentSkip to conversation

clk: AST2600 and FSI: Add APLL to control FSI bus frequency

PATCH In review Eddie James 4 participants 10 messages Sep 10, 2020 – Sep 22, 2020

Subsystems (4)
lore
Share & tools
lore

Eddie James

author maintainer
#0 lore raw

cover clk: AST2600 and FSI: Add APLL to control FSI bus frequency

Add functionality to control the APLL clock on the AST2600. The APLL provides the clock for the FSI master on the AST2600. Then add a devicetree property to set the AST2600 FSI master bus frequency.

Eddie James (4):
dt-bindings: clock: Add AST2600 APLLdivN definition
clk: ast2600: Add functionality to the APLL clock
dt-bindings: fsi: Aspeed master: Add bus-frequency property
fsi: Aspeed master: Set bus frequency from devicetree

 .../bindings/fsi/fsi-master-aspeed.txt        |   1 +
 drivers/clk/clk-ast2600.c                     | 177 ++++++++++++++++--
 drivers/fsi/fsi-master-aspeed.c               |   5 +
 include/dt-bindings/clock/ast2600-clock.h     |   1 +
 4 files changed, 172 insertions(+), 12 deletions(-)
Signature · 1 line
2.26.2
author maintainer
#1 lore raw

PATCH 1/4 dt-bindings: clock: Add AST2600 APLLdivN definition

Add a new clock definition for the "APLLdivN" as described in the AST2600 specification. This clock is simply the APLL divided by a factor defined in the SCU registers. It is the input to the FSI bus.

---
 include/dt-bindings/clock/ast2600-clock.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/clock/ast2600-clock.h +1 -0
index 62b9520a00fd..5a9ae0a1d574 100644
@@ -87,6 +87,7 @@
 #define ASPEED_CLK_MAC2RCLK		68
 #define ASPEED_CLK_MAC3RCLK		69
 #define ASPEED_CLK_MAC4RCLK		70
+#define ASPEED_CLK_APLLN		71
 
 /* Only list resets here that are not part of a gate */
 #define ASPEED_RESET_ADC		55
Signature · 1 line
2.26.2
maintainer
#2 lore raw

Re: [PATCH 1/4] dt-bindings: clock: Add AST2600 APLLdivN definition

Quoted 5 lines · Eddie James

Add a new clock definition for the "APLLdivN" as described in the AST2600 specification. This clock is simply the APLL divided by a factor defined in the SCU registers. It is the input to the FSI bus.

Ah, that's where the name comes from. Would calling it APLL_DIV make more sense?

Quoted 21 lines
---
 include/dt-bindings/clock/ast2600-clock.h | 1 +
 1 file changed, 1 insertion(+)
include/dt-bindings/clock/ast2600-clock.h +1 -1
index 62b9520a00fd..5a9ae0a1d574 100644
@@ -87,6 +87,7 @@
 #define ASPEED_CLK_MAC2RCLK            68
 #define ASPEED_CLK_MAC3RCLK            69
 #define ASPEED_CLK_MAC4RCLK            70
+#define ASPEED_CLK_APLLN               71
 
 /* Only list resets here that are not part of a gate */
 #define ASPEED_RESET_ADC               55
--

2.26.2

author maintainer
#3 lore raw

PATCH 2/4 clk: ast2600: Add functionality to the APLL clock

In reply to Eddie James

Register a clock with it's own operations to describe the APLL on the AST2600. The clock is controlled by an SCU register containing a multiplier and divider of the 25MHz input clock.
The functionality to change the APLL is necessary to finely control the FSI bus frequency.

---
 drivers/clk/clk-ast2600.c | 177 +++++++++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 12 deletions(-)
drivers/clk/clk-ast2600.c +165 -12
index 177368cac6dd..a147dffbaccc 100644
@@ -4,6 +4,7 @@
 
 #define pr_fmt(fmt) "clk-ast2600: " fmt
 
+#include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
@@ -15,7 +16,7 @@
 
 #include "clk-aspeed.h"
 
-#define ASPEED_G6_NUM_CLKS		71
+#define ASPEED_G6_NUM_CLKS		72
 
 #define ASPEED_G6_SILICON_REV		0x004
 
@@ -31,6 +32,7 @@
 #define ASPEED_G6_CLK_SELECTION1	0x300
 #define ASPEED_G6_CLK_SELECTION2	0x304
 #define ASPEED_G6_CLK_SELECTION4	0x310
+#define ASPEED_G6_CLK_SELECTION5	0x314
 
 #define ASPEED_HPLL_PARAM		0x200
 #define ASPEED_APLL_PARAM		0x210
@@ -116,7 +118,7 @@ static const struct aspeed_gate_data aspeed_g6_gates[] = {
 	[ASPEED_CLK_GATE_UART11CLK]	= { 59,  -1, "uart11clk-gate",	"uartx", 0 },	/* UART11 */
 	[ASPEED_CLK_GATE_UART12CLK]	= { 60,  -1, "uart12clk-gate",	"uartx", 0 },	/* UART12 */
 	[ASPEED_CLK_GATE_UART13CLK]	= { 61,  -1, "uart13clk-gate",	"uartx", 0 },	/* UART13 */
-	[ASPEED_CLK_GATE_FSICLK]	= { 62,  59, "fsiclk-gate",	NULL,	 0 },	/* FSI */
+	[ASPEED_CLK_GATE_FSICLK]	= { 62,  59, "fsiclk-gate",	"aplln", CLK_SET_RATE_PARENT },	/* FSI */
 };
 
 static const struct clk_div_table ast2600_eclk_div_table[] = {
@@ -187,24 +189,166 @@ static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
 			mult, div);
 };
 
-static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
+/*
+ * APLL Frequency: F = 25MHz * (2 - od) * [(m + 2) / (n + 1)]
+ */
+static void ast2600_apll_get_params(unsigned int *div, unsigned int *mul)
 {
-	unsigned int mult, div;
+	u32 val = readl(scu_g6_base + ASPEED_APLL_PARAM);
 
 	if (val & BIT(20)) {
 		/* Pass through mode */
-		mult = div = 1;
+		*mul = *div = 1;
 	} else {
-		/* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
 		u32 m = (val >> 5) & 0x3f;
 		u32 od = (val >> 4) & 0x1;
 		u32 n = val & 0xf;
 
-		mult = (2 - od) * (m + 2);
-		div = n + 1;
+		*mul = (2 - od) * (m + 2);
+		*div = n + 1;
 	}
-	return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
-			mult, div);
+}
+
+static long ast2600_apll_best(unsigned long ul_rate, unsigned long ul_prate,
+			      unsigned int *out_div, unsigned int *out_mul,
+			      unsigned int *output_divider)
+{
+#define min_mult 2ULL
+#define max_mult 65ULL
+#define min_div 1ULL
+#define max_div 16ULL
+	int i;
+	unsigned int bod = 0;
+	unsigned long long rem = 1ULL;
+	unsigned long long brem = ~(0ULL);
+	unsigned long long bdiv = 1ULL;
+	unsigned long long tdiv;
+	unsigned long long bmul = 16ULL;
+	unsigned long long tmul;
+	long brate = -ERANGE;
+	unsigned long long trate;
+	unsigned long long rate = ul_rate;
+	unsigned long long prate = ul_prate;
+
+	for (i = 0; i < 2; ++i, prate *= 2ULL) {
+		for (tdiv = min_div; tdiv <= max_div; ++tdiv) {
+			tmul = DIV_ROUND_CLOSEST_ULL(rate * tdiv, prate);
+			if (tmul <  min_mult || tmul > max_mult)
+				continue;
+
+			trate = DIV_ROUND_CLOSEST_ULL(prate * tmul, tdiv);
+			if (trate > rate)
+				rem = trate - rate;
+			else
+				rem = rate - trate;
+
+			if (rem < brem) {
+				bod = !i;
+				brem = rem;
+				bdiv = tdiv;
+				bmul = tmul;
+				brate = (long)trate;
+			}
+
+			if (!rem)
+				break;
+		}
+
+		if (!rem)
+			break;
+	}
+
+	if (out_div)
+		*out_div = (unsigned int)bdiv;
+
+	if (out_mul)
+		*out_mul = (unsigned int)bmul;
+
+	if (output_divider)
+		*output_divider = bod;
+
+	return brate;
+#undef min_mult
+#undef max_mult
+#undef min_div
+#undef max_div
+}
+
+static unsigned long ast2600_apll_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	unsigned int div;
+	unsigned int mul;
+	unsigned long long rate;
+	unsigned long long prate = (unsigned long long)parent_rate;
+
+	ast2600_apll_get_params(&div, &mul);
+
+	rate = DIV_ROUND_CLOSEST_ULL(prate * (unsigned long long)mul, div);
+	return (unsigned long)rate;
+}
+
+static long ast2600_apll_round_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long *parent_rate)
+{
+	return ast2600_apll_best(rate, *parent_rate, NULL, NULL, NULL);
+}
+
+static int ast2600_apll_set_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long parent_rate)
+{
+	u32 val;
+	unsigned int od;
+	unsigned int div;
+	unsigned int mul;
+	long brate = ast2600_apll_best(rate, parent_rate, &div, &mul, &od);
+
+	if (brate < 0)
+		return brate;
+
+	val = readl(scu_g6_base + ASPEED_APLL_PARAM);
+	val &= ~0x7ff;
+	val |= (div - 1) & 0xf;
+	val |= ((mul - 2) & 0x3f) << 5;
+	if (od)
+		val |= 0x10;
+	writel(val, scu_g6_base + ASPEED_APLL_PARAM);
+
+	return 0;
+}
+
+static const struct clk_ops ast2600_apll_ops = {
+	.recalc_rate = ast2600_apll_recalc_rate,
+	.round_rate = ast2600_apll_round_rate,
+	.set_rate = ast2600_apll_set_rate,
+};
+
+static struct clk_hw *ast2600_create_apll(void)
+{
+	int rc;
+	const char *parent = "clkin";
+	struct clk_init_data init = {
+		.name = "apll",
+		.ops = &ast2600_apll_ops,
+		.parent_names = &parent,
+		.parent_data = NULL,
+		.parent_hws = NULL,
+		.num_parents = 1,
+		.flags = 0,
+	};
+	struct clk_hw *clk = kzalloc(sizeof(*clk), GFP_KERNEL);
+
+	if (!clk)
+		return ERR_PTR(-ENOMEM);
+
+	clk->init = &init;
+	rc = of_clk_hw_register(NULL, clk);
+	if (rc) {
+		kfree(clk);
+		clk = ERR_PTR(rc);
+	}
+
+	return clk;
 };
 
 static u32 get_bit(u8 idx)
@@ -630,6 +774,16 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(hw);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_ECLK] = hw;
 
+	hw = clk_hw_register_divider_table(dev, "aplln", "apll",
+					   CLK_SET_RATE_PARENT,
+					   scu_g6_base + ASPEED_G6_CLK_SELECTION5,
+					   28, 3, CLK_DIVIDER_READ_ONLY,
+					   ast2600_eclk_div_table,
+					   &aspeed_g6_clk_lock);
+	if (IS_ERR(hw))
+		return PTR_ERR(hw);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_APLLN] = hw;
+
 	for (i = 0; i < ARRAY_SIZE(aspeed_g6_gates); i++) {
 		const struct aspeed_gate_data *gd = &aspeed_g6_gates[i];
 		u32 gate_flags;
@@ -710,8 +864,7 @@ static void __init aspeed_g6_cc(struct regmap *map)
 	regmap_read(map, ASPEED_EPLL_PARAM, &val);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_EPLL] = ast2600_calc_pll("epll", val);
 
-	regmap_read(map, ASPEED_APLL_PARAM, &val);
-	aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] = ast2600_calc_apll("apll", val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] = ast2600_create_apll();
 
 	/* Strap bits 12:11 define the AXI/AHB clock frequency ratio (aka HCLK)*/
 	regmap_read(map, ASPEED_G6_STRAP1, &val);
Signature · 1 line
2.26.2
maintainer
#4 lore raw

Re: [PATCH 2/4] clk: ast2600: Add functionality to the APLL clock

Ryan,

This change adds support for setting the A-PLL in the 2600 so we can control the FSI frequency. Can your team please review it? If it is okay please reply with your Reviewed-by.

Quoted 42 lines · Eddie James

Register a clock with it's own operations to describe the APLL on the AST2600. The clock is controlled by an SCU register containing a multiplier and divider of the 25MHz input clock.
The functionality to change the APLL is necessary to finely control the FSI bus frequency.

---
 drivers/clk/clk-ast2600.c | 177 +++++++++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 12 deletions(-)
drivers/clk/clk-ast2600.c +3 -1
index 177368cac6dd..a147dffbaccc 100644
@@ -4,6 +4,7 @@
 
 #define pr_fmt(fmt) "clk-ast2600: " fmt
 
+#include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
@@ -15,7 +16,7 @@
 
 #include "clk-aspeed.h"
 
-#define ASPEED_G6_NUM_CLKS             71
+#define ASPEED_G6_NUM_CLKS             72
 
 #define ASPEED_G6_SILICON_REV          0x004
 
@@ -31,6 +32,7 @@
 #define ASPEED_G6_CLK_SELECTION1       0x300
 #define ASPEED_G6_CLK_SELECTION2       0x304
 #define ASPEED_G6_CLK_SELECTION4       0x310
+#define ASPEED_G6_CLK_SELECTION5       0x314
 
 #define ASPEED_HPLL_PARAM              0x200
 #define ASPEED_APLL_PARAM              0x210

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data aspeed_g6_gates[] = { · line 121

Quoted 5 lines
drivers/clk/clk-ast2600.c +1 -1
@@ -116,7 +118,7 @@ static const struct aspeed_gate_data aspeed_g6_gates[] = {
        [ASPEED_CLK_GATE_UART11CLK]     = { 59,  -1, "uart11clk-gate",  "uartx", 0 },   /* UART11 */
        [ASPEED_CLK_GATE_UART12CLK]     = { 60,  -1, "uart12clk-gate",  "uartx", 0 },   /* UART12 */
        [ASPEED_CLK_GATE_UART13CLK]     = { 61,  -1, "uart13clk-gate",  "uartx", 0 },   /* UART13 */
-       [ASPEED_CLK_GATE_FSICLK]        = { 62,  59, "fsiclk-gate",     NULL,    0 },   /* FSI */
+       [ASPEED_CLK_GATE_FSICLK]        = { 62,  59, "fsiclk-gate",     "aplln", CLK_SET_RATE_PARENT }, /* FSI */

Why do we call this apll*n* ?

I believe the apll is also the parent of the sdclk.

Designs that use FSI do not use the sdclk, but it should be added to the table for completeness.

Quoted 29 lines
drivers/clk/clk-ast2600.c +7 -6
 };
 
 static const struct clk_div_table ast2600_eclk_div_table[] = {
@@ -187,24 +189,166 @@ static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
                        mult, div);
 };
 
-static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
+/*
+ * APLL Frequency: F = 25MHz * (2 - od) * [(m + 2) / (n + 1)]
+ */
+static void ast2600_apll_get_params(unsigned int *div, unsigned int *mul)
 {
-       unsigned int mult, div;
+       u32 val = readl(scu_g6_base + ASPEED_APLL_PARAM);
 
        if (val & BIT(20)) {
                /* Pass through mode */
-               mult = div = 1;
+               *mul = *div = 1;
        } else {
-               /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
                u32 m = (val >> 5) & 0x3f;
                u32 od = (val >> 4) & 0x1;
                u32 n = val & 0xf;
 
-               mult = (2 - od) * (m + 2);
-               div = n + 1;
+               *mul = (2 - od) * (m + 2);

drivers/clk/clk-ast2600.c static struct clk_hw *ast2600_calc_pll(const char *name, u32 val) · line 215

Quoted 10 lines
drivers/clk/clk-ast2600.c +7 -2
+               *div = n + 1;
        }
-       return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
-                       mult, div);
+}
+
+static long ast2600_apll_best(unsigned long ul_rate, unsigned long ul_prate,
+                             unsigned int *out_div, unsigned int *out_mul,
+                             unsigned int *output_divider)
+{

I look to our clk maintainers for review of this bit. Eddie has done a good job, but if there's a better way to do this then please let us know.

In practice we will run the APLL at one of two rates, so doing a search during every boot seems unnecessary to me.

Quoted 169 lines
drivers/clk/clk-ast2600.c +147 -3
+#define min_mult 2ULL
+#define max_mult 65ULL
+#define min_div 1ULL
+#define max_div 16ULL
+       int i;
+       unsigned int bod = 0;
+       unsigned long long rem = 1ULL;
+       unsigned long long brem = ~(0ULL);
+       unsigned long long bdiv = 1ULL;
+       unsigned long long tdiv;
+       unsigned long long bmul = 16ULL;
+       unsigned long long tmul;
+       long brate = -ERANGE;
+       unsigned long long trate;
+       unsigned long long rate = ul_rate;
+       unsigned long long prate = ul_prate;
+
+       for (i = 0; i < 2; ++i, prate *= 2ULL) {
+               for (tdiv = min_div; tdiv <= max_div; ++tdiv) {
+                       tmul = DIV_ROUND_CLOSEST_ULL(rate * tdiv, prate);
+                       if (tmul <  min_mult || tmul > max_mult)
+                               continue;
+
+                       trate = DIV_ROUND_CLOSEST_ULL(prate * tmul, tdiv);
+                       if (trate > rate)
+                               rem = trate - rate;
+                       else
+                               rem = rate - trate;
+
+                       if (rem < brem) {
+                               bod = !i;
+                               brem = rem;
+                               bdiv = tdiv;
+                               bmul = tmul;
+                               brate = (long)trate;
+                       }
+
+                       if (!rem)
+                               break;
+               }
+
+               if (!rem)
+                       break;
+       }
+
+       if (out_div)
+               *out_div = (unsigned int)bdiv;
+
+       if (out_mul)
+               *out_mul = (unsigned int)bmul;
+
+       if (output_divider)
+               *output_divider = bod;
+
+       return brate;
+#undef min_mult
+#undef max_mult
+#undef min_div
+#undef max_div
+}
+
+static unsigned long ast2600_apll_recalc_rate(struct clk_hw *hw,
+                                             unsigned long parent_rate)
+{
+       unsigned int div;
+       unsigned int mul;
+       unsigned long long rate;
+       unsigned long long prate = (unsigned long long)parent_rate;
+
+       ast2600_apll_get_params(&div, &mul);
+
+       rate = DIV_ROUND_CLOSEST_ULL(prate * (unsigned long long)mul, div);
+       return (unsigned long)rate;
+}
+
+static long ast2600_apll_round_rate(struct clk_hw *hw, unsigned long rate,
+                                   unsigned long *parent_rate)
+{
+       return ast2600_apll_best(rate, *parent_rate, NULL, NULL, NULL);
+}
+
+static int ast2600_apll_set_rate(struct clk_hw *hw, unsigned long rate,
+                                unsigned long parent_rate)
+{
+       u32 val;
+       unsigned int od;
+       unsigned int div;
+       unsigned int mul;
+       long brate = ast2600_apll_best(rate, parent_rate, &div, &mul, &od);
+
+       if (brate < 0)
+               return brate;
+
+       val = readl(scu_g6_base + ASPEED_APLL_PARAM);
+       val &= ~0x7ff;
+       val |= (div - 1) & 0xf;
+       val |= ((mul - 2) & 0x3f) << 5;
+       if (od)
+               val |= 0x10;
+       writel(val, scu_g6_base + ASPEED_APLL_PARAM);
+
+       return 0;
+}
+
+static const struct clk_ops ast2600_apll_ops = {
+       .recalc_rate = ast2600_apll_recalc_rate,
+       .round_rate = ast2600_apll_round_rate,
+       .set_rate = ast2600_apll_set_rate,
+};
+
+static struct clk_hw *ast2600_create_apll(void)
+{
+       int rc;
+       const char *parent = "clkin";
+       struct clk_init_data init = {
+               .name = "apll",
+               .ops = &ast2600_apll_ops,
+               .parent_names = &parent,
+               .parent_data = NULL,
+               .parent_hws = NULL,
+               .num_parents = 1,
+               .flags = 0,
+       };
+       struct clk_hw *clk = kzalloc(sizeof(*clk), GFP_KERNEL);
+
+       if (!clk)
+               return ERR_PTR(-ENOMEM);
+
+       clk->init = &init;
+       rc = of_clk_hw_register(NULL, clk);
+       if (rc) {
+               kfree(clk);
+               clk = ERR_PTR(rc);
+       }
+
+       return clk;
 };
 
 static u32 get_bit(u8 idx)
@@ -630,6 +774,16 @@ static int aspeed_g6_clk_probe(struct platform_device *pdev)
                return PTR_ERR(hw);
        aspeed_g6_clk_data->hws[ASPEED_CLK_ECLK] = hw;
 
+       hw = clk_hw_register_divider_table(dev, "aplln", "apll",
+                                          CLK_SET_RATE_PARENT,
+                                          scu_g6_base + ASPEED_G6_CLK_SELECTION5,
+                                          28, 3, CLK_DIVIDER_READ_ONLY,
+                                          ast2600_eclk_div_table,
+                                          &aspeed_g6_clk_lock);
+       if (IS_ERR(hw))
+               return PTR_ERR(hw);
+       aspeed_g6_clk_data->hws[ASPEED_CLK_APLLN] = hw;
+
        for (i = 0; i < ARRAY_SIZE(aspeed_g6_gates); i++) {
                const struct aspeed_gate_data *gd = &aspeed_g6_gates[i];
                u32 gate_flags;
@@ -710,8 +864,7 @@ static void __init aspeed_g6_cc(struct regmap *map)
        regmap_read(map, ASPEED_EPLL_PARAM, &val);
        aspeed_g6_clk_data->hws[ASPEED_CLK_EPLL] = ast2600_calc_pll("epll", val);
 
-       regmap_read(map, ASPEED_APLL_PARAM, &val);
-       aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] = ast2600_calc_apll("apll", val);
+       aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] = ast2600_create_apll();
 
        /* Strap bits 12:11 define the AXI/AHB clock frequency ratio (aka HCLK)*/
        regmap_read(map, ASPEED_G6_STRAP1, &val);
--

2.26.2

reviewer
#5 lore raw

RE: [PATCH 2/4] clk: ast2600: Add functionality to the APLL clock

Hello Eddie,
For FSI design is request for two clock source.
one for FSI core, one for FSI bus clock.
In AST2600 FSI core clk is apll, FSI bus clock need apll/4.
So, you should fix for fsi clock input. Not set from fsi-master-driver.
And fsi-master driver div from clock source apll/4. Should not set for apll from fsi-master driver.

Quoted 310 lines

-----Original Message-----
From: Joel Stanley <[email protected]>
Sent: Friday, September 11, 2020 9:30 AM
To: Eddie James <[email protected]>; Ryan Chen <[email protected]>; Billy Tsai <[email protected]>
Cc: linux-clk at vger.kernel.org; Linux Kernel Mailing List <[email protected]>; devicetree <[email protected]>; linux-aspeed <[email protected]>; linux-fsi at lists.ozlabs.org; Andrew Jeffery <[email protected]>; Rob Herring <[email protected]>; Stephen Boyd <[email protected]>; Michael Turquette <[email protected]>; Alistair Popple <[email protected]>; Jeremy Kerr <[email protected]>
Subject: Re: [PATCH 2/4] clk: ast2600: Add functionality to the APLL clock

Ryan,

This change adds support for setting the A-PLL in the 2600 so we can control the FSI frequency. Can your team please review it? If it is okay please reply with your Reviewed-by.

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data

Quoted 43 lines · Eddie James

Register a clock with it's own operations to describe the APLL on the AST2600. The clock is controlled by an SCU register containing a multiplier and divider of the 25MHz input clock.
The functionality to change the APLL is necessary to finely control the FSI bus frequency.

---
 drivers/clk/clk-ast2600.c | 177
+++++++++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 12 deletions(-)
drivers/clk/clk-ast2600.c +3 -1
index 177368cac6dd..a147dffbaccc 100644
@@ -4,6 +4,7 @@
 
 #define pr_fmt(fmt) "clk-ast2600: " fmt
 
+#include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
@@ -15,7 +16,7 @@
 
 #include "clk-aspeed.h"
 
-#define ASPEED_G6_NUM_CLKS             71
+#define ASPEED_G6_NUM_CLKS             72
 
 #define ASPEED_G6_SILICON_REV          0x004
 
@@ -31,6 +32,7 @@
 #define ASPEED_G6_CLK_SELECTION1       0x300
 #define ASPEED_G6_CLK_SELECTION2       0x304
 #define ASPEED_G6_CLK_SELECTION4       0x310
+#define ASPEED_G6_CLK_SELECTION5       0x314
 
 #define ASPEED_HPLL_PARAM              0x200
 #define ASPEED_APLL_PARAM              0x210
@@ -116,7 +118,7 @@ static const struct aspeed_gate_data

aspeed_g6_gates[] = {

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data · line 118

Quoted 1 line
drivers/clk/clk-ast2600.c +0 -0
        [ASPEED_CLK_GATE_UART11CLK]     = { 59,  -1,

"uart11clk-gate", "uartx", 0 }, /* UART11 */

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data · line 119

Quoted 1 line
drivers/clk/clk-ast2600.c +0 -0
        [ASPEED_CLK_GATE_UART12CLK]     = { 60,  -1,

"uart12clk-gate", "uartx", 0 }, /* UART12 */

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data · line 120

Quoted 1 line
drivers/clk/clk-ast2600.c +0 -0
        [ASPEED_CLK_GATE_UART13CLK]     = { 61,  -1,

"uart13clk-gate", "uartx", 0 }, /* UART13 */

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data · line 119

Quoted 1 line
drivers/clk/clk-ast2600.c +0 -1
-       [ASPEED_CLK_GATE_FSICLK]        = { 62,  59, "fsiclk-gate",

NULL, 0 }, /* FSI */

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data · line 121

Quoted 1 line
drivers/clk/clk-ast2600.c +1 -0
+       [ASPEED_CLK_GATE_FSICLK]        = { 62,  59, "fsiclk-gate",

"aplln", CLK_SET_RATE_PARENT }, /* FSI */

Why do we call this apll*n* ?

I believe the apll is also the parent of the sdclk.

Designs that use FSI do not use the sdclk, but it should be added to the table for completeness.

drivers/clk/clk-ast2600.c static const struct aspeed_gate_data

Quoted 4 lines
drivers/clk/clk-ast2600.c +0 -1
 };
 
 static const struct clk_div_table ast2600_eclk_div_table[] = { @@
-187,24 +189,166 @@ static struct clk_hw *ast2600_calc_pll(const char

*name, u32 val)

Quoted 22 lines
                        mult, div);

};

drivers/clk/clk-ast2600.c +6 -4
-static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
+/*
+ * APLL Frequency: F = 25MHz * (2 - od) * [(m + 2) / (n + 1)]  */
+static void ast2600_apll_get_params(unsigned int *div, unsigned int
+*mul)
 {
-       unsigned int mult, div;
+       u32 val = readl(scu_g6_base + ASPEED_APLL_PARAM);
 
        if (val & BIT(20)) {
                /* Pass through mode */
-               mult = div = 1;
+               *mul = *div = 1;
        } else {
-               /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
                u32 m = (val >> 5) & 0x3f;
                u32 od = (val >> 4) & 0x1;
                u32 n = val & 0xf;
 

drivers/clk/clk-ast2600.c static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)

Quoted 10 lines
drivers/clk/clk-ast2600.c +5 -4
-               mult = (2 - od) * (m + 2);
-               div = n + 1;
+               *mul = (2 - od) * (m + 2);
+               *div = n + 1;
        }
-       return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
-                       mult, div);
+}
+
+static long ast2600_apll_best(unsigned long ul_rate, unsigned long

ul_prate,

Quoted 1 line

+ unsigned int *out_div, unsigned int

*out_mul,

Quoted 1 line

+ unsigned int *output_divider) {

I look to our clk maintainers for review of this bit. Eddie has done a good job, but if there's a better way to do this then please let us know.

In practice we will run the APLL at one of two rates, so doing a search during every boot seems unnecessary to me.

Quoted 10 lines
drivers/clk/clk-ast2600.c +10 -0
+#define min_mult 2ULL
+#define max_mult 65ULL
+#define min_div 1ULL
+#define max_div 16ULL
+       int i;
+       unsigned int bod = 0;
+       unsigned long long rem = 1ULL;
+       unsigned long long brem = ~(0ULL);
+       unsigned long long bdiv = 1ULL;
+       unsigned long long tdiv;

drivers/clk/clk-ast2600.c static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)

Quoted 10 lines
drivers/clk/clk-ast2600.c +10 -0
+       unsigned long long bmul = 16ULL;
+       unsigned long long tmul;
+       long brate = -ERANGE;
+       unsigned long long trate;
+       unsigned long long rate = ul_rate;
+       unsigned long long prate = ul_prate;
+
+       for (i = 0; i < 2; ++i, prate *= 2ULL) {
+               for (tdiv = min_div; tdiv <= max_div; ++tdiv) {
+                       tmul = DIV_ROUND_CLOSEST_ULL(rate * tdiv,

prate);

drivers/clk/clk-ast2600.c static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)

Quoted 4 lines
drivers/clk/clk-ast2600.c +4 -0
+                       if (tmul <  min_mult || tmul > max_mult)
+                               continue;
+
+                       trate = DIV_ROUND_CLOSEST_ULL(prate * tmul,

tdiv);

Quoted 38 lines
drivers/clk/clk-ast2600.c +38 -0
+                       if (trate > rate)
+                               rem = trate - rate;
+                       else
+                               rem = rate - trate;
+
+                       if (rem < brem) {
+                               bod = !i;
+                               brem = rem;
+                               bdiv = tdiv;
+                               bmul = tmul;
+                               brate = (long)trate;
+                       }
+
+                       if (!rem)
+                               break;
+               }
+
+               if (!rem)
+                       break;
+       }
+
+       if (out_div)
+               *out_div = (unsigned int)bdiv;
+
+       if (out_mul)
+               *out_mul = (unsigned int)bmul;
+
+       if (output_divider)
+               *output_divider = bod;
+
+       return brate;
+#undef min_mult
+#undef max_mult
+#undef min_div
+#undef max_div
+}
+
+static unsigned long ast2600_apll_recalc_rate(struct clk_hw *hw,

drivers/clk/clk-ast2600.c static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)

Quoted 10 lines
drivers/clk/clk-ast2600.c +10 -0
+                                             unsigned long
+parent_rate) {
+       unsigned int div;
+       unsigned int mul;
+       unsigned long long rate;
+       unsigned long long prate = (unsigned long long)parent_rate;
+
+       ast2600_apll_get_params(&div, &mul);
+
+       rate = DIV_ROUND_CLOSEST_ULL(prate * (unsigned long long)mul,

div);

drivers/clk/clk-ast2600.c static int aspeed_g6_clk_probe(struct

Quoted 65 lines
drivers/clk/clk-ast2600.c +61 -0
+       return (unsigned long)rate;
+}
+
+static long ast2600_apll_round_rate(struct clk_hw *hw, unsigned long rate,
+                                   unsigned long *parent_rate) {
+       return ast2600_apll_best(rate, *parent_rate, NULL, NULL,
+NULL); }
+
+static int ast2600_apll_set_rate(struct clk_hw *hw, unsigned long rate,
+                                unsigned long parent_rate) {
+       u32 val;
+       unsigned int od;
+       unsigned int div;
+       unsigned int mul;
+       long brate = ast2600_apll_best(rate, parent_rate, &div, &mul,
+&od);
+
+       if (brate < 0)
+               return brate;
+
+       val = readl(scu_g6_base + ASPEED_APLL_PARAM);
+       val &= ~0x7ff;
+       val |= (div - 1) & 0xf;
+       val |= ((mul - 2) & 0x3f) << 5;
+       if (od)
+               val |= 0x10;
+       writel(val, scu_g6_base + ASPEED_APLL_PARAM);
+
+       return 0;
+}
+
+static const struct clk_ops ast2600_apll_ops = {
+       .recalc_rate = ast2600_apll_recalc_rate,
+       .round_rate = ast2600_apll_round_rate,
+       .set_rate = ast2600_apll_set_rate, };
+
+static struct clk_hw *ast2600_create_apll(void) {
+       int rc;
+       const char *parent = "clkin";
+       struct clk_init_data init = {
+               .name = "apll",
+               .ops = &ast2600_apll_ops,
+               .parent_names = &parent,
+               .parent_data = NULL,
+               .parent_hws = NULL,
+               .num_parents = 1,
+               .flags = 0,
+       };
+       struct clk_hw *clk = kzalloc(sizeof(*clk), GFP_KERNEL);
+
+       if (!clk)
+               return ERR_PTR(-ENOMEM);
+
+       clk->init = &init;
+       rc = of_clk_hw_register(NULL, clk);
+       if (rc) {
+               kfree(clk);
+               clk = ERR_PTR(rc);
+       }
+
+       return clk;
 };
 
 static u32 get_bit(u8 idx)
@@ -630,6 +774,16 @@ static int aspeed_g6_clk_probe(struct

platform_device *pdev)

drivers/clk/clk-ast2600.c static int aspeed_g6_clk_probe(struct platform_device *pdev)

Quoted 6 lines
drivers/clk/clk-ast2600.c +3 -0
                return PTR_ERR(hw);
        aspeed_g6_clk_data->hws[ASPEED_CLK_ECLK] = hw;
 
+       hw = clk_hw_register_divider_table(dev, "aplln", "apll",
+                                          CLK_SET_RATE_PARENT,
+                                          scu_g6_base +

ASPEED_G6_CLK_SELECTION5,

drivers/clk/clk-ast2600.c static int aspeed_g6_clk_probe(struct · line 780

Quoted 1 line
drivers/clk/clk-ast2600.c +1 -0
+                                          28, 3,

CLK_DIVIDER_READ_ONLY,

drivers/clk/clk-ast2600.c static int aspeed_g6_clk_probe(struct platform_device *pdev)

Quoted 8 lines
drivers/clk/clk-ast2600.c +6 -0
+                                          ast2600_eclk_div_table,
+                                          &aspeed_g6_clk_lock);
+       if (IS_ERR(hw))
+               return PTR_ERR(hw);
+       aspeed_g6_clk_data->hws[ASPEED_CLK_APLLN] = hw;
+
        for (i = 0; i < ARRAY_SIZE(aspeed_g6_gates); i++) {
                const struct aspeed_gate_data *gd =

&aspeed_g6_gates[i];

drivers/clk/clk-ast2600.c static void __init aspeed_g6_cc(struct regmap

Quoted 2 lines
drivers/clk/clk-ast2600.c +0 -0
                u32 gate_flags;
@@ -710,8 +864,7 @@ static void __init aspeed_g6_cc(struct regmap

*map)

drivers/clk/clk-ast2600.c static void __init aspeed_g6_cc(struct regmap *map)

Quoted 6 lines
drivers/clk/clk-ast2600.c +0 -2
        regmap_read(map, ASPEED_EPLL_PARAM, &val);
        aspeed_g6_clk_data->hws[ASPEED_CLK_EPLL] =
 ast2600_calc_pll("epll", val);
 
-       regmap_read(map, ASPEED_APLL_PARAM, &val);
-       aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] =

ast2600_calc_apll("apll", val);

drivers/clk/clk-ast2600.c static void __init aspeed_g6_cc(struct regmap

Quoted 4 lines
drivers/clk/clk-ast2600.c +2 -0
+       aspeed_g6_clk_data->hws[ASPEED_CLK_APLL] =
+ ast2600_create_apll();
 
        /* Strap bits 12:11 define the AXI/AHB clock frequency ratio (aka

HCLK)*/

Quoted 4 lines
        regmap_read(map, ASPEED_G6_STRAP1, &val);

--
2.26.2

author maintainer
#6 lore raw

PATCH 3/4 dt-bindings: fsi: Aspeed master: Add bus-frequency property

In reply to Eddie James

Document the bus-frequency property.

---
 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt +1 -0
index a513e65ec0c9..d84bd19526ca 100644
@@ -17,6 +17,7 @@ Optional properties:
 
  - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled)
  - fsi-mux-gpios: GPIO for detecting the desired FSI mux state
+ - bus-frequency: the frequency of the FSI bus
 
 
 Examples:
Signature · 1 line
2.26.2
maintainer Reviewed-by on v1 3/4
#7 lore raw

Re: [PATCH 3/4] dt-bindings: fsi: Aspeed master: Add bus-frequency property

Quoted 4 lines · Eddie James

Document the bus-frequency property.

I think this is good terminology, and it's consistent with similar protocols such as i2c.

Quoted 19 lines
---
 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt +1 -1
index a513e65ec0c9..d84bd19526ca 100644
@@ -17,6 +17,7 @@ Optional properties:
 
  - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled)
  - fsi-mux-gpios: GPIO for detecting the desired FSI mux state
+ - bus-frequency: the frequency of the FSI bus
 
 
 Examples:
--

2.26.2

author maintainer
#9 lore raw

PATCH 4/4 fsi: Aspeed master: Set bus frequency from devicetree

In reply to Eddie James

Set the FSI bus frequency based on the value encoded in the devicetree. The default value is 166MHz, which is the highest frequency some FSI slaves can accept.

---
 drivers/fsi/fsi-master-aspeed.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/fsi/fsi-master-aspeed.c +5 -0
index c006ec008a1a..be19fee0bece 100644
@@ -515,6 +515,7 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
 	struct fsi_master_aspeed *aspeed;
 	struct resource *res;
 	int rc, links, reg;
+	u32 bus_freq = 166666666;
 	__be32 raw;
 
 	rc = tacoma_cabled_fsi_fixup(&pdev->dev);
@@ -539,6 +540,10 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev)
 		dev_err(aspeed->dev, "couldn't get clock\n");
 		return PTR_ERR(aspeed->clk);
 	}
+
+	of_property_read_u32(pdev->dev.of_node, "bus-frequency", &bus_freq);
+	clk_set_rate(aspeed->clk, bus_freq);
+
 	rc = clk_prepare_enable(aspeed->clk);
 	if (rc) {
 		dev_err(aspeed->dev, "couldn't enable clock\n");
Signature · 1 line
2.26.2