2013-06-27 14:04:33

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Applies to next-20130619

drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 142 insertions(+)
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f41e3e3..92e4532 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += zynq/
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-$(CONFIG_PLAT_SAMSUNG) += samsung/
+obj-$(CONFIG_ARCH_MOXART) += clk-moxart.o

obj-$(CONFIG_X86) += x86/

diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..893a63b
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,141 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static DEFINE_SPINLOCK(_lock);
+
+struct clk_device {
+ struct clk_hw hw;
+ void __iomem *reg_pmu;
+ spinlock_t *lock;
+};
+
+static unsigned long moxart_recalc_rate(struct clk_hw *c_hw,
+ unsigned long parent_rate)
+{
+ unsigned int mul, val, div;
+ unsigned long ret;
+ struct clk_device *dev_clk = container_of(c_hw, struct clk_device, hw);
+
+ mul = (readl(dev_clk->reg_pmu + 0x30) >> 3) & 0x1ff;
+ val = (readl(dev_clk->reg_pmu + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 0:
+ div = 2;
+ break;
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ ret = (mul * 1200000 / div);
+
+ /* UC-7112-LX: ret=48000000 mul=80 div=2 val=0 */
+ pr_debug("%s: ret=%lu mul=%d div=%d val=%d\n",
+ __func__, ret, mul, div, val);
+ return ret;
+}
+
+static const struct clk_ops moxart_clk_ops = {
+ .recalc_rate = moxart_recalc_rate,
+};
+
+static const struct of_device_id moxart_pmu_match[] = {
+ { .compatible = "moxa,moxart-pmu" },
+ { },
+};
+
+static const struct of_device_id moxart_sysclk_match[] = {
+ { .compatible = "moxa,moxart-sysclk" },
+ { }
+};
+
+void __init moxart_of_clk_init(void)
+{
+ struct device_node *node, *clk_node;
+ struct clk *clk;
+ struct clk_device *dev_clk;
+ struct clk_init_data init;
+ int err;
+ const char *clk_name;
+
+ dev_clk = kzalloc(sizeof(*dev_clk), GFP_KERNEL);
+ if (WARN_ON(!dev_clk))
+ return;
+
+ dev_clk->lock = &_lock;
+
+ node = of_find_matching_node(NULL, moxart_pmu_match);
+ if (!node) {
+ pr_err("%s: can't find PMU DT node\n", __func__);
+ return;
+ }
+
+ dev_clk->reg_pmu = of_iomap(node, 0);
+ if (IS_ERR(dev_clk->reg_pmu)) {
+ pr_err("%s: of_iomap failed\n", __func__);
+ return;
+ }
+
+ clk_node = of_find_matching_node(NULL, moxart_sysclk_match);
+ if (!clk_node) {
+ pr_err("%s: can't find sys_clk DT node\n", __func__);
+ return;
+ }
+
+ clk_name = clk_node->name;
+
+ of_property_read_string(clk_node, "clock-output-names",
+ &clk_name);
+
+ init.name = clk_name;
+ init.ops = &moxart_clk_ops;
+ init.flags = CLK_IS_ROOT;
+ init.num_parents = 0;
+
+ dev_clk->hw.init = &init;
+
+ clk = clk_register(NULL, &dev_clk->hw);
+
+ if (WARN_ON(IS_ERR(clk))) {
+ kfree(dev_clk);
+ return;
+ }
+
+ clk_register_clkdev(clk, NULL, clk_name);
+
+ err = of_clk_add_provider(clk_node, of_clk_src_simple_get, clk);
+
+ pr_info("%s: %s finished\n", node->full_name, __func__);
+}
--
1.8.2.1


2013-07-04 13:08:27

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v2] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
of_clk_init(NULL) is now called from moxart machine descriptor
.init_time hook. clocksource now depend on this to be loaded first.

Applies to next-20130703

Changes since v1:
1. rewritten to register one fixed rate clock "clkapb"
2. use CLK_OF_DECLARE

drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..933622f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..d61e305
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,82 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static const struct of_device_id moxart_tclk_match[] = {
+ { .compatible = "moxa,moxart-apb-clock" },
+ { }
+};
+
+void __init moxart_of_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct device_node *clk_node;
+ struct clk *clk;
+ unsigned long rate;
+ unsigned int mul, val, div;
+
+ base = of_iomap(node, 0);
+ if (IS_ERR(base))
+ panic("%s: of_iomap failed\n", node->full_name);
+
+ clk_node = of_find_matching_node(NULL, moxart_tclk_match);
+ if (!clk_node)
+ panic("%s: can't find clock DT node\n", node->full_name);
+
+ mul = (readl(base + 0x30) >> 3) & 0x1ff;
+ val = (readl(base + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ /*
+ * the rate calculation below is only tested and proven
+ * to be true for UC-7112-LX
+ *
+ * UC-7112-LX: mul=80 val=0
+ *
+ * to support other moxart SoC hardware, this may need
+ * a change, though it's possible it works there too
+ */
+ rate = (mul * 1200000 / div);
+
+ clk = clk_register_fixed_rate(NULL, "clkapb", NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, "clkapb");
+ of_clk_add_provider(clk_node, of_clk_src_simple_get, clk);
+
+ iounmap(base);
+}
+CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
--
1.8.2.1

2013-07-17 13:23:40

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v3] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Changes since v2:

1. add devicetree bindings document

Applies to next-20130716

.../bindings/clock/moxa,moxart-core-clock.txt | 19 +++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 82 ++++++++++++++++++++++
3 files changed, 102 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
new file mode 100644
index 0000000..cf69361
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
@@ -0,0 +1,19 @@
+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 core clock frequencies by reading
+a register.
+
+Required properties:
+- compatible : Should be "moxa,moxart-core-clock"
+- reg : Should contain registers location and length
+
+For example:
+
+ clk: core-clock@98100000 {
+ compatible = "moxa,moxart-core-clock";
+ reg = <0x98100000 0x34>;
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..933622f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..79c27f4
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,82 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+static const struct of_device_id moxart_apb_clock_match[] = {
+ { .compatible = "moxa,moxart-apb-clock" },
+ { }
+};
+
+void __init moxart_of_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct device_node *clk_node;
+ struct clk *clk;
+ unsigned long rate;
+ unsigned int mul, val, div;
+
+ base = of_iomap(node, 0);
+ if (IS_ERR(base))
+ panic("%s: of_iomap failed\n", node->full_name);
+
+ clk_node = of_find_matching_node(NULL, moxart_apb_clock_match);
+ if (!clk_node)
+ panic("%s: can't find clock DT node\n", node->full_name);
+
+ mul = (readl(base + 0x30) >> 3) & 0x1ff;
+ val = (readl(base + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ /*
+ * the rate calculation below is only tested and proven
+ * to be true for UC-7112-LX
+ *
+ * UC-7112-LX: mul=80 val=0
+ *
+ * to support other moxart SoC hardware, this may need
+ * a change, though it's possible it works there too
+ */
+ rate = (mul * 1200000 / div);
+
+ clk = clk_register_fixed_rate(NULL, "clkapb", NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, "clkapb");
+ of_clk_add_provider(clk_node, of_clk_src_simple_get, clk);
+
+ iounmap(base);
+}
+CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
--
1.8.2.1

2013-07-18 09:51:10

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

On Wed, Jul 17, 2013 at 02:23:16PM +0100, Jonas Jensen wrote:
> This patch adds MOXA ART SoCs clock driver support.
>
> Signed-off-by: Jonas Jensen <[email protected]>
> ---
>
> Notes:
> Changes since v2:
>
> 1. add devicetree bindings document
>
> Applies to next-20130716
>
> .../bindings/clock/moxa,moxart-core-clock.txt | 19 +++++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-moxart.c | 82 ++++++++++++++++++++++
> 3 files changed, 102 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> create mode 100644 drivers/clk/clk-moxart.c
>
> diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> new file mode 100644
> index 0000000..cf69361
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> @@ -0,0 +1,19 @@
> +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 core clock frequencies by reading
> +a register.
> +
> +Required properties:
> +- compatible : Should be "moxa,moxart-core-clock"
> +- reg : Should contain registers location and length
> +
> +For example:
> +
> + clk: core-clock@98100000 {
> + compatible = "moxa,moxart-core-clock";
> + reg = <0x98100000 0x34>;
> + };
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 4038c2b..933622f 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
>
> # SoCs specific
> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
> diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> new file mode 100644
> index 0000000..79c27f4
> --- /dev/null
> +++ b/drivers/clk/clk-moxart.c
> @@ -0,0 +1,82 @@
> +/*
> + * MOXA ART SoCs clock driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <[email protected]>
> + *
> + * 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/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/clkdev.h>
> +
> +static const struct of_device_id moxart_apb_clock_match[] = {
> + { .compatible = "moxa,moxart-apb-clock" },
> + { }
> +};
> +
> +void __init moxart_of_clk_init(struct device_node *node)
> +{
> + static void __iomem *base;
> + struct device_node *clk_node;
> + struct clk *clk;
> + unsigned long rate;
> + unsigned int mul, val, div;
> +
> + base = of_iomap(node, 0);
> + if (IS_ERR(base))
> + panic("%s: of_iomap failed\n", node->full_name);
> +
> + clk_node = of_find_matching_node(NULL, moxart_apb_clock_match);
> + if (!clk_node)
> + panic("%s: can't find clock DT node\n", node->full_name);
> +
> + mul = (readl(base + 0x30) >> 3) & 0x1ff;
> + val = (readl(base + 0x0c) >> 4) & 0x7;
> +
> + switch (val) {
> + case 1:
> + div = 3;
> + break;
> + case 2:
> + div = 4;
> + break;
> + case 3:
> + div = 6;
> + break;
> + case 4:
> + div = 8;
> + break;
> + default:
> + div = 2;
> + break;
> + }
> +
> + /*
> + * the rate calculation below is only tested and proven
> + * to be true for UC-7112-LX
> + *
> + * UC-7112-LX: mul=80 val=0
> + *
> + * to support other moxart SoC hardware, this may need
> + * a change, though it's possible it works there too
> + */
> + rate = (mul * 1200000 / div);
> +
> + clk = clk_register_fixed_rate(NULL, "clkapb", NULL, CLK_IS_ROOT, rate);
> + clk_register_clkdev(clk, NULL, "clkapb");
> + of_clk_add_provider(clk_node, of_clk_src_simple_get, clk);

This confuses me. moxart_of_clk_init gets called because there was a
"moxa,moxart-core-clock", node in the dt, but the driver only seems to
use the information to figure out the configuration of another clock
("moxa,moxart-apb-clock"), and never registers a clock specifically for
the core-clock.

I couldn't find "moxa,moxart-apb-clock" described in mainline. COuld you
describe the relationship between core-clock and apb-clock?

Thanks,
Mark.

> +
> + iounmap(base);
> +}
> +CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
> --
> 1.8.2.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

2013-07-18 10:40:45

by Jonas Jensen

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

Hi Mark, thanks for taking a look at this.

On 18 July 2013 11:50, Mark Rutland <[email protected]> wrote:
> This confuses me. moxart_of_clk_init gets called because there was a
> "moxa,moxart-core-clock", node in the dt, but the driver only seems to
> use the information to figure out the configuration of another clock
> ("moxa,moxart-apb-clock"), and never registers a clock specifically for
> the core-clock.
>
> I couldn't find "moxa,moxart-apb-clock" described in mainline. COuld you
> describe the relationship between core-clock and apb-clock?

It's true core-clock exist only so the register can be mapped.

apb-clock is part of a patch set that will add new device tree files for the
MOXA ART SoC, but it's not in mainline:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181757.html

apb-clock could be a fixed rate 48MHz DT only clock, but because we can't
be sure it's 48MHz on all platforms, reading it from a register with core-clock
is more portable.


Best regards,
Jonas

2013-07-18 11:02:40

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

On Thu, Jul 18, 2013 at 11:36:40AM +0100, Jonas Jensen wrote:
> Hi Mark, thanks for taking a look at this.
>
> On 18 July 2013 11:50, Mark Rutland <[email protected]> wrote:
> > This confuses me. moxart_of_clk_init gets called because there was a
> > "moxa,moxart-core-clock", node in the dt, but the driver only seems to
> > use the information to figure out the configuration of another clock
> > ("moxa,moxart-apb-clock"), and never registers a clock specifically for
> > the core-clock.
> >
> > I couldn't find "moxa,moxart-apb-clock" described in mainline. COuld you
> > describe the relationship between core-clock and apb-clock?
>
> It's true core-clock exist only so the register can be mapped.

Ok. I'm just concerned that the linkage isn't explicit or obvious.

>
> apb-clock is part of a patch set that will add new device tree files for the
> MOXA ART SoC, but it's not in mainline:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-July/181757.html
>
> apb-clock could be a fixed rate 48MHz DT only clock, but because we can't
> be sure it's 48MHz on all platforms, reading it from a register with core-clock
> is more portable.

This does leave apb-clock completely dependent on core-clock, and unless
I've missed something there's no linkage between the two described in
the dt.

How does core-clock physically relate to apb-clock? Does it feed or is
it fed by apb-clock?

Are we always guaranteed to have core-clock if we have apb-clock, and is
it part of the same block in hardware? If so we could describe the
amalgamation as a provider with two clock outputs, with core-clock's
registers for configuration at probe-time.

Thanks,
Mark.

>
>
> Best regards,
> Jonas
>

2013-07-18 11:55:28

by Jonas Jensen

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

On 18 July 2013 13:02, Mark Rutland <[email protected]> wrote:
> Ok. I'm just concerned that the linkage isn't explicit or obvious.

> This does leave apb-clock completely dependent on core-clock, and unless
> I've missed something there's no linkage between the two described in
> the dt.

I can add a description in the core-clock binding and also for
apb-clock pointing
out that it's set from core-clock.

> How does core-clock physically relate to apb-clock? Does it feed or is
> it fed by apb-clock?

apb-clock is entirely a DT construct used by drivers to get the fixed
rate 48MHz.
It's not fed by core-clock more than what happens in probe.

For UC-7112-LX, drivers using apb-clock are: clocksource, MMC, watchdog

Because clocksource relies on apb-clock, a successful probe of
core-clock is critical.

Commonly, drivers look up the apb-clock node and call clk_get_rate.

> Are we always guaranteed to have core-clock if we have apb-clock, and is
> it part of the same block in hardware? If so we could describe the
> amalgamation as a provider with two clock outputs, with core-clock's
> registers for configuration at probe-time.

Yes, as described above, there can not be a apb-clock without core-clock.


I think drivers could find and use core-clock instead. Maybe the abstraction
of apb-clock is unnecessary?


Best regards,
Jonas

2013-07-18 13:57:25

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

On Thu, Jul 18, 2013 at 12:55:26PM +0100, Jonas Jensen wrote:
> On 18 July 2013 13:02, Mark Rutland <[email protected]> wrote:
> > Ok. I'm just concerned that the linkage isn't explicit or obvious.
>
> > This does leave apb-clock completely dependent on core-clock, and unless
> > I've missed something there's no linkage between the two described in
> > the dt.
>
> I can add a description in the core-clock binding and also for
> apb-clock pointing
> out that it's set from core-clock.
>
> > How does core-clock physically relate to apb-clock? Does it feed or is
> > it fed by apb-clock?
>
> apb-clock is entirely a DT construct used by drivers to get the fixed
> rate 48MHz.
> It's not fed by core-clock more than what happens in probe.
>
> For UC-7112-LX, drivers using apb-clock are: clocksource, MMC, watchdog
>
> Because clocksource relies on apb-clock, a successful probe of
> core-clock is critical.
>
> Commonly, drivers look up the apb-clock node and call clk_get_rate.

When you say look up the apb-clock node, I assume they find it via their
clocks property with of_clk_get or similar, rather than scanning the
tree for a "moxart,moxa-apb-clock" node specifically?

>
> > Are we always guaranteed to have core-clock if we have apb-clock, and is
> > it part of the same block in hardware? If so we could describe the
> > amalgamation as a provider with two clock outputs, with core-clock's
> > registers for configuration at probe-time.
>
> Yes, as described above, there can not be a apb-clock without core-clock.
>
>
> I think drivers could find and use core-clock instead. Maybe the abstraction
> of apb-clock is unnecessary?

Given the above, I think it would make more sense to have core-clock
registered, and have it passed to the devices currently passed
apb-clock.

Thanks,
Mark.

>
>
> Best regards,
> Jonas
>

2013-07-18 14:25:29

by Jonas Jensen

[permalink] [raw]
Subject: Re: [PATCH v3] clk: add MOXA ART SoCs clock driver

On 18 July 2013 15:56, Mark Rutland <[email protected]> wrote:
> When you say look up the apb-clock node, I assume they find it via their
> clocks property with of_clk_get or similar, rather than scanning the
> tree for a "moxart,moxa-apb-clock" node specifically?

Yes, all use of_clk_get(node, 0), which is fortunate because e.g.
clocksource is already merged with linaro's clockevent drivers tree.

That should mean I can get away with only changing the DT files :)

> Given the above, I think it would make more sense to have core-clock
> registered, and have it passed to the devices currently passed
> apb-clock.

OK, I'll return with an updated version.


Best regards,
Jonas

2013-07-19 08:09:01

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v4] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Changes since v3:

