2013-09-24 04:05:34

by Takashi Yoshii

[permalink] [raw]
Subject: [PATCH 0/6] ARM: shmobile: kzm9d-reference: migrate to common clock framework with DT

This patch series makes kzm9d-reference to move to new clk implementation
based on the common clock framework and device tree.

First three are for preparation.
[PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare
[PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare
[PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively

Next one is a proposal document of DT bindings.
[PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

This is the implementation, and dts which carries clock tree description.
[PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

Last one is small modification needed for migration.
[PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock

These patches do not remove sh-clkfwk version, even on emev2.
kzm9d(without -reference) target still uses sh-clkfwk.
I believe this method encourages step-by-step migration on other sh-mobile
targets, too.

Patches should be applied and compiled both on
v3.12-rc2 and
kernel/git/horms/renesas.git tag:renesas-devel-20130922

Confirmed on kernel/git/horms/renesas.git tag:renesas-devel-20130922
for boot up to command prompt.
- kzm9d board / multiplatform + kzm9d-reference configuration
- kzm9d board / kzm9d-reference configuration
- kzm9d board / kzm9d (with sh-clkfwk) configuration for regression.
- ape6evm (with sh-clkfwk) for regression.
for compile only
- sh7757lcr (in arch/sh with sh-clkfwk) for regression.


Shinya Kuribayashi (2):
clocksource: em_sti: convert to clk_prepare/unprepare
serial8250-em: convert to clk_prepare/unprepare

Takashi YOSHII (4):
sh: clkfwk: Select sh-/common- clkfwk alternatively
ARM: shmobile: emev2: Define SMU clock DT bindings
clk: emev2: Add support for emev2 SMU clocks with DT
ARM: shmobile: kzm9d-reference: Use common clock framework

.../devicetree/bindings/clock/emev2-clock.txt | 99 ++++++++++++++++++++
arch/arm/Kconfig | 2 +-
arch/arm/boot/dts/emev2.dtsi | 84 +++++++++++++++++
arch/arm/mach-shmobile/Kconfig | 1 +
arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 +-
arch/sh/Kconfig | 1 +
drivers/clk/Makefile | 2 +
drivers/clk/shmobile/Makefile | 5 +
drivers/clk/shmobile/clk-emev2.c | 104 +++++++++++++++++++++
drivers/clocksource/em_sti.c | 4 +-
drivers/sh/clk/Makefile | 3 +-
drivers/tty/serial/8250/8250_em.c | 6 +-
12 files changed, 305 insertions(+), 11 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/emev2-clock.txt
create mode 100644 drivers/clk/shmobile/Makefile
create mode 100644 drivers/clk/shmobile/clk-emev2.c

--
1.8.1.5


2013-09-24 04:16:08

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

Common clock framework version of emev2 clock support.
smu_clkdiv and smu_gclk are handled.
So far, reparent is not implemented, and is fixed to index #0.
SMU and small numbers of clocks are described in emev2.dtsi.

That function and numbers of clocks are equivalent to current
sh-clkfwk version. It is just enough to run kzm9d-reference.

Signed-off-by: Takashi Yoshii <[email protected]>
---
arch/arm/boot/dts/emev2.dtsi | 84 +++++++++++++++++++++++++++++++
drivers/clk/Makefile | 2 +
drivers/clk/shmobile/Makefile | 5 ++
drivers/clk/shmobile/clk-emev2.c | 104 +++++++++++++++++++++++++++++++++++++++
4 files changed, 195 insertions(+)
create mode 100644 drivers/clk/shmobile/Makefile
create mode 100644 drivers/clk/shmobile/clk-emev2.c

diff --git a/arch/arm/boot/dts/emev2.dtsi b/arch/arm/boot/dts/emev2.dtsi
index 9063a44..c6a4767 100644
--- a/arch/arm/boot/dts/emev2.dtsi
+++ b/arch/arm/boot/dts/emev2.dtsi
@@ -52,34 +52,118 @@
<0 121 4>;
};

+ smu {
+ compatible = "renesas,emev2-smu";
+ reg = <0xe0110000 0x10000>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ c32ki: c32ki {
+ compatible = "fixed-clock";
+ clock-frequency = <32768>;
+ #clock-cells = <0>;
+ };
+ pll3_fo: pll3_fo {
+ compatible = "fixed-factor-clock";
+ clocks = <&c32ki>;
+ clock-div = <1>;
+ clock-mult = <7000>;
+ #clock-cells = <0>;
+ };
+ usia_u0_sclkdiv: usia_u0_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x610 0>;
+ clocks = <&pll3_fo>;
+ #clock-cells = <0>;
+ };
+ usib_u1_sclkdiv: usib_u1_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x65c 0>;
+ clocks = <&pll3_fo>;
+ #clock-cells = <0>;
+ };
+ usib_u2_sclkdiv: usib_u2_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x65c 16>;
+ clocks = <&pll3_fo>;
+ #clock-cells = <0>;
+ };
+ usib_u3_sclkdiv: usib_u3_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x660 0>;
+ clocks = <&pll3_fo>;
+ #clock-cells = <0>;
+ };
+ usia_u0_sclk: usia_u0_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4a0 1>;
+ clocks = <&usia_u0_sclkdiv>;
+ #clock-cells = <0>;
+ };
+ usib_u1_sclk: usib_u1_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4b8 1>;
+ clocks = <&usib_u1_sclkdiv>;
+ #clock-cells = <0>;
+ };
+ usib_u2_sclk: usib_u2_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4bc 1>;
+ clocks = <&usib_u2_sclkdiv>;
+ #clock-cells = <0>;
+ };
+ usib_u3_sclk: usib_u3_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4c0 1>;
+ clocks = <&usib_u3_sclkdiv>;
+ #clock-cells = <0>;
+ };
+ sti_sclk: sti_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x528 1>;
+ clocks = <&c32ki>;
+ #clock-cells = <0>;
+ };
+ };
+
sti@e0180000 {
compatible = "renesas,em-sti";
reg = <0xe0180000 0x54>;
interrupts = <0 125 0>;
+ clocks = <&sti_sclk>;
+ clock-names = "sclk";
};

uart@e1020000 {
compatible = "renesas,em-uart";
reg = <0xe1020000 0x38>;
interrupts = <0 8 0>;
+ clocks = <&usia_u0_sclk>;
+ clock-names = "sclk";
};

uart@e1030000 {
compatible = "renesas,em-uart";
reg = <0xe1030000 0x38>;
interrupts = <0 9 0>;
+ clocks = <&usib_u1_sclk>;
+ clock-names = "sclk";
};

uart@e1040000 {
compatible = "renesas,em-uart";
reg = <0xe1040000 0x38>;
interrupts = <0 10 0>;
+ clocks = <&usib_u2_sclk>;
+ clock-names = "sclk";
};

uart@e1050000 {
compatible = "renesas,em-uart";
reg = <0xe1050000 0x38>;
interrupts = <0 11 0>;
+ clocks = <&usib_u3_sclk>;
+ clock-names = "sclk";
};

gpio0: gpio@e0050000 {
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 7b11106..3e64ac4 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,8 @@ 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_SHMOBILE) += shmobile/
+obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/

obj-$(CONFIG_X86) += x86/

diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
new file mode 100644
index 0000000..6a26eb6
--- /dev/null
+++ b/drivers/clk/shmobile/Makefile
@@ -0,0 +1,5 @@
+ifeq ($(CONFIG_COMMON_CLK), y)
+obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o
+endif
+# for emply built-in.o
+obj-n := dummy
diff --git a/drivers/clk/shmobile/clk-emev2.c b/drivers/clk/shmobile/clk-emev2.c
new file mode 100644
index 0000000..6c7c929
--- /dev/null
+++ b/drivers/clk/shmobile/clk-emev2.c
@@ -0,0 +1,104 @@
+/*
+ * EMMA Mobile EV2 common clock framework support
+ *
+ * Copyright (C) 2013 Takashi Yoshii <[email protected]>
+ * Copyright (C) 2012 Magnus Damm
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+/* EMEV2 SMU registers */
+#define USIAU0_RSTCTRL 0x094
+#define USIBU1_RSTCTRL 0x0ac
+#define USIBU2_RSTCTRL 0x0b0
+#define USIBU3_RSTCTRL 0x0b4
+#define STI_RSTCTRL 0x124
+#define STI_CLKSEL 0x688
+
+static DEFINE_SPINLOCK(lock);
+
+/* not pretty, but hey */
+void __iomem *smu_base;
+
+static void __init emev2_smu_write(unsigned long value, int offs)
+{
+ BUG_ON(!smu_base || (offs >= PAGE_SIZE));
+ writel_relaxed(value, smu_base + offs);
+}
+
+static const struct of_device_id smu_id[] __initconst = {
+ { .compatible = "renesas,emev2-smu", },
+ {},
+};
+
+static void __init emev2_smu_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_matching_node(NULL, smu_id);
+ BUG_ON(!np);
+ smu_base = of_iomap(np, 0);
+ BUG_ON(!smu_base);
+ of_node_put(np);
+
+ /* setup STI timer to run on 32.768 kHz and deassert reset */
+ emev2_smu_write(0, STI_CLKSEL);
+ emev2_smu_write(1, STI_RSTCTRL);
+
+ /* deassert reset for UART0->UART3 */
+ emev2_smu_write(2, USIAU0_RSTCTRL);
+ emev2_smu_write(2, USIBU1_RSTCTRL);
+ emev2_smu_write(2, USIBU2_RSTCTRL);
+ emev2_smu_write(2, USIBU3_RSTCTRL);
+}
+
+static void __init emev2_smu_clkdiv_init(struct device_node *np)
+{
+ u32 reg[2];
+ struct clk *clk;
+ const char *parent_name = of_clk_get_parent_name(np, 0);
+ if (WARN_ON(of_property_read_u32_array(np, "reg", reg, 2)))
+ return;
+ if (!smu_base)
+ emev2_smu_init();
+ clk = clk_register_divider(NULL, np->name, parent_name, 0,
+ smu_base + reg[0], reg[1], 8, 0, &lock);
+ of_clk_add_provider(np, of_clk_src_simple_get, clk);
+ clk_register_clkdev(clk, np->name, NULL);
+ pr_debug("## %s %s %p\n", __func__, np->name, clk);
+}
+CLK_OF_DECLARE(emev2_smu_clkdiv, "renesas,emev2-smu-clkdiv",
+ emev2_smu_clkdiv_init);
+
+static void __init emev2_smu_gclk_init(struct device_node *np)
+{
+ u32 reg[2];
+ struct clk *clk;
+ const char *parent_name = of_clk_get_parent_name(np, 0);
+ if (WARN_ON(of_property_read_u32_array(np, "reg", reg, 2)))
+ return;
+ if (!smu_base)
+ emev2_smu_init();
+ clk = clk_register_gate(NULL, np->name, parent_name, 0,
+ smu_base + reg[0], reg[1], 0, &lock);
+ of_clk_add_provider(np, of_clk_src_simple_get, clk);
+ clk_register_clkdev(clk, np->name, NULL);
+ pr_debug("## %s %s %p\n", __func__, np->name, clk);
+}
+CLK_OF_DECLARE(emev2_smu_gclk, "renesas,emev2-smu-gclk", emev2_smu_gclk_init);
--
1.8.1.5

2013-09-24 04:17:27

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock framework

Use common clock framework version of clock
drivers/clk/shmobile/clk-emev2.c
instead of sh-clkfwk version
arch/arm/mach-shmobile/clock-emev2.c

kzm9d(without -reference) still uses sh-clkfwk version.

Because two of that framework can not live in one kernel binary,
there will be SoCs and Boards that can not be in one binary as
multiplatform binary or so.
For example, kzm9d and kzm9d-reference is now exclusive.

Signed-off-by: Takashi Yoshii <[email protected]>
---
arch/arm/mach-shmobile/Kconfig | 1 +
arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 50bab8d..d20d4ce 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -237,6 +237,7 @@ config MACH_KZM9D_REFERENCE
depends on ARCH_EMEV2
select REGULATOR_FIXED_VOLTAGE if REGULATOR
select USE_OF
+ select COMMON_CLK
---help---
Use reference implementation of KZM9D board support
which makes a greater use of device tree at the expense
diff --git a/arch/arm/mach-shmobile/board-kzm9d-reference.c b/arch/arm/mach-shmobile/board-kzm9d-reference.c
index 8f8bb2f..e0b8317 100644
--- a/arch/arm/mach-shmobile/board-kzm9d-reference.c
+++ b/arch/arm/mach-shmobile/board-kzm9d-reference.c
@@ -20,15 +20,14 @@

#include <linux/init.h>
#include <linux/of_platform.h>
+#include <linux/clk-provider.h>
#include <mach/emev2.h>
#include <mach/common.h>
#include <asm/mach/arch.h>

static void __init kzm9d_add_standard_devices(void)
{
- if (!IS_ENABLED(CONFIG_COMMON_CLK))
- emev2_clock_init();
-
+ of_clk_init(NULL);
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}

--
1.8.1.5

2013-09-24 04:18:53

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

From: Shinya Kuribayashi <[email protected]>

Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
migrate to the common clock framework.

Signed-off-by: Shinya Kuribayashi <[email protected]>
[[email protected]: edited for conflicts]
Signed-off-by: Takashi Yoshii <[email protected]>
---
drivers/tty/serial/8250/8250_em.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c
index 5f3bba1..d1a9078 100644
--- a/drivers/tty/serial/8250/8250_em.c
+++ b/drivers/tty/serial/8250/8250_em.c
@@ -122,7 +122,7 @@ static int serial8250_em_probe(struct platform_device *pdev)
up.port.dev = &pdev->dev;
up.port.private_data = priv;

- clk_enable(priv->sclk);
+ clk_prepare_enable(priv->sclk);
up.port.uartclk = clk_get_rate(priv->sclk);

up.port.iotype = UPIO_MEM32;
@@ -134,7 +134,7 @@ static int serial8250_em_probe(struct platform_device *pdev)
ret = serial8250_register_8250_port(&up);
if (ret < 0) {
dev_err(&pdev->dev, "unable to register 8250 port\n");
- clk_disable(priv->sclk);
+ clk_disable_unprepare(priv->sclk);
return ret;
}

@@ -148,7 +148,7 @@ static int serial8250_em_remove(struct platform_device *pdev)
struct serial8250_em_priv *priv = platform_get_drvdata(pdev);

serial8250_unregister_port(priv->line);
- clk_disable(priv->sclk);
+ clk_disable_unprepare(priv->sclk);
return 0;
}

--
1.8.1.5

2013-09-24 04:18:51

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

From: Shinya Kuribayashi <[email protected]>

Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
migrate to the common clock framework.

Signed-off-by: Shinya Kuribayashi <[email protected]>
---
drivers/clocksource/em_sti.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index 3a5909c..9d17083 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
int ret;

/* enable clock */
- ret = clk_enable(p->clk);
+ ret = clk_prepare_enable(p->clk);
if (ret) {
dev_err(&p->pdev->dev, "cannot enable clock\n");
return ret;
@@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
em_sti_write(p, STI_INTENCLR, 3);

/* stop clock */
- clk_disable(p->clk);
+ clk_disable_unprepare(p->clk);
}

static cycle_t em_sti_count(struct em_sti_priv *p)
--
1.8.1.5

2013-09-24 04:33:55

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

Device tree clock binding document for EMMA Mobile EV2 SMU.
Following nodes are defined to describe clock tree.
- renesas,emev2-smu
- renesas,emev2-smu-clkdiv
- renesas,emev2-smu-gclk

These bindings are designed manually based on
19UH0037EJ1000_SMU : System Management Unit User's Manual

