[PATCH 0/4] clk: Add drivers for Aspeed BMC SoCs

[PATCH 0/4] clk: Add drivers for Aspeed BMC SoCs

[Joel Stanley]

Hello,

This adds support for the Aspeed ast2400 and ast2500 BMC SoCs. The drivers are
basic in that they only create clock devices so other drivers know what
frequency they are clocked at; there is no support for modifying the clocks.

Heiko and I exchanged some mail about the structure of the drivers. Thanks for
the review Hieko! However, I still think the layout matches the hardware as we
understand it so I did not make significant changes.

Cheers,

Joel

Joel Stanley (4):
  doc/devicetree: Add Aspeed clock bindings
  drvers/clk: Support fourth generation Aspeed SoCs
  drvers/clk: Support fifth generation Aspeed SoCs
  drivers/clk: Support Aspeed UART clock divisor

 .../devicetree/bindings/clock/aspeed-clock.txt     | 156 +++++++++++++++++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/aspeed/Makefile                        |   3 +
 drivers/clk/aspeed/clk-g4.c                        | 109 ++++++++++++
 drivers/clk/aspeed/clk-g5.c                        | 189 +++++++++++++++++++++
 drivers/clk/aspeed/clk-uart.c                      |  52 ++++++
 6 files changed, 510 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/aspeed-clock.txt
 create mode 100644 drivers/clk/aspeed/Makefile
 create mode 100644 drivers/clk/aspeed/clk-g4.c
 create mode 100644 drivers/clk/aspeed/clk-g5.c
 create mode 100644 drivers/clk/aspeed/clk-uart.c

-- 
2.8.1

[PATCH 1/4] doc/devicetree: Add Aspeed clock bindings

[Joel Stanley]

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .../devicetree/bindings/clock/aspeed-clock.txt     | 156 +++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/aspeed-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/aspeed-clock.txt b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
new file mode 100644
index 000000000000..968329406435
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
@@ -0,0 +1,156 @@
+Device Tree Clock bindings for the Aspeed SoCs
+
+Aspeed SoCs have a fixed frequency input osciallator is used to create the PLL
+and APB clocks. We can determine these frequencies by reading registers that
+are set according to strapping bits.
+
+Forth generation boards
+-----------------------
+
+eg, ast2400.
+
+CLKIN:
+ - compatible : Must be "fixed-clock"
+ - #clock-cells : Should be 0
+ - clock-frequency: 48e6, 25e6 or 24e6 depending on the input clock
+
+PLL:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-hpll-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the input clock (clkin)
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-apb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+
+For example:
+
+	clk_clkin: clk_clkin {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <48000000>;
+	};
+
+	clk_hpll: clk_hpll {
+		compatible = "aspeed,g4-hpll-clock";
+		#clock-cells = <0>;
+		reg = <0x1e6e2008 0x4>;
+	};
+
+	clk_apb: clk_apb@1e6e2008 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g4-apb-clock";
+		reg = <0x1e6e2008 0x4>;
+		clocks = <&clk_hpll>;
+	};
+
+
+
+Fifth generation boards
+-----------------------
+
+eg, ast2500.
+
+CLKIN:
+Required properties:
+ - compatible : Must be "fixed-clock"
+ - #clock-cells : Should be 0
+ - clock-frequency: 25000000 or 24000000 depending on the input clock
+
+H-PLL:
+
+Required properties:
+ - compatible : Must be "aspeed,g5-hpll-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the input clock (clkin)
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+AHB:
+
+Required properties:
+ - compatible : Must be "aspeed,g5-ahb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+APB:
+
+Required properties:
+ - compatible : Must be "aspeed,g4-apb-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+ - clocks : Should contain phandle + clock-specifier for the the h-pll
+
+Optional properties:
+ - clock-output-names : Should contain clock name
+
+For example:
+	clk_clkin: clk_clkin@1e6e2070 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-clkin-clock";
+		reg = <0x1e6e2070 0x04>;
+	};
+
+	clk_hpll: clk_hpll@1e6e2024 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-hpll-clock";
+		reg = <0x1e6e2024 0x4>;
+		clocks = <&clk_clkin>;
+	};
+
+	clk_ahb: clk_ahb@1e6e2070 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-ahb-clock";
+		reg = <0x1e6e2070 0x4>;
+		clocks = <&clk_hpll>;
+	};
+
+	clk_apb: clk_apb@1e6e2008 {
+		#clock-cells = <0>;
+		compatible = "aspeed,g5-apb-clock";
+		reg = <0x1e6e2008 0x4>;
+		clocks = <&clk_hpll>;
+	};
+
+Common clocks
+-------------
+
+UART:
+
+All supported boards have a uart clock that is derived from a 24MHz reference.
+We have a clock driver for it as there is a register in the SCU that controls
+weather it will be divided down by 13 or not.
+
+Required properties:
+ - compatible : Must be "aspeed,uart-clock"
+ - #clock-cells : Should be 0
+ - reg : Should contain registers location and length
+
+For example:
+
+	clk_uart: clk_uart@1e6e2008 {
+		#clock-cells = <0>;
+		compatible = "aspeed,uart-clock";
+		reg = <0x1e6e202c 0x4>;
+	};
-- 
2.8.1

