On Fri, 3 Feb 2023 at 03:14, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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@xxxxxxxxxx> > > --- > > 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 >