diff mbox

[25/26] clk: boston: Add a driver for MIPS Boston board clocks

Message ID 20160826153725.11629-26-paul.burton@imgtec.com (mailing list archive)
State Changes Requested, archived
Delegated to: Stephen Boyd
Headers show

Commit Message

Paul Burton Aug. 26, 2016, 3:37 p.m. UTC 
Add a driver for the clocks provided by the MIPS Boston board from
Imagination Technologies. 2 clocks are provided - the system clock & the
CPU clock - and each is a simple fixed rate clock whose frequency can be
determined by reading a register provided by the board.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 drivers/clk/Kconfig      |   9 ++++
 drivers/clk/Makefile     |   1 +
 drivers/clk/clk-boston.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)
 create mode 100644 drivers/clk/clk-boston.c

Comments

Stephen Boyd Aug. 26, 2016, 5:41 p.m. UTC | #1 
On 08/26, Paul Burton wrote:
> 
>  drivers/clk/Kconfig      |   9 ++++
>  drivers/clk/Makefile     |   1 +
>  drivers/clk/clk-boston.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++

Maybe a vendor subdirectory is appropriate? imgtec?

> +
> +struct clk_boston_state {
> +	struct clk *clk[BOSTON_CLK_COUNT];
> +	struct clk_boston clk_boston[BOSTON_CLK_COUNT];
> +	struct clk_onecell_data onecell_data[BOSTON_CLK_COUNT];
> +};
> +
> +static const char *clk_names[BOSTON_CLK_COUNT] = {

const char * const?

> +	[BOSTON_CLK_SYS] = "sys",
> +	[BOSTON_CLK_CPU] = "cpu",
> +};
> +
> +#define BOSTON_PLAT_MMCMDIV		0x30
> +# define BOSTON_PLAT_MMCMDIV_CLK0DIV	(0xff << 0)
> +# define BOSTON_PLAT_MMCMDIV_INPUT	(0xff << 8)
> +# define BOSTON_PLAT_MMCMDIV_MUL	(0xff << 16)
> +# define BOSTON_PLAT_MMCMDIV_CLK1DIV	(0xff << 24)
> +
> +static struct clk_boston *to_clk_boston(struct clk_hw *hw)
> +{
> +	return container_of(hw, struct clk_boston, hw);
> +}
> +
> +static uint32_t ext_field(uint32_t val, uint32_t mask)

Please use u32 instead of uint32_t in drivers.

> +{
> +	return (val & mask) >> (ffs(mask) - 1);
> +}
> +
> +static unsigned long clk_boston_recalc_rate(struct clk_hw *hw,
> +					    unsigned long parent_rate)
> +{
> +	struct clk_boston *state = to_clk_boston(hw);
> +	uint32_t in_rate, mul, div;
> +	uint mmcmdiv;

unsigned int?

> +	int err;
> +
> +	err = regmap_read(state->regmap, BOSTON_PLAT_MMCMDIV, &mmcmdiv);
> +	if (err)
> +		return 0;
> +
> +	in_rate = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_INPUT);

This sounds like a parent rate? Should there be another clk
created for that so that parent_rate in this function is useful?

> +	mul = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_MUL);
> +
> +	switch (state->id) {
> +	case BOSTON_CLK_SYS:
> +		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK0DIV);
> +		break;
> +	case BOSTON_CLK_CPU:
> +		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV);

Why not put the CLK0DIV or CLK1DIV offset in state->id instead?
That way this function just read in_rate, mul, and div and then
does the math?

> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return (in_rate * mul * 1000000) / div;

Is this always fixed at boot? It may be easier to populate fixed
rate clks during probe with the rate calculated there. Then there
aren't any clk_ops to implement.

