2023-02-03 06:09:07

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 0/6] clk: samsung: Add PM support for ARM64 Exynos chips

In order to prepare for PM enablement in clk-exynos850, common PM code
was extracted from clk-exynos5433 to clk-exynos-arm64. Also some related
cleanups were done prior to that. More specifically:

- patches #1..5: cleanups
- patch #6: PM code extraction

During the extraction of the exynos5433_cmu_probe() content to
exynos_arm64_register_cmu_pm() some code was reworked a bit, and also
split into smaller functions. In particular:

- cmu_data structure now contains a pointer to ctx, which is now
allocated in samsung_clk_init()
- cmu_data structure initialization was moved into separate function
- code for configuring gate clocks was added (optional)

Which in turn resulted in somehow modified code of probe function:

Original
--------

...
devm_platform_ioremap_resource(...);
samsung_clk_init(...);
exynos_arm64_cmu_prepare_pm(...);
exynos_arm64_enable_bus_clk(...);
platform_set_drvdata(...);
...

Modified
--------

...
platform_set_drvdata(...);
exynos_arm64_cmu_prepare_pm(...);
exynos_arm64_enable_bus_clk(...);
exynos_arm64_init_clocks(...);
devm_platform_ioremap_resource(...);
samsung_clk_init(...);
...

That shouldn't really change the logic or mode of operation. It was
preliminary tested on Exynos850 based board, with some extra patches on
top of this series (will be submitted later).

Marek, could you please test these patches on Exynos5433 platform, and
also review accordingly?

Thanks!

Sam Protsenko (6):
clk: samsung: Don't pass reg_base to samsung_clk_register_pll()
clk: samsung: Remove np argument from samsung_clk_init()
clk: samsung: Set dev in samsung_clk_init()
clk: samsung: Extract clocks registration to common function
clk: samsung: Extract parent clock enabling to common function
clk: samsung: exynos5433: Extract PM support to common ARM64 layer

drivers/clk/samsung/clk-exynos-arm64.c | 219 +++++++++++++++++++++--
drivers/clk/samsung/clk-exynos-arm64.h | 3 +
drivers/clk/samsung/clk-exynos4.c | 6 +-
drivers/clk/samsung/clk-exynos4412-isp.c | 3 +-
drivers/clk/samsung/clk-exynos5250.c | 5 +-
drivers/clk/samsung/clk-exynos5420.c | 5 +-
drivers/clk/samsung/clk-exynos5433.c | 157 +---------------
drivers/clk/samsung/clk-pll.c | 11 +-
drivers/clk/samsung/clk-s3c2410.c | 6 +-
drivers/clk/samsung/clk-s3c2412.c | 5 +-
drivers/clk/samsung/clk-s3c2443.c | 6 +-
drivers/clk/samsung/clk-s3c64xx.c | 4 +-
drivers/clk/samsung/clk-s5pv210.c | 6 +-
drivers/clk/samsung/clk.c | 64 ++++---
drivers/clk/samsung/clk.h | 10 +-
15 files changed, 282 insertions(+), 228 deletions(-)

--
2.39.0



2023-02-03 06:09:11

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 1/6] clk: samsung: Don't pass reg_base to samsung_clk_register_pll()

Base address can be derived from context structure. Remove `base'
argument from samsung_clk_register_pll() and use `ctx->reg_base'
instead, as it's done in other clock registering functions.

No functional change.

Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 4 ++--
drivers/clk/samsung/clk-exynos5250.c | 3 +--
drivers/clk/samsung/clk-exynos5420.c | 3 +--
drivers/clk/samsung/clk-exynos5433.c | 4 ++--
drivers/clk/samsung/clk-pll.c | 11 +++++------
drivers/clk/samsung/clk-s3c2410.c | 4 ++--
drivers/clk/samsung/clk-s3c2412.c | 3 +--
drivers/clk/samsung/clk-s3c2443.c | 4 ++--
drivers/clk/samsung/clk-s3c64xx.c | 2 +-
drivers/clk/samsung/clk-s5pv210.c | 4 ++--
drivers/clk/samsung/clk.c | 3 +--
drivers/clk/samsung/clk.h | 2 +-
12 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 22009cb53428..244f74f31746 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1276,7 +1276,7 @@ static void __init exynos4_clk_init(struct device_node *np,
exynos4210_vpll_rates;

samsung_clk_register_pll(ctx, exynos4210_plls,
- ARRAY_SIZE(exynos4210_plls), reg_base);
+ ARRAY_SIZE(exynos4210_plls));
} else {
if (clk_hw_get_rate(hws[CLK_FIN_PLL]) == 24000000) {
exynos4x12_plls[apll].rate_table =
@@ -1288,7 +1288,7 @@ static void __init exynos4_clk_init(struct device_node *np,
}

samsung_clk_register_pll(ctx, exynos4x12_plls,
- ARRAY_SIZE(exynos4x12_plls), reg_base);
+ ARRAY_SIZE(exynos4x12_plls));
}

samsung_clk_register_fixed_rate(ctx, exynos4_fixed_rate_clks,
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 113df773ee44..69862ab6dc52 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -815,8 +815,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
exynos5250_plls[vpll].rate_table = vpll_24mhz_tbl;

samsung_clk_register_pll(ctx, exynos5250_plls,
- ARRAY_SIZE(exynos5250_plls),
- reg_base);
+ ARRAY_SIZE(exynos5250_plls));
samsung_clk_register_fixed_rate(ctx, exynos5250_fixed_rate_clks,
ARRAY_SIZE(exynos5250_fixed_rate_clks));
samsung_clk_register_fixed_factor(ctx, exynos5250_fixed_factor_clks,
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index caad74dee297..df9e93d6adf2 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1606,8 +1606,7 @@ static void __init exynos5x_clk_init(struct device_node *np,
else
exynos5x_plls[bpll].rate_table = exynos5422_bpll_rate_table;

- samsung_clk_register_pll(ctx, exynos5x_plls, ARRAY_SIZE(exynos5x_plls),
- reg_base);
+ samsung_clk_register_pll(ctx, exynos5x_plls, ARRAY_SIZE(exynos5x_plls));
samsung_clk_register_fixed_rate(ctx, exynos5x_fixed_rate_clks,
ARRAY_SIZE(exynos5x_fixed_rate_clks));
samsung_clk_register_fixed_factor(ctx, exynos5x_fixed_factor_clks,
diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index f9daae20f393..eb72bf2aaee8 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5610,8 +5610,8 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev)
pm_runtime_enable(dev);

if (info->pll_clks)
- samsung_clk_register_pll(ctx, info->pll_clks, info->nr_pll_clks,
- reg_base);
+ samsung_clk_register_pll(ctx, info->pll_clks,
+ info->nr_pll_clks);
if (info->mux_clks)
samsung_clk_register_mux(ctx, info->mux_clks,
info->nr_mux_clks);
diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
index 0ff28938943f..96b2b15fafc3 100644
--- a/drivers/clk/samsung/clk-pll.c
+++ b/drivers/clk/samsung/clk-pll.c
@@ -1422,8 +1422,7 @@ static const struct clk_ops samsung_pll2650xx_clk_min_ops = {
};

static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,
- const struct samsung_pll_clock *pll_clk,
- void __iomem *base)
+ const struct samsung_pll_clock *pll_clk)
{
struct samsung_clk_pll *pll;
struct clk_init_data init;
@@ -1576,8 +1575,8 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,

pll->hw.init = &init;
pll->type = pll_clk->type;
- pll->lock_reg = base + pll_clk->lock_offset;
- pll->con_reg = base + pll_clk->con_offset;
+ pll->lock_reg = ctx->reg_base + pll_clk->lock_offset;
+ pll->con_reg = ctx->reg_base + pll_clk->con_offset;

ret = clk_hw_register(ctx->dev, &pll->hw);
if (ret) {
@@ -1593,10 +1592,10 @@ static void __init _samsung_clk_register_pll(struct samsung_clk_provider *ctx,

void __init samsung_clk_register_pll(struct samsung_clk_provider *ctx,
const struct samsung_pll_clock *pll_list,
- unsigned int nr_pll, void __iomem *base)
+ unsigned int nr_pll)
{
int cnt;

for (cnt = 0; cnt < nr_pll; cnt++)
- _samsung_clk_register_pll(ctx, &pll_list[cnt], base);
+ _samsung_clk_register_pll(ctx, &pll_list[cnt]);
}
diff --git a/drivers/clk/samsung/clk-s3c2410.c b/drivers/clk/samsung/clk-s3c2410.c
index 3d152a46169b..d72c79fa9b63 100644
--- a/drivers/clk/samsung/clk-s3c2410.c
+++ b/drivers/clk/samsung/clk-s3c2410.c
@@ -347,7 +347,7 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f,

/* Register PLLs. */
samsung_clk_register_pll(ctx, s3c2410_plls,
- ARRAY_SIZE(s3c2410_plls), reg_base);
+ ARRAY_SIZE(s3c2410_plls));

} else { /* S3C2440, S3C2442 */
if (clk_hw_get_rate(hws[XTI]) == 12 * MHZ) {
@@ -363,7 +363,7 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f,

/* Register PLLs. */
samsung_clk_register_pll(ctx, s3c244x_common_plls,
- ARRAY_SIZE(s3c244x_common_plls), reg_base);
+ ARRAY_SIZE(s3c244x_common_plls));
}

