Received-SPF: pass (google.com: domain of linux-kernel+bounces-108036-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240301015118.30072-1-semen.protsenko@linaro.org>
In-Reply-To: <20240301015118.30072-1-semen.protsenko@linaro.org>
From: Sam Protsenko <semen.protsenko@linaro.org>
Date: Tue, 19 Mar 2024 13:47:03 -0500
Message-ID: <CAPLW+4=_yD3ShU5DvLWFyEzVrVHNVCsB+4bVkP+x_boRmC-vEw@mail.gmail.com>
Subject: Re: [PATCH v4 1/3] clk: samsung: Implement manual PLL control for
 ARM64 SoCs
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, 
	Alim Akhtar <alim.akhtar@samsung.com>, Michael Turquette <mturquette@baylibre.com>, 
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh@kernel.org>, Conor Dooley <conor+dt@kernel.org>, 
	Tomasz Figa <tomasz.figa@gmail.com>, linux-samsung-soc@vger.kernel.org, 
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org, 
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 29, 2024 at 7:51=E2=80=AFPM Sam Protsenko
<semen.protsenko@linaro.org> wrote:
>
> Some ARM64 Exynos chips are capable to control PLL clocks automatically.
> For those chips, whether the PLL is controlled automatically or manually
> is chosen in PLL_CON1 register with next bits:
>
>     [28]  ENABLE_AUTOMATIC_CLKGATING
>     [1]   MANUAL_PLL_CTRL
>     [0]   AUTO_PLL_CTRL
>
> The bl2 bootloader sets 0x10000001 value for some PLL_CON1 registers,
> which means any attempt to control those PLLs manually (e.g.
> disabling/enabling those PLLs or changing MUX parent clocks) would lead
> to PLL lock timeout with error message like this:
>
>     Could not lock PLL ...
>
> At the moment, all Samsung clock drivers implement manual clock control.
> So in order to make it possible to control PLLs, corresponding PLL_CON1
> registers should be set to 0x2 first.
>
> Some older ARM64 chips don't implement the automatic clock control
> though. It also might be desirable to configure some PLLs for manual
> control, while keeping the default configuration for the rest. So it'd
> convenient to choose this PLL mode for each CMU separately. Introduce
> .manual_plls field to CMU structure to choose the PLL control mode.
> Because it'll be initialized with "false" in all existing CMU
> structures by default, it won't affect any existing clock drivers,
> allowing for this feature to be enabled gradually when it's needed with
> no change for the rest of users. In case .manual_plls is set, set
> PLL_CON1 registers to manual control, akin to what's already done for
> gate clocks in exynos_arm64_init_clocks(). Of course, PLL_CON1 registers
> should be added to corresponding struct samsung_cmu_info::clk_regs array
> to make sure they get initialized.
>
> No functional change. This patch adds a feature, but doesn't enable it
> for any users.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---

Hi Krzysztof,

If it looks ok to you, can you please apply this series?

    [PATCH 1/3] clk: samsung: Implement manual PLL control for ARM64 SoCs
    [PATCH 2/3] clk: samsung: exynos850: Add CMU_CPUCL0 and CMU_CPUCL1
    [PATCH 3/3] arm64: dts: exynos: Add CPU clocks for Exynos850

That concludes my efforts on CPU clock enablement in Exynos850.

Thanks!

> Changes in v4:
>   - Turned register checking macros into static functions
>
> Changes in v3:
>   - none
>
> Changes in v2:
>   - none
>
>  drivers/clk/samsung/clk-exynos-arm64.c | 56 +++++++++++++++++++-------
>  drivers/clk/samsung/clk.h              |  4 ++
>  2 files changed, 45 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung=
/clk-exynos-arm64.c
> index 6fb7194df7ab..bf7de21f329e 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -17,10 +17,17 @@
>
>  #include "clk-exynos-arm64.h"
>
> +/* PLL register bits */
> +#define PLL_CON1_MANUAL                BIT(1)
> +
>  /* Gate register bits */
>  #define GATE_MANUAL            BIT(20)
>  #define GATE_ENABLE_HWACG      BIT(28)
>
> +/* PLL_CONx_PLL register offsets range */
> +#define PLL_CON_OFF_START      0x100
> +#define PLL_CON_OFF_END                0x600
> +
>  /* Gate register offsets range */
>  #define GATE_OFF_START         0x2000
>  #define GATE_OFF_END           0x2fff
> @@ -38,17 +45,36 @@ struct exynos_arm64_cmu_data {
>         struct samsung_clk_provider *ctx;
>  };
>
> +/* Check if the register offset is a GATE register */
> +static bool is_gate_reg(unsigned long off)
> +{
> +       return off >=3D GATE_OFF_START && off <=3D GATE_OFF_END;
> +}
> +
> +/* Check if the register offset is a PLL_CONx register */
> +static bool is_pll_conx_reg(unsigned long off)
> +{
> +       return off >=3D PLL_CON_OFF_START && off <=3D PLL_CON_OFF_END;
> +}
> +
> +/* Check if the register offset is a PLL_CON1 register */
> +static bool is_pll_con1_reg(unsigned long off)
> +{
> +       return is_pll_conx_reg(off) && (off & 0xf) =3D=3D 0x4 && !(off & =
0x10);
> +}
> +
>  /**
>   * exynos_arm64_init_clocks - Set clocks initial configuration
> - * @np:                        CMU device tree node with "reg" property =
(CMU addr)
> - * @reg_offs:          Register offsets array for clocks to init
> - * @reg_offs_len:      Number of register offsets in reg_offs array
> + * @np:                CMU device tree node with "reg" property (CMU add=
r)
> + * @cmu:       CMU data
>   *
> - * Set manual control mode for all gate clocks.
> + * Set manual control mode for all gate and PLL clocks.
>   */
>  static void __init exynos_arm64_init_clocks(struct device_node *np,
> -               const unsigned long *reg_offs, size_t reg_offs_len)
> +                                           const struct samsung_cmu_info=
 *cmu)
