diff mbox series

[RFC,1/5] mips: ralink: add rt2880-clock driver

Message ID 20190330123317.16821-2-drvlabo@gmail.com (mailing list archive)
State Superseded
Headers show
Series MIPS: ralink: peripheral clock gating driver | expand

Commit Message

NOGUCHI Hiroshi March 30, 2019, 12:33 p.m. UTC 
This patch adds SoC peripheral clock gating driver.
The driver loads gating clock table from of_device_id.data in individual SoC sources.

Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
---
 arch/mips/ralink/rt2880-clk_internal.h |  21 ++++
 arch/mips/ralink/rt2880-clock.c        | 134 +++++++++++++++++++++++++
 2 files changed, 155 insertions(+)
 create mode 100644 arch/mips/ralink/rt2880-clk_internal.h
 create mode 100644 arch/mips/ralink/rt2880-clock.c

Comments

Stephen Boyd April 2, 2019, 8:08 p.m. UTC | #1 
Quoting NOGUCHI Hiroshi (2019-03-30 05:33:13)
> This patch adds SoC peripheral clock gating driver.
> The driver loads gating clock table from of_device_id.data in individual SoC sources.

Please line wrap this to the column length standard of something like 76
columns.

> 
> Signed-off-by: NOGUCHI Hiroshi <drvlabo@gmail.com>
> diff --git a/arch/mips/ralink/rt2880-clk_internal.h b/arch/mips/ralink/rt2880-clk_internal.h
> new file mode 100644
> index 000000000000..9d5dded16a80
> --- /dev/null
> +++ b/arch/mips/ralink/rt2880-clk_internal.h
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
> + */
> +
> +#ifndef __RT2880_CLOCK_INTERNAL_H
> +
> +
> +#define GATE_CLK_NUM   (32)
> +
> +struct gate_clk_desc {
> +       const char *name;
> +       const char *parent_name;
> +};
> +
> +extern const struct of_device_id __initconst of_match_rt2880_clk[];

Why is it extern?

> +
> +
> +#endif
> +
> +
> diff --git a/arch/mips/ralink/rt2880-clock.c b/arch/mips/ralink/rt2880-clock.c
> new file mode 100644
> index 000000000000..46cc067225ab
> --- /dev/null
> +++ b/arch/mips/ralink/rt2880-clock.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>

Used?

> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/clk.h>

Used?

> +#include <linux/clkdev.h>

Used?

> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/bug.h>

Drop? Don't use panic().

> +
> +#include "rt2880-clk_internal.h"
> +
> +
> +/* clock configuration 1 */
> +#define        SYSC_REG_CLKCFG1        0x30
> +
> +struct rt2880_gate {
> +       struct clk_hw   hw;
> +       u8      shift;
> +};
> +
> +#define        to_rt2880_gate(_hw)     container_of(_hw, struct rt2880_gate, hw)
> +
> +static struct clk_onecell_data clk_data;
> +static struct clk *clks[GATE_CLK_NUM];

Why does it need to be static? Why not allocate at runtime?

> +
> +static struct regmap *syscon_regmap;
> +
> +static int rt2880_gate_enable(struct clk_hw *hw)
> +{
> +       struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> +       u32 val = 0x01UL << clk_gate->shift;

Just write BIT(clk_gate->shift) instead.

> +
> +       regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, val);

return regmap_udpate_bits()?