1. remove apb-clock
2. read clock-output-names of property
3. update device tree bindings document

Applies to next-20130716

.../bindings/clock/moxa,moxart-core-clock.txt | 23 +++++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 74 ++++++++++++++++++++++
3 files changed, 98 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
new file mode 100644
index 0000000..528c8b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
@@ -0,0 +1,23 @@
+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 core clock frequencies by reading
+a register.
+
+Required properties:
+- compatible : Should be "moxa,moxart-core-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clock-output-names : Should be "coreclk"
+
+For example:
+
+ coreclk: core-clock@98100000 {
+ compatible = "moxa,moxart-core-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clock-output-names = "coreclk";
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..933622f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..a7a1e7b
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,74 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/clkdev.h>
+
+void __init moxart_of_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct clk *clk;
+ unsigned long rate;
+ unsigned int mul, val, div;
+ const char *name;
+
+ base = of_iomap(node, 0);
+ if (IS_ERR(base))
+ panic("%s: of_iomap failed\n", node->full_name);
+
+ mul = (readl(base + 0x30) >> 3) & 0x1ff;
+ val = (readl(base + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ /*
+ * the rate calculation below is only tested and proven
+ * to be true for UC-7112-LX
+ *
+ * UC-7112-LX: mul=80 val=0
+ *
+ * to support other moxart SoC hardware, this may need
+ * a change, though it's possible it works there too
+ */
+ rate = (mul * 1200000 / div);
+
+ of_property_read_string(node, "clock-output-names", &name);
+ clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ iounmap(base);
+}
+CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
--
1.8.2.1

2013-07-19 08:17:56

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v5] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Changes since v4:

1. remove unnecessary header includes

Applies to next-20130716

.../bindings/clock/moxa,moxart-core-clock.txt | 23 ++++++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 69 ++++++++++++++++++++++
3 files changed, 93 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
new file mode 100644
index 0000000..528c8b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
@@ -0,0 +1,23 @@
+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 core clock frequencies by reading
+a register.
+
+Required properties:
+- compatible : Should be "moxa,moxart-core-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clock-output-names : Should be "coreclk"
+
+For example:
+
+ coreclk: core-clock@98100000 {
+ compatible = "moxa,moxart-core-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clock-output-names = "coreclk";
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..933622f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..5559371
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,69 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct clk *clk;
+ unsigned long rate;
+ unsigned int mul, val, div;
+ const char *name;
+
+ base = of_iomap(node, 0);
+ if (IS_ERR(base))
+ panic("%s: of_iomap failed\n", node->full_name);
+
+ mul = (readl(base + 0x30) >> 3) & 0x1ff;
+ val = (readl(base + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ /*
+ * the rate calculation below is only tested and proven
+ * to be true for UC-7112-LX
+ *
+ * UC-7112-LX: mul=80 val=0
+ *
+ * to support other moxart SoC hardware, this may need
+ * a change, though it's possible it works there too
+ */
+ rate = (mul * 1200000 / div);
+
+ of_property_read_string(node, "clock-output-names", &name);
+ clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ iounmap(base);
+}
+CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
--
1.8.2.1

2013-07-22 09:22:10

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v5] clk: add MOXA ART SoCs clock driver

On Fri, Jul 19, 2013 at 09:17:17AM +0100, Jonas Jensen wrote:
> This patch adds MOXA ART SoCs clock driver support.
>
> Signed-off-by: Jonas Jensen <[email protected]>
> ---
>
> Notes:
> Changes since v4:
>
> 1. remove unnecessary header includes
>
> Applies to next-20130716
>
> .../bindings/clock/moxa,moxart-core-clock.txt | 23 ++++++++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-moxart.c | 69 ++++++++++++++++++++++
> 3 files changed, 93 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> create mode 100644 drivers/clk/clk-moxart.c
>
> diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> new file mode 100644
> index 0000000..528c8b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> @@ -0,0 +1,23 @@
> +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 core clock frequencies by reading
> +a register.
> +
> +Required properties:
> +- compatible : Should be "moxa,moxart-core-clock"
> +- #clock-cells : Should be 0
> +- reg : Should contain registers location and length
> +- clock-output-names : Should be "coreclk"
> +
> +For example:
> +
> + coreclk: core-clock@98100000 {
> + compatible = "moxa,moxart-core-clock";
> + #clock-cells = <0>;
> + reg = <0x98100000 0x34>;
> + clock-output-names = "coreclk";
> + };
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 4038c2b..933622f 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
>
> # SoCs specific
> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
> diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> new file mode 100644
> index 0000000..5559371
> --- /dev/null
> +++ b/drivers/clk/clk-moxart.c
> @@ -0,0 +1,69 @@
> +/*
> + * MOXA ART SoCs clock driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <[email protected]>
> + *
> + * 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_clk_init(struct device_node *node)
> +{
> + static void __iomem *base;
> + struct clk *clk;
> + unsigned long rate;
> + unsigned int mul, val, div;
> + const char *name;
> +
> + base = of_iomap(node, 0);
> + if (IS_ERR(base))
> + panic("%s: of_iomap failed\n", node->full_name);

As far as I'm aware, of_iomap returns NULL, not an ERR_PTR value.

Also, is it necessary to panic() ? In v1 of the patch you used pr_err...

Thanks,
Mark.

> +
> + mul = (readl(base + 0x30) >> 3) & 0x1ff;
> + val = (readl(base + 0x0c) >> 4) & 0x7;
> +
> + switch (val) {
> + case 1:
> + div = 3;
> + break;
> + case 2:
> + div = 4;
> + break;
> + case 3:
> + div = 6;
> + break;
> + case 4:
> + div = 8;
> + break;
> + default:
> + div = 2;
> + break;
> + }
> +
> + /*
> + * the rate calculation below is only tested and proven
> + * to be true for UC-7112-LX
> + *
> + * UC-7112-LX: mul=80 val=0
> + *
> + * to support other moxart SoC hardware, this may need
> + * a change, though it's possible it works there too
> + */
> + rate = (mul * 1200000 / div);
> +
> + of_property_read_string(node, "clock-output-names", &name);
> + clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
> + clk_register_clkdev(clk, NULL, name);
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +
> + iounmap(base);
> +}
> +CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
> --
> 1.8.2.1
>
>

2013-07-23 08:10:18

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v5] clk: add MOXA ART SoCs clock driver