>  {
> +       const unsigned long *reg_offs =3D cmu->clk_regs;
> +       size_t reg_offs_len =3D cmu->nr_clk_regs;
>         void __iomem *reg_base;
>         size_t i;
>
> @@ -60,14 +86,14 @@ static void __init exynos_arm64_init_clocks(struct de=
vice_node *np,
>                 void __iomem *reg =3D reg_base + reg_offs[i];
>                 u32 val;
>
> -               /* Modify only gate clock registers */
> -               if (reg_offs[i] < GATE_OFF_START || reg_offs[i] > GATE_OF=
F_END)
> -                       continue;
> -
> -               val =3D readl(reg);
> -               val |=3D GATE_MANUAL;
> -               val &=3D ~GATE_ENABLE_HWACG;
> -               writel(val, reg);
> +               if (cmu->manual_plls && is_pll_con1_reg(reg_offs[i])) {
> +                       writel(PLL_CON1_MANUAL, reg);
> +               } else if (is_gate_reg(reg_offs[i])) {
> +                       val =3D readl(reg);
> +                       val |=3D GATE_MANUAL;
> +                       val &=3D ~GATE_ENABLE_HWACG;
> +                       writel(val, reg);
> +               }
>         }
>
>         iounmap(reg_base);
> @@ -177,7 +203,7 @@ void __init exynos_arm64_register_cmu(struct device *=
dev,
>                 pr_err("%s: could not enable bus clock %s; err =3D %d\n",
>                        __func__, cmu->clk_name, err);
>
> -       exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
> +       exynos_arm64_init_clocks(np, cmu);
>         samsung_cmu_register_one(np, cmu);
>  }
>
> @@ -224,7 +250,7 @@ int __init exynos_arm64_register_cmu_pm(struct platfo=
rm_device *pdev,
>                        __func__, cmu->clk_name, ret);
>
>         if (set_manual)
> -               exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_r=
egs);
> +               exynos_arm64_init_clocks(np, cmu);
>
>         reg_base =3D devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(reg_base))
> diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
> index a763309e6f12..a70bd7cce39f 100644
> --- a/drivers/clk/samsung/clk.h
> +++ b/drivers/clk/samsung/clk.h
> @@ -330,6 +330,7 @@ struct samsung_clock_reg_cache {
>   * @suspend_regs: list of clock registers to set before suspend
>   * @nr_suspend_regs: count of clock registers in @suspend_regs
>   * @clk_name: name of the parent clock needed for CMU register access
> + * @manual_plls: Enable manual control for PLL clocks
>   */
>  struct samsung_cmu_info {
>         const struct samsung_pll_clock *pll_clks;
> @@ -354,6 +355,9 @@ struct samsung_cmu_info {
>         const struct samsung_clk_reg_dump *suspend_regs;
>         unsigned int nr_suspend_regs;
>         const char *clk_name;
> +
> +       /* ARM64 Exynos CMUs */
> +       bool manual_plls;
>  };
>
>  struct samsung_clk_provider *samsung_clk_init(struct device *dev,
> --
> 2.39.2
>