Re: [PATCH v10] clk: add MOXA ART SoCs clock driver

2014-01-27 Thread Mark Rutland
On Tue, Jan 21, 2014 at 12:44:21PM +, Jonas Jensen wrote:
> MOXA ART SoCs allow to determine PLL output and APB frequencies
> by reading registers holding multiplier and divisor information.
> 
> Add a clock driver for this SoC.
> 
> Signed-off-by: Jonas Jensen 
> ---
> 
> Notes:
> Thanks for the reply Sudeep, changes are in v10.
> 
> Changes since v9:
> 
> 1. rebase drivers/clk/Makefile to next-20140121
> 2. remove unnecessary switch
> 3. use a more elaborate commit message
> 
> Applies to next-20140121
> 
>  .../bindings/clock/moxa,moxart-clock.txt   | 48 +++
>  drivers/clk/Makefile   |  1 +
>  drivers/clk/clk-moxart.c   | 99 
> ++
>  3 files changed, 148 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
>  create mode 100644 drivers/clk/clk-moxart.c
> 
> diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt 
> b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
> new file mode 100644
> index 000..242e3fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
> @@ -0,0 +1,48 @@
> +Device Tree Clock bindings for arch-moxart
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +MOXA ART SoCs allow to determine PLL output and APB frequencies
> +by reading registers holding multiplier and divisor information.
> +
> +
> +PLL:
> +
> +Required properties:
> +- compatible : Must be "moxa,moxart-pll-clock"
> +- #clock-cells : Should be 0
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle to parent clock

Nit: clocks are referenced with a clock-specifier (which might be zero
cells), not just a phandle.

How about:

clocks: should contain a phandle + clock-specifier for the parent clock

> +
> +Optional properties:
> +- clock-output-names : Should contain clock name
> +
> +
> +APB:
> +
> +Required properties:
> +- compatible : Must be "moxa,moxart-apb-clock"
> +- #clock-cells : Should be 0
> +- reg : Should contain registers location and length
> +- clocks : Should contain phandle to parent clock

Likewise.

> +
> +Optional properties:
> +- clock-output-names : Should contain clock name
> +
> +
> +For example:
> +
> + clk_pll: clk_pll@9810 {
> + compatible = "moxa,moxart-pll-clock";
> + #clock-cells = <0>;
> + reg = <0x9810 0x34>;
> + };
> +
> + clk_apb: clk_apb@9810 {
> + compatible = "moxa,moxart-apb-clock";
> + #clock-cells = <0>;
> + reg = <0x9810 0x34>;
> + clocks = <_pll>;
> + };
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 58b2d72..24361bf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK)+= clk-composite.o
>  # SoCs specific
>  obj-$(CONFIG_ARCH_BCM2835)   += clk-bcm2835.o
>  obj-$(CONFIG_ARCH_EFM32) += clk-efm32gg.o
> +obj-$(CONFIG_ARCH_MOXART)+= clk-moxart.o
>  obj-$(CONFIG_ARCH_NOMADIK)   += clk-nomadik.o
>  obj-$(CONFIG_ARCH_HIGHBANK)  += clk-highbank.o
>  obj-$(CONFIG_ARCH_HI3xxx)+= hisilicon/
> diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> new file mode 100644
> index 000..7021748
> --- /dev/null
> +++ b/drivers/clk/clk-moxart.c
> @@ -0,0 +1,99 @@
> +/*
> + * MOXA ART SoCs clock driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen 
> + *
> + * 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.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +void __init moxart_of_pll_clk_init(struct device_node *node)
> +{
> + static void __iomem *base;
> + struct clk *clk, *ref_clk;
> + unsigned long rate;
> + unsigned int mul;
> + const char *name = node->name;
> +
> + of_property_read_string(node, "clock-output-names", );
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s: of_iomap failed\n", node->full_name);
> + return;
> + }
> +
> + mul = readl(base + 0x30) >> 3 & 0x3f;
> + iounmap(base);
> +
> + ref_clk = of_clk_get(node, 0);
> + if (IS_ERR(ref_clk)) {
> + pr_err("%s: of_clk_get failed\n", node->full_name);
> + return;
> + }
> +
> + rate = mul * clk_get_rate(ref_clk);
> +
> + clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);

If this clock has a parent, Surely CLK_IS_ROOT is not appropriate by its
definition:

#define CLK_IS_ROOT BIT(4) /* root clk, has no parent */