> +
> +       return 0;
> +}
> +
> +static void rt2880_gate_disable(struct clk_hw *hw)
> +{
> +       struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> +       u32 val = 0x01UL << clk_gate->shift;

Same, use BIT() macro.

> +
> +       regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, 0);
> +}
> +
> +static int rt2880_gate_is_enabled(struct clk_hw *hw)
> +{
> +       struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
> +       unsigned int rdval;
> +
> +       if (regmap_read(syscon_regmap, SYSC_REG_CLKCFG1, &rdval))
> +               return 0;
> +
> +       return (!!(rdval & (0x01UL << clk_gate->shift)));

Doesn't need to be a bool. Just 'return rdval & BIT(clk_gate->shift)'

> +}
> +
> +static const struct clk_ops rt2880_gate_ops = {
> +       .enable = rt2880_gate_enable,
> +       .disable = rt2880_gate_disable,
> +       .is_enabled = rt2880_gate_is_enabled,
> +};
> +
> +static struct clk * __init
> +rt2880_register_gate(const char *name, const char *parent_name, u8 shift)
> +{
> +       struct rt2880_gate      *clk_gate;
> +       struct clk              *clk;
> +       struct clk_init_data    init;
> +       const char *_parent_names[1] = { parent_name };
> +
> +       clk_gate = kzalloc(sizeof(*clk_gate), GFP_KERNEL);
> +       if (!clk_gate)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = name;
> +       init.ops = &rt2880_gate_ops;
> +       init.flags = 0;
> +       init.parent_names = parent_name ? _parent_names : NULL;
> +       init.num_parents = parent_name ? 1 : 0;
> +
> +       clk_gate->hw.init = &init;
> +       clk_gate->shift = shift;
> +
> +       clk = clk_register(NULL, &clk_gate->hw);

Please use clk_hw_register() instead of clk_register().

> +       if (IS_ERR(clk))
> +               kfree(clk_gate);
> +
> +       return clk;
> +}
> +
> +static void __init rt2880_clkctrl_init_dt(struct device_node *np)
> +{
> +       struct clk *clk;
> +       int i;
> +       const struct of_device_id *match;
> +       struct gate_clk_desc *clk_tbl;
> +
> +       match = of_match_node(of_match_rt2880_clk, np);
> +       if (!match) {
> +               pr_info("rt2880-clock: could not get compatible node");

How is this possible?

> +               return;
> +       }
> +       clk_tbl = (struct gate_clk_desc *)match->data;

Drop useless cast from void.

> +
> +       syscon_regmap = syscon_regmap_lookup_by_phandle(np, "ralink,sysctl");
> +       if (IS_ERR(syscon_regmap)) {
> +               pr_info("rt2880-clock: could not get syscon regmap");

Why are error messages at 'info' print level? Should be pr_err().

> +               return;
> +       }
> +
> +       clk_data.clk_num = GATE_CLK_NUM;
> +       clk_data.clks = clks;
> +
> +       for (i = 0; i < GATE_CLK_NUM; i++) {
> +               if (clk_tbl[i].name) {
> +                       clk = rt2880_register_gate(
> +                               clk_tbl[i].name, clk_tbl[i].parent_name, i);
> +                       if (IS_ERR_OR_NULL(clk))
> +                               panic("rt2880-clock : could not register gate clock");

Do we need to panic here? Maybe things will still work?

> +                       clk_data.clks[i] = clk;
> +               }
> +       }
> +
> +       of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);

Please add a clk_hw provider instead of a clk provider.

> +}
> +CLK_OF_DECLARE(rt2880, "ralink,rt2880-clock", rt2880_clkctrl_init_dt);

Can this be a platform driver?
diff mbox series

Patch

