[PATCH 0/4] clk: AST2600 and FSI: Add APLL to control FSI bus frequency

[PATCH 0/4] clk: AST2600 and FSI: Add APLL to control FSI bus frequency

[Eddie James]

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(-)

-- 
2.26.2

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

[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.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 include/dt-bindings/clock/ast2600-clock.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/ast2600-clock.h b/include/dt-bindings/clock/ast2600-clock.h
index 62b9520a00fd..5a9ae0a1d574 100644
--- a/include/dt-bindings/clock/ast2600-clock.h
+++ b/include/dt-bindings/clock/ast2600-clock.h
@@ -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

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

[Joel Stanley]

On Thu, 10 Sep 2020 at 15:18, Eddie James <eajames@linux.ibm.com> wrote:
>
> 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?

>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  include/dt-bindings/clock/ast2600-clock.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/dt-bindings/clock/ast2600-clock.h b/include/dt-bindings/clock/ast2600-clock.h
> index 62b9520a00fd..5a9ae0a1d574 100644
> --- a/include/dt-bindings/clock/ast2600-clock.h
> +++ b/include/dt-bindings/clock/ast2600-clock.h
> @@ -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
>

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

[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.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/clk/clk-ast2600.c | 177 +++++++++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index 177368cac6dd..a147dffbaccc 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -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);
-- 
2.26.2

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

[Joel Stanley]

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.


On Thu, 10 Sep 2020 at 15:18, Eddie James <eajames@linux.ibm.com> wrote:
>
> 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.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/clk/clk-ast2600.c | 177 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 165 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index 177368cac6dd..a147dffbaccc 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -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 */

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.

>  };
>
>  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)
> +{

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.

> +#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
>

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

[Ryan Chen]

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.  

> -----Original Message-----
> From: Joel Stanley <joel@jms.id.au>
> Sent: Friday, September 11, 2020 9:30 AM
> To: Eddie James <eajames@linux.ibm.com>; Ryan Chen
> <ryan_chen@aspeedtech.com>; Billy Tsai <billy_tsai@aspeedtech.com>
> Cc: linux-clk@vger.kernel.org; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; devicetree <devicetree@vger.kernel.org>;
> linux-aspeed <linux-aspeed@lists.ozlabs.org>; linux-fsi@lists.ozlabs.org;
> Andrew Jeffery <andrew@aj.id.au>; Rob Herring <robh+dt@kernel.org>;
> Stephen Boyd <sboyd@kernel.org>; Michael Turquette
> <mturquette@baylibre.com>; Alistair Popple <alistair@popple.id.au>; Jeremy
> Kerr <jk@ozlabs.org>
> 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.
> 
> 
> On Thu, 10 Sep 2020 at 15:18, Eddie James <eajames@linux.ibm.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > ---
> >  drivers/clk/clk-ast2600.c | 177
> > +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 165 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> > index 177368cac6dd..a147dffbaccc 100644
> > --- a/drivers/clk/clk-ast2600.c
> > +++ b/drivers/clk/clk-ast2600.c
> > @@ -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 */
> 
> 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.
> 
> >  };
> >
> >  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) {
> 
> 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.
> 
> > +#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
> >

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

[Eddie James]

Document the bus-frequency property.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
index a513e65ec0c9..d84bd19526ca 100644
--- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
+++ b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
@@ -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

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

[Joel Stanley]

On Thu, 10 Sep 2020 at 15:18, Eddie James <eajames@linux.ibm.com> wrote:
>
> Document the bus-frequency property.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

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

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> index a513e65ec0c9..d84bd19526ca 100644
> --- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> +++ b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt
> @@ -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
>

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

[Rob Herring]

On Thu, 10 Sep 2020 10:18:39 -0500, Eddie James wrote:
> Document the bus-frequency property.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

[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.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 drivers/fsi/fsi-master-aspeed.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index c006ec008a1a..be19fee0bece 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -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");
-- 
2.26.2