> +}
> +
> +static const struct clk_ops clk_boston_ops = {
> +	.recalc_rate = clk_boston_recalc_rate,
> +};
> +
> +static void __init clk_boston_setup(struct device_node *np)
> +{
> +	struct clk_boston_state *state;
> +	struct clk_init_data init;
> +	struct regmap *regmap;
> +	int i, err;
> +
> +	state = kzalloc(sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(np, "regmap");
> +	if (IS_ERR(regmap)) {
> +		pr_err("failed to find regmap\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < BOSTON_CLK_COUNT; i++) {
> +		memset(&init, 0, sizeof(init));
> +		init.flags = CLK_IS_BASIC;

Please drop this flag unless you really need it for something. As
far as I know CLK_IS_BASIC is just for OMAP code.

> +		init.name = clk_names[i];
> +		init.ops = &clk_boston_ops;
> +
> +		state->clk_boston[i].hw.init = &init;
> +		state->clk_boston[i].id = i;
> +		state->clk_boston[i].regmap = regmap;
> +
> +		state->clk[i] = clk_register(NULL, &state->clk_boston[i].hw);

Please use clk_hw_register() instead.

> +		if (IS_ERR(state->clk[i])) {
> +			pr_err("failed to register clock: %ld\n",
> +			       PTR_ERR(state->clk[i]));
> +			return;
> +		}
> +	}
> +
> +	state->onecell_data->clks = state->clk;
> +	state->onecell_data->clk_num = BOSTON_CLK_COUNT;
> +
> +	err = of_clk_add_provider(np, of_clk_src_onecell_get,
> +				  state->onecell_data);

Please use of_clk_add_hw_provider() instead.

> +	if (err)
> +		pr_err("failed to add DT provider: %d\n", err);
> +}
> +CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);

Please make this into a platform driver.
Paul Burton Aug. 30, 2016, 3:06 p.m. UTC | #2 
On 26/08/16 18:41, Stephen Boyd wrote:
>> +	if (err)
>> +		pr_err("failed to add DT provider: %d\n", err);
>> +}
>> +CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);
> 
> Please make this into a platform driver.

Hi Stephen,

The problem with this would be that we need to obtain the CPU clock rate
fairly early during boot in order to set up the clocksource & delay loop
etc. Using CLK_OF_DECLARE allows for that but if this were a platform
driver my understanding is that the clocks wouldn't become available
until some point later in boot. If I'm wrong & there's a way to avoid
that please let me know.

Also: why? If CLK_OF_DECLARE isn't liked, shouldn't that be documented
somewhere (ideally next to the declaration of CLK_OF_DECLARE in
include/linux/clk-provider.h)?

Thanks,
    Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index e2d9bd7..2680343 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -208,6 +208,15 @@  config COMMON_CLK_OXNAS
 	---help---
 	  Support for the OXNAS SoC Family clocks.
 
+config COMMON_CLK_BOSTON
+	bool "Clock driver for MIPS Boston boards"
+	select MFD_SYSCON
+	---help---
+	  Enable this to support the system & CPU clocks on the MIPS Boston
+	  development board from Imagination Technologies. These are simple
+	  fixed rate clocks whose rate is determined by reading a platform
+	  provided register.
+
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/meson/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 3b6f9cf..3b78e515 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -20,6 +20,7 @@  endif
 obj-$(CONFIG_MACH_ASM9260)		+= clk-asm9260.o
 obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN)	+= clk-axi-clkgen.o
 obj-$(CONFIG_ARCH_AXXIA)		+= clk-axm5516.o
+obj-$(CONFIG_COMMON_CLK_BOSTON)		+= clk-boston.o
 obj-$(CONFIG_COMMON_CLK_CDCE706)	+= clk-cdce706.o
 obj-$(CONFIG_COMMON_CLK_CDCE925)	+= clk-cdce925.o
 obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
