MIME-Version: 1.0
References: <20211206153124.427102-1-virag.david003@gmail.com> <20211206153124.427102-5-virag.david003@gmail.com>
In-Reply-To: <20211206153124.427102-5-virag.david003@gmail.com>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Tue, 7 Dec 2021 20:53:47 +0200
Message-ID: <CAPLW+4m6aMx3T7Rsh39zuNqrd1r_US1WWeU+5KRR5GGjOvyDcA@mail.gmail.com>
Subject: Re: [PATCH v4 4/7] clk: samsung: Make exynos850_register_cmu shared
To:     David Virag <virag.david003@gmail.com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Rob Herring <robh+dt@kernel.org>,
        Sylwester Nawrocki <s.nawrocki@samsung.com>,
        Tomasz Figa <tomasz.figa@gmail.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, 6 Dec 2021 at 17:32, David Virag <virag.david003@gmail.com> wrote:
>
> Rename exynos850_register_cmu to exynos_arm64_register_cmu and move it
> to a new file called "clk-exynos-arm64.c".
>
> This should have no functional changes, but it will allow this code to
> be shared between other arm64 Exynos SoCs, like the Exynos7885 and
> possibly ExynosAuto V9.
>
> Signed-off-by: David Virag <virag.david003@gmail.com>
> ---
> Changes in v2:
>   - New patch
>
> Changes in v3:
>   - Fix SPDX comment style in clk-exynos-arm64.h
>
> Changes in v4:
>   - Fix missing headers but still remove of_address.h
>   - "__SAMSUNG_CLK_ARM64_H" -> "__CLK_EXYNOS_ARM64_H" in
>     clk-exynos-arm64.h everywhere (only the comment at the end had the
>     latter by accident)
>
>  drivers/clk/samsung/Makefile           |  1 +
>  drivers/clk/samsung/clk-exynos-arm64.c | 94 ++++++++++++++++++++++++++
>  drivers/clk/samsung/clk-exynos-arm64.h | 20 ++++++
>  drivers/clk/samsung/clk-exynos850.c    | 88 ++----------------------
>  4 files changed, 119 insertions(+), 84 deletions(-)
>  create mode 100644 drivers/clk/samsung/clk-exynos-arm64.c
>  create mode 100644 drivers/clk/samsung/clk-exynos-arm64.h
>
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index c46cf11e4d0b..901e6333c5f0 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_EXYNOS_5420_COMMON_CLK)  +=3D clk-exynos5-=
subcmu.o
>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)  +=3D clk-exynos5433.o
>  obj-$(CONFIG_EXYNOS_AUDSS_CLK_CON) +=3D clk-exynos-audss.o
>  obj-$(CONFIG_EXYNOS_CLKOUT)    +=3D clk-exynos-clkout.o
> +obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)  +=3D clk-exynos-arm64.o
>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)  +=3D clk-exynos7.o
>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)  +=3D clk-exynos850.o
>  obj-$(CONFIG_S3C2410_COMMON_CLK)+=3D clk-s3c2410.o
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung=
/clk-exynos-arm64.c
> new file mode 100644
> index 000000000000..b921b9a1134a
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 Linaro Ltd.
> + * Copyright (C) 2021 D=C3=A1vid Vir=C3=A1g <virag.david003@gmail.com>
> + * Author: Sam Protsenko <semen.protsenko@linaro.org>
> + * Author: D=C3=A1vid Vir=C3=A1g <virag.david003@gmail.com>
> + *
> + * This file contains shared functions used by some arm64 Exynos SoCs,
> + * such as Exynos7885 or Exynos850 to register and init CMUs.
> + */