Surely this is a fixed factor clock rather than a fixed rate clock?

> + if (IS_ERR(clk)) {
> + 

Re: [PATCH v10] clk: add MOXA ART SoCs clock driver

2014-01-27 Thread Mark Rutland
On Tue, Jan 21, 2014 at 12:44:21PM +, Jonas Jensen wrote:
 MOXA ART SoCs allow to determine PLL output and APB frequencies
 by reading registers holding multiplier and divisor information.
 
 Add a clock driver for this SoC.
 
 Signed-off-by: Jonas Jensen jonas.jen...@gmail.com
 ---
 
 Notes:
 Thanks for the reply Sudeep, changes are in v10.
 
 Changes since v9:
 
 1. rebase drivers/clk/Makefile to next-20140121
 2. remove unnecessary switch
 3. use a more elaborate commit message
 
 Applies to next-20140121
 
  .../bindings/clock/moxa,moxart-clock.txt   | 48 +++
  drivers/clk/Makefile   |  1 +
  drivers/clk/clk-moxart.c   | 99 
 ++
  3 files changed, 148 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
  create mode 100644 drivers/clk/clk-moxart.c
 
 diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt 
 b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
 new file mode 100644
 index 000..242e3fc
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
 @@ -0,0 +1,48 @@
 +Device Tree Clock bindings for arch-moxart
 +
 +This binding uses the common clock binding[1].
 +
 +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
 +
 +MOXA ART SoCs allow to determine PLL output and APB frequencies
 +by reading registers holding multiplier and divisor information.
 +
 +
 +PLL:
 +
 +Required properties:
 +- compatible : Must be moxa,moxart-pll-clock
 +- #clock-cells : Should be 0
 +- reg : Should contain registers location and length
 +- clocks : Should contain phandle to parent clock

Nit: clocks are referenced with a clock-specifier (which might be zero
cells), not just a phandle.

How about:

clocks: should contain a phandle + clock-specifier for the parent clock

 +
 +Optional properties:
 +- clock-output-names : Should contain clock name
 +
 +
 +APB:
 +
 +Required properties:
 +- compatible : Must be moxa,moxart-apb-clock
 +- #clock-cells : Should be 0
 +- reg : Should contain registers location and length
 +- clocks : Should contain phandle to parent clock

Likewise.

 +
 +Optional properties:
 +- clock-output-names : Should contain clock name
 +
 +
 +For example:
 +
 + clk_pll: clk_pll@9810 {
 + compatible = moxa,moxart-pll-clock;
 + #clock-cells = 0;
 + reg = 0x9810 0x34;
 + };
 +
 + clk_apb: clk_apb@9810 {
 + compatible = moxa,moxart-apb-clock;
 + #clock-cells = 0;
 + reg = 0x9810 0x34;
 + clocks = clk_pll;
 + };
 diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
 index 58b2d72..24361bf 100644
 --- a/drivers/clk/Makefile
 +++ b/drivers/clk/Makefile
 @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK)+= clk-composite.o
  # SoCs specific
  obj-$(CONFIG_ARCH_BCM2835)   += clk-bcm2835.o
  obj-$(CONFIG_ARCH_EFM32) += clk-efm32gg.o
 +obj-$(CONFIG_ARCH_MOXART)+= clk-moxart.o
  obj-$(CONFIG_ARCH_NOMADIK)   += clk-nomadik.o
  obj-$(CONFIG_ARCH_HIGHBANK)  += clk-highbank.o
  obj-$(CONFIG_ARCH_HI3xxx)+= hisilicon/
 diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
 new file mode 100644
 index 000..7021748
 --- /dev/null
 +++ b/drivers/clk/clk-moxart.c
 @@ -0,0 +1,99 @@
 +/*
 + * MOXA ART SoCs clock driver.
 + *
 + * Copyright (C) 2013 Jonas Jensen
 + *
 + * Jonas Jensen jonas.jen...@gmail.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.
 + */
 +
 +#include linux/clk-provider.h
 +#include linux/io.h
 +#include linux/of_address.h
 +#include linux/clkdev.h
 +
 +void __init moxart_of_pll_clk_init(struct device_node *node)
 +{
 + static void __iomem *base;
 + struct clk *clk, *ref_clk;
 + unsigned long rate;
 + unsigned int mul;
 + 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;
 + }
 +
 + mul = readl(base + 0x30)  3  0x3f;
 + iounmap(base);
 +
 + ref_clk = of_clk_get(node, 0);
 + if (IS_ERR(ref_clk)) {
 + pr_err(%s: of_clk_get failed\n, node-full_name);
 + return;
 + }
 +
 + rate = mul * clk_get_rate(ref_clk);
 +
 + clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);

