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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <155389787963.20095.9145169286940814698@swboyd.mtv.corp.google.com>
Date:   Fri, 29 Mar 2019 15:17:59 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Dan Carpenter <dan.carpenter@...cle.com>,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        qiaozhou <qiaozhou@...micro.com>
Cc:     Qiao Zhou <qiaozhou@...micro.com>
Subject: Re: [PATCH] clk: asr: clock driver support for ASR AquilaC Soc

Quoting qiaozhou (2019-03-23 07:08:35)
> From: Qiao Zhou <qiaozhou@...micro.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@...micro.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@...micro.com>
> + * Qiao Zhou <qiaozhou@...micro.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.