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

["Zhou Qiao(周侨)" <qiaozhou@asrmicro.com>]

On 2019/3/30 上午6:18, Stephen Boyd wrote:
> 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.
I'll change it.
>
>> +
>> +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?
It could be a platform driver. We wrote the code by taking a reference
of Hisicon/Samsung clock drivers, and wrote it like this. I can change
it to a platform driver, just that I don't know the benefits that it's a
platform driver though. Is there any guide? Thanks.
>
>> 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'll fix it.
>
>> + */
>> +
> I'm not going to review anymore as I'll wait for a resend for Dan's
> comments.
Thanks a lot for your reviewing. I'll refine and resend the patch set.
>
>