/* Register common internal clocks. */
diff --git a/drivers/clk/samsung/clk-s3c2412.c b/drivers/clk/samsung/clk-s3c2412.c
index 724ef642f048..367f3ecb68e8 100644
--- a/drivers/clk/samsung/clk-s3c2412.c
+++ b/drivers/clk/samsung/clk-s3c2412.c
@@ -223,8 +223,7 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
s3c2412_common_clk_register_fixed_ext(ctx, xti_f, ext_f);

/* Register PLLs. */
- samsung_clk_register_pll(ctx, s3c2412_plls, ARRAY_SIZE(s3c2412_plls),
- reg_base);
+ samsung_clk_register_pll(ctx, s3c2412_plls, ARRAY_SIZE(s3c2412_plls));

/* Register common internal clocks. */
samsung_clk_register_mux(ctx, s3c2412_muxes, ARRAY_SIZE(s3c2412_muxes));
diff --git a/drivers/clk/samsung/clk-s3c2443.c b/drivers/clk/samsung/clk-s3c2443.c
index a827d63766d1..bfcad2a24dbe 100644
--- a/drivers/clk/samsung/clk-s3c2443.c
+++ b/drivers/clk/samsung/clk-s3c2443.c
@@ -362,10 +362,10 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
/* Register PLLs. */
if (current_soc == S3C2416 || current_soc == S3C2450)
samsung_clk_register_pll(ctx, s3c2416_pll_clks,
- ARRAY_SIZE(s3c2416_pll_clks), reg_base);
+ ARRAY_SIZE(s3c2416_pll_clks));
else
samsung_clk_register_pll(ctx, s3c2443_pll_clks,
- ARRAY_SIZE(s3c2443_pll_clks), reg_base);
+ ARRAY_SIZE(s3c2443_pll_clks));