Signed-off-by: Takashi Yoshii <[email protected]>
---
.../devicetree/bindings/clock/emev2-clock.txt | 99 ++++++++++++++++++++++
1 file changed, 99 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/emev2-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/emev2-clock.txt b/Documentation/devicetree/bindings/clock/emev2-clock.txt
new file mode 100644
index 0000000..f8649eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/emev2-clock.txt
@@ -0,0 +1,99 @@
+Device tree Clock bindings for Renesas EMMA Mobile EV2
+
+This binding uses the common clock binding.
+
+* SMU
+System Management Unit described in user's manual R19UH0037EJ1000_SMU.
+This is not a clock provider, but clocks under SMU depend on it.
+
+Required properties:
+- compatible: Should be "renesas,emev2-smu"
+- reg: Address and Size of SMU registers
+
+* SMU_CLKDIV
+Function block with an input mux and a divider, which corresponds to
+"Serial clock generator" in fig."Clock System Overview" of the manual,
+and "xxx frequency division setting register" (XXXCLKDIV) registers.
+This makes internal (neither input nor output) clock that is provided
+to input of xxxGCLK block.
+
+Required properties:
+- compatible: Should be "renesas,emev2-smu-clkdiv"
+- reg: Byte offset from SMU base and Bit position in the register
+- clocks: Parent clocks. Input clocks as described in clock-bindings.txt
+- #clock-cells: Should be <0>
+
+* SMU_GCLK
+Clock gating node shown as "Clock stop processing block" in the
+fig."Clock System Overview" of the manual.
+Registers are "xxx clock gate control register" (XXXGCLKCTRL).
+
+Required properties:
+- compatible: Should be "renesas,emev2-smu-gclk"
+- reg: Byte offset from SMU base and Bit position in the register
+- clocks: Input clock as described in clock-bindings.txt
+- #clock-cells: Should be <0>
+
+Example of provider:
+
+usia_u0_sclkdiv: usia_u0_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x610 0>;
+ clocks = <&pll3_fo>, <&pll4_fo>, <&pll1_fo>, <&osc1_fo>;
+ #clock-cells = <0>;
+};
+
+usia_u0_sclk: usia_u0_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4a0 1>;
+ clocks = <&usia_u0_sclkdiv>;
+ #clock-cells = <0>;
+};
+
+Example of consumer:
+
+uart@e1020000 {
+ compatible = "renesas,em-uart";
+ reg = <0xe1020000 0x38>;
+ interrupts = <0 8 0>;
+ clocks = <&usia_u0_sclk>;
+ clock-names = "sclk";
+};
+
+Example of clock-tree description:
+
+ This describes a clock path in the clock tree
+ c32ki -> pll3_fo -> usia_u0_sclkdiv -> usia_u0_sclk
+
+smu {
+ compatible = "renesas,emev2-smu";
+ reg = <0xe0110000 0x10000>;
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ c32ki: c32ki {
+ compatible = "fixed-clock";
+ clock-frequency = <32768>;
+ #clock-cells = <0>;
+ };
+ pll3_fo: pll3_fo {
+ compatible = "fixed-factor-clock";
+ clocks = <&c32ki>;
+ clock-div = <1>;
+ clock-mult = <7000>;
+ #clock-cells = <0>;
+ };
+ usia_u0_sclkdiv: usia_u0_sclkdiv {
+ compatible = "renesas,emev2-smu-clkdiv";
+ reg = <0x610 0>;
+ clocks = <&pll3_fo>;
+ #clock-cells = <0>;
+ };
+ usia_u0_sclk: usia_u0_sclk {
+ compatible = "renesas,emev2-smu-gclk";
+ reg = <0x4a0 1>;
+ clocks = <&usia_u0_sclkdiv>;
+ #clock-cells = <0>;
+ };
+};
+
--
1.8.1.5

2013-09-24 04:33:52

by Takashi YOSHII

[permalink] [raw]
Subject: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively

Make sh clock framework core depend on HAVE_MACH_CLKDEV, and
set it
- y on sh for backward compatibility
- !CONFIG_COMMON_CLK on sh-mobile
This is a preparation for migration to common clock framework
from sh clock framework on sh-mobile.

Signed-off-by: Takashi Yoshii <[email protected]>
---
arch/arm/Kconfig | 2 +-
arch/sh/Kconfig | 1 +
drivers/sh/clk/Makefile | 3 +--
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 3f7714d..53044ca 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -650,7 +650,7 @@ config ARCH_SHMOBILE
select HAVE_ARM_SCU if SMP
select HAVE_ARM_TWD if SMP
select HAVE_CLK
- select HAVE_MACH_CLKDEV
+ select HAVE_MACH_CLKDEV if !COMMON_CLK
select HAVE_SMP
select MIGHT_HAVE_CACHE_L2X0
select MULTI_IRQ_HANDLER
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 224f4bc..f57e47f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -41,6 +41,7 @@ config SUPERH
select MODULES_USE_ELF_RELA
select OLD_SIGSUSPEND
select OLD_SIGACTION
+ select HAVE_MACH_CLKDEV
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/drivers/sh/clk/Makefile b/drivers/sh/clk/Makefile
index 5d15ebf..e73b031 100644
--- a/drivers/sh/clk/Makefile
+++ b/drivers/sh/clk/Makefile
@@ -1,3 +1,2 @@
-obj-y := core.o
-
+obj-$(CONFIG_HAVE_MACH_CLKDEV) += core.o
obj-$(CONFIG_SH_CLK_CPG) += cpg.o
--
1.8.1.5

2013-09-24 04:42:43

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

[ CCed Daniel Lezcano and Thomas Gleixner, the clocksource maintainers ]

On Tue, Sep 24, 2013 at 01:09:24PM +0900, [email protected] wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>
> ---
> drivers/clocksource/em_sti.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
> index 3a5909c..9d17083 100644
> --- a/drivers/clocksource/em_sti.c
> +++ b/drivers/clocksource/em_sti.c
> @@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
> int ret;
>
> /* enable clock */
> - ret = clk_enable(p->clk);
> + ret = clk_prepare_enable(p->clk);
> if (ret) {
> dev_err(&p->pdev->dev, "cannot enable clock\n");
> return ret;
> @@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
> em_sti_write(p, STI_INTENCLR, 3);
>
> /* stop clock */
> - clk_disable(p->clk);
> + clk_disable_unprepare(p->clk);
> }
>
> static cycle_t em_sti_count(struct em_sti_priv *p)
> --
> 1.8.1.5
>

2013-09-24 04:44:39

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

[ CCed Greg Kroah-Hartman, the serial maintainer for his review ]

On Tue, Sep 24, 2013 at 01:10:39PM +0900, [email protected] wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>
> [[email protected]: edited for conflicts]
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> drivers/tty/serial/8250/8250_em.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_em.c b/drivers/tty/serial/8250/8250_em.c
> index 5f3bba1..d1a9078 100644
> --- a/drivers/tty/serial/8250/8250_em.c
> +++ b/drivers/tty/serial/8250/8250_em.c
> @@ -122,7 +122,7 @@ static int serial8250_em_probe(struct platform_device *pdev)
> up.port.dev = &pdev->dev;
> up.port.private_data = priv;
>
> - clk_enable(priv->sclk);
> + clk_prepare_enable(priv->sclk);
> up.port.uartclk = clk_get_rate(priv->sclk);
>
> up.port.iotype = UPIO_MEM32;
> @@ -134,7 +134,7 @@ static int serial8250_em_probe(struct platform_device *pdev)
> ret = serial8250_register_8250_port(&up);
> if (ret < 0) {
> dev_err(&pdev->dev, "unable to register 8250 port\n");
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return ret;
> }
>
> @@ -148,7 +148,7 @@ static int serial8250_em_remove(struct platform_device *pdev)
> struct serial8250_em_priv *priv = platform_get_drvdata(pdev);
>
> serial8250_unregister_port(priv->line);
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return 0;
> }
>
> --
> 1.8.1.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-09-24 04:52:20

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

[ Cc Laurent ]

On Tue, Sep 24, 2013 at 01:13:31PM +0900, [email protected] wrote:
> Device tree clock binding document for EMMA Mobile EV2 SMU.
> Following nodes are defined to describe clock tree.
> - renesas,emev2-smu
> - renesas,emev2-smu-clkdiv
> - renesas,emev2-smu-gclk

I realise this has been entirely consistent in the past and
even as recently as Linus' pre v3.12-rc2 master branch.
However, after some recent discussion we are now trying to make our
compatibility strings consistently of the form renesas,<unit>-<soc>.

With this in mind I believe the strings should be:

- renesas,smu-emev2
- renesas,smu-clkdiv-emev2
- renesas,smu-gclk-emev2

To be honest I am not quite sure about the "-clkdiv" and "-gclk"
bits and I would appreciate some review from others.

I have CCed Laurent as he may have some advice to offer here.