Please add empty line here (if you're going to send another version).
Other than that:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> +#include <linux/clk.h>
> +#include <linux/of_address.h>
> +
> +#include "clk-exynos-arm64.h"
> +
> +/* Gate register bits */
> +#define GATE_MANUAL            BIT(20)
> +#define GATE_ENABLE_HWACG      BIT(28)
> +
> +/* Gate register offsets range */
> +#define GATE_OFF_START         0x2000
> +#define GATE_OFF_END           0x2fff
> +
> +/**
> + * 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
> + *
> + * Set manual control mode for all gate clocks.
> + */
> +static void __init exynos_arm64_init_clocks(struct device_node *np,
> +               const unsigned long *reg_offs, size_t reg_offs_len)
> +{
> +       void __iomem *reg_base;
> +       size_t i;
> +
> +       reg_base =3D of_iomap(np, 0);
> +       if (!reg_base)
> +               panic("%s: failed to map registers\n", __func__);
> +
> +       for (i =3D 0; i < reg_offs_len; ++i) {
> +               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);
> +       }
> +
> +       iounmap(reg_base);
> +}
> +
> +/**
> + * exynos_arm64_register_cmu - Register specified Exynos CMU domain
> + * @dev:       Device object; may be NULL if this function is not being
> + *             called from platform driver probe function
> + * @np:                CMU device tree node
> + * @cmu:       CMU data
> + *
> + * Register specified CMU domain, which includes next steps:
> + *
> + * 1. Enable parent clock of @cmu CMU
> + * 2. Set initial registers configuration for @cmu CMU clocks
> + * 3. Register @cmu CMU clocks using Samsung clock framework API
> + */
> +void __init exynos_arm64_register_cmu(struct device *dev,
> +               struct device_node *np, const struct samsung_cmu_info *cm=
u)
> +{
> +       /* Keep CMU parent clock running (needed for CMU registers access=
) */
> +       if (cmu->clk_name) {
> +               struct clk *parent_clk;
> +
> +               if (dev)
> +                       parent_clk =3D clk_get(dev, cmu->clk_name);
> +               else
> +                       parent_clk =3D of_clk_get_by_name(np, cmu->clk_na=
me);
> +
> +               if (IS_ERR(parent_clk)) {
> +                       pr_err("%s: could not find bus clock %s; err =3D =
%ld\n",
> +                              __func__, cmu->clk_name, PTR_ERR(parent_cl=
k));
> +               } else {
> +                       clk_prepare_enable(parent_clk);
> +               }
> +       }
> +
> +       exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
> +       samsung_cmu_register_one(np, cmu);
> +}
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.h b/drivers/clk/samsung=
/clk-exynos-arm64.h
> new file mode 100644
> index 000000000000..0dd174693935
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos-arm64.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 Linaro Ltd.
> + * Copyright (C) 2021 D=C3=A1vid Vir=C3=A1g <virag.david003@gmail.com>
> + * Author: Sam Protsenko <semen.protsenko@linaro.org>
> + * Author: D=C3=A1vid Vir=C3=A1g <virag.david003@gmail.com>
> + *
> + * This file contains shared functions used by some arm64 Exynos SoCs,
> + * such as Exynos7885 or Exynos850 to register and init CMUs.
> + */
> +
> +#ifndef __CLK_EXYNOS_ARM64_H
> +#define __CLK_EXYNOS_ARM64_H
> +
> +#include "clk.h"
> +
> +void exynos_arm64_register_cmu(struct device *dev,
> +               struct device_node *np, const struct samsung_cmu_info *cm=
u);
> +
> +#endif /* __CLK_EXYNOS_ARM64_H */
> diff --git a/drivers/clk/samsung/clk-exynos850.c b/drivers/clk/samsung/cl=
k-exynos850.c
> index 568ac97c8120..17413135196d 100644
> --- a/drivers/clk/samsung/clk-exynos850.c
> +++ b/drivers/clk/samsung/clk-exynos850.c
> @@ -9,93 +9,13 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/of.h>
> -#include <linux/of_address.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>
>  #include <dt-bindings/clock/exynos850.h>
>
>  #include "clk.h"
> -
> -/* Gate register bits */
> -#define GATE_MANUAL            BIT(20)
> -#define GATE_ENABLE_HWACG      BIT(28)
> -
> -/* Gate register offsets range */
> -#define GATE_OFF_START         0x2000
> -#define GATE_OFF_END           0x2fff
> -
> -/**
> - * exynos850_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
> - *
> - * Set manual control mode for all gate clocks.
> - */
> -static void __init exynos850_init_clocks(struct device_node *np,
> -               const unsigned long *reg_offs, size_t reg_offs_len)
> -{
> -       void __iomem *reg_base;
> -       size_t i;
> -
> -       reg_base =3D of_iomap(np, 0);
> -       if (!reg_base)
> -               panic("%s: failed to map registers\n", __func__);
> -
> -       for (i =3D 0; i < reg_offs_len; ++i) {
> -               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);
> -       }
> -
> -       iounmap(reg_base);
> -}
> -
> -/**
> - * exynos850_register_cmu - Register specified Exynos850 CMU domain
> - * @dev:       Device object; may be NULL if this function is not being
> - *             called from platform driver probe function
> - * @np:                CMU device tree node
> - * @cmu:       CMU data
> - *
> - * Register specified CMU domain, which includes next steps:
> - *
> - * 1. Enable parent clock of @cmu CMU
> - * 2. Set initial registers configuration for @cmu CMU clocks
> - * 3. Register @cmu CMU clocks using Samsung clock framework API
> - */
> -static void __init exynos850_register_cmu(struct device *dev,
> -               struct device_node *np, const struct samsung_cmu_info *cm=
u)
> -{
> -       /* Keep CMU parent clock running (needed for CMU registers access=
) */
> -       if (cmu->clk_name) {
> -               struct clk *parent_clk;
> -
> -               if (dev)
> -                       parent_clk =3D clk_get(dev, cmu->clk_name);
> -               else
> -                       parent_clk =3D of_clk_get_by_name(np, cmu->clk_na=
me);
> -
> -               if (IS_ERR(parent_clk)) {
> -                       pr_err("%s: could not find bus clock %s; err =3D =
%ld\n",
> -                              __func__, cmu->clk_name, PTR_ERR(parent_cl=
k));
> -               } else {
> -                       clk_prepare_enable(parent_clk);
> -               }
> -       }
> -
> -       exynos850_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
> -       samsung_cmu_register_one(np, cmu);
> -}
> +#include "clk-exynos-arm64.h"
>
>  /* ---- CMU_TOP --------------------------------------------------------=
----- */
>
> @@ -404,7 +324,7 @@ static const struct samsung_cmu_info top_cmu_info __i=
nitconst =3D {
>
>  static void __init exynos850_cmu_top_init(struct device_node *np)
>  {
> -       exynos850_register_cmu(NULL, np, &top_cmu_info);
> +       exynos_arm64_register_cmu(NULL, np, &top_cmu_info);
>  }
>
>  /* Register CMU_TOP early, as it's a dependency for other early domains =
*/
> @@ -892,7 +812,7 @@ static const struct samsung_cmu_info peri_cmu_info __=
initconst =3D {
>
>  static void __init exynos850_cmu_peri_init(struct device_node *np)
>  {
> -       exynos850_register_cmu(NULL, np, &peri_cmu_info);
> +       exynos_arm64_register_cmu(NULL, np, &peri_cmu_info);
>  }
>
>  /* Register CMU_PERI early, as it's needed for MCT timer */
> @@ -1069,7 +989,7 @@ static int __init exynos850_cmu_probe(struct platfor=
m_device *pdev)
>         struct device *dev =3D &pdev->dev;
>
>         info =3D of_device_get_match_data(dev);
> -       exynos850_register_cmu(dev, dev->of_node, info);
> +       exynos_arm64_register_cmu(dev, dev->of_node, info);
>
>         return 0;
>  }
> --
> 2.34.1
>