Re: [PATCH] clk: asr: clock driver support for ASR AquilaC Soc

[Stephen Boyd <sboyd@kernel.org>]

Quoting qiaozhou (2019-03-23 07:08:35)
> From: Qiao Zhou <qiaozhou@asrmicro.com>
> 
> add clock driver support for ASR AquilaC SoC.
> 
> We add clk-gate, clk-mix, and clk-pll drivers:
> 1. clk-gate driver is for regisers which have different enable/disable bits
> to control gating.
> 2. clk-mix driver is for registers which request to set div and mux
> bits at the same time.
> 3. clk-pll driver is for pll configuration.
> 
> Signed-off-by: qiaozhou <qiaozhou@asrmicro.com>

Please make this match your "From" line above.

> +
> +static void __init aquilac_clk_init(struct device_node *np)
> +{
> +       int ret;
> +       struct asr_clk_data *clock_data;
> +
> +       clock_data = kzalloc(sizeof(*clock_data), GFP_KERNEL);
> +       if (!clock_data)
> +               return;
> +
> +       ret = aquilac_clk_of_iomap(np, clock_data);
> +       if (ret < 0)
> +               goto out;
> +
> +       ret = asr_clk_init(np, &clock_data->unit, ASR_NR_CLKS);
> +       if (ret < 0)
> +               goto out;
> +
> +       aquilac_general_clk_init(clock_data);
> +
> +       aquilac_pll_init(clock_data);
> +
> +       aquilac_mix_clk_init(clock_data);
> +
> +       aquilac_periph_clk_init(clock_data);
> +
> +       asr_clks_enable((const char **)keep_on_clocks_tbl, ARRAY_SIZE(keep_on_clocks_tbl));
> +
> +       return;
> +out:
> +       kfree(clock_data);
> +}
> +CLK_OF_DECLARE(aquilac_clk, "asr,8751c-clock", aquilac_clk_init);

Any reason this can't be a platform driver?

> diff --git a/drivers/clk/asr/clk-gate.c b/drivers/clk/asr/clk-gate.c
> new file mode 100644
> index 0000000..4ba5587
> --- /dev/null
> +++ b/drivers/clk/asr/clk-gate.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * asr gate clock operation source file
> + *
> + * Copyright (C) 2019 ASR Microelectronics(Shanghai) Co., Ltd.
> + * Gang Wu <gangwu@asrmicro.com>
> + * Qiao Zhou <qiaozhou@asrmicro.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.

Please only have the SPDX tag and not this above paragraph.

> + */
> +

I'm not going to review anymore as I'll wait for a resend for Dan's
comments.