On Monday 22 of July 2013 10:21:38 Mark Rutland wrote:
> On Fri, Jul 19, 2013 at 09:17:17AM +0100, Jonas Jensen wrote:
> > This patch adds MOXA ART SoCs clock driver support.
> >
> > Signed-off-by: Jonas Jensen <[email protected]>
> > ---
> >
> > Notes:
> > Changes since v4:
> >
> > 1. remove unnecessary header includes
> >
> > Applies to next-20130716
> >
> > .../bindings/clock/moxa,moxart-core-clock.txt | 23 ++++++++
> > drivers/clk/Makefile | 1 +
> > drivers/clk/clk-moxart.c | 69
> > ++++++++++++++++++++++ 3 files changed, 93 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > create mode 100644 drivers/clk/clk-moxart.c
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > new file mode 100644
> > index 0000000..528c8b0
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > @@ -0,0 +1,23 @@
> > +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 core clock frequencies by reading
> > +a register.
> > +
> > +Required properties:
> > +- compatible : Should be "moxa,moxart-core-clock"
> > +- #clock-cells : Should be 0
> > +- reg : Should contain registers location and length
> > +- clock-output-names : Should be "coreclk"
> > +
> > +For example:
> > +
> > + coreclk: core-clock@98100000 {
> > + compatible = "moxa,moxart-core-clock";
> > + #clock-cells = <0>;
> > + reg = <0x98100000 0x34>;
> > + clock-output-names = "coreclk";
> > + };
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 4038c2b..933622f 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
> >
> > # SoCs specific
> > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
> >
> > diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> > new file mode 100644
> > index 0000000..5559371
> > --- /dev/null
> > +++ b/drivers/clk/clk-moxart.c
> > @@ -0,0 +1,69 @@
> > +/*
> > + * MOXA ART SoCs clock driver.
> > + *
> > + * Copyright (C) 2013 Jonas Jensen
> > + *
> > + * Jonas Jensen <[email protected]>
> > + *
> > + * 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_clk_init(struct device_node *node)
> > +{
> > + static void __iomem *base;
> > + struct clk *clk;
> > + unsigned long rate;
> > + unsigned int mul, val, div;
> > + const char *name;
> > +
> > + base = of_iomap(node, 0);
> > + if (IS_ERR(base))
> > + panic("%s: of_iomap failed\n", node->full_name);
>
> As far as I'm aware, of_iomap returns NULL, not an ERR_PTR value.
>
> Also, is it necessary to panic() ? In v1 of the patch you used pr_err...

This is a good question in general, not only for this driver.

IMHO if you know early that something is really wrong, to the point that
the system won't be able to work normally (and failing to initialize
clocks looks like it) it is better to let the user/developer know that
something is utterly wrong at this point without letting the system run
for a bit more and fail anyway with a harder to track down error message.

It's not so obvious, though, what kinds of failure should be considered
critical. IMHO any failure in low level initialization code, but this is
debatable, I guess.

Best regards,
Tomasz

2013-07-26 22:32:41

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v5] clk: add MOXA ART SoCs clock driver

Quoting Tomasz Figa (2013-07-23 01:09:06)
> On Monday 22 of July 2013 10:21:38 Mark Rutland wrote:
> > On Fri, Jul 19, 2013 at 09:17:17AM +0100, Jonas Jensen wrote:
> > > This patch adds MOXA ART SoCs clock driver support.
> > >
> > > Signed-off-by: Jonas Jensen <[email protected]>
> > > ---
> > >
> > > Notes:
> > > Changes since v4:
> > >
> > > 1. remove unnecessary header includes
> > >
> > > Applies to next-20130716
> > >
> > > .../bindings/clock/moxa,moxart-core-clock.txt | 23 ++++++++
> > > drivers/clk/Makefile | 1 +
> > > drivers/clk/clk-moxart.c | 69
> > > ++++++++++++++++++++++ 3 files changed, 93 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > > create mode 100644 drivers/clk/clk-moxart.c
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > > b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > > new file mode 100644
> > > index 0000000..528c8b0
> > > --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> > > @@ -0,0 +1,23 @@
> > > +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 core clock frequencies by reading
> > > +a register.
> > > +
> > > +Required properties:
> > > +- compatible : Should be "moxa,moxart-core-clock"
> > > +- #clock-cells : Should be 0
> > > +- reg : Should contain registers location and length
> > > +- clock-output-names : Should be "coreclk"
> > > +
> > > +For example:
> > > +
> > > + coreclk: core-clock@98100000 {
> > > + compatible = "moxa,moxart-core-clock";
> > > + #clock-cells = <0>;
> > > + reg = <0x98100000 0x34>;
> > > + clock-output-names = "coreclk";
> > > + };
> > > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > > index 4038c2b..933622f 100644
> > > --- a/drivers/clk/Makefile
> > > +++ b/drivers/clk/Makefile
> > > @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
> > >
> > > # SoCs specific
> > > obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
> > >
> > > diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> > > new file mode 100644
> > > index 0000000..5559371
> > > --- /dev/null
> > > +++ b/drivers/clk/clk-moxart.c
> > > @@ -0,0 +1,69 @@
> > > +/*
> > > + * MOXA ART SoCs clock driver.
> > > + *
> > > + * Copyright (C) 2013 Jonas Jensen
> > > + *
> > > + * Jonas Jensen <[email protected]>
> > > + *
> > > + * 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_clk_init(struct device_node *node)
> > > +{
> > > + static void __iomem *base;
> > > + struct clk *clk;
> > > + unsigned long rate;
> > > + unsigned int mul, val, div;
> > > + const char *name;
> > > +
> > > + base = of_iomap(node, 0);
> > > + if (IS_ERR(base))
> > > + panic("%s: of_iomap failed\n", node->full_name);
> >
> > As far as I'm aware, of_iomap returns NULL, not an ERR_PTR value.
> >
> > Also, is it necessary to panic() ? In v1 of the patch you used pr_err...
>
> This is a good question in general, not only for this driver.
>
> IMHO if you know early that something is really wrong, to the point that
> the system won't be able to work normally (and failing to initialize
> clocks looks like it) it is better to let the user/developer know that
> something is utterly wrong at this point without letting the system run
> for a bit more and fail anyway with a harder to track down error message.

You can let the user/developer know that something is utterly wrong with
a WARN.

Where panics and BUGs come back to haunt you is when you are bringing up
a new platform or SoC that uses this clock driver, but this clock driver
fails pending some subtle change for the new platform/SoC.

But perhaps your bootloader at least set the clocks up for you sensibly
and your other device drivers might bail out due to not having clocks
but at least you could boot your new system. But putting a panic/BUG
here makes the above scenario impossible.

>
> It's not so obvious, though, what kinds of failure should be considered
> critical. IMHO any failure in low level initialization code, but this is
> debatable, I guess.

Agreed that's it's hard to know when to use them. But if you can get
away with a WARN then I always err on the cautious side and try to
prevent panicing the system explicitly.

Regards,
Mike

>
> Best regards,
> Tomasz

2013-07-29 09:44:44

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v6] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Changes since v5:

1. corrected of_iomap return value check
2. don't panic, print the error and return

Applies to next-20130729

.../bindings/clock/moxa,moxart-core-clock.txt | 23 +++++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 71 ++++++++++++++++++++++
3 files changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
create mode 100644 drivers/clk/clk-moxart.c

diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
new file mode 100644
index 0000000..379ae79
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
@@ -0,0 +1,23 @@
+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 core clock frequencies by reading
+a register.
+
+Required properties:
+- compatible : Must be "moxa,moxart-core-clock"
+- #clock-cells : Should be 0
+- reg : Should contain registers location and length
+- clock-output-names : Should be "coreclk"
+
+For example:
+
+ coreclk: core-clock@98100000 {
+ compatible = "moxa,moxart-core-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clock-output-names = "coreclk";
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4038c2b..933622f 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..14d5b26
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,71 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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_clk_init(struct device_node *node)
+{
+ static void __iomem *base;
+ struct clk *clk;
+ unsigned long rate;
+ unsigned int mul, val, div;
+ const char *name;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s: of_iomap failed\n", node->full_name);
+ return;
+ }
+
+ mul = (readl(base + 0x30) >> 3) & 0x1ff;
+ val = (readl(base + 0x0c) >> 4) & 0x7;
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ /*
+ * the rate calculation below is only tested and proven
+ * to be true for UC-7112-LX
+ *
+ * UC-7112-LX: mul=80 val=0
+ *
+ * to support other moxart SoC hardware, this may need
+ * a change, though it's possible it works there too
+ */
+ rate = (mul * 1200000 / div);
+
+ of_property_read_string(node, "clock-output-names", &name);
+ clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ iounmap(base);
+}
+CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
--
1.8.2.1

