[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1jy1oab06v.fsf@starbuckisacylon.baylibre.com>
Date: Mon, 06 Mar 2023 12:09:35 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Dmitry Rokosov <ddrokosov@...rdevices.ru>,
neil.armstrong@...aro.org, mturquette@...libre.com,
sboyd@...nel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, khilman@...libre.com,
martin.blumenstingl@...glemail.com
Cc: jian.hu@...ogic.com, kernel@...rdevices.ru, rockosov@...il.com,
linux-amlogic@...ts.infradead.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v9 1/5] clk: meson: add support for A1 PLL clock ops
On Wed 01 Mar 2023 at 21:37, Dmitry Rokosov <ddrokosov@...rdevices.ru> wrote:
> From: Jian Hu <jian.hu@...ogic.com>
>
> Modern meson PLL IPs are a little bit different from early known PLLs.
> The main difference is located in the init/enable/disable sequences; the
> rate logic is the same.
For the record, I find very odd that PLLs used to have an 'rst' bit in
CTRL0:29 (see g12 for example), this bit goes un-documented in the a1
datasheet, and following SoCs like s4 still have a rst bit, still in
CTRL0:29
I would not be surpized if the rst is actually still there in the a1.
It is just my guess ...
> Compared with the previous SoCs, self-adaption current module
> is newly added for A1, and there is no reset parameter except the
> fixed pll. In A1 PLL, the PLL enable sequence is different, using
> the new power-on sequence to enable the PLL.
Please split this patch:
#1 make the rst optional (if you must)
#2 add the self current adapt param.
Apart from this, it looks good
>
> Signed-off-by: Jian Hu <jian.hu@...ogic.com>
> Acked-by: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
> Signed-off-by: Dmitry Rokosov <ddrokosov@...rdevices.ru>
> ---
> drivers/clk/meson/clk-pll.c | 47 +++++++++++++++++++++++++++++++------
> drivers/clk/meson/clk-pll.h | 2 ++
> 2 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index afefeba6e458..56ec2210f1ad 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -295,10 +295,14 @@ static int meson_clk_pll_init(struct clk_hw *hw)
> struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>
> if (pll->init_count) {
> - meson_parm_write(clk->map, &pll->rst, 1);
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 1);
> +
> regmap_multi_reg_write(clk->map, pll->init_regs,
> pll->init_count);
> - meson_parm_write(clk->map, &pll->rst, 0);
> +
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 0);
> }
>
> return 0;
> @@ -309,8 +313,11 @@ static int meson_clk_pll_is_enabled(struct clk_hw *hw)
> struct clk_regmap *clk = to_clk_regmap(hw);
> struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>
> - if (meson_parm_read(clk->map, &pll->rst) ||
> - !meson_parm_read(clk->map, &pll->en) ||
> + if (MESON_PARM_APPLICABLE(&pll->rst) &&
> + meson_parm_read(clk->map, &pll->rst))
> + return 0;
> +
> + if (!meson_parm_read(clk->map, &pll->en) ||
> !meson_parm_read(clk->map, &pll->l))
> return 0;
>
> @@ -341,13 +348,34 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
> return 0;
>
> /* Make sure the pll is in reset */
> - meson_parm_write(clk->map, &pll->rst, 1);
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 1);
>
> /* Enable the pll */
> meson_parm_write(clk->map, &pll->en, 1);
>
> /* Take the pll out reset */
> - meson_parm_write(clk->map, &pll->rst, 0);
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 0);
> +
> + /*
> + * Compared with the previous SoCs, self-adaption current module
> + * is newly added for A1, keep the new power-on sequence to enable the
> + * PLL. The sequence is:
> + * 1. enable the pll, delay for 10us
> + * 2. enable the pll self-adaption current module, delay for 40us
> + * 3. enable the lock detect module
> + */
> + if (MESON_PARM_APPLICABLE(&pll->current_en)) {
> + usleep_range(10, 20);
> + meson_parm_write(clk->map, &pll->current_en, 1);
> + usleep_range(40, 50);
> + };
> +
> + if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
> + meson_parm_write(clk->map, &pll->l_detect, 1);
> + meson_parm_write(clk->map, &pll->l_detect, 0);
> + }
>
> if (meson_clk_pll_wait_lock(hw))
> return -EIO;
> @@ -361,10 +389,15 @@ static void meson_clk_pll_disable(struct clk_hw *hw)
> struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>
> /* Put the pll is in reset */
> - meson_parm_write(clk->map, &pll->rst, 1);
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 1);
>
> /* Disable the pll */
> meson_parm_write(clk->map, &pll->en, 0);
> +
> + /* Disable PLL internal self-adaption current module */
> + if (MESON_PARM_APPLICABLE(&pll->current_en))
> + meson_parm_write(clk->map, &pll->current_en, 0);
> }
>
> static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
> index 367efd0f6410..a2228c0fdce5 100644
> --- a/drivers/clk/meson/clk-pll.h
> +++ b/drivers/clk/meson/clk-pll.h
> @@ -36,6 +36,8 @@ struct meson_clk_pll_data {
> struct parm frac;
> struct parm l;
> struct parm rst;
> + struct parm current_en;
> + struct parm l_detect;
> const struct reg_sequence *init_regs;
> unsigned int init_count;
> const struct pll_params_table *table;
Powered by blists - more mailing lists