If this clock has a parent, Surely CLK_IS_ROOT is not appropriate by its
definition:

#define CLK_IS_ROOT BIT(4) /* root clk, has no parent */

Surely this is a fixed factor clock rather than a fixed rate clock?

 + if (IS_ERR(clk)) {
 + pr_err(%s: clk_register_fixed_rate failed\n, node-full_name);
 + 

[PATCH v10] clk: add MOXA ART SoCs clock driver

2014-01-21 Thread Jonas Jensen
MOXA ART SoCs allow to determine PLL output and APB frequencies
by reading registers holding multiplier and divisor information.

Add a clock driver for this SoC.

Signed-off-by: Jonas Jensen 
---

Notes:
Thanks for the reply Sudeep, changes are in v10.

Changes since v9:

1. rebase drivers/clk/Makefile to next-20140121
2. remove unnecessary switch
3. use a more elaborate commit message

Applies to next-20140121

 .../bindings/clock/moxa,moxart-clock.txt   | 48 +++
 drivers/clk/Makefile   |  1 +
 drivers/clk/clk-moxart.c   | 99 ++
 3 files changed, 148 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
 create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt 
b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
new file mode 100644
index 000..242e3fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
@@ -0,0 +1,48 @@
+Device Tree Clock bindings for arch-moxart
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+MOXA ART SoCs allow to determine PLL output and APB frequencies
+by reading registers holding multiplier and divisor information.
+
+
+PLL:
+
+Required properties:
+- compatible : Must be "moxa,moxart-pll-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle to parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+- compatible : Must be "moxa,moxart-apb-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle to parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+For example:
+
+   clk_pll: clk_pll@9810 {
+   compatible = "moxa,moxart-pll-clock";
+   #clock-cells = <0>;
+   reg = <0x9810 0x34>;
+   };
+
+   clk_apb: clk_apb@9810 {
+   compatible = "moxa,moxart-apb-clock";
+   #clock-cells = <0>;
+   reg = <0x9810 0x34>;
+   clocks = <_pll>;
+   };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 58b2d72..24361bf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK)  += clk-composite.o
 # SoCs specific
 obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
 obj-$(CONFIG_ARCH_EFM32)   += clk-efm32gg.o