>
> These bindings are designed manually based on
> 19UH0037EJ1000_SMU : System Management Unit User's Manual
>
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> .../devicetree/bindings/clock/emev2-clock.txt | 99 ++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/emev2-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/emev2-clock.txt b/Documentation/devicetree/bindings/clock/emev2-clock.txt
> new file mode 100644
> index 0000000..f8649eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/emev2-clock.txt
> @@ -0,0 +1,99 @@
> +Device tree Clock bindings for Renesas EMMA Mobile EV2
> +
> +This binding uses the common clock binding.
> +
> +* SMU
> +System Management Unit described in user's manual R19UH0037EJ1000_SMU.
> +This is not a clock provider, but clocks under SMU depend on it.
> +
> +Required properties:
> +- compatible: Should be "renesas,emev2-smu"
> +- reg: Address and Size of SMU registers
> +
> +* SMU_CLKDIV
> +Function block with an input mux and a divider, which corresponds to
> +"Serial clock generator" in fig."Clock System Overview" of the manual,
> +and "xxx frequency division setting register" (XXXCLKDIV) registers.
> +This makes internal (neither input nor output) clock that is provided
> +to input of xxxGCLK block.
> +
> +Required properties:
> +- compatible: Should be "renesas,emev2-smu-clkdiv"
> +- reg: Byte offset from SMU base and Bit position in the register
> +- clocks: Parent clocks. Input clocks as described in clock-bindings.txt
> +- #clock-cells: Should be <0>
> +
> +* SMU_GCLK
> +Clock gating node shown as "Clock stop processing block" in the
> +fig."Clock System Overview" of the manual.
> +Registers are "xxx clock gate control register" (XXXGCLKCTRL).
> +
> +Required properties:
> +- compatible: Should be "renesas,emev2-smu-gclk"
> +- reg: Byte offset from SMU base and Bit position in the register
> +- clocks: Input clock as described in clock-bindings.txt
> +- #clock-cells: Should be <0>
> +
> +Example of provider:
> +
> +usia_u0_sclkdiv: usia_u0_sclkdiv {
> + compatible = "renesas,emev2-smu-clkdiv";
> + reg = <0x610 0>;
> + clocks = <&pll3_fo>, <&pll4_fo>, <&pll1_fo>, <&osc1_fo>;
> + #clock-cells = <0>;
> +};
> +
> +usia_u0_sclk: usia_u0_sclk {
> + compatible = "renesas,emev2-smu-gclk";
> + reg = <0x4a0 1>;
> + clocks = <&usia_u0_sclkdiv>;
> + #clock-cells = <0>;
> +};
> +
> +Example of consumer:
> +
> +uart@e1020000 {
> + compatible = "renesas,em-uart";
> + reg = <0xe1020000 0x38>;
> + interrupts = <0 8 0>;
> + clocks = <&usia_u0_sclk>;
> + clock-names = "sclk";
> +};
> +
> +Example of clock-tree description:
> +
> + This describes a clock path in the clock tree
> + c32ki -> pll3_fo -> usia_u0_sclkdiv -> usia_u0_sclk
> +
> +smu {
> + compatible = "renesas,emev2-smu";
> + reg = <0xe0110000 0x10000>;
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + c32ki: c32ki {
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + #clock-cells = <0>;
> + };
> + pll3_fo: pll3_fo {
> + compatible = "fixed-factor-clock";
> + clocks = <&c32ki>;
> + clock-div = <1>;
> + clock-mult = <7000>;
> + #clock-cells = <0>;
> + };
> + usia_u0_sclkdiv: usia_u0_sclkdiv {
> + compatible = "renesas,emev2-smu-clkdiv";
> + reg = <0x610 0>;
> + clocks = <&pll3_fo>;
> + #clock-cells = <0>;
> + };
> + usia_u0_sclk: usia_u0_sclk {
> + compatible = "renesas,emev2-smu-gclk";
> + reg = <0x4a0 1>;
> + clocks = <&usia_u0_sclkdiv>;
> + #clock-cells = <0>;
> + };
> +};
> +
> --
> 1.8.1.5
>

2013-09-24 04:55:40

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock framework

On Tue, Sep 24, 2013 at 01:17:03PM +0900, [email protected] wrote:
> Use common clock framework version of clock
> drivers/clk/shmobile/clk-emev2.c
> instead of sh-clkfwk version
> arch/arm/mach-shmobile/clock-emev2.c
>
> kzm9d(without -reference) still uses sh-clkfwk version.
>
> Because two of that framework can not live in one kernel binary,
> there will be SoCs and Boards that can not be in one binary as
> multiplatform binary or so.
> For example, kzm9d and kzm9d-reference is now exclusive.
>
> Signed-off-by: Takashi Yoshii <[email protected]>

Magnus, could you confirm whether or not you would like
common clocks to be mandatory for -reference in the near-term?

> ---
> arch/arm/mach-shmobile/Kconfig | 1 +
> arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 ++---
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 50bab8d..d20d4ce 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -237,6 +237,7 @@ config MACH_KZM9D_REFERENCE
> depends on ARCH_EMEV2
> select REGULATOR_FIXED_VOLTAGE if REGULATOR
> select USE_OF
> + select COMMON_CLK
> ---help---
> Use reference implementation of KZM9D board support
> which makes a greater use of device tree at the expense
> diff --git a/arch/arm/mach-shmobile/board-kzm9d-reference.c b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> index 8f8bb2f..e0b8317 100644
> --- a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> +++ b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> @@ -20,15 +20,14 @@
>
> #include <linux/init.h>
> #include <linux/of_platform.h>
> +#include <linux/clk-provider.h>
> #include <mach/emev2.h>
> #include <mach/common.h>
> #include <asm/mach/arch.h>
>
> static void __init kzm9d_add_standard_devices(void)
> {
> - if (!IS_ENABLED(CONFIG_COMMON_CLK))
> - emev2_clock_init();
> -
> + of_clk_init(NULL);
> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
>
> --
> 1.8.1.5
>

2013-09-24 04:56:48

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 0/6] ARM: shmobile: kzm9d-reference: migrate to common clock framework with DT

[ Cc Laurent ]

On Tue, Sep 24, 2013 at 01:05:26PM +0900, Takashi Yoshii wrote:
> This patch series makes kzm9d-reference to move to new clk implementation
> based on the common clock framework and device tree.

Magnus, Laurent,

I would appreciate it if you could take a look over the approach taken
here.

Thanks