/* Register common internal clocks. */
samsung_clk_register_mux(ctx, s3c2443_common_muxes,
diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c
index d6b432a26d63..d582bac68bb7 100644
--- a/drivers/clk/samsung/clk-s3c64xx.c
+++ b/drivers/clk/samsung/clk-s3c64xx.c
@@ -414,7 +414,7 @@ void __init s3c64xx_clk_init(struct device_node *np, unsigned long xtal_f,

/* Register PLLs. */
samsung_clk_register_pll(ctx, s3c64xx_pll_clks,
- ARRAY_SIZE(s3c64xx_pll_clks), reg_base);
+ ARRAY_SIZE(s3c64xx_pll_clks));

/* Register common internal clocks. */
samsung_clk_register_fixed_rate(ctx, s3c64xx_fixed_rate_clks,
diff --git a/drivers/clk/samsung/clk-s5pv210.c b/drivers/clk/samsung/clk-s5pv210.c
index 4425186bdcab..b91d20994bf5 100644
--- a/drivers/clk/samsung/clk-s5pv210.c
+++ b/drivers/clk/samsung/clk-s5pv210.c
@@ -753,7 +753,7 @@ static void __init __s5pv210_clk_init(struct device_node *np,
samsung_clk_register_fixed_rate(ctx, s5p6442_frate_clks,
ARRAY_SIZE(s5p6442_frate_clks));
samsung_clk_register_pll(ctx, s5p6442_pll_clks,
- ARRAY_SIZE(s5p6442_pll_clks), reg_base);
+ ARRAY_SIZE(s5p6442_pll_clks));
samsung_clk_register_mux(ctx, s5p6442_mux_clks,
ARRAY_SIZE(s5p6442_mux_clks));
samsung_clk_register_div(ctx, s5p6442_div_clks,
@@ -764,7 +764,7 @@ static void __init __s5pv210_clk_init(struct device_node *np,
samsung_clk_register_fixed_rate(ctx, s5pv210_frate_clks,
ARRAY_SIZE(s5pv210_frate_clks));
samsung_clk_register_pll(ctx, s5pv210_pll_clks,
- ARRAY_SIZE(s5pv210_pll_clks), reg_base);
+ ARRAY_SIZE(s5pv210_pll_clks));
samsung_clk_register_mux(ctx, s5pv210_mux_clks,
ARRAY_SIZE(s5pv210_mux_clks));
samsung_clk_register_div(ctx, s5pv210_div_clks,
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index bca4731b14ea..e132d63933c3 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -344,8 +344,7 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
ctx = samsung_clk_init(np, reg_base, cmu->nr_clk_ids);

if (cmu->pll_clks)
- samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks,
- reg_base);
+ samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
if (cmu->mux_clks)
samsung_clk_register_mux(ctx, cmu->mux_clks,
cmu->nr_mux_clks);
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index b46e83a2581f..1b7315063edd 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -373,7 +373,7 @@ void samsung_clk_register_gate(struct samsung_clk_provider *ctx,
unsigned int nr_clk);
void samsung_clk_register_pll(struct samsung_clk_provider *ctx,
const struct samsung_pll_clock *pll_list,
- unsigned int nr_clk, void __iomem *base);
+ unsigned int nr_clk);
void samsung_clk_register_cpu(struct samsung_clk_provider *ctx,
const struct samsung_cpu_clock *list, unsigned int nr_clk);

--
2.39.0


2023-02-03 06:09:15

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 2/6] clk: samsung: Remove np argument from samsung_clk_init()

The code using `np' argument was removed from samsung_clk_init(). Remove
that leftover parameter as well.

No functional change.

Fixes: d5e136a21b20 ("clk: samsung: Register clk provider only after registering its all clocks")
Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 2 +-
drivers/clk/samsung/clk-exynos4412-isp.c | 2 +-
drivers/clk/samsung/clk-exynos5250.c | 2 +-
drivers/clk/samsung/clk-exynos5420.c | 2 +-
drivers/clk/samsung/clk-s3c2410.c | 2 +-
drivers/clk/samsung/clk-s3c2412.c | 2 +-
drivers/clk/samsung/clk-s3c2443.c | 2 +-
drivers/clk/samsung/clk-s3c64xx.c | 2 +-
drivers/clk/samsung/clk-s5pv210.c | 2 +-
drivers/clk/samsung/clk.c | 6 +++---
drivers/clk/samsung/clk.h | 3 +--
11 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 244f74f31746..7a9994144d72 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1251,7 +1251,7 @@ static void __init exynos4_clk_init(struct device_node *np,
if (!reg_base)
panic("%s: failed to map registers\n", __func__);

- ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos4_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
index 471a6fb82670..0b6390e04533 100644
--- a/drivers/clk/samsung/clk-exynos4412-isp.c
+++ b/drivers/clk/samsung/clk-exynos4412-isp.c
@@ -121,7 +121,7 @@ static int __init exynos4x12_isp_clk_probe(struct platform_device *pdev)
if (!exynos4x12_save_isp)
return -ENOMEM;

- ctx = samsung_clk_init(np, reg_base, CLK_NR_ISP_CLKS);
+ ctx = samsung_clk_init(reg_base, CLK_NR_ISP_CLKS);
ctx->dev = dev;

platform_set_drvdata(pdev, ctx);
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 69862ab6dc52..f1cb69aea10e 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -797,7 +797,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
panic("%s: unable to determine soc\n", __func__);
}

- ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos5250_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index df9e93d6adf2..46cac4980be2 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1587,7 +1587,7 @@ static void __init exynos5x_clk_init(struct device_node *np,

exynos5x_soc = soc;

- ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos5x_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-s3c2410.c b/drivers/clk/samsung/clk-s3c2410.c
index d72c79fa9b63..8483d4a2be03 100644
--- a/drivers/clk/samsung/clk-s3c2410.c
+++ b/drivers/clk/samsung/clk-s3c2410.c
@@ -332,7 +332,7 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(np, reg_base, NR_CLKS);
+ ctx = samsung_clk_init(reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

/* Register external clocks only in non-dt cases */
diff --git a/drivers/clk/samsung/clk-s3c2412.c b/drivers/clk/samsung/clk-s3c2412.c
index 367f3ecb68e8..8f12a40790d4 100644
--- a/drivers/clk/samsung/clk-s3c2412.c
+++ b/drivers/clk/samsung/clk-s3c2412.c
@@ -216,7 +216,7 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(np, reg_base, NR_CLKS);
+ ctx = samsung_clk_init(reg_base, NR_CLKS);

/* Register external clocks only in non-dt cases */
if (!np)
diff --git a/drivers/clk/samsung/clk-s3c2443.c b/drivers/clk/samsung/clk-s3c2443.c
index bfcad2a24dbe..6db8147f0aa2 100644
--- a/drivers/clk/samsung/clk-s3c2443.c
+++ b/drivers/clk/samsung/clk-s3c2443.c
@@ -353,7 +353,7 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(np, reg_base, NR_CLKS);
+ ctx = samsung_clk_init(reg_base, NR_CLKS);

/* Register external clocks only in non-dt cases */
if (!np)
diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c
index d582bac68bb7..47e9d19486dc 100644
--- a/drivers/clk/samsung/clk-s3c64xx.c
+++ b/drivers/clk/samsung/clk-s3c64xx.c
@@ -405,7 +405,7 @@ void __init s3c64xx_clk_init(struct device_node *np, unsigned long xtal_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(np, reg_base, NR_CLKS);
+ ctx = samsung_clk_init(reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

/* Register external clocks. */
diff --git a/drivers/clk/samsung/clk-s5pv210.c b/drivers/clk/samsung/clk-s5pv210.c
index b91d20994bf5..b0ab6bc9d21d 100644
--- a/drivers/clk/samsung/clk-s5pv210.c
+++ b/drivers/clk/samsung/clk-s5pv210.c
@@ -743,7 +743,7 @@ static void __init __s5pv210_clk_init(struct device_node *np,
struct samsung_clk_provider *ctx;
struct clk_hw **hws;

- ctx = samsung_clk_init(np, reg_base, NR_CLKS);
+ ctx = samsung_clk_init(reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_register_mux(ctx, early_mux_clks,
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index e132d63933c3..2436223aac1a 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -54,8 +54,8 @@ struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
}

/* setup the essentials required to support clock lookup using ccf */
-struct samsung_clk_provider *__init samsung_clk_init(struct device_node *np,
- void __iomem *base, unsigned long nr_clks)
+struct samsung_clk_provider * __init samsung_clk_init(void __iomem *base,
+ unsigned long nr_clks)
{
struct samsung_clk_provider *ctx;
int i;
@@ -341,7 +341,7 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
return NULL;
}

- ctx = samsung_clk_init(np, reg_base, cmu->nr_clk_ids);
+ ctx = samsung_clk_init(reg_base, cmu->nr_clk_ids);

if (cmu->pll_clks)
samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 1b7315063edd..98753b0e5055 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -337,8 +337,7 @@ struct samsung_cmu_info {
const char *clk_name;
};

-struct samsung_clk_provider * samsung_clk_init(
- struct device_node *np, void __iomem *base,
+struct samsung_clk_provider *samsung_clk_init(void __iomem *base,
unsigned long nr_clks);
void samsung_clk_of_add_provider(struct device_node *np,
struct samsung_clk_provider *ctx);
--
2.39.0


2023-02-03 06:09:22

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 3/6] clk: samsung: Set dev in samsung_clk_init()

Some drivers set dev to context in order to implement PM. Make that part
of samsung_clk_init() instead of assigning `ctx->dev = dev' separately.

No functional change.

Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 2 +-
drivers/clk/samsung/clk-exynos4412-isp.c | 3 +--
drivers/clk/samsung/clk-exynos5250.c | 2 +-
drivers/clk/samsung/clk-exynos5420.c | 2 +-
drivers/clk/samsung/clk-s3c2410.c | 2 +-
drivers/clk/samsung/clk-s3c2412.c | 2 +-
drivers/clk/samsung/clk-s3c2443.c | 2 +-
drivers/clk/samsung/clk-s3c64xx.c | 2 +-
drivers/clk/samsung/clk-s5pv210.c | 2 +-
drivers/clk/samsung/clk.c | 19 +++++++++++++++----
drivers/clk/samsung/clk.h | 5 +++--
11 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 7a9994144d72..d7dbb3858347 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -1251,7 +1251,7 @@ static void __init exynos4_clk_init(struct device_node *np,
if (!reg_base)
panic("%s: failed to map registers\n", __func__);

- ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos4_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-exynos4412-isp.c b/drivers/clk/samsung/clk-exynos4412-isp.c
index 0b6390e04533..1470c15e95da 100644
--- a/drivers/clk/samsung/clk-exynos4412-isp.c
+++ b/drivers/clk/samsung/clk-exynos4412-isp.c
@@ -121,8 +121,7 @@ static int __init exynos4x12_isp_clk_probe(struct platform_device *pdev)
if (!exynos4x12_save_isp)
return -ENOMEM;

- ctx = samsung_clk_init(reg_base, CLK_NR_ISP_CLKS);
- ctx->dev = dev;
+ ctx = samsung_clk_init(dev, reg_base, CLK_NR_ISP_CLKS);

platform_set_drvdata(pdev, ctx);

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index f1cb69aea10e..92fb09922f28 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -797,7 +797,7 @@ static void __init exynos5250_clk_init(struct device_node *np)
panic("%s: unable to determine soc\n", __func__);
}

- ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos5250_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c
index 46cac4980be2..1e0cbf762408 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1587,7 +1587,7 @@ static void __init exynos5x_clk_init(struct device_node *np,

exynos5x_soc = soc;

- ctx = samsung_clk_init(reg_base, CLK_NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, CLK_NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_of_register_fixed_ext(ctx, exynos5x_fixed_rate_ext_clks,
diff --git a/drivers/clk/samsung/clk-s3c2410.c b/drivers/clk/samsung/clk-s3c2410.c
index 8483d4a2be03..a30db5b5b0d3 100644
--- a/drivers/clk/samsung/clk-s3c2410.c
+++ b/drivers/clk/samsung/clk-s3c2410.c
@@ -332,7 +332,7 @@ void __init s3c2410_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(reg_base, NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

/* Register external clocks only in non-dt cases */
diff --git a/drivers/clk/samsung/clk-s3c2412.c b/drivers/clk/samsung/clk-s3c2412.c
index 8f12a40790d4..c4a9abbdd1e8 100644
--- a/drivers/clk/samsung/clk-s3c2412.c
+++ b/drivers/clk/samsung/clk-s3c2412.c
@@ -216,7 +216,7 @@ void __init s3c2412_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(reg_base, NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, NR_CLKS);

/* Register external clocks only in non-dt cases */
if (!np)
diff --git a/drivers/clk/samsung/clk-s3c2443.c b/drivers/clk/samsung/clk-s3c2443.c
index 6db8147f0aa2..be0a378d0a92 100644
--- a/drivers/clk/samsung/clk-s3c2443.c
+++ b/drivers/clk/samsung/clk-s3c2443.c
@@ -353,7 +353,7 @@ void __init s3c2443_common_clk_init(struct device_node *np, unsigned long xti_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(reg_base, NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, NR_CLKS);

/* Register external clocks only in non-dt cases */
if (!np)
diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c
index 47e9d19486dc..d27a1f73f077 100644
--- a/drivers/clk/samsung/clk-s3c64xx.c
+++ b/drivers/clk/samsung/clk-s3c64xx.c
@@ -405,7 +405,7 @@ void __init s3c64xx_clk_init(struct device_node *np, unsigned long xtal_f,
panic("%s: failed to map registers\n", __func__);
}

- ctx = samsung_clk_init(reg_base, NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

/* Register external clocks. */
diff --git a/drivers/clk/samsung/clk-s5pv210.c b/drivers/clk/samsung/clk-s5pv210.c
index b0ab6bc9d21d..cd85342e4ddb 100644
--- a/drivers/clk/samsung/clk-s5pv210.c
+++ b/drivers/clk/samsung/clk-s5pv210.c
@@ -743,7 +743,7 @@ static void __init __s5pv210_clk_init(struct device_node *np,
struct samsung_clk_provider *ctx;
struct clk_hw **hws;

- ctx = samsung_clk_init(reg_base, NR_CLKS);
+ ctx = samsung_clk_init(NULL, reg_base, NR_CLKS);
hws = ctx->clk_data.hws;

samsung_clk_register_mux(ctx, early_mux_clks,
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index 2436223aac1a..912dfbe5ac20 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -53,9 +53,19 @@ struct samsung_clk_reg_dump *samsung_clk_alloc_reg_dump(
return rd;
}

-/* setup the essentials required to support clock lookup using ccf */
-struct samsung_clk_provider * __init samsung_clk_init(void __iomem *base,
- unsigned long nr_clks)
+/**
+ * samsung_clk_init() - Create and initialize a clock provider object
+ * @dev: CMU device to enable runtime PM, or NULL if RPM is not needed
+ * @base: Start address (mapped) of CMU registers
+ * @nr_clks: Total clock count to allocate in clock provider object
+ *
+ * Setup the essentials required to support clock lookup using Common Clock
+ * Framework.
+ *
+ * Return: Allocated and initialized clock provider object.
+ */
+struct samsung_clk_provider * __init samsung_clk_init(struct device *dev,
+ void __iomem *base, unsigned long nr_clks)
{
struct samsung_clk_provider *ctx;
int i;
@@ -67,6 +77,7 @@ struct samsung_clk_provider * __init samsung_clk_init(void __iomem *base,
for (i = 0; i < nr_clks; ++i)
ctx->clk_data.hws[i] = ERR_PTR(-ENOENT);

+ ctx->dev = dev;
ctx->reg_base = base;
ctx->clk_data.num = nr_clks;
spin_lock_init(&ctx->lock);
@@ -341,7 +352,7 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
return NULL;
}

- ctx = samsung_clk_init(reg_base, cmu->nr_clk_ids);
+ ctx = samsung_clk_init(NULL, reg_base, cmu->nr_clk_ids);

if (cmu->pll_clks)
samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 98753b0e5055..3fd6c0868921 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -16,6 +16,7 @@
/**
* struct samsung_clk_provider: information about clock provider
* @reg_base: virtual address for the register base.
+ * @dev: clock provider device needed for runtime PM.
* @lock: maintains exclusion between callbacks for a given clock-provider.
* @clk_data: holds clock related data like clk_hw* and number of clocks.
*/
@@ -337,8 +338,8 @@ struct samsung_cmu_info {
const char *clk_name;
};

-struct samsung_clk_provider *samsung_clk_init(void __iomem *base,
- unsigned long nr_clks);
+struct samsung_clk_provider *samsung_clk_init(struct device *dev,
+ void __iomem *base, unsigned long nr_clks);
void samsung_clk_of_add_provider(struct device_node *np,
struct samsung_clk_provider *ctx);
void samsung_clk_of_register_fixed_ext(
--
2.39.0


2023-02-03 06:09:26

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 4/6] clk: samsung: Extract clocks registration to common function

It might be useful to have a separate clocks registration function, so
it can be called from different users. Extract that common code from
samsung_cmu_register_one() to samsung_cmu_register_clocks(). Also make
that new function global as it's going to be used in other modules
further.

Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk.c | 46 ++++++++++++++++++++++++---------------
drivers/clk/samsung/clk.h | 2 ++
2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index 912dfbe5ac20..b6701905f254 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -335,6 +335,33 @@ void samsung_clk_extended_sleep_init(void __iomem *reg_base,
}
#endif

+/**
+ * samsung_cmu_register_clocks() - Register all clocks provided in CMU object
+ * @ctx: Clock provider object
+ * @cmu: CMU object with clocks to register
+ */
+void __init samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
+ const struct samsung_cmu_info *cmu)
+{
+ if (cmu->pll_clks)
+ samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
+ if (cmu->mux_clks)
+ samsung_clk_register_mux(ctx, cmu->mux_clks, cmu->nr_mux_clks);
+ if (cmu->div_clks)
+ samsung_clk_register_div(ctx, cmu->div_clks, cmu->nr_div_clks);
+ if (cmu->gate_clks)
+ samsung_clk_register_gate(ctx, cmu->gate_clks,
+ cmu->nr_gate_clks);
+ if (cmu->fixed_clks)
+ samsung_clk_register_fixed_rate(ctx, cmu->fixed_clks,
+ cmu->nr_fixed_clks);
+ if (cmu->fixed_factor_clks)
+ samsung_clk_register_fixed_factor(ctx, cmu->fixed_factor_clks,
+ cmu->nr_fixed_factor_clks);
+ if (cmu->cpu_clks)
+ samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);
+}
+
/*
* Common function which registers plls, muxes, dividers and gates
* for each CMU. It also add CMU register list to register cache.
@@ -353,29 +380,12 @@ struct samsung_clk_provider * __init samsung_cmu_register_one(
}

ctx = samsung_clk_init(NULL, reg_base, cmu->nr_clk_ids);
+ samsung_cmu_register_clocks(ctx, cmu);

- if (cmu->pll_clks)
- samsung_clk_register_pll(ctx, cmu->pll_clks, cmu->nr_pll_clks);
- if (cmu->mux_clks)
- samsung_clk_register_mux(ctx, cmu->mux_clks,
- cmu->nr_mux_clks);
- if (cmu->div_clks)
- samsung_clk_register_div(ctx, cmu->div_clks, cmu->nr_div_clks);
- if (cmu->gate_clks)
- samsung_clk_register_gate(ctx, cmu->gate_clks,
- cmu->nr_gate_clks);
- if (cmu->fixed_clks)
- samsung_clk_register_fixed_rate(ctx, cmu->fixed_clks,
- cmu->nr_fixed_clks);
- if (cmu->fixed_factor_clks)
- samsung_clk_register_fixed_factor(ctx, cmu->fixed_factor_clks,
- cmu->nr_fixed_factor_clks);
if (cmu->clk_regs)
samsung_clk_extended_sleep_init(reg_base,
cmu->clk_regs, cmu->nr_clk_regs,
cmu->suspend_regs, cmu->nr_suspend_regs);
- if (cmu->cpu_clks)
- samsung_clk_register_cpu(ctx, cmu->cpu_clks, cmu->nr_cpu_clks);

samsung_clk_of_add_provider(np, ctx);

diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 3fd6c0868921..ab9c3d7a25b3 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -377,6 +377,8 @@ void samsung_clk_register_pll(struct samsung_clk_provider *ctx,
void samsung_clk_register_cpu(struct samsung_clk_provider *ctx,
const struct samsung_cpu_clock *list, unsigned int nr_clk);

+void samsung_cmu_register_clocks(struct samsung_clk_provider *ctx,
+ const struct samsung_cmu_info *cmu);
struct samsung_clk_provider *samsung_cmu_register_one(
struct device_node *,
const struct samsung_cmu_info *);
--
2.39.0


2023-02-03 06:09:31

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 5/6] clk: samsung: Extract parent clock enabling to common function

Extract parent clock enabling from exynos_arm64_register_cmu() to
dedicated function. No functional change.

No functional change.

Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk-exynos-arm64.c | 53 +++++++++++++++++---------
1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
index b921b9a1134a..361663223a24 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.c
+++ b/drivers/clk/samsung/clk-exynos-arm64.c
@@ -56,6 +56,41 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
iounmap(reg_base);
}

+/**
+ * exynos_arm64_enable_bus_clk - Enable parent clock of specified CMU
+ *
+ * @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
+ *
+ * Keep CMU parent clock running (needed for CMU registers access).
+ *
+ * Return: 0 on success or negative error code on failure.
+ */
+static int __init exynos_arm64_enable_bus_clk(struct device *dev,
+ struct device_node *np, const struct samsung_cmu_info *cmu)
+{
+ struct clk *parent_clk;
+
+ if (!cmu->clk_name)
+ return 0;
+
+ if (dev)
+ parent_clk = clk_get(dev, cmu->clk_name);
+ else
+ parent_clk = of_clk_get_by_name(np, cmu->clk_name);
+
+ if (IS_ERR(parent_clk)) {
+ pr_err("%s: could not find bus clock %s; err = %ld\n",
+ __func__, cmu->clk_name, PTR_ERR(parent_clk));
+ return PTR_ERR(parent_clk);
+ }
+
+ clk_prepare_enable(parent_clk);
+ return 0;
+}
+
/**
* exynos_arm64_register_cmu - Register specified Exynos CMU domain
* @dev: Device object; may be NULL if this function is not being
@@ -72,23 +107,7 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
void __init exynos_arm64_register_cmu(struct device *dev,
struct device_node *np, const struct samsung_cmu_info *cmu)
{
- /* Keep CMU parent clock running (needed for CMU registers access) */
- if (cmu->clk_name) {
- struct clk *parent_clk;
-
- if (dev)
- parent_clk = clk_get(dev, cmu->clk_name);
- else
- parent_clk = of_clk_get_by_name(np, cmu->clk_name);
-
- if (IS_ERR(parent_clk)) {
- pr_err("%s: could not find bus clock %s; err = %ld\n",
- __func__, cmu->clk_name, PTR_ERR(parent_clk));
- } else {
- clk_prepare_enable(parent_clk);
- }
- }
-
+ exynos_arm64_enable_bus_clk(dev, np, cmu);
exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
samsung_cmu_register_one(np, cmu);
}
--
2.39.0


2023-02-03 06:09:38

by Sam Protsenko

[permalink] [raw]
Subject: [PATCH 6/6] clk: samsung: exynos5433: Extract PM support to common ARM64 layer

Exynos5433 clock driver implements PM support internally, which might be
also useful for other Exynos clock drivers. Extract all PM related code
from clk-exynos5433 to common ARM64 functions.

Signed-off-by: Sam Protsenko <[email protected]>
---
drivers/clk/samsung/clk-exynos-arm64.c | 170 ++++++++++++++++++++++++-
drivers/clk/samsung/clk-exynos-arm64.h | 3 +
drivers/clk/samsung/clk-exynos5433.c | 157 +----------------------
3 files changed, 174 insertions(+), 156 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
index 361663223a24..4479c1c54dd2 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.c
+++ b/drivers/clk/samsung/clk-exynos-arm64.c
@@ -10,6 +10,8 @@
*/
#include <linux/clk.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/pm_runtime.h>

#include "clk-exynos-arm64.h"

@@ -21,6 +23,19 @@
#define GATE_OFF_START 0x2000
#define GATE_OFF_END 0x2fff

+struct exynos_arm64_cmu_data {
+ struct samsung_clk_reg_dump *clk_save;
+ unsigned int nr_clk_save;
+ const struct samsung_clk_reg_dump *clk_suspend;
+ unsigned int nr_clk_suspend;
+
+ struct clk *clk;
+ struct clk **pclks;
+ int nr_pclks;
+
+ struct samsung_clk_provider *ctx;
+};
+
/**
* exynos_arm64_init_clocks - Set clocks initial configuration
* @np: CMU device tree node with "reg" property (CMU addr)
@@ -76,10 +91,16 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
if (!cmu->clk_name)
return 0;

- if (dev)
+ if (dev) {
+ struct exynos_arm64_cmu_data *data;
+
parent_clk = clk_get(dev, cmu->clk_name);
- else
+ data = dev_get_drvdata(dev);
+ if (data)
+ data->clk = parent_clk;
+ } else {
parent_clk = of_clk_get_by_name(np, cmu->clk_name);
+ }

if (IS_ERR(parent_clk)) {
pr_err("%s: could not find bus clock %s; err = %ld\n",
@@ -91,6 +112,46 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
return 0;
}

+static int __init exynos_arm64_cmu_prepare_pm(struct device *dev,
+ const struct samsung_cmu_info *cmu)
+{
+ struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
+ int i;
+
+ data->clk_save = samsung_clk_alloc_reg_dump(cmu->clk_regs,
+ cmu->nr_clk_regs);
+ if (!data->clk_save)
+ return -ENOMEM;
+
+ data->nr_clk_save = cmu->nr_clk_regs;
+ data->clk_suspend = cmu->suspend_regs;
+ data->nr_clk_suspend = cmu->nr_suspend_regs;
+ data->nr_pclks = of_clk_get_parent_count(dev->of_node);
+ if (!data->nr_pclks)
+ return 0;
+
+ data->pclks = devm_kcalloc(dev, sizeof(struct clk *), data->nr_pclks,
+ GFP_KERNEL);
+ if (!data->pclks) {
+ kfree(data->clk_save);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < data->nr_pclks; i++) {
+ struct clk *clk = of_clk_get(dev->of_node, i);
+
+ if (IS_ERR(clk)) {
+ kfree(data->clk_save);
+ while (--i >= 0)
+ clk_put(data->pclks[i]);
+ return PTR_ERR(clk);
+ }
+ data->pclks[i] = clk;
+ }
+
+ return 0;
+}
+
/**
* exynos_arm64_register_cmu - Register specified Exynos CMU domain
* @dev: Device object; may be NULL if this function is not being
@@ -111,3 +172,108 @@ void __init exynos_arm64_register_cmu(struct device *dev,
exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
samsung_cmu_register_one(np, cmu);
}
+
+/**
+ * exynos_arm64_register_cmu_pm - Register Exynos CMU domain with PM support
+ *
+ * @pdev: Platform device object
+ * @set_manual: If true, set gate clocks to manual mode
+ *
+ * It's a version of exynos_arm64_register_cmu() with PM support. Should be
+ * called from probe function of platform driver.
+ *
+ * Return: 0 on success, or negative error code on error.
+ */
+int __init exynos_arm64_register_cmu_pm(struct platform_device *pdev,
+ bool set_manual)
+{
+ const struct samsung_cmu_info *cmu;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct exynos_arm64_cmu_data *data;
+ void __iomem *reg_base;
+ int ret;
+
+ cmu = of_device_get_match_data(dev);
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, data);
+
+ ret = exynos_arm64_cmu_prepare_pm(dev, cmu);
+ if (ret)
+ return ret;
+
+ ret = exynos_arm64_enable_bus_clk(dev, NULL, cmu);
+ if (ret)
+ return ret;
+
+ if (set_manual)
+ exynos_arm64_init_clocks(np, cmu->clk_regs, cmu->nr_clk_regs);
+
+ reg_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(reg_base))
+ return PTR_ERR(reg_base);
+
+ data->ctx = samsung_clk_init(dev, reg_base, cmu->nr_clk_ids);
+
+ /*
+ * Enable runtime PM here to allow the clock core using runtime PM
+ * for the registered clocks. Additionally, we increase the runtime
+ * PM usage count before registering the clocks, to prevent the
+ * clock core from runtime suspending the device.
+ */
+ pm_runtime_get_noresume(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
+ samsung_cmu_register_clocks(data->ctx, cmu);
+ samsung_clk_of_add_provider(dev->of_node, data->ctx);
+ pm_runtime_put_sync(dev);
+
+ return 0;
+}
+
+int exynos_arm64_cmu_suspend(struct device *dev)
+{
+ struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
+ int i;
+
+ samsung_clk_save(data->ctx->reg_base, data->clk_save,
+ data->nr_clk_save);
+
+ for (i = 0; i < data->nr_pclks; i++)
+ clk_prepare_enable(data->pclks[i]);
+
+ /* For suspend some registers have to be set to certain values */
+ samsung_clk_restore(data->ctx->reg_base, data->clk_suspend,
+ data->nr_clk_suspend);
+
+ for (i = 0; i < data->nr_pclks; i++)
+ clk_disable_unprepare(data->pclks[i]);
+
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+int exynos_arm64_cmu_resume(struct device *dev)
+{
+ struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
+ int i;
+
+ clk_prepare_enable(data->clk);
+
+ for (i = 0; i < data->nr_pclks; i++)
+ clk_prepare_enable(data->pclks[i]);
+
+ samsung_clk_restore(data->ctx->reg_base, data->clk_save,
+ data->nr_clk_save);
+
+ for (i = 0; i < data->nr_pclks; i++)
+ clk_disable_unprepare(data->pclks[i]);
+
+ return 0;
+}
diff --git a/drivers/clk/samsung/clk-exynos-arm64.h b/drivers/clk/samsung/clk-exynos-arm64.h
index 0dd174693935..969979e714bc 100644
--- a/drivers/clk/samsung/clk-exynos-arm64.h
+++ b/drivers/clk/samsung/clk-exynos-arm64.h
@@ -16,5 +16,8 @@

void exynos_arm64_register_cmu(struct device *dev,
struct device_node *np, const struct samsung_cmu_info *cmu);
+int exynos_arm64_register_cmu_pm(struct platform_device *pdev, bool set_manual);
+int exynos_arm64_cmu_suspend(struct device *dev);
+int exynos_arm64_cmu_resume(struct device *dev);

#endif /* __CLK_EXYNOS_ARM64_H */
diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index eb72bf2aaee8..ed43233649ae 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -10,7 +10,6 @@
#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 <linux/pm_runtime.h>
#include <linux/slab.h>
@@ -19,6 +18,7 @@

#include "clk.h"
#include "clk-cpu.h"
+#include "clk-exynos-arm64.h"
#include "clk-pll.h"

/*
@@ -5478,160 +5478,9 @@ static const struct samsung_cmu_info imem_cmu_info __initconst = {
.clk_name = "aclk_imem_200",
};

-struct exynos5433_cmu_data {
- struct samsung_clk_reg_dump *clk_save;
- unsigned int nr_clk_save;
- const struct samsung_clk_reg_dump *clk_suspend;
- unsigned int nr_clk_suspend;
-
- struct clk *clk;
- struct clk **pclks;
- int nr_pclks;
-
- /* must be the last entry */
- struct samsung_clk_provider ctx;
-};
-
-static int __maybe_unused exynos5433_cmu_suspend(struct device *dev)
-{
- struct exynos5433_cmu_data *data = dev_get_drvdata(dev);
- int i;
-
- samsung_clk_save(data->ctx.reg_base, data->clk_save,
- data->nr_clk_save);
-
- for (i = 0; i < data->nr_pclks; i++)
- clk_prepare_enable(data->pclks[i]);
-
- /* for suspend some registers have to be set to certain values */
- samsung_clk_restore(data->ctx.reg_base, data->clk_suspend,
- data->nr_clk_suspend);
-
- for (i = 0; i < data->nr_pclks; i++)
- clk_disable_unprepare(data->pclks[i]);
-
- clk_disable_unprepare(data->clk);
-
- return 0;
-}
-
-static int __maybe_unused exynos5433_cmu_resume(struct device *dev)
-{
- struct exynos5433_cmu_data *data = dev_get_drvdata(dev);
- int i;
-
- clk_prepare_enable(data->clk);
-
- for (i = 0; i < data->nr_pclks; i++)
- clk_prepare_enable(data->pclks[i]);
-
- samsung_clk_restore(data->ctx.reg_base, data->clk_save,
- data->nr_clk_save);
-
- for (i = 0; i < data->nr_pclks; i++)
- clk_disable_unprepare(data->pclks[i]);
-
- return 0;
-}
-
static int __init exynos5433_cmu_probe(struct platform_device *pdev)
{
- const struct samsung_cmu_info *info;
- struct exynos5433_cmu_data *data;
- struct samsung_clk_provider *ctx;
- struct device *dev = &pdev->dev;
- void __iomem *reg_base;
- int i;
-
- info = of_device_get_match_data(dev);
-
- data = devm_kzalloc(dev,
- struct_size(data, ctx.clk_data.hws, info->nr_clk_ids),
- GFP_KERNEL);
- if (!data)
- return -ENOMEM;
- ctx = &data->ctx;
-
- reg_base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(reg_base))
- return PTR_ERR(reg_base);
-
- for (i = 0; i < info->nr_clk_ids; ++i)
- ctx->clk_data.hws[i] = ERR_PTR(-ENOENT);
-
- ctx->clk_data.num = info->nr_clk_ids;
- ctx->reg_base = reg_base;
- ctx->dev = dev;
- spin_lock_init(&ctx->lock);
-
- data->clk_save = samsung_clk_alloc_reg_dump(info->clk_regs,
- info->nr_clk_regs);
- if (!data->clk_save)
- return -ENOMEM;
- data->nr_clk_save = info->nr_clk_regs;
- data->clk_suspend = info->suspend_regs;
- data->nr_clk_suspend = info->nr_suspend_regs;
- data->nr_pclks = of_clk_get_parent_count(dev->of_node);
-
- if (data->nr_pclks > 0) {
- data->pclks = devm_kcalloc(dev, sizeof(struct clk *),
- data->nr_pclks, GFP_KERNEL);
- if (!data->pclks) {
- kfree(data->clk_save);
- return -ENOMEM;
- }
- for (i = 0; i < data->nr_pclks; i++) {
- struct clk *clk = of_clk_get(dev->of_node, i);
-
- if (IS_ERR(clk)) {
- kfree(data->clk_save);
- while (--i >= 0)
- clk_put(data->pclks[i]);
- return PTR_ERR(clk);
- }
- data->pclks[i] = clk;
- }
- }
-
- if (info->clk_name)
- data->clk = clk_get(dev, info->clk_name);
- clk_prepare_enable(data->clk);
-
- platform_set_drvdata(pdev, data);
-
- /*
- * Enable runtime PM here to allow the clock core using runtime PM
- * for the registered clocks. Additionally, we increase the runtime
- * PM usage count before registering the clocks, to prevent the
- * clock core from runtime suspending the device.
- */
- pm_runtime_get_noresume(dev);
- pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
-
- if (info->pll_clks)
- samsung_clk_register_pll(ctx, info->pll_clks,
- info->nr_pll_clks);
- if (info->mux_clks)
- samsung_clk_register_mux(ctx, info->mux_clks,
- info->nr_mux_clks);
- if (info->div_clks)
- samsung_clk_register_div(ctx, info->div_clks,
- info->nr_div_clks);
- if (info->gate_clks)
- samsung_clk_register_gate(ctx, info->gate_clks,
- info->nr_gate_clks);
- if (info->fixed_clks)
- samsung_clk_register_fixed_rate(ctx, info->fixed_clks,
- info->nr_fixed_clks);
- if (info->fixed_factor_clks)
- samsung_clk_register_fixed_factor(ctx, info->fixed_factor_clks,
- info->nr_fixed_factor_clks);
-
- samsung_clk_of_add_provider(dev->of_node, ctx);
- pm_runtime_put_sync(dev);
-
- return 0;
+ return exynos_arm64_register_cmu_pm(pdev, false);
}

static const struct of_device_id exynos5433_cmu_of_match[] = {
@@ -5679,7 +5528,7 @@ static const struct of_device_id exynos5433_cmu_of_match[] = {
};

static const struct dev_pm_ops exynos5433_cmu_pm_ops = {
- SET_RUNTIME_PM_OPS(exynos5433_cmu_suspend, exynos5433_cmu_resume,
+ SET_RUNTIME_PM_OPS(exynos_arm64_cmu_suspend, exynos_arm64_cmu_resume,
NULL)
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
--
2.39.0


2023-02-03 09:14:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: samsung: Extract parent clock enabling to common function

On 03/02/2023 07:09, Sam Protsenko wrote:
> Extract parent clock enabling from exynos_arm64_register_cmu() to
> dedicated function. No functional change.
>
> No functional change.
>
> Signed-off-by: Sam Protsenko <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos-arm64.c | 53 +++++++++++++++++---------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> index b921b9a1134a..361663223a24 100644
> --- a/drivers/clk/samsung/clk-exynos-arm64.c
> +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> @@ -56,6 +56,41 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> iounmap(reg_base);
> }
>
> +/**
> + * exynos_arm64_enable_bus_clk - Enable parent clock of specified CMU
> + *
> + * @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
> + *
> + * Keep CMU parent clock running (needed for CMU registers access).
> + *
> + * Return: 0 on success or negative error code on failure.
> + */
> +static int __init exynos_arm64_enable_bus_clk(struct device *dev,
> + struct device_node *np, const struct samsung_cmu_info *cmu)

Align the arguments.

> +{
> + struct clk *parent_clk;
> +
> + if (!cmu->clk_name)
> + return 0;
> +
> + if (dev)
> + parent_clk = clk_get(dev, cmu->clk_name);
> + else
> + parent_clk = of_clk_get_by_name(np, cmu->clk_name);
> +
> + if (IS_ERR(parent_clk)) {
> + pr_err("%s: could not find bus clock %s; err = %ld\n",
> + __func__, cmu->clk_name, PTR_ERR(parent_clk));
> + return PTR_ERR(parent_clk);
> + }
> +
> + clk_prepare_enable(parent_clk);
> + return 0;

You do not check the return value in exynos_arm64_register_cmu() below,
so either make it a void or add the check.


Best regards,
Krzysztof


2023-02-03 09:20:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 6/6] clk: samsung: exynos5433: Extract PM support to common ARM64 layer

On 03/02/2023 07:09, Sam Protsenko wrote:
> Exynos5433 clock driver implements PM support internally, which might be
> also useful for other Exynos clock drivers. Extract all PM related code
> from clk-exynos5433 to common ARM64 functions.
>
> Signed-off-by: Sam Protsenko <[email protected]>
> ---


>
> if (IS_ERR(parent_clk)) {
> pr_err("%s: could not find bus clock %s; err = %ld\n",
> @@ -91,6 +112,46 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
> return 0;
> }
>
> +static int __init exynos_arm64_cmu_prepare_pm(struct device *dev,
> + const struct samsung_cmu_info *cmu)

Align the arguments.

Rest looks good to me.

Best regards,
Krzysztof


2023-02-03 12:20:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/6] clk: samsung: exynos5433: Extract PM support to common ARM64 layer

Hi Sam,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on krzk/for-next]
[also build test ERROR on linus/master v6.2-rc6]
[cannot apply to next-20230203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Sam-Protsenko/clk-samsung-Don-t-pass-reg_base-to-samsung_clk_register_pll/20230203-141059
base: https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git for-next
patch link: https://lore.kernel.org/r/20230203060924.8257-7-semen.protsenko%40linaro.org
patch subject: [PATCH 6/6] clk: samsung: exynos5433: Extract PM support to common ARM64 layer
config: openrisc-randconfig-r003-20230202 (https://download.01.org/0day-ci/archive/20230203/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/7fc08f82f3096e6f1c747f00b9d56c029b0b7a0f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sam-Protsenko/clk-samsung-Don-t-pass-reg_base-to-samsung_clk_register_pll/20230203-141059
git checkout 7fc08f82f3096e6f1c747f00b9d56c029b0b7a0f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash drivers/clk/samsung/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/clk/samsung/clk-exynos-arm64.c: In function 'exynos_arm64_cmu_prepare_pm':
>> drivers/clk/samsung/clk-exynos-arm64.c:136:17: error: implicit declaration of function 'kfree'; did you mean 'vfree'? [-Werror=implicit-function-declaration]
136 | kfree(data->clk_save);
| ^~~~~
| vfree
cc1: some warnings being treated as errors


vim +136 drivers/clk/samsung/clk-exynos-arm64.c

114
115 static int __init exynos_arm64_cmu_prepare_pm(struct device *dev,
116 const struct samsung_cmu_info *cmu)
117 {
118 struct exynos_arm64_cmu_data *data = dev_get_drvdata(dev);
119 int i;
120
121 data->clk_save = samsung_clk_alloc_reg_dump(cmu->clk_regs,
122 cmu->nr_clk_regs);
123 if (!data->clk_save)
124 return -ENOMEM;
125
126 data->nr_clk_save = cmu->nr_clk_regs;
127 data->clk_suspend = cmu->suspend_regs;
128 data->nr_clk_suspend = cmu->nr_suspend_regs;
129 data->nr_pclks = of_clk_get_parent_count(dev->of_node);
130 if (!data->nr_pclks)
131 return 0;
132
133 data->pclks = devm_kcalloc(dev, sizeof(struct clk *), data->nr_pclks,
134 GFP_KERNEL);
135 if (!data->pclks) {
> 136 kfree(data->clk_save);
137 return -ENOMEM;
138 }
139
140 for (i = 0; i < data->nr_pclks; i++) {
141 struct clk *clk = of_clk_get(dev->of_node, i);
142
143 if (IS_ERR(clk)) {
144 kfree(data->clk_save);
145 while (--i >= 0)
146 clk_put(data->pclks[i]);
147 return PTR_ERR(clk);
148 }
149 data->pclks[i] = clk;
150 }
151
152 return 0;
153 }
154

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-02-03 14:33:54

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH 0/6] clk: samsung: Add PM support for ARM64 Exynos chips

On 03.02.2023 07:09, Sam Protsenko wrote:
> In order to prepare for PM enablement in clk-exynos850, common PM code
> was extracted from clk-exynos5433 to clk-exynos-arm64. Also some related
> cleanups were done prior to that. More specifically:
>
> - patches #1..5: cleanups
> - patch #6: PM code extraction
>
> During the extraction of the exynos5433_cmu_probe() content to
> exynos_arm64_register_cmu_pm() some code was reworked a bit, and also
> split into smaller functions. In particular:
>
> - cmu_data structure now contains a pointer to ctx, which is now
> allocated in samsung_clk_init()
> - cmu_data structure initialization was moved into separate function
> - code for configuring gate clocks was added (optional)
>
> Which in turn resulted in somehow modified code of probe function:
>
> Original
> --------
>
> ...
> devm_platform_ioremap_resource(...);
> samsung_clk_init(...);
> exynos_arm64_cmu_prepare_pm(...);
> exynos_arm64_enable_bus_clk(...);
> platform_set_drvdata(...);
> ...
>
> Modified
> --------
>
> ...
> platform_set_drvdata(...);
> exynos_arm64_cmu_prepare_pm(...);
> exynos_arm64_enable_bus_clk(...);
> exynos_arm64_init_clocks(...);
> devm_platform_ioremap_resource(...);
> samsung_clk_init(...);
> ...
>
> That shouldn't really change the logic or mode of operation. It was
> preliminary tested on Exynos850 based board, with some extra patches on
> top of this series (will be submitted later).
>
> Marek, could you please test these patches on Exynos5433 platform, and
> also review accordingly?

I've tested and Exynos5433 still works fine after applying your changes.
Btw, you should probably rebase your changes onto arm-soc tree (or
recent linux-next), as there are some conflicts between your changes and
the s3c24xx removal merged there.

Feel free to add:

Tested-by: Marek Szyprowski <[email protected]>

I will try to review the individual patches on Monday though.


> Thanks!
>
> Sam Protsenko (6):
> clk: samsung: Don't pass reg_base to samsung_clk_register_pll()
> clk: samsung: Remove np argument from samsung_clk_init()
> clk: samsung: Set dev in samsung_clk_init()
> clk: samsung: Extract clocks registration to common function
> clk: samsung: Extract parent clock enabling to common function
> clk: samsung: exynos5433: Extract PM support to common ARM64 layer
>
> drivers/clk/samsung/clk-exynos-arm64.c | 219 +++++++++++++++++++++--
> drivers/clk/samsung/clk-exynos-arm64.h | 3 +
> drivers/clk/samsung/clk-exynos4.c | 6 +-
> drivers/clk/samsung/clk-exynos4412-isp.c | 3 +-
> drivers/clk/samsung/clk-exynos5250.c | 5 +-
> drivers/clk/samsung/clk-exynos5420.c | 5 +-
> drivers/clk/samsung/clk-exynos5433.c | 157 +---------------
> drivers/clk/samsung/clk-pll.c | 11 +-
> drivers/clk/samsung/clk-s3c2410.c | 6 +-
> drivers/clk/samsung/clk-s3c2412.c | 5 +-
> drivers/clk/samsung/clk-s3c2443.c | 6 +-
> drivers/clk/samsung/clk-s3c64xx.c | 4 +-
> drivers/clk/samsung/clk-s5pv210.c | 6 +-
> drivers/clk/samsung/clk.c | 64 ++++---
> drivers/clk/samsung/clk.h | 10 +-
> 15 files changed, 282 insertions(+), 228 deletions(-)
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland


2023-02-08 06:00:33

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: samsung: Extract parent clock enabling to common function

On Fri, 3 Feb 2023 at 03:14, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 03/02/2023 07:09, Sam Protsenko wrote:
> > Extract parent clock enabling from exynos_arm64_register_cmu() to
> > dedicated function. No functional change.
> >
> > No functional change.
> >
> > Signed-off-by: Sam Protsenko <[email protected]>
> > ---
> > drivers/clk/samsung/clk-exynos-arm64.c | 53 +++++++++++++++++---------
> > 1 file changed, 36 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/clk/samsung/clk-exynos-arm64.c b/drivers/clk/samsung/clk-exynos-arm64.c
> > index b921b9a1134a..361663223a24 100644
> > --- a/drivers/clk/samsung/clk-exynos-arm64.c
> > +++ b/drivers/clk/samsung/clk-exynos-arm64.c
> > @@ -56,6 +56,41 @@ static void __init exynos_arm64_init_clocks(struct device_node *np,
> > iounmap(reg_base);
> > }
> >
> > +/**
> > + * exynos_arm64_enable_bus_clk - Enable parent clock of specified CMU
> > + *
> > + * @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
> > + *
> > + * Keep CMU parent clock running (needed for CMU registers access).
> > + *
> > + * Return: 0 on success or negative error code on failure.
> > + */
> > +static int __init exynos_arm64_enable_bus_clk(struct device *dev,
> > + struct device_node *np, const struct samsung_cmu_info *cmu)
>
> Align the arguments.
>

In this particular case, if I align arguments to match the open
parentheses, the last argument declaration won't fit the 80 characters
limit. I know it's not a strict requirement anymore, but the whole
point of breaking the line is to kind of keep it short. So I suggest
keeping this one as is, if you don't have a strong opinion about it.

> > +{
> > + struct clk *parent_clk;
> > +
> > + if (!cmu->clk_name)
> > + return 0;
> > +
> > + if (dev)
> > + parent_clk = clk_get(dev, cmu->clk_name);
> > + else
> > + parent_clk = of_clk_get_by_name(np, cmu->clk_name);
> > +
> > + if (IS_ERR(parent_clk)) {
> > + pr_err("%s: could not find bus clock %s; err = %ld\n",
> > + __func__, cmu->clk_name, PTR_ERR(parent_clk));
> > + return PTR_ERR(parent_clk);
> > + }
> > +
> > + clk_prepare_enable(parent_clk);
> > + return 0;
>
> You do not check the return value in exynos_arm64_register_cmu() below,
> so either make it a void or add the check.
>

Thanks, will add the check in v2.

>
> Best regards,
> Krzysztof
>

2023-02-08 06:05:24

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 0/6] clk: samsung: Add PM support for ARM64 Exynos chips

On Fri, 3 Feb 2023 at 08:33, Marek Szyprowski <[email protected]> wrote:
>
> On 03.02.2023 07:09, Sam Protsenko wrote:
> > In order to prepare for PM enablement in clk-exynos850, common PM code
> > was extracted from clk-exynos5433 to clk-exynos-arm64. Also some related
> > cleanups were done prior to that. More specifically:
> >
> > - patches #1..5: cleanups
> > - patch #6: PM code extraction
> >
> > During the extraction of the exynos5433_cmu_probe() content to
> > exynos_arm64_register_cmu_pm() some code was reworked a bit, and also
> > split into smaller functions. In particular:
> >
> > - cmu_data structure now contains a pointer to ctx, which is now
> > allocated in samsung_clk_init()
> > - cmu_data structure initialization was moved into separate function
> > - code for configuring gate clocks was added (optional)
> >
> > Which in turn resulted in somehow modified code of probe function:
> >
> > Original
> > --------
> >
> > ...
> > devm_platform_ioremap_resource(...);
> > samsung_clk_init(...);
> > exynos_arm64_cmu_prepare_pm(...);
> > exynos_arm64_enable_bus_clk(...);
> > platform_set_drvdata(...);
> > ...
> >
> > Modified
> > --------
> >
> > ...
> > platform_set_drvdata(...);
> > exynos_arm64_cmu_prepare_pm(...);
> > exynos_arm64_enable_bus_clk(...);
> > exynos_arm64_init_clocks(...);
> > devm_platform_ioremap_resource(...);
> > samsung_clk_init(...);
> > ...
> >
> > That shouldn't really change the logic or mode of operation. It was
> > preliminary tested on Exynos850 based board, with some extra patches on
> > top of this series (will be submitted later).
> >
> > Marek, could you please test these patches on Exynos5433 platform, and
> > also review accordingly?
>
> I've tested and Exynos5433 still works fine after applying your changes.
> Btw, you should probably rebase your changes onto arm-soc tree (or
> recent linux-next), as there are some conflicts between your changes and
> the s3c24xx removal merged there.
>

Thanks, Marek! Will send v2 soon, which will be rebased on top of
soc/for-next tree.

> Feel free to add:
>
> Tested-by: Marek Szyprowski <[email protected]>
>

I have some (small) functional changes in v2, so I guess it'll nice
ideally to re-test those first, if possible.

> I will try to review the individual patches on Monday though.
>
>
> > Thanks!
> >
> > Sam Protsenko (6):
> > clk: samsung: Don't pass reg_base to samsung_clk_register_pll()
> > clk: samsung: Remove np argument from samsung_clk_init()
> > clk: samsung: Set dev in samsung_clk_init()
> > clk: samsung: Extract clocks registration to common function
> > clk: samsung: Extract parent clock enabling to common function
> > clk: samsung: exynos5433: Extract PM support to common ARM64 layer
> >
> > drivers/clk/samsung/clk-exynos-arm64.c | 219 +++++++++++++++++++++--
> > drivers/clk/samsung/clk-exynos-arm64.h | 3 +
> > drivers/clk/samsung/clk-exynos4.c | 6 +-
> > drivers/clk/samsung/clk-exynos4412-isp.c | 3 +-
> > drivers/clk/samsung/clk-exynos5250.c | 5 +-
> > drivers/clk/samsung/clk-exynos5420.c | 5 +-
> > drivers/clk/samsung/clk-exynos5433.c | 157 +---------------
> > drivers/clk/samsung/clk-pll.c | 11 +-
> > drivers/clk/samsung/clk-s3c2410.c | 6 +-
> > drivers/clk/samsung/clk-s3c2412.c | 5 +-
> > drivers/clk/samsung/clk-s3c2443.c | 6 +-
> > drivers/clk/samsung/clk-s3c64xx.c | 4 +-
> > drivers/clk/samsung/clk-s5pv210.c | 6 +-
> > drivers/clk/samsung/clk.c | 64 ++++---
> > drivers/clk/samsung/clk.h | 10 +-
> > 15 files changed, 282 insertions(+), 228 deletions(-)
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

2023-02-08 06:08:15

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 6/6] clk: samsung: exynos5433: Extract PM support to common ARM64 layer

On Fri, 3 Feb 2023 at 03:18, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 03/02/2023 07:09, Sam Protsenko wrote:
> > Exynos5433 clock driver implements PM support internally, which might be
> > also useful for other Exynos clock drivers. Extract all PM related code
> > from clk-exynos5433 to common ARM64 functions.
> >
> > Signed-off-by: Sam Protsenko <[email protected]>
> > ---
>
>
> >
> > if (IS_ERR(parent_clk)) {
> > pr_err("%s: could not find bus clock %s; err = %ld\n",
> > @@ -91,6 +112,46 @@ static int __init exynos_arm64_enable_bus_clk(struct device *dev,
> > return 0;
> > }
> >
> > +static int __init exynos_arm64_cmu_prepare_pm(struct device *dev,
> > + const struct samsung_cmu_info *cmu)
>
> Align the arguments.
>

The same issue here as in my previous answer, unfortunately: when I
try to align the `cmu' argument to match the open parentheses, it
doesn't fit 80 characters limit, which doesn't look nice to me. Do you
mind if I leave it as is?

> Rest looks good to me.
>
> Best regards,
> Krzysztof
>