From: Stephen Boyd <sboyd@kernel.org>
To: Claudiu Beznea <claudiu.beznea@microchip.com>,
alexandre.belloni@bootlin.com, ludovic.desroches@microchip.com,
mturquette@baylibre.com, nicolas.ferre@microchip.com,
robh+dt@kernel.org
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-clk@vger.kernel.org, eugen.hristev@microchip.com,
Claudiu Beznea <claudiu.beznea@microchip.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 06/11] clk: at91: clk-sam9x60-pll: allow runtime changes for pll
Date: Sat, 14 Nov 2020 13:14:59 -0800 [thread overview]
Message-ID: <160538849947.60232.12002724470272520124@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <1604655988-353-7-git-send-email-claudiu.beznea@microchip.com>
Quoting Claudiu Beznea (2020-11-06 01:46:23)
> diff --git a/drivers/clk/at91/clk-sam9x60-pll.c b/drivers/clk/at91/clk-sam9x60-pll.c
> index 78f458a7b2ef..6fe5d8530a0c 100644
> --- a/drivers/clk/at91/clk-sam9x60-pll.c
> +++ b/drivers/clk/at91/clk-sam9x60-pll.c
> @@ -225,8 +225,51 @@ static int sam9x60_frac_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long parent_rate)
> {
> struct sam9x60_pll_core *core = to_sam9x60_pll_core(hw);
> + struct sam9x60_frac *frac = to_sam9x60_frac(core);
> + struct regmap *regmap = core->regmap;
> + unsigned long irqflags, clkflags = clk_hw_get_flags(hw);
> + unsigned int val, cfrac, cmul;
> + long ret;
> +
> + ret = sam9x60_frac_pll_compute_mul_frac(core, rate, parent_rate, true);
> + if (ret <= 0 || (clkflags & CLK_SET_RATE_GATE))
Is this function being called when the clk is enabled and it has the
CLK_SET_RATE_GATE flag set? I'm confused why this driver needs to check
this flag.
> + return ret;
> +
> + spin_lock_irqsave(core->lock, irqflags);
> +
> + regmap_update_bits(regmap, AT91_PMC_PLL_UPDT, AT91_PMC_PLL_UPDT_ID_MSK,
> + core->id);
> + regmap_read(regmap, AT91_PMC_PLL_CTRL1, &val);
> + cmul = (val & core->layout->mul_mask) >> core->layout->mul_shift;
> + cfrac = (val & core->layout->frac_mask) >> core->layout->frac_shift;
> +
> + if (cmul == frac->mul && cfrac == frac->frac)
> + goto unlock;
> +
> + regmap_write(regmap, AT91_PMC_PLL_CTRL1,
> + (frac->mul << core->layout->mul_shift) |
> + (frac->frac << core->layout->frac_shift));
> +
> + regmap_update_bits(regmap, AT91_PMC_PLL_UPDT,
> + AT91_PMC_PLL_UPDT_UPDATE | AT91_PMC_PLL_UPDT_ID_MSK,
> + AT91_PMC_PLL_UPDT_UPDATE | core->id);
> +
> + regmap_update_bits(regmap, AT91_PMC_PLL_CTRL0,
> + AT91_PMC_PLL_CTRL0_ENLOCK | AT91_PMC_PLL_CTRL0_ENPLL,
> + AT91_PMC_PLL_CTRL0_ENLOCK |
> + AT91_PMC_PLL_CTRL0_ENPLL);
> +
> + regmap_update_bits(regmap, AT91_PMC_PLL_UPDT,
> + AT91_PMC_PLL_UPDT_UPDATE | AT91_PMC_PLL_UPDT_ID_MSK,
> + AT91_PMC_PLL_UPDT_UPDATE | core->id);
>
> - return sam9x60_frac_pll_compute_mul_frac(core, rate, parent_rate, true);
> + while (!sam9x60_pll_ready(regmap, core->id))
> + cpu_relax();
> +
> +unlock:
> + spin_unlock_irqrestore(core->lock, irqflags);
> +
> + return ret;
> }
>
> static const struct clk_ops sam9x60_frac_pll_ops = {
> @@ -378,9 +421,39 @@ static int sam9x60_div_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> {
> struct sam9x60_pll_core *core = to_sam9x60_pll_core(hw);
> struct sam9x60_div *div = to_sam9x60_div(core);
> + struct regmap *regmap = core->regmap;
> + unsigned long irqflags, clkflags = clk_hw_get_flags(hw);
> + unsigned int val, cdiv;
>
> div->div = DIV_ROUND_CLOSEST(parent_rate, rate) - 1;
>
> + if (clkflags & CLK_SET_RATE_GATE)
Same comment.
> diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
> index d685e22b2014..33faf7c6d9fb 100644
> --- a/drivers/clk/at91/sama7g5.c
> +++ b/drivers/clk/at91/sama7g5.c
> @@ -95,15 +95,15 @@ static const struct clk_pll_layout pll_layout_divio = {
> * @p: clock parent
> * @l: clock layout
> * @t: clock type
> - * @f: true if clock is critical and cannot be disabled
> + * @f: clock flags
> * @eid: export index in sama7g5->chws[] array
> */
> static const struct {
> const char *n;
> const char *p;
> const struct clk_pll_layout *l;
> + u32 f;
Why not unsigned long?
> u8 t;
> - u8 c;
> u8 eid;
> } sama7g5_plls[][PLL_ID_MAX] = {
> [PLL_ID_CPU] = {
> @@ -111,13 +111,13 @@ static const struct {
> .p = "mainck",
> .l = &pll_layout_frac,
> .t = PLL_TYPE_FRAC,
> - .c = 1, },
> + .f = CLK_IS_CRITICAL, },
>
> { .n = "cpupll_divpmcck",
> .p = "cpupll_fracck",
> .l = &pll_layout_divpmc,
> .t = PLL_TYPE_DIV,
> - .c = 1,
> + .f = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
> .eid = PMC_CPUPLL, },
> },
>
> @@ -126,13 +126,13 @@ static const struct {
> .p = "mainck",
> .l = &pll_layout_frac,
> .t = PLL_TYPE_FRAC,
> - .c = 1, },
> + .f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE, },
>
> { .n = "syspll_divpmcck",
> .p = "syspll_fracck",
> .l = &pll_layout_divpmc,
> .t = PLL_TYPE_DIV,
> - .c = 1,
> + .f = CLK_IS_CRITICAL | CLK_SET_RATE_GATE,
Please indicate why clks are critical. Whenever the CLK_IS_CRITICAL flag
is used we should have a comment indicating why.
> .eid = PMC_SYSPLL, },
> },
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-14 21:15 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-06 9:46 [PATCH v4 00/11] clk: at91: adapt for dvfs Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 01/11] clk: at91: sama7g5: fix compilation error Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 02/11] dt-bindings: clock: at91: add sama7g5 pll defines Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 03/11] clk: at91: sama7g5: allow SYS and CPU PLLs to be exported and referenced in DT Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 04/11] clk: at91: clk-master: add 5th divisor for mck master Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 05/11] clk: at91: sama7g5: add 5th divisor for mck0 layout and characteristics Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 06/11] clk: at91: clk-sam9x60-pll: allow runtime changes for pll Claudiu Beznea
2020-11-14 21:14 ` Stephen Boyd [this message]
2020-11-16 11:24 ` Claudiu.Beznea
2020-11-18 1:49 ` Stephen Boyd
2020-11-18 8:58 ` Claudiu.Beznea
2020-11-06 9:46 ` [PATCH v4 07/11] clk: at91: sama7g5: remove mck0 from parent list of other clocks Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 08/11] clk: at91: sama7g5: decrease lower limit for MCK0 rate Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 09/11] clk: at91: sama7g5: do not allow cpu pll to go higher than 1GHz Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 10/11] clk: at91: clk-master: re-factor master clock Claudiu Beznea
2020-11-06 9:46 ` [PATCH v4 11/11] clk: at91: sama7g5: register cpu clock Claudiu Beznea
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=160538849947.60232.12002724470272520124@swboyd.mtv.corp.google.com \
--to=sboyd@kernel.org \
--cc=alexandre.belloni@bootlin.com \
--cc=claudiu.beznea@microchip.com \
--cc=devicetree@vger.kernel.org \
--cc=eugen.hristev@microchip.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ludovic.desroches@microchip.com \
--cc=mturquette@baylibre.com \
--cc=nicolas.ferre@microchip.com \
--cc=robh+dt@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).