[PATCH v4 4/5] clk: hi6220: Clock driver support for Hisilicon hi6220 SoC
From: Stephen Boyd <hidden>
Date: 2015-05-15 00:25:33
Also in:
linux-devicetree, lkml
On 05/05, Bintian Wang wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 9897f35..935c44b 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig@@ -152,6 +152,8 @@ config COMMON_CLK_CDCE706 source "drivers/clk/qcom/Kconfig" +source "drivers/clk/hisilicon/Kconfig" +
Please move this above qcom to maintain alphabet sort.
quoted hunk ↗ jump to hunk
endmenu source "drivers/clk/bcm/Kconfig"diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig new file mode 100644 index 0000000..8034739 --- /dev/null +++ b/drivers/clk/hisilicon/Kconfig@@ -0,0 +1,6 @@ +config COMMON_CLK_HI6220 + bool "Hi6220 Clock Driver" + depends on OF && ARCH_HISI + default y
Can this be depends on ARCH_HISI || COMPILE_TEST default ARCH_HISI instead? I'd like to increase build coverage.
quoted hunk ↗ jump to hunk
+ help + Build the Hisilicon Hi6220 clock driver based on the common clock framework.diff --git a/drivers/clk/hisilicon/clk-hi6220.c b/drivers/clk/hisilicon/clk-hi6220.c new file mode 100644 index 0000000..91b1cd7 --- /dev/null +++ b/drivers/clk/hisilicon/clk-hi6220.c@@ -0,0 +1,292 @@ +/* + * Hisilicon Hi6220 clock driver + * + * Copyright (c) 2015 Hisilicon Limited. + * + * Author: Bintian Wang <bintian.wang@huawei.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_device.h> +#include <linux/slab.h> +#include <linux/clk.h>
Do we need to include linux/clk.h? I don't see any consumer usage here.
quoted hunk ↗ jump to hunk
+ +#include <dt-bindings/clock/hi6220-clock.h> + +#include "clk.h" +diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c index a078e84..5d2305c 100644 --- a/drivers/clk/hisilicon/clk.c +++ b/drivers/clk/hisilicon/clk.c@@ -108,4 +123,6 @@ void __init hisi_clk_register_gate(struct hisi_gate_clock *, int, struct hisi_clock_data *); void __init hisi_clk_register_gate_sep(struct hisi_gate_clock *, int, struct hisi_clock_data *); +void __init hi6220_clk_register_divider(struct hi6220_divider_clock *, + int, struct hisi_clock_data *);
__init markings on function prototypes are useless. Please remove them.
quoted hunk ↗ jump to hunk
#endif /* __HISI_CLK_H */diff --git a/drivers/clk/hisilicon/clkdivider-hi6220.c b/drivers/clk/hisilicon/clkdivider-hi6220.c + +/** + * struct hi6220_clk_divider - divider clock for hi6220 + * + * @hw: handle between common and hardware-specific interfaces + * @reg: register containing divider + * @shift: shift to the divider bit field + * @width: width of the divider bit field + * @mask: mask for setting divider rate + * @table: the div table that the divider supports + * @lock: register lock + */ +struct hi6220_clk_divider { + struct clk_hw hw; + void __iomem *reg; + u8 shift; + u8 width; + u32 mask; + const struct clk_div_table *table; + spinlock_t *lock; +};
The clk-divider.c code has been made "reusable". Can you please try to use the functions that it now exposes instead of copy/pasting it and modifying it to suit your needs? A lot of this code looks the same.
+ +#define to_hi6220_clk_divider(_hw) \
[..]
+
+static struct clk_ops hi6220_clkdiv_ops = {const?
+ .recalc_rate = hi6220_clkdiv_recalc_rate,
+ .round_rate = hi6220_clkdiv_round_rate,
+ .set_rate = hi6220_clkdiv_set_rate,
+};
+
+struct clk *hi6220_register_clkdiv(struct device *dev, const char *name,
+ const char *parent_name, unsigned long flags, void __iomem *reg,
+ u8 shift, u8 width, u32 mask_bit, spinlock_t *lock)
+{
+ struct hi6220_clk_divider *div;
+ struct clk *clk;
+ struct clk_init_data init;
+ struct clk_div_table *table;
+ u32 max_div, min_div;
+ int i;
+
+ /* allocate the divider */
+ div = kzalloc(sizeof(struct hi6220_clk_divider), GFP_KERNEL);
+ if (!div) {
+ pr_err("%s: could not allocate divider clk\n", __func__);Useless error message on allocation failure. Please run checkpatch. I've sent a patch to remove these from basic clock types.
+ return ERR_PTR(-ENOMEM);
+ }
+
+ /* Init the divider table */
+ max_div = div_mask(width) + 1;
+ min_div = 1;
+
+ table = kzalloc(sizeof(struct clk_div_table) * (max_div + 1), GFP_KERNEL);
+ if (!table) {
+ kfree(div);
+ pr_err("%s: failed to alloc divider table!\n", __func__);ditto.
+ return ERR_PTR(-ENOMEM);
+ }
+
+ for (i = 0; i < max_div; i++) {
+ table[i].div = min_div + i;
+ table[i].val = table[i].div - 1;
+ }
+
+ init.name = name;
+ init.ops = &hi6220_clkdiv_ops;
+ init.flags = flags | CLK_IS_BASIC;It's basic?
+ init.parent_names = parent_name ? &parent_name : NULL; + init.num_parents = parent_name ? 1 : 0;
-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project