Re: [PATCH 1/4] doc/devicetree: Add Aspeed clock bindings

[Rob Herring]

On Mon, May 09, 2016 at 10:01:48PM +0930, Joel Stanley wrote:
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  .../devicetree/bindings/clock/aspeed-clock.txt     | 156 +++++++++++++++++++++
>  1 file changed, 156 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/aspeed-clock.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/aspeed-clock.txt b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
> new file mode 100644
> index 000000000000..968329406435
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/aspeed-clock.txt
> @@ -0,0 +1,156 @@
> +Device Tree Clock bindings for the Aspeed SoCs
> +
> +Aspeed SoCs have a fixed frequency input osciallator is used to create the PLL
> +and APB clocks. We can determine these frequencies by reading registers that
> +are set according to strapping bits.
> +
> +Forth generation boards
> +-----------------------
> +
> +eg, ast2400.
> +
> +CLKIN:
> + - compatible : Must be "fixed-clock"
> + - #clock-cells : Should be 0
> + - clock-frequency: 48e6, 25e6 or 24e6 depending on the input clock
> +
> +PLL:
> +
> +Required properties:
> + - compatible : Must be "aspeed,g4-hpll-clock"
> + - #clock-cells : Should be 0
> + - reg : Should contain registers location and length
> + - clocks : Should contain phandle + clock-specifier for the input clock (clkin)
> +
> +Optional properties:
> + - clock-output-names : Should contain clock name
> +
> +
> +APB:
> +
> +Required properties:
> + - compatible : Must be "aspeed,g4-apb-clock"
> + - #clock-cells : Should be 0
> + - reg : Should contain registers location and length
> + - clocks : Should contain phandle + clock-specifier for the h-pll
> +
> +Optional properties:
> + - clock-output-names : Should contain clock name
> +
> +
> +For example:
> +
> +	clk_clkin: clk_clkin {
> +		#clock-cells = <0>;
> +		compatible = "fixed-clock";
> +		clock-frequency = <48000000>;
> +	};
> +
> +	clk_hpll: clk_hpll {
> +		compatible = "aspeed,g4-hpll-clock";
> +		#clock-cells = <0>;
> +		reg = <0x1e6e2008 0x4>;
> +	};
> +
> +	clk_apb: clk_apb@1e6e2008 {
> +		#clock-cells = <0>;
> +		compatible = "aspeed,g4-apb-clock";
> +		reg = <0x1e6e2008 0x4>;

You have overlapping register regions which we try to avoid (it would 
through errors, but doesn't because of existing platforms).

Just define the h/w block controlling these clocks and support multiple 
clocks.

Is this really all the clocks the SOC has?

Rob

[PATCH 2/4] drvers/clk: Support fourth generation Aspeed SoCs

[Joel Stanley]

A basic driver to create fixed rate clock devices from strapping
registers.

The clocks on the ast2400 are derived from an external oscillator and
the frequency of this can be derived from the strapping of the
processor. The frequency of internal clocks can be derived from other
registers in the SCU (System Control Unit).

Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
---
 drivers/clk/Makefile        |   1 +
 drivers/clk/aspeed/Makefile |   1 +
 drivers/clk/aspeed/clk-g4.c | 109 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 drivers/clk/aspeed/Makefile
 create mode 100644 drivers/clk/aspeed/clk-g4.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 46869d696e4d..8eaaef013a28 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -84,3 +84,4 @@ obj-$(CONFIG_X86)			+= x86/
 obj-$(CONFIG_ARCH_ZX)			+= zte/
 obj-$(CONFIG_ARCH_ZYNQ)			+= zynq/
 obj-$(CONFIG_H8300)		+= h8300/
+obj-$(CONFIG_ARCH_ASPEED)		+= aspeed/
diff --git a/drivers/clk/aspeed/Makefile b/drivers/clk/aspeed/Makefile
new file mode 100644
index 000000000000..d3457fbe3019
--- /dev/null
+++ b/drivers/clk/aspeed/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MACH_ASPEED_G4) += clk-g4.o
diff --git a/drivers/clk/aspeed/clk-g4.c b/drivers/clk/aspeed/clk-g4.c
new file mode 100644
index 000000000000..91677e9177f5
--- /dev/null
+++ b/drivers/clk/aspeed/clk-g4.c
@@ -0,0 +1,109 @@
+/*
+ * Copyright 2016 IBM Corporation
+ *
+ * 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.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static void __init aspeed_of_hpll_clk_init(struct device_node *node)
+{
+	struct clk *clk, *clkin_clk;
+	void __iomem *base;
+	int reg, rate, clkin;
+	const char *name = node->name;
+	const char *parent_name;
+	const int rates[][4] = {
+		{384, 360, 336, 408},
+		{400, 375, 350, 425},
+	};
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	reg = readl(base);
+	iounmap(base);
+
+	clkin_clk = of_clk_get(node, 0);
+	if (IS_ERR(clkin_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	clkin = clk_get_rate(clkin_clk);
+
+	reg = (reg >> 8) & 0x2;
+
+	if (clkin == 48000000 || clkin == 24000000)
+		rate = rates[0][reg] * 1000000;
+	else if (clkin == 25000000)
+		rate = rates[1][reg] * 1000000;
+	else {
+		pr_err("%s: unknown clkin frequency %dHz\n",
+				node->full_name, clkin);
+		WARN_ON(1);
+	}
+
+	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_hpll_clock, "aspeed,g4-hpll-clock",
+	       aspeed_of_hpll_clk_init);
+
+static void __init aspeed_of_apb_clk_init(struct device_node *node)
+{
+	struct clk *clk, *hpll_clk;
+	void __iomem *base;
+	int reg, rate;
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	reg = readl(base);
+	iounmap(base);
+
+	hpll_clk = of_clk_get(node, 0);
+	if (IS_ERR(hpll_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	reg = (reg >> 23) & 0x3;
+	rate = clk_get_rate(hpll_clk) / (2 + 2 * reg);
+
+	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_apb_clock, "aspeed,g4-apb-clock",
+	       aspeed_of_apb_clk_init);
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] drvers/clk: Support fourth generation Aspeed SoCs

[Stephen Boyd]

On 05/09, Joel Stanley wrote:
> diff --git a/drivers/clk/aspeed/clk-g4.c b/drivers/clk/aspeed/clk-g4.c
> new file mode 100644
> index 000000000000..91677e9177f5
> --- /dev/null
> +++ b/drivers/clk/aspeed/clk-g4.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright 2016 IBM Corporation
> + *
> + * 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.h>

Hopefully this include isn't needed.

> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/clkdev.h>
> +
> +static void __init aspeed_of_hpll_clk_init(struct device_node *node)
> +{
> +	struct clk *clk, *clkin_clk;
> +	void __iomem *base;
> +	int reg, rate, clkin;
> +	const char *name = node->name;
> +	const char *parent_name;
> +	const int rates[][4] = {
> +		{384, 360, 336, 408},
> +		{400, 375, 350, 425},
> +	};
> +
> +	of_property_read_string(node, "clock-output-names", &name);
> +	parent_name = of_clk_get_parent_name(node, 0);
> +
> +	base = of_iomap(node, 0);
> +	if (!base) {
> +		pr_err("%s: of_iomap failed\n", node->full_name);
> +		return;
> +	}
> +	reg = readl(base);
> +	iounmap(base);
> +
> +	clkin_clk = of_clk_get(node, 0);
> +	if (IS_ERR(clkin_clk)) {
> +		pr_err("%s: of_clk_get failed\n", node->full_name);
> +		return;
> +	}
> +
> +	clkin = clk_get_rate(clkin_clk);
> +
> +	reg = (reg >> 8) & 0x2;
> +
> +	if (clkin == 48000000 || clkin == 24000000)
> +		rate = rates[0][reg] * 1000000;
> +	else if (clkin == 25000000)
> +		rate = rates[1][reg] * 1000000;
> +	else {
> +		pr_err("%s: unknown clkin frequency %dHz\n",
> +				node->full_name, clkin);
> +		WARN_ON(1);
> +	}
> +
> +	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);

Please write a proper clk driver that sets clkin to be the parent
of this clk registered here and then calculates the frequency
based on the parent clk rate and the strapping registers via the
recalc rate callback. Also please move this to a platform driver
instead of using CLK_OF_DECLARE assuming this isn't required for
any timers in the system.

> +	if (IS_ERR(clk)) {
> +		pr_err("%s: failed to register clock\n", node->full_name);
> +		return;
> +	}
> +
> +	clk_register_clkdev(clk, NULL, name);

Do you have clkdev users? If not then please drop this.

> +	of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +}
> +CLK_OF_DECLARE(aspeed_hpll_clock, "aspeed,g4-hpll-clock",
> +	       aspeed_of_hpll_clk_init);
> +
> +static void __init aspeed_of_apb_clk_init(struct device_node *node)
> +{
> +	struct clk *clk, *hpll_clk;
> +	void __iomem *base;
> +	int reg, rate;
> +	const char *name = node->name;
> +	const char *parent_name;
> +
> +	of_property_read_string(node, "clock-output-names", &name);
> +	parent_name = of_clk_get_parent_name(node, 0);
> +
> +	base = of_iomap(node, 0);
> +	if (!base) {
> +		pr_err("%s: of_iomap failed\n", node->full_name);
> +		return;
> +	}
> +	reg = readl(base);
> +	iounmap(base);
> +
> +	hpll_clk = of_clk_get(node, 0);
> +	if (IS_ERR(hpll_clk)) {
> +		pr_err("%s: of_clk_get failed\n", node->full_name);
> +		return;
> +	}
> +
> +	reg = (reg >> 23) & 0x3;
> +	rate = clk_get_rate(hpll_clk) / (2 + 2 * reg);
> +
> +	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
> +	if (IS_ERR(clk)) {
> +		pr_err("%s: failed to register clock\n", node->full_name);
> +		return;
> +	}
> +
> +	clk_register_clkdev(clk, NULL, name);
> +	of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +}
> +CLK_OF_DECLARE(aspeed_apb_clock, "aspeed,g4-apb-clock",
> +	       aspeed_of_apb_clk_init);

Following on Rob's comment, please combine this into one driver
instead of having different DT nodes for different clks that are
all really part of one clock controller hw block.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/4] drvers/clk: Support fourth generation Aspeed SoCs

[Joel Stanley]

On Tue, May 10, 2016 at 8:19 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/09, Joel Stanley wrote:
>> diff --git a/drivers/clk/aspeed/clk-g4.c b/drivers/clk/aspeed/clk-g4.c
>> new file mode 100644
>> index 000000000000..91677e9177f5
>> --- /dev/null
>> +++ b/drivers/clk/aspeed/clk-g4.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Copyright 2016 IBM Corporation
>> + *
>> + * 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.h>
>
> Hopefully this include isn't needed.

It was required for clk_get_rate in aspeed_of_pclk_clk_init. As I
mention below, I reworked it to avoid this.

>
>> +#include <linux/clk-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/clkdev.h>
>> +
>> +static void __init aspeed_of_hpll_clk_init(struct device_node *node)
>> +{
>> +     struct clk *clk, *clkin_clk;
>> +     void __iomem *base;
>> +     int reg, rate, clkin;
>> +     const char *name = node->name;
>> +     const char *parent_name;
>> +     const int rates[][4] = {
>> +             {384, 360, 336, 408},
>> +             {400, 375, 350, 425},
>> +     };
>> +
>> +     of_property_read_string(node, "clock-output-names", &name);
>> +     parent_name = of_clk_get_parent_name(node, 0);
>> +
>> +     base = of_iomap(node, 0);
>> +     if (!base) {
>> +             pr_err("%s: of_iomap failed\n", node->full_name);
>> +             return;
>> +     }
>> +     reg = readl(base);
>> +     iounmap(base);
>> +
>> +     clkin_clk = of_clk_get(node, 0);
>> +     if (IS_ERR(clkin_clk)) {
>> +             pr_err("%s: of_clk_get failed\n", node->full_name);
>> +             return;
>> +     }
>> +
>> +     clkin = clk_get_rate(clkin_clk);
>> +
>> +     reg = (reg >> 8) & 0x2;
>> +
>> +     if (clkin == 48000000 || clkin == 24000000)
>> +             rate = rates[0][reg] * 1000000;
>> +     else if (clkin == 25000000)
>> +             rate = rates[1][reg] * 1000000;
>> +     else {
>> +             pr_err("%s: unknown clkin frequency %dHz\n",
>> +                             node->full_name, clkin);
>> +             WARN_ON(1);
>> +     }
>> +
>> +     clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
>
> Please write a proper clk driver that sets clkin to be the parent
> of this clk registered here and then calculates the frequency
> based on the parent clk rate and the strapping registers via the
> recalc rate callback.

After a few goes I understood what you meant here.

Is the pclk part okay? I modified the pclk part use a fixed factor
clock instead of the fixed clock, so I could drop the pclk_get_rate
call below.

> Also please move this to a platform driver
> instead of using CLK_OF_DECLARE assuming this isn't required for
> any timers in the system.

Can you clarify here? We do use the rate of pclk (the apb clock) in
the clocksource driver to calculate our count_per_tick value.

>
>> +     if (IS_ERR(clk)) {
>> +             pr_err("%s: failed to register clock\n", node->full_name);
>> +             return;
>> +     }
>> +
>> +     clk_register_clkdev(clk, NULL, name);
>
> Do you have clkdev users? If not then please drop this.

I think the answer is no. I will drop.

>> +     of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> +}
>> +CLK_OF_DECLARE(aspeed_hpll_clock, "aspeed,g4-hpll-clock",
>> +            aspeed_of_hpll_clk_init);
>> +
>> +static void __init aspeed_of_apb_clk_init(struct device_node *node)
>> +{
>> +     struct clk *clk, *hpll_clk;
>> +     void __iomem *base;
>> +     int reg, rate;
>> +     const char *name = node->name;
>> +     const char *parent_name;
>> +
>> +     of_property_read_string(node, "clock-output-names", &name);
>> +     parent_name = of_clk_get_parent_name(node, 0);
>> +
>> +     base = of_iomap(node, 0);
>> +     if (!base) {
>> +             pr_err("%s: of_iomap failed\n", node->full_name);
>> +             return;
>> +     }
>> +     reg = readl(base);
>> +     iounmap(base);
>> +
>> +     hpll_clk = of_clk_get(node, 0);
>> +     if (IS_ERR(hpll_clk)) {
>> +             pr_err("%s: of_clk_get failed\n", node->full_name);
>> +             return;
>> +     }
>> +
>> +     reg = (reg >> 23) & 0x3;
>> +     rate = clk_get_rate(hpll_clk) / (2 + 2 * reg);
>> +
>> +     clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
>> +     if (IS_ERR(clk)) {
>> +             pr_err("%s: failed to register clock\n", node->full_name);
>> +             return;
>> +     }
>> +
>> +     clk_register_clkdev(clk, NULL, name);
>> +     of_clk_add_provider(node, of_clk_src_simple_get, clk);
>> +}
>> +CLK_OF_DECLARE(aspeed_apb_clock, "aspeed,g4-apb-clock",
>> +            aspeed_of_apb_clk_init);
>
> Following on Rob's comment, please combine this into one driver
> instead of having different DT nodes for different clks that are
> all really part of one clock controller hw block.

Ok, I have had a go at this. In our case the clock registers are part
of the "SCU" IP; a collection of disparate functions that happen to
include clock control. Is syscon the right thing to use here?

regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2400-syscon-scu");
ret = regmap_read(hpll->regmap, 0x70, &reg);

Cheers,

Joel

Re: [PATCH 2/4] drvers/clk: Support fourth generation Aspeed SoCs

[Stephen Boyd]

On 05/10, Joel Stanley wrote:
> On Tue, May 10, 2016 at 8:19 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > Please write a proper clk driver that sets clkin to be the parent
> > of this clk registered here and then calculates the frequency
> > based on the parent clk rate and the strapping registers via the
> > recalc rate callback.
> 
> After a few goes I understood what you meant here.
> 
> Is the pclk part okay? I modified the pclk part use a fixed factor
> clock instead of the fixed clock, so I could drop the pclk_get_rate
> call below.

Sorry what's the pclk part? Using a fixed factor is OK, as long
as we don't have to call clk_get_rate() in this driver it's a
better design.

> 
> > Also please move this to a platform driver
> > instead of using CLK_OF_DECLARE assuming this isn't required for
> > any timers in the system.
> 
> Can you clarify here? We do use the rate of pclk (the apb clock) in
> the clocksource driver to calculate our count_per_tick value.

Ok. I was hoping that this wasn't providing any clks required in
the timer driver. Even if that's true, we could just register the
part of the clk tree that needs to be ready for the timer from
CLK_OF_DECLARE() and then register the other clks from a regular
platform driver. Some people have already started doing this, so
there are some examples around (clk/nxp/clk-lpc18xx-creg.c is one
example).

> > Following on Rob's comment, please combine this into one driver
> > instead of having different DT nodes for different clks that are
> > all really part of one clock controller hw block.
> 
> Ok, I have had a go at this. In our case the clock registers are part
> of the "SCU" IP; a collection of disparate functions that happen to
> include clock control. Is syscon the right thing to use here?
> 
> regmap = syscon_regmap_lookup_by_compatible("aspeed,ast2400-syscon-scu");
> ret = regmap_read(hpll->regmap, 0x70, &reg);
> 

In that case I don't have a problem with a toplevel MFD node in
DT with a child node for the clk part and a child node for other
logical driver parts. Or it could all be one MFD driver and node
that knows to add a clk device as a child of the MFD purely in
software so that we can fork control of the clk part over to the
drivers/clk directory. If you have the clk driver attach to some
child device then you can always get the regmap from the parent
MFD via the dev->parent pointer.

Maybe Rob can chime in here too with the proper design, because
I've seen both styles.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/4] drvers/clk: Support fifth generation Aspeed SoCs

[Joel Stanley]

A basic driver to create fixed rate clock devices from strapping
registers.

Like the ast2400, the ast2500 clocks are derived from an external
oscillator and the frequency of this can be determined from the
strapping of the processor.

The frequency of internal clocks can be derived from other registers in
the SCU (System Control Unit). The layout of the internal clocks is a
bit different to the ast2400, as are the divisor reigisters, so it has
it's own driver.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/clk/aspeed/Makefile |   1 +
 drivers/clk/aspeed/clk-g5.c | 189 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 190 insertions(+)
 create mode 100644 drivers/clk/aspeed/clk-g5.c

diff --git a/drivers/clk/aspeed/Makefile b/drivers/clk/aspeed/Makefile
index d3457fbe3019..9ddb0f8f4356 100644
--- a/drivers/clk/aspeed/Makefile
+++ b/drivers/clk/aspeed/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_MACH_ASPEED_G4) += clk-g4.o
+obj-$(CONFIG_MACH_ASPEED_G5) += clk-g5.o
diff --git a/drivers/clk/aspeed/clk-g5.c b/drivers/clk/aspeed/clk-g5.c
new file mode 100644
index 000000000000..812fd9accf30
--- /dev/null
+++ b/drivers/clk/aspeed/clk-g5.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright 2016 IBM Corporation
+ *
+ * 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.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static void __init aspeed_of_clkin_clk_init(struct device_node *node)
+{
+	struct clk *clk;
+	void __iomem *base;
+	int reg, rate;
+	const char *name = node->name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	/* Strap register SCU70 */
+	reg = readl(base) & BIT(23);
+	iounmap(base);
+
+	if (reg)
+		rate = 25 * 1000 * 1000;
+	else
+		rate = 24 * 1000 * 1000;
+
+	clk = clk_register_fixed_rate(NULL, name, NULL, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_clkin_clock, "aspeed,g5-clkin-clock",
+	       aspeed_of_clkin_clk_init);
+
+
+static void __init aspeed_of_hpll_clk_init(struct device_node *node)
+{
+	struct clk *clk, *clkin_clk;
+	void __iomem *base;
+	int reg, rate, clkin;
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	/* H-PLL parameter register SCU24 */
+	reg = readl(base);
+	iounmap(base);
+
+	clkin_clk = of_clk_get(node, 0);
+	if (IS_ERR(clkin_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	clkin = clk_get_rate(clkin_clk);
+
+	if (reg & BIT(21)) {
+		rate = 0;
+	} else if (reg & BIT(20)) {
+		rate = clkin;
+	} else {
+		int p = (reg >> 13) & 0x3f;
+		int m = (reg >> 5) & 0xff;
+		int n = reg & 0x1f;
+
+		rate = clkin * ((m + 1) / (n + 1)) / (p + 1);
+	}
+
+	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_hpll_clock, "aspeed,g5-hpll-clock",
+	       aspeed_of_hpll_clk_init);
+
+
+static void __init aspeed_of_ahb_clk_init(struct device_node *node)
+{
+	struct clk *clk, *hpll_clk;
+	void __iomem *base;
+	int reg, rate, hpll;
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	/* Strap register SCU70 */
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	reg = (readl(base) >> 9) & 0x03;
+	iounmap(base);
+
+	/* A value of zero is undefined */
+	WARN_ON(reg == 0);
+
+	hpll_clk = of_clk_get(node, 0);
+	if (IS_ERR(hpll_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	hpll = clk_get_rate(hpll_clk);
+
+	rate = hpll / 2 / (reg + 1);
+
+	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_ahb_clock, "aspeed,g5-ahb-clock",
+	       aspeed_of_ahb_clk_init);
+
+
+static void __init aspeed_of_apb_clk_init(struct device_node *node)
+{
+	struct clk *clk, *hpll_clk;
+	void __iomem *base;
+	int reg, rate;
+	const char *name = node->name;
+	const char *parent_name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	/* Clock selection register SCU08 */
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	reg = readl(base) >> 23 & 0x3;
+	iounmap(base);
+
+	hpll_clk = of_clk_get(node, 0);
+	if (IS_ERR(hpll_clk)) {
+		pr_err("%s: of_clk_get failed\n", node->full_name);
+		return;
+	}
+
+	rate = clk_get_rate(hpll_clk) / (4 * (reg + 1));
+
+	clk = clk_register_fixed_rate(NULL, name, parent_name, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_apb_clock, "aspeed,g5-apb-clock",
+	       aspeed_of_apb_clk_init);
-- 
2.8.1

[PATCH 4/4] drivers/clk: Support Aspeed UART clock divisor

[Joel Stanley]

The Aspeed BMC SoCs have UART IP that derive their clocks from
a 24MHz reference. It's not clear where this reference comes from, so it
is hard coded in the driver.

This clock may be divided down by 13 if a certain register is set. This
driver reads this register and creates a struct clk for the UARTs to
consume.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/clk/aspeed/Makefile   |  1 +
 drivers/clk/aspeed/clk-uart.c | 52 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 drivers/clk/aspeed/clk-uart.c

diff --git a/drivers/clk/aspeed/Makefile b/drivers/clk/aspeed/Makefile
index 9ddb0f8f4356..9d5754c086eb 100644
--- a/drivers/clk/aspeed/Makefile
+++ b/drivers/clk/aspeed/Makefile
@@ -1,2 +1,3 @@
+obj-y += clk-uart.o
 obj-$(CONFIG_MACH_ASPEED_G4) += clk-g4.o
 obj-$(CONFIG_MACH_ASPEED_G5) += clk-g5.o
diff --git a/drivers/clk/aspeed/clk-uart.c b/drivers/clk/aspeed/clk-uart.c
new file mode 100644
index 000000000000..8cd23a758887
--- /dev/null
+++ b/drivers/clk/aspeed/clk-uart.c
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2016 IBM Corporation
+ *
+ * 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.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static void __init aspeed_of_uart_clk_init(struct device_node *node)
+{
+	struct clk *clk;
+	void __iomem *base;
+	int reg, rate;
+	const char *name = node->name;
+
+	of_property_read_string(node, "clock-output-names", &name);
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: of_iomap failed\n", node->full_name);
+		return;
+	}
+	reg = readl(base);
+	iounmap(base);
+
+	/*
+	 * The documentation does not indicate where this 24MHz clock is
+	 * derived from.
+	 */
+	rate = 24000000;
+
+	if (reg & BIT(12))
+		rate /= 13;
+
+	clk = clk_register_fixed_rate(NULL, name, NULL, 0, rate);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to register clock\n", node->full_name);
+		return;
+	}
+
+	clk_register_clkdev(clk, NULL, name);
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(aspeed_uart_clock, "aspeed,uart-clock",
+	       aspeed_of_uart_clk_init);
-- 
2.8.1