diff --git a/drivers/clk/clk-boston.c b/drivers/clk/clk-boston.c
new file mode 100644
index 0000000..4fa3fad
--- /dev/null
+++ b/drivers/clk/clk-boston.c
@@ -0,0 +1,131 @@ 
+/*
+ * Copyright (C) 2016 Imagination Technologies
+ * Author: Paul Burton <paul.burton@imgtec.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/mfd/syscon.h>
+
+#include <dt-bindings/clock/boston-clock.h>
+
+#define BOSTON_CLK_COUNT 2
+
+struct clk_boston {
+	struct clk_hw hw;
+	struct regmap *regmap;
+	unsigned int id;
+};
+
+struct clk_boston_state {
+	struct clk *clk[BOSTON_CLK_COUNT];
+	struct clk_boston clk_boston[BOSTON_CLK_COUNT];
+	struct clk_onecell_data onecell_data[BOSTON_CLK_COUNT];
+};
+
+static const char *clk_names[BOSTON_CLK_COUNT] = {
+	[BOSTON_CLK_SYS] = "sys",
+	[BOSTON_CLK_CPU] = "cpu",
+};
+
+#define BOSTON_PLAT_MMCMDIV		0x30
+# define BOSTON_PLAT_MMCMDIV_CLK0DIV	(0xff << 0)
+# define BOSTON_PLAT_MMCMDIV_INPUT	(0xff << 8)
+# define BOSTON_PLAT_MMCMDIV_MUL	(0xff << 16)
+# define BOSTON_PLAT_MMCMDIV_CLK1DIV	(0xff << 24)
+
+static struct clk_boston *to_clk_boston(struct clk_hw *hw)
+{
+	return container_of(hw, struct clk_boston, hw);
+}
+
+static uint32_t ext_field(uint32_t val, uint32_t mask)
+{
+	return (val & mask) >> (ffs(mask) - 1);
+}
+
+static unsigned long clk_boston_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct clk_boston *state = to_clk_boston(hw);
+	uint32_t in_rate, mul, div;
+	uint mmcmdiv;
+	int err;
+
+	err = regmap_read(state->regmap, BOSTON_PLAT_MMCMDIV, &mmcmdiv);
+	if (err)
+		return 0;
+
+	in_rate = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_INPUT);
+	mul = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_MUL);
+
+	switch (state->id) {
+	case BOSTON_CLK_SYS:
+		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK0DIV);
+		break;
+	case BOSTON_CLK_CPU:
+		div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV);
+		break;
+	default:
+		return 0;
+	}
+
+	return (in_rate * mul * 1000000) / div;
+}
+
+static const struct clk_ops clk_boston_ops = {
+	.recalc_rate = clk_boston_recalc_rate,
+};
+
+static void __init clk_boston_setup(struct device_node *np)
+{
+	struct clk_boston_state *state;
+	struct clk_init_data init;
+	struct regmap *regmap;
+	int i, err;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "regmap");
+	if (IS_ERR(regmap)) {
+		pr_err("failed to find regmap\n");
+		return;
+	}
+
+	for (i = 0; i < BOSTON_CLK_COUNT; i++) {
+		memset(&init, 0, sizeof(init));
+		init.flags = CLK_IS_BASIC;
+		init.name = clk_names[i];
+		init.ops = &clk_boston_ops;
+
+		state->clk_boston[i].hw.init = &init;
+		state->clk_boston[i].id = i;
+		state->clk_boston[i].regmap = regmap;
+
+		state->clk[i] = clk_register(NULL, &state->clk_boston[i].hw);
+		if (IS_ERR(state->clk[i])) {
+			pr_err("failed to register clock: %ld\n",
+			       PTR_ERR(state->clk[i]));
+			return;
+		}
+	}
+
+	state->onecell_data->clks = state->clk;
+	state->onecell_data->clk_num = BOSTON_CLK_COUNT;
+
+	err = of_clk_add_provider(np, of_clk_src_onecell_get,
+				  state->onecell_data);
+	if (err)
+		pr_err("failed to add DT provider: %d\n", err);
+}
+CLK_OF_DECLARE(clk_boston, "img,boston-clock", clk_boston_setup);