2013-10-07 04:47:08

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH v6] clk: add MOXA ART SoCs clock driver

Quoting Jonas Jensen (2013-07-29 02:44:22)
> This patch adds MOXA ART SoCs clock driver support.
>
> Signed-off-by: Jonas Jensen <[email protected]>

I've taken this patch into clk-next. Thanks for the rework.

Is it possible for parent clocks of these moxa core clocks to change
rate? It might make sense for your driver to provide a .recalc_rate
callback in a future patch.

Regards,
Mike

> ---
>
> Notes:
> Changes since v5:
>
> 1. corrected of_iomap return value check
> 2. don't panic, print the error and return
>
> Applies to next-20130729
>
> .../bindings/clock/moxa,moxart-core-clock.txt | 23 +++++++
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-moxart.c | 71 ++++++++++++++++++++++
> 3 files changed, 95 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> create mode 100644 drivers/clk/clk-moxart.c
>
> diff --git a/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> new file mode 100644
> index 0000000..379ae79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/moxa,moxart-core-clock.txt
> @@ -0,0 +1,23 @@
> +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 core clock frequencies by reading
> +a register.
> +
> +Required properties:
> +- compatible : Must be "moxa,moxart-core-clock"
> +- #clock-cells : Should be 0
> +- reg : Should contain registers location and length
> +- clock-output-names : Should be "coreclk"
> +
> +For example:
> +
> + coreclk: core-clock@98100000 {
> + compatible = "moxa,moxart-core-clock";
> + #clock-cells = <0>;
> + reg = <0x98100000 0x34>;
> + clock-output-names = "coreclk";
> + };
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 4038c2b..933622f 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o
>
> # SoCs specific
> obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
> diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
> new file mode 100644
> index 0000000..14d5b26
> --- /dev/null
> +++ b/drivers/clk/clk-moxart.c
> @@ -0,0 +1,71 @@
> +/*
> + * MOXA ART SoCs clock driver.
> + *
> + * Copyright (C) 2013 Jonas Jensen
> + *
> + * Jonas Jensen <[email protected]>
> + *
> + * 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_clk_init(struct device_node *node)
> +{
> + static void __iomem *base;
> + struct clk *clk;
> + unsigned long rate;
> + unsigned int mul, val, div;
> + const char *name;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s: of_iomap failed\n", node->full_name);
> + return;
> + }
> +
> + mul = (readl(base + 0x30) >> 3) & 0x1ff;
> + val = (readl(base + 0x0c) >> 4) & 0x7;
> +
> + switch (val) {
> + case 1:
> + div = 3;
> + break;
> + case 2:
> + div = 4;
> + break;
> + case 3:
> + div = 6;
> + break;
> + case 4:
> + div = 8;
> + break;
> + default:
> + div = 2;
> + break;
> + }
> +
> + /*
> + * the rate calculation below is only tested and proven
> + * to be true for UC-7112-LX
> + *
> + * UC-7112-LX: mul=80 val=0
> + *
> + * to support other moxart SoC hardware, this may need
> + * a change, though it's possible it works there too
> + */
> + rate = (mul * 1200000 / div);
> +
> + of_property_read_string(node, "clock-output-names", &name);
> + clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
> + clk_register_clkdev(clk, NULL, name);
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +
> + iounmap(base);
> +}
> +CLK_OF_DECLARE(moxart_core_clock, "moxa,moxart-core-clock", moxart_of_clk_init);
> --
> 1.8.2.1

