From: Joel Stanley <joel@jms.id.au>
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
Date: Fri, 11 Sep 2020 01:29:40 +0000 [thread overview]
Message-ID: <CACPK8XeMcf3B+5gt-d=+4dbNindiAHA4XjVdtOOcpU3Fr0191Q@mail.gmail.com> (raw)
In-Reply-To: <20200910151840.25333-3-eajames@linux.ibm.com>
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
>
next prev parent reply other threads:[~2020-09-11 1:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-10 15:18 [PATCH 0/4] clk: AST2600 and FSI: Add APLL to control FSI bus frequency Eddie James
2020-09-10 15:18 ` [PATCH 1/4] dt-bindings: clock: Add AST2600 APLLdivN definition Eddie James
2020-09-11 1:33 ` Joel Stanley
2020-09-10 15:18 ` [PATCH 2/4] clk: ast2600: Add functionality to the APLL clock Eddie James
2020-09-11 1:29 ` Joel Stanley [this message]
2020-09-11 9:11 ` Ryan Chen
2020-09-10 15:18 ` [PATCH 3/4] dt-bindings: fsi: Aspeed master: Add bus-frequency property Eddie James
2020-09-11 1:16 ` Joel Stanley
2020-09-22 22:19 ` Rob Herring
2020-09-10 15:18 ` [PATCH 4/4] fsi: Aspeed master: Set bus frequency from devicetree Eddie James
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACPK8XeMcf3B+5gt-d=+4dbNindiAHA4XjVdtOOcpU3Fr0191Q@mail.gmail.com' \
--to=joel@jms.id.au \
--cc=alistair@popple.id.au \
--cc=andrew@aj.id.au \
--cc=billy_tsai@aspeedtech.com \
--cc=devicetree@vger.kernel.org \
--cc=eajames@linux.ibm.com \
--cc=jk@ozlabs.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-fsi@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=ryan_chen@aspeedtech.com \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).