>
> First three are for preparation.
> [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare
> [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare
> [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively
>
> Next one is a proposal document of DT bindings.
> [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings
>
> This is the implementation, and dts which carries clock tree description.
> [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT
>
> Last one is small modification needed for migration.
> [PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock
>
> These patches do not remove sh-clkfwk version, even on emev2.
> kzm9d(without -reference) target still uses sh-clkfwk.
> I believe this method encourages step-by-step migration on other sh-mobile
> targets, too.
>
> Patches should be applied and compiled both on
> v3.12-rc2 and
> kernel/git/horms/renesas.git tag:renesas-devel-20130922
>
> Confirmed on kernel/git/horms/renesas.git tag:renesas-devel-20130922
> for boot up to command prompt.
> - kzm9d board / multiplatform + kzm9d-reference configuration
> - kzm9d board / kzm9d-reference configuration
> - kzm9d board / kzm9d (with sh-clkfwk) configuration for regression.
> - ape6evm (with sh-clkfwk) for regression.
> for compile only
> - sh7757lcr (in arch/sh with sh-clkfwk) for regression.
>
>
> Shinya Kuribayashi (2):
> clocksource: em_sti: convert to clk_prepare/unprepare
> serial8250-em: convert to clk_prepare/unprepare
>
> Takashi YOSHII (4):
> sh: clkfwk: Select sh-/common- clkfwk alternatively
> ARM: shmobile: emev2: Define SMU clock DT bindings
> clk: emev2: Add support for emev2 SMU clocks with DT
> ARM: shmobile: kzm9d-reference: Use common clock framework
>
> .../devicetree/bindings/clock/emev2-clock.txt | 99 ++++++++++++++++++++
> arch/arm/Kconfig | 2 +-
> arch/arm/boot/dts/emev2.dtsi | 84 +++++++++++++++++
> arch/arm/mach-shmobile/Kconfig | 1 +
> arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 +-
> arch/sh/Kconfig | 1 +
> drivers/clk/Makefile | 2 +
> drivers/clk/shmobile/Makefile | 5 +
> drivers/clk/shmobile/clk-emev2.c | 104 +++++++++++++++++++++
> drivers/clocksource/em_sti.c | 4 +-
> drivers/sh/clk/Makefile | 3 +-
> drivers/tty/serial/8250/8250_em.c | 6 +-
> 12 files changed, 305 insertions(+), 11 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/emev2-clock.txt
> create mode 100644 drivers/clk/shmobile/Makefile
> create mode 100644 drivers/clk/shmobile/clk-emev2.c
>
> --
> 1.8.1.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-09-24 09:01:26

by Takashi Yoshii

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

Hi Simon,
# I had added wrong From:, sorry.

> With this in mind I believe the strings should be:
>
> - renesas,smu-emev2
> - renesas,smu-clkdiv-emev2
> - renesas,smu-gclk-emev2

Thank you for pointed it out. I'll fix it in v2.

> To be honest I am not quite sure about the "-clkdiv" and "-gclk"
Neither am I.
I'm afread there are many 'not sure' thing for me about DT.
I know some part might be wrong, which does not follow ePAPR at least.
But, even I don't know whether I should follow ePAPR or not.

I will wait for response for a while.
/yoshii

2013-09-24 13:40:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

On Tue, Sep 24, 2013 at 01:44:32PM +0900, Simon Horman wrote:
> [ CCed Greg Kroah-Hartman, the serial maintainer for his review ]
>
> On Tue, Sep 24, 2013 at 01:10:39PM +0900, [email protected] wrote:
> > From: Shinya Kuribayashi <[email protected]>
> >
> > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> > migrate to the common clock framework.
> >
> > Signed-off-by: Shinya Kuribayashi <[email protected]>
> > [[email protected]: edited for conflicts]
> > Signed-off-by: Takashi Yoshii <[email protected]>

Acked-by: Greg Kroah-Hartman <[email protected]>

2013-09-25 07:17:22

by Kuninori Morimoto

[permalink] [raw]
Subject: Re: [PATCH 0/6] ARM: shmobile: kzm9d-reference: migrate to common clock framework with DT


Hi Yoshii-san

> Next one is a proposal document of DT bindings.
> [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings
>
> This is the implementation, and dts which carries clock tree description.
> [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

Documentation and implementation in one patch,
and dts in other patch, are normal style IMO

I mean
4/6 + 5/6's implementation part,
5/6's dts part


Best regards
---
Kuninori Morimoto

2013-09-26 10:18:52

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

On 09/24/2013 06:42 AM, Simon Horman wrote:
> [ CCed Daniel Lezcano and Thomas Gleixner, the clocksource maintainers ]
>
> On Tue, Sep 24, 2013 at 01:09:24PM +0900, [email protected] wrote:
>> From: Shinya Kuribayashi <[email protected]>
>>
>> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
>> migrate to the common clock framework.
>>
>> Signed-off-by: Shinya Kuribayashi <[email protected]>

I am not a clk framework expert but at the first glance sounds correct.

Acked-by: Daniel Lezcano <[email protected]>

>> ---
>> drivers/clocksource/em_sti.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
>> index 3a5909c..9d17083 100644
>> --- a/drivers/clocksource/em_sti.c
>> +++ b/drivers/clocksource/em_sti.c
>> @@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
>> int ret;
>>
>> /* enable clock */
>> - ret = clk_enable(p->clk);
>> + ret = clk_prepare_enable(p->clk);
>> if (ret) {
>> dev_err(&p->pdev->dev, "cannot enable clock\n");
>> return ret;
>> @@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
>> em_sti_write(p, STI_INTENCLR, 3);
>>
>> /* stop clock */
>> - clk_disable(p->clk);
>> + clk_disable_unprepare(p->clk);
>> }
>>
>> static cycle_t em_sti_count(struct em_sti_priv *p)
>> --
>> 1.8.1.5
>>


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2013-09-30 15:25:42

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

Hi Yoshii-san,

Thank you for the patch.

On Tuesday 24 September 2013 13:10:39 [email protected] wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>
> [[email protected]: edited for conflicts]
> Signed-off-by: Takashi Yoshii <[email protected]>

Acked-by: Laurent Pinchart <[email protected]>

> ---
> drivers/tty/serial/8250/8250_em.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_em.c
> b/drivers/tty/serial/8250/8250_em.c index 5f3bba1..d1a9078 100644
> --- a/drivers/tty/serial/8250/8250_em.c
> +++ b/drivers/tty/serial/8250/8250_em.c
> @@ -122,7 +122,7 @@ static int serial8250_em_probe(struct platform_device
> *pdev) up.port.dev = &pdev->dev;
> up.port.private_data = priv;
>
> - clk_enable(priv->sclk);
> + clk_prepare_enable(priv->sclk);
> up.port.uartclk = clk_get_rate(priv->sclk);
>
> up.port.iotype = UPIO_MEM32;
> @@ -134,7 +134,7 @@ static int serial8250_em_probe(struct platform_device
> *pdev) ret = serial8250_register_8250_port(&up);
> if (ret < 0) {
> dev_err(&pdev->dev, "unable to register 8250 port\n");
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return ret;
> }
>
> @@ -148,7 +148,7 @@ static int serial8250_em_remove(struct platform_device
> *pdev) struct serial8250_em_priv *priv = platform_get_drvdata(pdev);
>
> serial8250_unregister_port(priv->line);
> - clk_disable(priv->sclk);
> + clk_disable_unprepare(priv->sclk);
> return 0;
> }
--
Regards,

Laurent Pinchart

2013-09-30 15:25:57

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

Hi Yoshii-san,

Thank you for the patch.

On Tuesday 24 September 2013 13:09:24 [email protected] wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>

Acked-by: Laurent Pinchart <[email protected]>

> ---
> drivers/clocksource/em_sti.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
> index 3a5909c..9d17083 100644
> --- a/drivers/clocksource/em_sti.c
> +++ b/drivers/clocksource/em_sti.c
> @@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
> int ret;
>
> /* enable clock */
> - ret = clk_enable(p->clk);
> + ret = clk_prepare_enable(p->clk);
> if (ret) {
> dev_err(&p->pdev->dev, "cannot enable clock\n");
> return ret;
> @@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
> em_sti_write(p, STI_INTENCLR, 3);
>
> /* stop clock */
> - clk_disable(p->clk);
> + clk_disable_unprepare(p->clk);
> }
>
> static cycle_t em_sti_count(struct em_sti_priv *p)
--
Regards,

Laurent Pinchart

2013-09-30 18:40:05

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively

Hi Yoshii-san,

Thank you for the patch.

On Tuesday 24 September 2013 13:12:06 [email protected] wrote:
> Make sh clock framework core depend on HAVE_MACH_CLKDEV, and
> set it
> - y on sh for backward compatibility
> - !CONFIG_COMMON_CLK on sh-mobile
> This is a preparation for migration to common clock framework
> from sh clock framework on sh-mobile.

While I agree with this patch, I believe the use of the HAVE_MACH_CLKDEV
configuration option to select whether to compile core.o in is a bit of an
abuse. I would have created a new configuration option (SH_CLK_CORE in
drivers/sh/Kconfig for instance) as 'def_bool y' that depends on SH || (ARM &&
!COMMON_CLK).

However, as all ARCH_SHMOBILE platforms should be converted to the common
clock framework, this is only temporary and could be revisited later, so I'm
fine with keeping the patch as-is.

> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> arch/arm/Kconfig | 2 +-
> arch/sh/Kconfig | 1 +
> drivers/sh/clk/Makefile | 3 +--
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3f7714d..53044ca 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -650,7 +650,7 @@ config ARCH_SHMOBILE
> select HAVE_ARM_SCU if SMP
> select HAVE_ARM_TWD if SMP
> select HAVE_CLK
> - select HAVE_MACH_CLKDEV
> + select HAVE_MACH_CLKDEV if !COMMON_CLK
> select HAVE_SMP
> select MIGHT_HAVE_CACHE_L2X0
> select MULTI_IRQ_HANDLER
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 224f4bc..f57e47f 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -41,6 +41,7 @@ config SUPERH
> select MODULES_USE_ELF_RELA
> select OLD_SIGSUSPEND
> select OLD_SIGACTION
> + select HAVE_MACH_CLKDEV
> help
> The SuperH is a RISC processor targeted for use in embedded systems
> and consumer electronics; it was also used in the Sega Dreamcast
> diff --git a/drivers/sh/clk/Makefile b/drivers/sh/clk/Makefile
> index 5d15ebf..e73b031 100644
> --- a/drivers/sh/clk/Makefile
> +++ b/drivers/sh/clk/Makefile
> @@ -1,3 +1,2 @@
> -obj-y := core.o
> -
> +obj-$(CONFIG_HAVE_MACH_CLKDEV) += core.o
> obj-$(CONFIG_SH_CLK_CPG) += cpg.o
--
Regards,

Laurent Pinchart

2013-10-01 09:05:55

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

On Tue, Sep 24, 2013 at 1:09 PM, <[email protected]> wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>
> ---
> drivers/clocksource/em_sti.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Looking good, thanks!

Acked-by: Magnus Damm <[email protected]>

2013-10-01 09:07:13

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

On Tue, Sep 24, 2013 at 1:10 PM, <[email protected]> wrote:
> From: Shinya Kuribayashi <[email protected]>
>
> Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> migrate to the common clock framework.
>
> Signed-off-by: Shinya Kuribayashi <[email protected]>
> [[email protected]: edited for conflicts]
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> drivers/tty/serial/8250/8250_em.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)

This looks fine to me, thanks!

Acked-by: Magnus Damm <[email protected]>

2013-10-01 09:15:32

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

On Tue, Sep 24, 2013 at 1:15 PM, <[email protected]> wrote:
> Common clock framework version of emev2 clock support.
> smu_clkdiv and smu_gclk are handled.
> So far, reparent is not implemented, and is fixed to index #0.
> SMU and small numbers of clocks are described in emev2.dtsi.
>
> That function and numbers of clocks are equivalent to current
> sh-clkfwk version. It is just enough to run kzm9d-reference.
>
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> arch/arm/boot/dts/emev2.dtsi | 84 +++++++++++++++++++++++++++++++
> drivers/clk/Makefile | 2 +
> drivers/clk/shmobile/Makefile | 5 ++
> drivers/clk/shmobile/clk-emev2.c | 104 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 195 insertions(+)
> create mode 100644 drivers/clk/shmobile/Makefile
> create mode 100644 drivers/clk/shmobile/clk-emev2.c

Hi Yoshii-san,

Thanks for your efforts on this. I'm very pleased to see that you
describe the clock topology using DT. In general I think your patch
looks fine, but I have some comment related to the multiplatform
integration, please see below.

> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 7b11106..3e64ac4 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -32,6 +32,8 @@ 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_SHMOBILE) += shmobile/
> +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/

Here I believe it is enough that you only use
CONFIG_ARCH_SHMOBILE_MULTI. Building common clocks to coexist with the
old legacy board code does not make any sense IMO. If you think it
makes sense for some reason, please explain why. =)

> diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> new file mode 100644
> index 0000000..6a26eb6
> --- /dev/null
> +++ b/drivers/clk/shmobile/Makefile
> @@ -0,0 +1,5 @@
> +ifeq ($(CONFIG_COMMON_CLK), y)
> +obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o
> +endif

I don't think you would need the above ifeq/endif wrapper if you only
used CONFIG_ARCH_SHMOBILE_MULTI above.

Apart from that it looks good to me. And, yes, I have tested this on
my KZM9D board together with multiplatform and it works very well!

Cheers,

/ magnus

2013-10-01 09:23:44

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock framework

On Tue, Sep 24, 2013 at 1:17 PM, <[email protected]> wrote:
> Use common clock framework version of clock
> drivers/clk/shmobile/clk-emev2.c
> instead of sh-clkfwk version
> arch/arm/mach-shmobile/clock-emev2.c
>
> kzm9d(without -reference) still uses sh-clkfwk version.
>
> Because two of that framework can not live in one kernel binary,
> there will be SoCs and Boards that can not be in one binary as
> multiplatform binary or so.
> For example, kzm9d and kzm9d-reference is now exclusive.
>
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> arch/arm/mach-shmobile/Kconfig | 1 +
> arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 ++---
> 2 files changed, 3 insertions(+), 3 deletions(-)

Hi Yoshii-san,

Thanks for your patch. I have some comments on this portion to try to
simplify things, please see below.

> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 50bab8d..d20d4ce 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -237,6 +237,7 @@ config MACH_KZM9D_REFERENCE
> depends on ARCH_EMEV2
> select REGULATOR_FIXED_VOLTAGE if REGULATOR
> select USE_OF
> + select COMMON_CLK

I don't think this hunk is needed. This is probably the
ARCH_SHMOBILE_MULTI case, and if so then ARCH_MULTIPLATFORM in
arch/arm/Kconfig already selects COMMON_CLK. And if it's the
ARCH_SHMOBILE case you're aiming at then I recommend you to only focus
on ARCH_SHMOBILE_MULTI instead.

> diff --git a/arch/arm/mach-shmobile/board-kzm9d-reference.c b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> index 8f8bb2f..e0b8317 100644
> --- a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> +++ b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> @@ -20,15 +20,14 @@
>
> #include <linux/init.h>
> #include <linux/of_platform.h>
> +#include <linux/clk-provider.h>
> #include <mach/emev2.h>
> #include <mach/common.h>
> #include <asm/mach/arch.h>
>
> static void __init kzm9d_add_standard_devices(void)
> {
> - if (!IS_ENABLED(CONFIG_COMMON_CLK))
> - emev2_clock_init();
> -
> + of_clk_init(NULL);
> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }

To keep on allowing build of board-kzm9d-reference.c for both
ARCH_SHMOBILE_MULTI and ARCH_SHMOBILE I recommend you to adjust your
code into something liket this instead:

@@ -18,6 +18,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

+#include <linux/clk-provider.h>
#include <linux/init.h>
#include <linux/of_platform.h>
#include <mach/emev2.h>
@@ -26,9 +27,11 @@

static void __init kzm9d_add_standard_devices(void)
{
- if (!IS_ENABLED(CONFIG_COMMON_CLK))
- emev2_clock_init();
-
+#ifdef CONFIG_COMMON_CLK
+ of_clk_init(NULL);
+#else
+ emev2_clock_init();
+#endif
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}

Cheers,

/ magnus

2013-10-01 09:30:46

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively

On Tue, Sep 24, 2013 at 1:12 PM, <[email protected]> wrote:
> Make sh clock framework core depend on HAVE_MACH_CLKDEV, and
> set it
> - y on sh for backward compatibility
> - !CONFIG_COMMON_CLK on sh-mobile
> This is a preparation for migration to common clock framework
> from sh clock framework on sh-mobile.
>
> Signed-off-by: Takashi Yoshii <[email protected]>
> ---
> arch/arm/Kconfig | 2 +-
> arch/sh/Kconfig | 1 +
> drivers/sh/clk/Makefile | 3 +--
> 3 files changed, 3 insertions(+), 3 deletions(-)

Hi Yoshii-san,

Thanks for your patch. I'm sure there is a reason behind this, but I'm
trying to understand why you need this modification. It looks to me
like you're trying to enable COMMON_CLK on ARCH_SHMOBILE but in my
mind it is enough to only enable COMMON_CLK in the case of
ARCH_SHMOBILE_MULTI.

During my test using ARCH_SHMOBILE_MULTI on KZM9D I omitted this patch
and only used patch 1, 2, 4, 5, and a modified 6 from your series.
This worked just fine, but I may be missing something.

Let me know if you really want to keep this patch or not. If it's not
needed for MULTIPLATFORM then I suggest that we just drop it.

Cheers,

/ magnus

2013-10-01 11:36:31

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

Hi Yoshii-san and Simon,

On Tuesday 24 September 2013 13:52:15 Simon Horman wrote:
> [ Cc Laurent ]
>
> On Tue, Sep 24, 2013 at 01:13:31PM +0900, [email protected] wrote:
> > Device tree clock binding document for EMMA Mobile EV2 SMU.
> > Following nodes are defined to describe clock tree.
> > - renesas,emev2-smu
> > - renesas,emev2-smu-clkdiv
> > - renesas,emev2-smu-gclk
>
> I realise this has been entirely consistent in the past and
> even as recently as Linus' pre v3.12-rc2 master branch.
> However, after some recent discussion we are now trying to make our
> compatibility strings consistently of the form renesas,<unit>-<soc>.
>
> With this in mind I believe the strings should be:
>
> - renesas,smu-emev2
> - renesas,smu-clkdiv-emev2
> - renesas,smu-gclk-emev2
>
> To be honest I am not quite sure about the "-clkdiv" and "-gclk"
> bits and I would appreciate some review from others.

I don't have access to the EMEV2 datasheet so my ability to comment on this is
somehow limited. However, given that the clock hardware is very SoC-specific,
it might make sense to keep the names proposed by Yoshii-san. This would be
consistent with the other clock bindings.

> I have CCed Laurent as he may have some advice to offer here.
>
> > These bindings are designed manually based on
> >
> > 19UH0037EJ1000_SMU : System Management Unit User's Manual
> >
> > Signed-off-by: Takashi Yoshii <[email protected]>
> > ---
> >
> > .../devicetree/bindings/clock/emev2-clock.txt | 99 +++++++++++++++++
> > 1 file changed, 99 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/clock/emev2-clock.txt
> >
> > diff --git a/Documentation/devicetree/bindings/clock/emev2-clock.txt
> > b/Documentation/devicetree/bindings/clock/emev2-clock.txt new file mode
> > 100644
> > index 0000000..f8649eb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/emev2-clock.txt
> > @@ -0,0 +1,99 @@
> > +Device tree Clock bindings for Renesas EMMA Mobile EV2
> > +
> > +This binding uses the common clock binding.
> > +
> > +* SMU
> > +System Management Unit described in user's manual R19UH0037EJ1000_SMU.
> > +This is not a clock provider, but clocks under SMU depend on it.
> > +
> > +Required properties:
> > +- compatible: Should be "renesas,emev2-smu"
> > +- reg: Address and Size of SMU registers
> > +
> > +* SMU_CLKDIV
> > +Function block with an input mux and a divider, which corresponds to
> > +"Serial clock generator" in fig."Clock System Overview" of the manual,
> > +and "xxx frequency division setting register" (XXXCLKDIV) registers.
> > +This makes internal (neither input nor output) clock that is provided
> > +to input of xxxGCLK block.
> > +
> > +Required properties:
> > +- compatible: Should be "renesas,emev2-smu-clkdiv"
> > +- reg: Byte offset from SMU base and Bit position in the register
> > +- clocks: Parent clocks. Input clocks as described in clock-bindings.txt
> > +- #clock-cells: Should be <0>
> > +
> > +* SMU_GCLK
> > +Clock gating node shown as "Clock stop processing block" in the
> > +fig."Clock System Overview" of the manual.
> > +Registers are "xxx clock gate control register" (XXXGCLKCTRL).
> > +
> > +Required properties:
> > +- compatible: Should be "renesas,emev2-smu-gclk"
> > +- reg: Byte offset from SMU base and Bit position in the register
> > +- clocks: Input clock as described in clock-bindings.txt
> > +- #clock-cells: Should be <0>
> > +
> > +Example of provider:
> > +
> > +usia_u0_sclkdiv: usia_u0_sclkdiv {
> > + compatible = "renesas,emev2-smu-clkdiv";
> > + reg = <0x610 0>;

Is the registers space really 0 bytes long ?

> > + clocks = <&pll3_fo>, <&pll4_fo>, <&pll1_fo>, <&osc1_fo>;
> > + #clock-cells = <0>;
> > +};
> > +
> > +usia_u0_sclk: usia_u0_sclk {
> > + compatible = "renesas,emev2-smu-gclk";
> > + reg = <0x4a0 1>;
> > + clocks = <&usia_u0_sclkdiv>;
> > + #clock-cells = <0>;
> > +};
> > +
> > +Example of consumer:
> > +
> > +uart@e1020000 {
> > + compatible = "renesas,em-uart";
> > + reg = <0xe1020000 0x38>;
> > + interrupts = <0 8 0>;
> > + clocks = <&usia_u0_sclk>;
> > + clock-names = "sclk";
> > +};
> > +
> > +Example of clock-tree description:
> > +
> > + This describes a clock path in the clock tree
> > + c32ki -> pll3_fo -> usia_u0_sclkdiv -> usia_u0_sclk
> > +
> > +smu {
> > + compatible = "renesas,emev2-smu";
> > + reg = <0xe0110000 0x10000>;
> > + #address-cells = <2>;
> > + #size-cells = <0>;
> > +
> > + c32ki: c32ki {
> > + compatible = "fixed-clock";
> > + clock-frequency = <32768>;
> > + #clock-cells = <0>;
> > + };
> > + pll3_fo: pll3_fo {
> > + compatible = "fixed-factor-clock";
> > + clocks = <&c32ki>;
> > + clock-div = <1>;
> > + clock-mult = <7000>;
> > + #clock-cells = <0>;
> > + };
> > + usia_u0_sclkdiv: usia_u0_sclkdiv {
> > + compatible = "renesas,emev2-smu-clkdiv";
> > + reg = <0x610 0>;
> > + clocks = <&pll3_fo>;
> > + #clock-cells = <0>;
> > + };
> > + usia_u0_sclk: usia_u0_sclk {
> > + compatible = "renesas,emev2-smu-gclk";
> > + reg = <0x4a0 1>;
> > + clocks = <&usia_u0_sclkdiv>;
> > + #clock-cells = <0>;
> > + };
> > +};
> > +

There's an extra blank line at the end of the file.

--
Regards,

Laurent Pinchart

2013-10-01 12:27:55

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

Hi Yoshii-san,

Thank you for the patch.

(CC'ing LAMK as a generic CCF question follows)

On Tuesday 01 October 2013 18:15:26 Magnus Damm wrote:
> On Tue, Sep 24, 2013 at 1:15 PM, <[email protected]> wrote:
> > Common clock framework version of emev2 clock support.
> > smu_clkdiv and smu_gclk are handled.
> > So far, reparent is not implemented, and is fixed to index #0.
> > SMU and small numbers of clocks are described in emev2.dtsi.
> >
> > That function and numbers of clocks are equivalent to current
> > sh-clkfwk version. It is just enough to run kzm9d-reference.
> >
> > Signed-off-by: Takashi Yoshii <[email protected]>
> > ---
> >
> > arch/arm/boot/dts/emev2.dtsi | 84 +++++++++++++++++++++++++++++++
> > drivers/clk/Makefile | 2 +
> > drivers/clk/shmobile/Makefile | 5 ++
> > drivers/clk/shmobile/clk-emev2.c | 104 ++++++++++++++++++++++++++++++++++
> > 4 files changed, 195 insertions(+)
> > create mode 100644 drivers/clk/shmobile/Makefile
> > create mode 100644 drivers/clk/shmobile/clk-emev2.c
>
> Hi Yoshii-san,
>
> Thanks for your efforts on this. I'm very pleased to see that you describe
> the clock topology using DT.

What is the generally accepted practice when an IP core provides a large
number of clocks, with either one register or one register bit dedicated to
each clock ? Should each clock be described as one DT node, or should the IP
core be described by a single DT node ?

I also see both platforms using CLK_OF_DECLARE and platforms calling a clock
init function provided by drivers/clk/<platform>.c in the SoC setup code in
arch/arm/. What is the preferred practice there ?

> In general I think your patch looks fine, but I have some comment related to
> the multiplatform integration, please see below.
>
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 7b11106..3e64ac4 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -32,6 +32,8 @@ 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_SHMOBILE) += shmobile/
> > +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/
>
> Here I believe it is enough that you only use
> CONFIG_ARCH_SHMOBILE_MULTI. Building common clocks to coexist with the
> old legacy board code does not make any sense IMO. If you think it
> makes sense for some reason, please explain why. =)
>
> > diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile
> > new file mode 100644
> > index 0000000..6a26eb6
> > --- /dev/null
> > +++ b/drivers/clk/shmobile/Makefile
> > @@ -0,0 +1,5 @@
> > +ifeq ($(CONFIG_COMMON_CLK), y)
> > +obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o
> > +endif
>
> I don't think you would need the above ifeq/endif wrapper if you only
> used CONFIG_ARCH_SHMOBILE_MULTI above.
>
> Apart from that it looks good to me. And, yes, I have tested this on
> my KZM9D board together with multiplatform and it works very well!

--
Regards,

Laurent Pinchart

2013-10-02 01:28:44

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

On Tue, Oct 01, 2013 at 01:36:33PM +0200, Laurent Pinchart wrote:
> Hi Yoshii-san and Simon,
>
> On Tuesday 24 September 2013 13:52:15 Simon Horman wrote:
> > [ Cc Laurent ]
> >
> > On Tue, Sep 24, 2013 at 01:13:31PM +0900, [email protected] wrote:
> > > Device tree clock binding document for EMMA Mobile EV2 SMU.
> > > Following nodes are defined to describe clock tree.
> > > - renesas,emev2-smu
> > > - renesas,emev2-smu-clkdiv
> > > - renesas,emev2-smu-gclk
> >
> > I realise this has been entirely consistent in the past and
> > even as recently as Linus' pre v3.12-rc2 master branch.
> > However, after some recent discussion we are now trying to make our
> > compatibility strings consistently of the form renesas,<unit>-<soc>.
> >
> > With this in mind I believe the strings should be:
> >
> > - renesas,smu-emev2
> > - renesas,smu-clkdiv-emev2
> > - renesas,smu-gclk-emev2
> >
> > To be honest I am not quite sure about the "-clkdiv" and "-gclk"
> > bits and I would appreciate some review from others.
>
> I don't have access to the EMEV2 datasheet so my ability to comment on this is
> somehow limited. However, given that the clock hardware is very SoC-specific,
> it might make sense to keep the names proposed by Yoshii-san. This would be
> consistent with the other clock bindings.

Thanks Laurent. If that seems fine by you then it is fine by me.

2013-10-03 02:30:06

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare

On Thu, Sep 26, 2013 at 12:18:47PM +0200, Daniel Lezcano wrote:
> On 09/24/2013 06:42 AM, Simon Horman wrote:
> >[ CCed Daniel Lezcano and Thomas Gleixner, the clocksource maintainers ]
> >
> >On Tue, Sep 24, 2013 at 01:09:24PM +0900, [email protected] wrote:
> >>From: Shinya Kuribayashi <[email protected]>
> >>
> >>Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> >>migrate to the common clock framework.
> >>
> >>Signed-off-by: Shinya Kuribayashi <[email protected]>
>
> I am not a clk framework expert but at the first glance sounds correct.
>
> Acked-by: Daniel Lezcano <[email protected]>

On Mon, Sep 30, 2013 at 05:25:41PM +0200, Laurent Pinchart wrote:
> Hi Yoshii-san,
>
> Thank you for the patch.
>
> On Tuesday 24 September 2013 13:09:24 [email protected] wrote:
> > From: Shinya Kuribayashi <[email protected]>
> >
> > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> > migrate to the common clock framework.
> >
> > Signed-off-by: Shinya Kuribayashi <[email protected]>
>
> Acked-by: Laurent Pinchart <[email protected]>

Thanks, I have queued this up in the clocksource branch of the renesas
tree. I'll send a pull request to Daniel in the not to distant future.

2013-10-03 02:30:47

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare

On Tue, Sep 24, 2013 at 06:41:13AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Sep 24, 2013 at 01:44:32PM +0900, Simon Horman wrote:
> > [ CCed Greg Kroah-Hartman, the serial maintainer for his review ]
> >
> > On Tue, Sep 24, 2013 at 01:10:39PM +0900, [email protected] wrote:
> > > From: Shinya Kuribayashi <[email protected]>
> > >
> > > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> > > migrate to the common clock framework.
> > >
> > > Signed-off-by: Shinya Kuribayashi <[email protected]>
> > > [[email protected]: edited for conflicts]
> > > Signed-off-by: Takashi Yoshii <[email protected]>
>
> Acked-by: Greg Kroah-Hartman <[email protected]>
>

On Mon, Sep 30, 2013 at 05:25:40PM +0200, Laurent Pinchart wrote:
> Hi Yoshii-san,
>
> Thank you for the patch.
>
> On Tuesday 24 September 2013 13:10:39 [email protected] wrote:
> > From: Shinya Kuribayashi <[email protected]>
> >
> > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can
> > migrate to the common clock framework.
> >
> > Signed-off-by: Shinya Kuribayashi <[email protected]>
> > [[email protected]: edited for conflicts]
> > Signed-off-by: Takashi Yoshii <[email protected]>
>
> Acked-by: Laurent Pinchart <[email protected]>

Thanks, I have queued this up in the serial branch of the renesas tree.

2013-10-04 01:12:37

by Takashi YOSHII

[permalink] [raw]
Subject: Re: [PATCH 0/6] ARM: shmobile: kzm9d-reference: migrate to common clock framework with DT

Hi,

> Documentation and implementation in one patch,
> and dts in other patch, are normal style IMO

That's strange! Considering the nature of DT, it must be wrong.
... was what I thought at first, but you are right.
I found 3/4 (1021 in 1365) of commits (new + change - typo fix)
under Documentation/devicetree/bindings/ has come with .c code.

Thank you for let me know.
/yoshii

2013-10-04 05:25:44

by Takashi YOSHII

[permalink] [raw]
Subject: Re: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively

Hi Laurent,

> While I agree with this patch, I believe the use of the HAVE_MACH_CLKDEV
> configuration option to select whether to compile core.o in is a bit of an
> abuse. <snip>
Well, yes, indeed. It does not mean having include/mach/clkdev.h,
but means somewhat such like !COMMON_CLK.
# I think there is similar usage in arch/mips, though they define but not use.
# ... is an execution.

> However, as all ARCH_SHMOBILE platforms should be converted to the common
> clock framework, this is only temporary and could be revisited later, so I'm
> fine with keeping the patch as-is.
Sure.

Thank you for your review.
/yoshii

2013-10-04 05:45:05

by Takashi YOSHII

[permalink] [raw]
Subject: Re: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT

Hi Magnus,
Thank you for your commnets.

> > +obj-$(CONFIG_ARCH_SHMOBILE) += shmobile/
> > +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/
>
> Here I believe it is enough that you only use
> CONFIG_ARCH_SHMOBILE_MULTI. ...

That is because it supports three configuration,
1. MULTI + KZM9D_REFERENCE + COMMON_CLK
2. KZM9D_REFERENCE + COMMON_CLK
3. KZM9D
but,

> ... Building common clocks to coexist with the
> old legacy board code does not make any sense IMO. ...
I think so, too.
And from the view of testing, having smaller variants is better.
# easier :)

> > +ifeq ($(CONFIG_COMMON_CLK), y)
> > +obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o
> > +endif
>
> I don't think you would need the above ifeq/endif wrapper if you only
> used CONFIG_ARCH_SHMOBILE_MULTI above.
Right.

Anyway, because you have make kzm9d-reference things simpler,
now we don't need to worry about it.

Thanks,
/yoshii

2013-10-06 17:17:17

by Takashi YOSHII

[permalink] [raw]
Subject: Re: [PATCH 4/6] ARM: shmobile: emev2: Define SMU clock DT bindings

Hi Laurent,

> > > Device tree clock binding document for EMMA Mobile EV2 SMU.
> > > Following nodes are defined to describe clock tree.
> > > - renesas,emev2-smu
> > > - renesas,emev2-smu-clkdiv
> > > - renesas,emev2-smu-gclk
> >
> > I realise this has been entirely consistent in the past and
> > even as recently as Linus' pre v3.12-rc2 master branch.
> > However, after some recent discussion we are now trying to make our
> > compatibility strings consistently of the form renesas,<unit>-<soc>.
> >
> > With this in mind I believe the strings should be:
> >
> > - renesas,smu-emev2
> > - renesas,smu-clkdiv-emev2
> > - renesas,smu-gclk-emev2
> >
> > To be honest I am not quite sure about the "-clkdiv" and "-gclk"
> > bits and I would appreciate some review from others.
>
> I don't have access to the EMEV2 datasheet so my ability to comment on this is
> somehow limited. However, given that the clock hardware is very SoC-specific,
> it might make sense to keep the names proposed by Yoshii-san. This would be
> consistent with the other clock bindings.

Just for explanation:
EM/EV2(there is "/" according to its Users Manual) might be a little difficult
one for general discussion. It stands for "EMMA Mobile EV2", and is a SoC of
"EMMA Mobile" series, they say. So, possibly, the string was
emmamobile-smu or emmamobile-smu-ev2 if it is EV2 specific variant.

But, EMEV2 is the only SoC known in EMMA Mobile series, and no document found
to tell if some other one(if any. there used to be, they say) has SMU or not.
That's why I choose "emev2-smu". This is <unit> part, and no <soc> portion.

> > > +++ b/Documentation/devicetree/bindings/clock/emev2-clock.txt
...
> > > +Example of provider:
> > > +
> > > +usia_u0_sclkdiv: usia_u0_sclkdiv {
> > > + compatible = "renesas,emev2-smu-clkdiv";
> > > + reg = <0x610 0>;
>
> Is the registers space really 0 bytes long ?

Both those are address. as
> > +- reg: Byte offset from SMU base and Bit position in the register

The outer unit (smu) does
> > > + #address-cells = <2>;
> > > + #size-cells = <0>;
and no "ranges".
These clock node is defined not as memory-mapped.

Looks strange? I think so.
This _was introduced_ to comply ePAPR that requires the node name to consist
of generic name and @address to make it unique.
So, the first version was like this.
| usia_u0_sclkdiv: clock@610,0 {
| compatible = "renesas,emev2-smu-clkdiv";
| reg = <0x610 0>;

But later, I trashed it for the consistency with clock nodes without <reg>, say
> > > + c32ki: c32ki {
> > > + compatible = "fixed-clock";

I am still not sure what the clock nodes should be. But, I think what we are
discussing is out of the scope of current ePAPR, which does not give the answer.

> > > +
>
> There's an extra blank line at the end of the file.
Oops. Thank you.
/yoshii