diff --git a/arch/mips/ralink/rt2880-clk_internal.h b/arch/mips/ralink/rt2880-clk_internal.h
new file mode 100644
index 000000000000..9d5dded16a80
--- /dev/null
+++ b/arch/mips/ralink/rt2880-clk_internal.h
@@ -0,0 +1,21 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
+ */
+
+#ifndef __RT2880_CLOCK_INTERNAL_H
+
+
+#define GATE_CLK_NUM	(32)
+
+struct gate_clk_desc {
+	const char *name;
+	const char *parent_name;
+};
+
+extern const struct of_device_id __initconst of_match_rt2880_clk[];
+
+
+#endif
+
+
diff --git a/arch/mips/ralink/rt2880-clock.c b/arch/mips/ralink/rt2880-clock.c
new file mode 100644
index 000000000000..46cc067225ab
--- /dev/null
+++ b/arch/mips/ralink/rt2880-clock.c
@@ -0,0 +1,134 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 NOGUCHI Hiroshi <drvlabo@gmail.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/bug.h>
+
+#include "rt2880-clk_internal.h"
+
+
+/* clock configuration 1 */
+#define	SYSC_REG_CLKCFG1	0x30
+
+struct rt2880_gate {
+	struct clk_hw	hw;
+	u8	shift;
+};
+
+#define	to_rt2880_gate(_hw)	container_of(_hw, struct rt2880_gate, hw)
+
+static struct clk_onecell_data	clk_data;
+static struct clk *clks[GATE_CLK_NUM];
+
+static struct regmap *syscon_regmap;
+
+static int rt2880_gate_enable(struct clk_hw *hw)
+{
+	struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+	u32 val = 0x01UL << clk_gate->shift;
+
+	regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, val);
+
+	return 0;
+}
+
+static void rt2880_gate_disable(struct clk_hw *hw)
+{
+	struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+	u32 val = 0x01UL << clk_gate->shift;
+
+	regmap_update_bits(syscon_regmap, SYSC_REG_CLKCFG1, val, 0);
+}
+
+static int rt2880_gate_is_enabled(struct clk_hw *hw)
+{
+	struct rt2880_gate *clk_gate = to_rt2880_gate(hw);
+	unsigned int rdval;
+
+	if (regmap_read(syscon_regmap, SYSC_REG_CLKCFG1, &rdval))
+		return 0;
+
+	return (!!(rdval & (0x01UL << clk_gate->shift)));
+}
+
+static const struct clk_ops rt2880_gate_ops = {
+	.enable = rt2880_gate_enable,
+	.disable = rt2880_gate_disable,
+	.is_enabled = rt2880_gate_is_enabled,
+};
+
+static struct clk * __init
+rt2880_register_gate(const char *name, const char *parent_name, u8 shift)
+{
+	struct rt2880_gate	*clk_gate;
+	struct clk		*clk;
+	struct clk_init_data	init;
+	const char *_parent_names[1] = { parent_name };
+
+	clk_gate = kzalloc(sizeof(*clk_gate), GFP_KERNEL);
+	if (!clk_gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &rt2880_gate_ops;
+	init.flags = 0;
+	init.parent_names = parent_name ? _parent_names : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	clk_gate->hw.init = &init;
+	clk_gate->shift = shift;
+
+	clk = clk_register(NULL, &clk_gate->hw);
+	if (IS_ERR(clk))
+		kfree(clk_gate);
+
+	return clk;
+}
+
+static void __init rt2880_clkctrl_init_dt(struct device_node *np)
+{
+	struct clk *clk;
+	int i;
+	const struct of_device_id *match;
+	struct gate_clk_desc *clk_tbl;
+
+	match = of_match_node(of_match_rt2880_clk, np);
+	if (!match) {
+		pr_info("rt2880-clock: could not get compatible node");
+		return;
+	}
+	clk_tbl = (struct gate_clk_desc *)match->data;
+
+	syscon_regmap = syscon_regmap_lookup_by_phandle(np, "ralink,sysctl");
+	if (IS_ERR(syscon_regmap)) {
+		pr_info("rt2880-clock: could not get syscon regmap");
+		return;
+	}
+
+	clk_data.clk_num = GATE_CLK_NUM;
+	clk_data.clks = clks;
+
+	for (i = 0; i < GATE_CLK_NUM; i++) {
+		if (clk_tbl[i].name) {
+			clk = rt2880_register_gate(
+				clk_tbl[i].name, clk_tbl[i].parent_name, i);
+			if (IS_ERR_OR_NULL(clk))
+				panic("rt2880-clock : could not register gate clock");
+			clk_data.clks[i] = clk;
+		}
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+}
+CLK_OF_DECLARE(rt2880, "ralink,rt2880-clock", rt2880_clkctrl_init_dt);