| Message ID | 20230203060924.8257-6-semen.protsenko@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: samsung: Add PM support for ARM64 Exynos chips | expand |
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 <semen.protsenko@linaro.org> > --- > 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
On Fri, 3 Feb 2023 at 03:14, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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 <semen.protsenko@linaro.org> > > --- > > 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 >
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); }
Extract parent clock enabling from exynos_arm64_register_cmu() to dedicated function. No functional change. No functional change. Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- drivers/clk/samsung/clk-exynos-arm64.c | 53 +++++++++++++++++--------- 1 file changed, 36 insertions(+), 17 deletions(-)