2013-10-09 14:55:22

by Jonas Jensen

[permalink] [raw]
Subject: [PATCH v7] clk: add MOXA ART SoCs clock driver

This patch adds MOXA ART SoCs clock driver support.

Signed-off-by: Jonas Jensen <[email protected]>
---

Notes:
Since last version, Adam Jaremko has been helping providing info on
the clock registers and the SoC in general. He's been looking at
bootloader sources in particular, which seem to do the same things
with slight variations.

Two separate clocks are now registered: clk_pll and clk_apb

clk_apb provides the same 48MHz frequency as before but the value is
calculated from clk_pll (192MHz) which in turn is calculated from a
fixed rate reference (12MHz).

Device drivers (watchdog, clocksource, MMC) should generally be
interested in reading apb_clk, not clk_pll.

clk_pll is obtained by multiplying the content of 0x98100030 (16)
with the reference.

The reference represent the PCB oscillator crystal, on UC-7112-LX
this has part number "12.000 KCC 20GT".

apb_clk is obtained by getting the rate of clk_pll and dividing it.

I don't think the parent clock can change rate, not without modifying
the hardware.

Changes since v6:

1. all rates calculated from a fixed rate reference clock
2. register two separate clocks, clk_pll and clk_apb

Applies to next-20130927

.../bindings/clock/moxa,moxart-clock.txt | 43 ++++++++
drivers/clk/Makefile | 1 +
drivers/clk/clk-moxart.c | 109 +++++++++++++++++++++
3 files changed, 153 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 0000000..61fd327
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/moxa,moxart-clock.txt
@@ -0,0 +1,43 @@
+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.
+
+Required properties:
+- compatible : Must be "moxa,moxart-pll-clock" or "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 for "moxa,moxart-pll-clock":
+- multiplier-reg : Should contain register offset
+- multiplier-mask : Should contain register mask
+- multiplier-shift : Should contain number of places the register is shifted to the right
+- clock-output-names : Should contain clock name
+
+Optional properties for "moxa,moxart-apb-clock":
+- divisor-reg : Should contain register offset
+- divisor-mask : Should contain register mask
+- divisor-shift : Should contain number of places the register is shifted to the right
+- clock-output-names : Should contain clock name
+
+
+For example:
+
+ clk_pll: clk_pll0@98100000 {
+ compatible = "moxa,moxart-pll-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clocks = <&ref12>;
+ };
+
+ clk_apb: clk_apb@98100000 {
+ compatible = "moxa,moxart-apb-clock";
+ #clock-cells = <0>;
+ reg = <0x98100000 0x34>;
+ clocks = <&pll0>;
+ };
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 7b11106..4c8d857 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-composite.o