+obj-$(CONFIG_ARCH_MOXART)  += clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o
 obj-$(CONFIG_ARCH_HIGHBANK)+= clk-highbank.o
 obj-$(CONFIG_ARCH_HI3xxx)  += hisilicon/
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 000..7021748
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,99 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen 
+ *
+ * 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.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+void __init moxart_of_pll_clk_init(struct device_node *node)
+{
+   static void __iomem *base;
+   struct clk *clk, *ref_clk;
+   unsigned long rate;
+   unsigned int mul;
+   const char *name = node->name;
+
+   of_property_read_string(node, "clock-output-names", );
+
+   base = of_iomap(node, 0);
+   if (!base) {
+   pr_err("%s: of_iomap failed\n", node->full_name);
+   return;
+   }
+
+   mul = readl(base + 0x30) >> 3 & 0x3f;
+   iounmap(base);
+
+   ref_clk = of_clk_get(node, 0);
+   if (IS_ERR(ref_clk)) {
+   pr_err("%s: of_clk_get failed\n", node->full_name);
+   return;
+   }
+
+   rate = mul * clk_get_rate(ref_clk);
+
+   clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+   if (IS_ERR(clk)) {
+   pr_err("%s: clk_register_fixed_rate failed\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(moxart_pll_clock, "moxa,moxart-pll-clock",
+  moxart_of_pll_clk_init);
+
+void __init moxart_of_apb_clk_init(struct device_node *node)
+{
+   static void __iomem *base;
+   struct clk *clk, *pll_clk;
+   unsigned long rate;
+   unsigned int div, val;
+   unsigned int div_idx[] = { 2, 3, 4, 6, 8};
+   const char *name = node->name;
+
+   of_property_read_string(node, "clock-output-names", );
+
+   base = of_iomap(node, 0);
+   

[PATCH v10] clk: add MOXA ART SoCs clock driver

2014-01-21 Thread Jonas Jensen
MOXA ART SoCs allow to determine PLL output and APB frequencies
by reading registers holding multiplier and divisor information.

Add a clock driver for this SoC.

Signed-off-by: Jonas Jensen jonas.jen...@gmail.com
---

Notes:
Thanks for the reply Sudeep, changes are in v10.

Changes since v9:

1. rebase drivers/clk/Makefile to next-20140121
2. remove unnecessary switch
3. use a more elaborate commit message

Applies to next-20140121

 .../bindings/clock/moxa,moxart-clock.txt   | 48 +++
 drivers/clk/Makefile   |  1 +
 drivers/clk/clk-moxart.c   | 99 ++
 3 files changed, 148 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
 create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt 
b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
new file mode 100644
index 000..242e3fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
@@ -0,0 +1,48 @@
+Device Tree Clock bindings for arch-moxart
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+MOXA ART SoCs allow to determine PLL output and APB frequencies
+by reading registers holding multiplier and divisor information.
+
+
+PLL:
+
+Required properties:
+- compatible : Must be moxa,moxart-pll-clock
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle to parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+APB:
+
+Required properties:
+- compatible : Must be moxa,moxart-apb-clock
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clocks : Should contain phandle to parent clock
+
+Optional properties:
+- clock-output-names : Should contain clock name
+
+
+For example:
+
+   clk_pll: clk_pll@9810 {
+   compatible = moxa,moxart-pll-clock;
+   #clock-cells = 0;
+   reg = 0x9810 0x34;
+   };
+
+   clk_apb: clk_apb@9810 {
+   compatible = moxa,moxart-apb-clock;
+   #clock-cells = 0;
+   reg = 0x9810 0x34;
+   clocks = clk_pll;
+   };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 58b2d72..24361bf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK)  += clk-composite.o
 # SoCs specific
 obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.o
 obj-$(CONFIG_ARCH_EFM32)   += clk-efm32gg.o
+obj-$(CONFIG_ARCH_MOXART)  += clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK) += clk-nomadik.o
 obj-$(CONFIG_ARCH_HIGHBANK)+= clk-highbank.o
 obj-$(CONFIG_ARCH_HI3xxx)  += hisilicon/
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 000..7021748
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,99 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen jonas.jen...@gmail.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.
+ */
+
+#include linux/clk-provider.h
+#include linux/io.h
+#include linux/of_address.h
+#include linux/clkdev.h
+
+void __init moxart_of_pll_clk_init(struct device_node *node)
+{
+   static void __iomem *base;
+   struct clk *clk, *ref_clk;
+   unsigned long rate;
+   unsigned int mul;
+   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;
+   }
+
+   mul = readl(base + 0x30)  3  0x3f;
+   iounmap(base);
+
+   ref_clk = of_clk_get(node, 0);
+   if (IS_ERR(ref_clk)) {
+   pr_err(%s: of_clk_get failed\n, node-full_name);
+   return;
+   }
+
+   rate = mul * clk_get_rate(ref_clk);
+
+   clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+   if (IS_ERR(clk)) {
+   pr_err(%s: clk_register_fixed_rate failed\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(moxart_pll_clock, moxa,moxart-pll-clock,
+  moxart_of_pll_clk_init);
+
+void __init moxart_of_apb_clk_init(struct device_node *node)
+{
+   static void __iomem *base;
+   struct clk *clk, *pll_clk;
+   unsigned long rate;
+   unsigned int div, val;
+   unsigned int div_idx[] = { 2, 3, 4, 6, 8};
+   const char *name = node-name;
+
+