# SoCs specific
obj-$(CONFIG_ARCH_BCM2835) += clk-bcm2835.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_NSPIRE) += clk-nspire.o
diff --git a/drivers/clk/clk-moxart.c b/drivers/clk/clk-moxart.c
new file mode 100644
index 0000000..18b337a
--- /dev/null
+++ b/drivers/clk/clk-moxart.c
@@ -0,0 +1,109 @@
+/*
+ * MOXA ART SoCs clock driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <[email protected]>
+ *
+ * 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, reg_offset = 0x30, reg_mask = 0x3f, reg_shift = 3;
+ const char *name = node->name;
+
+ of_property_read_u32(node, "multiplier-reg", &reg_offset);
+ of_property_read_u32(node, "multiplier-mask", &reg_mask);
+ of_property_read_u32(node, "multiplier-shift", &reg_shift);
+ 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 + reg_offset) >> reg_shift) & reg_mask;
+ 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);
+ 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, reg_offset = 0xc, reg_mask = 0x7, reg_shift = 4;
+ const char *name = node->name;
+
+ of_property_read_u32(node, "divisor-reg", &reg_offset);
+ of_property_read_u32(node, "divisor-mask", &reg_mask);
+ of_property_read_u32(node, "divisor-shift", &reg_shift);
+ 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;
+ }
+
+ val = (readl(base + reg_offset) >> reg_shift) & reg_mask;
+ iounmap(base);
+
+ switch (val) {
+ case 1:
+ div = 3;
+ break;
+ case 2:
+ div = 4;
+ break;
+ case 3:
+ div = 6;
+ break;
+ case 4:
+ div = 8;
+ break;
+ default:
+ div = 2;
+ break;
+ }
+
+ pll_clk = of_clk_get(node, 0);
+ if (IS_ERR(pll_clk)) {
+ pr_err("%s: of_clk_get failed\n", node->full_name);
+ return;
+ }
+
+ rate = clk_get_rate(pll_clk) / (div * 2);
+
+ clk = clk_register_fixed_rate(NULL, name, NULL, CLK_IS_ROOT, rate);
+ clk_register_clkdev(clk, NULL, name);
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(moxart_apb_clock, "moxa,moxart-apb-clock",
+ moxart_of_apb_clk_init);
+
--
1.8.2.1