2013-04-12 22:55:07

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 0/6] ARM: OMAP3+: support cpufreq-cpu0 for device tree boot

The following version 4 of the series arose from trying to use BeagleBoard-XM
(OMAP3 variant) for doing CPU DVFS using cpufreq-cpu0. This series enables the
generic cpufreq-cpu0 driver to be used in device tree enabled boot while
maintaining support of the legacy omap-cpufreq driver when used in non device
tree enabled boot.

However, in order to enable complete SoC entitlement for OMAP platforms, with
this series, key features are still pending on device tree adaptation for OMAP:
A) clock framework data transition to DT - this should happen soon, so this
series hacks the clock node for the time being as suggested in review of
original series[1].
B) On processors that use voltage controller, voltage processor (VC/VP hardware
loop using I2C_SR path) - we have started work on transitioning them to
regulator framework driven by DT.
C) Adaptive Body Bias and SmartReflex AVS conversion to DT. [2]

Note: At this point in time, we do not have DT entries for clock on OMAP
platforms. Common Clock Framework(CCF) could also control regulators[3].
Once these conversions are complete, there might be minimal cleanup work to
switch to the new data structure changes.

Key benefit of the series is to allow all relevant TI platforms now to use a
single cpufreq driver and equivalent frameworks in addition be part of the
transition to device tree.
NOTE: As a result of this series:
1. omap-cpufreq will be used only in non device tree boot scenario. we should
delete this driver once the 100% DT conversion is complete.
2. Generic cpufreq-cpu0 will be used only in device tree boot scenario.
boot systems.

Key changes in version 4 since version 3:
- OMAP clock driver introduced as discussed in
http://marc.info/?t=136370325600009&r=1&w=2
- cpufreq-cpu0 dummy device introduced in pm.c late_initcall: discussed
in http://marc.info/?t=136450773400004&r=1&w=2
- updated test script (prints regulator voltage where available)
- parts of the series probably belong to different maintainer:
consolidated as a single series to aid review in-context.

version 3 of the series:
http://marc.info/?l=linux-pm&m=136450759315742&w=2
available at:
http://git.kernel.org/cgit/linux/kernel/git/bcousson/linux-omap-dt.git/log/?h=for_3.10/dts

version 2 of the series:
http://marc.info/?t=136371570200003&r=1&w=2
available at:
https://github.com/nmenon/linux-2.6-playground/commits/push/cpufreq-cpu0-omap-all-v2

version 1 of the series:
http://marc.info/?t=136329485400005&r=1&w=2
available at:
https://github.com/nmenon/linux-2.6-playground/commits/push/cpufreq-cpu0-omap-all-v1

[1] Original discussion thread which triggered this series:
http://marc.info/?l=linux-pm&m=136304313700602&w=2
https://patchwork.kernel.org/patch/2251841/
https://patchwork.kernel.org/patch/2251851/
[2] ABB RFC: http://marc.info/?l=linux-omap&m=136449099409794&w=2
[3] CCF DVFS patches:
https://patchwork.kernel.org/patch/2195431/
https://patchwork.kernel.org/patch/2195421/
https://patchwork.kernel.org/patch/2195451/
https://patchwork.kernel.org/patch/2195441/
https://patchwork.kernel.org/patch/2195461/

Version 4 is now based on a few branches to prevent merge conflicts(v3.9-rc6):
git://git.linaro.org/people/mturquette/linux.git
clk-next 38e4aa0 clk: sunxi: Unify oscillator clock
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
linux-next 584fa10 Merge branch 'pm-runtime-next'
git://git.kernel.org/pub/scm/linux/kernel/git/bcousson/linux-omap-dt.git
for_3.10/dts 1fac4ff ARM: dts: Add OMAP3430
(merged branch is called baseline-v4 on my tree)

Version 4 is also available at:
https://github.com/nmenon/linux-2.6-playground/commits/push/cpufreq-cpu0-omap-all-v4
git link: git://github.com/nmenon/linux-2.6-playground.git
branch: cpufreq-cpu0-omap-all-v4

Test coverage:
test script: http://pastebin.com/z4yqZrBg
Platforms verified:
beaglebone(rev A6a) - AM33xx compatible - http://pastebin.com/YnSNiTnx
beagleboard (rev C1D) - OMAP3430 compatible
- DT enabled boot: http://pastebin.com/eGaDD3B4
- No DT enabled boot: http://pastebin.com/5xqsNBA6
omap3-beagle-xm -OMAP3630 compatible - http://pastebin.com/FdrG4Rna
Pandaboard -(OMAP4430 ES2.1) - http://pastebin.com/1jjSRKQh
Pandaboard-ES -(OMAP4460 ES1.1) - http://pastebin.com/B5acTbch

Nishanth Menon (6):
clk: OMAP: introduce device tree binding to kernel clock data
[Clk probably belongs to mike's tree?]
ARM: dts: OMAP3: add clock nodes for CPU
ARM: dts: OMAP4: add clock nodes for CPU
ARM: dts: AM33XX: add clock nodes for CPU
[The above probably belong to Benoit's tree]
ARM: OMAP2+: AM33XX: add lateinit hook for calling pm late init
ARM: OMAP3+: use cpu0-cpufreq driver in device tree supported boot
[The above probably belong to Kevin/Tony's tree]

.../devicetree/bindings/clock/omap-clock.txt | 40 ++++++++
arch/arm/boot/dts/am33xx.dtsi | 7 ++
arch/arm/boot/dts/omap3.dtsi | 7 ++
arch/arm/boot/dts/omap4.dtsi | 7 ++
arch/arm/mach-omap2/board-generic.c | 1 +
arch/arm/mach-omap2/cclock33xx_data.c | 1 -
arch/arm/mach-omap2/common.h | 1 +
arch/arm/mach-omap2/io.c | 6 ++
arch/arm/mach-omap2/pm.c | 11 ++-
drivers/clk/Makefile | 1 +
drivers/clk/omap/Makefile | 1 +
drivers/clk/omap/clk.c | 96 ++++++++++++++++++++
12 files changed, 175 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/omap-clock.txt
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c

Regards,
Nishanth Menon
--
1.7.9.5


2013-04-12 22:55:09

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 2/6] ARM: dts: OMAP3: add clock nodes for CPU

OMAP34xx and OMAP36xx platforms use dpll1 clock. Add same to common
definition.

Cc: Benoit Cousson <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
RFC of this patch was discussed in http://marc.info/?t=136370325600009&r=1&w=2
along with the detailed context as to why this is an intermediate step.

previous revisions of cpufreq-cpu0 support do not use this approach.

[Probably belongs to Benoit's tree]
arch/arm/boot/dts/omap3.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 559b02f..58c6ed6 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -23,6 +23,8 @@
cpus {
[email protected] {
compatible = "arm,cortex-a8";
+ clocks = <&dpll1>;
+ clock-names = "cpu";
};
};

@@ -73,6 +75,11 @@
ti,hwmods = "counter_32k";
};

+ dpll1: dpll1 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+
intc: [email protected] {
compatible = "ti,omap2-intc";
interrupt-controller;
--
1.7.9.5

2013-04-12 22:55:21

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 4/6] ARM: dts: AM33XX: add clock nodes for CPU

AM33XX based platforms use dpll_mpu clock. Add same to common dtsi
and remove the dummy clock node entry as AM33XX platform supports
only device tree based boot.

Cc: Benoit Cousson <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
RFC of this patch was discussed in http://marc.info/?t=136370325600009&r=1&w=2
along with the detailed context as to why this is an intermediate step.

previous revisions of cpufreq-cpu0 support do not use this approach.

[Probably belongs to Benoit/Tony/Paul tree?]

arch/arm/boot/dts/am33xx.dtsi | 7 +++++++
arch/arm/mach-omap2/cclock33xx_data.c | 1 -
2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index df62830..3aed044 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -42,6 +42,8 @@
275000 1125000
>;
voltage-tolerance = <2>; /* 2 percentage */
+ clocks = <&dpll_mpu>;
+ clock-names = "cpu";
clock-latency = <300000>; /* From omap-cpufreq driver */
};
};
@@ -89,6 +91,11 @@
reg = <0x48200000 0x1000>;
};

+ dpll_mpu: dpll_mpu {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+
gpio0: [email protected] {
compatible = "ti,omap4-gpio";
ti,hwmods = "gpio1";
diff --git a/arch/arm/mach-omap2/cclock33xx_data.c b/arch/arm/mach-omap2/cclock33xx_data.c
index 7f091c8..8be6832 100644
--- a/arch/arm/mach-omap2/cclock33xx_data.c
+++ b/arch/arm/mach-omap2/cclock33xx_data.c
@@ -852,7 +852,6 @@ static struct omap_clk am33xx_clks[] = {
CLK(NULL, "dpll_core_m5_ck", &dpll_core_m5_ck, CK_AM33XX),
CLK(NULL, "dpll_core_m6_ck", &dpll_core_m6_ck, CK_AM33XX),
CLK(NULL, "dpll_mpu_ck", &dpll_mpu_ck, CK_AM33XX),
- CLK("cpu0", NULL, &dpll_mpu_ck, CK_AM33XX),
CLK(NULL, "dpll_mpu_m2_ck", &dpll_mpu_m2_ck, CK_AM33XX),
CLK(NULL, "dpll_ddr_ck", &dpll_ddr_ck, CK_AM33XX),
CLK(NULL, "dpll_ddr_m2_ck", &dpll_ddr_m2_ck, CK_AM33XX),
--
1.7.9.5

2013-04-12 22:55:18

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 5/6] ARM: OMAP2+: AM33XX: add lateinit hook for calling pm late init

Call OMAP2+ generic lateinit hook from AM specific late init hook.
This allows the generic late initializations such as cpufreq hooks
to be active.

Cc: Benoit Cousson <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
Introduced as a result of alignment in http://marc.info/?t=136450773400004&r=1&w=2
previous versions do not use this approach.

[Probably belongs to Kevin/Tony's tree?]
arch/arm/mach-omap2/board-generic.c | 1 +
arch/arm/mach-omap2/common.h | 1 +
arch/arm/mach-omap2/io.c | 6 ++++++
3 files changed, 8 insertions(+)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index afa509a..86e8479 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -141,6 +141,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)")
.init_irq = omap_intc_of_init,
.handle_irq = omap3_intc_handle_irq,
.init_machine = omap_generic_init,
+ .init_late = am33xx_init_late,
.init_time = omap3_am33xx_gptimer_timer_init,
.dt_compat = am33xx_boards_compat,
.restart = am33xx_restart,
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 14522d0..65f2ebc 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -108,6 +108,7 @@ void omap35xx_init_late(void);
void omap3630_init_late(void);
void am35xx_init_late(void);
void ti81xx_init_late(void);
+void am33xx_init_late(void);
int omap2_common_pm_late_init(void);

#if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430)
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 5c445ca..61f5b84 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -576,6 +576,12 @@ void __init am33xx_init_early(void)
omap_hwmod_init_postsetup();
omap_clk_init = am33xx_clk_init;
}
+
+void __init am33xx_init_late(void)
+{
+ omap2_common_pm_late_init();
+}
+
#endif

#ifdef CONFIG_ARCH_OMAP4
--
1.7.9.5

2013-04-12 22:55:16

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 6/6] ARM: OMAP3+: use cpu0-cpufreq driver in device tree supported boot

With OMAP3+ and AM33xx supported SoC having defined CPU device tree
entries with operating-points and clock nodes defined, we can now use
the SoC generic cpufreq-cpu0 driver by registering appropriate device.

Cc: Benoit Cousson <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
Introduced as a result of alignment in http://marc.info/?t=136450773400004&r=1&w=2
previous versions do not use this approach.

This would probably be good to get merged *after* Rafael's tree is merged in.

[Probably belongs to Kevin/Tony's tree?]

arch/arm/mach-omap2/pm.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 8d15f9a..6cf95160 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -267,7 +267,12 @@ static void __init omap4_init_voltages(void)

static inline void omap_init_cpufreq(void)
{
- struct platform_device_info devinfo = { .name = "omap-cpufreq", };
+ struct platform_device_info devinfo = { };
+
+ if (!of_have_populated_dt())
+ devinfo.name = "omap-cpufreq";
+ else
+ devinfo.name = "cpufreq-cpu0";
platform_device_register_full(&devinfo);
}

@@ -301,9 +306,9 @@ int __init omap2_common_pm_late_init(void)
/* Smartreflex device init */
omap_devinit_smartreflex();

- /* cpufreq dummy device instantiation */
- omap_init_cpufreq();
}
+ /* cpufreq dummy device instantiation */
+ omap_init_cpufreq();

#ifdef CONFIG_SUSPEND
suspend_set_ops(&omap_pm_ops);
--
1.7.9.5

2013-04-12 22:56:12

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 3/6] ARM: dts: OMAP4: add clock nodes for CPU

OMAP443x, OMAP446x and OMAP447x platforms use dpll_mpu clock.
Add same to common definition.

Cc: Benoit Cousson <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
RFC of this patch approach for OMAP3 was discussed in http://marc.info/?t=136370325600009&r=1&w=2
along with the detailed context as to why this is an intermediate step.

previous revisions of cpufreq-cpu0 support do not use this approach.
[Probably belongs to Benoit's tree]

arch/arm/boot/dts/omap4.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 2a56428..1c6d969 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -31,6 +31,8 @@
[email protected] {
compatible = "arm,cortex-a9";
next-level-cache = <&L2>;
+ clocks = <&dpll_mpu>;
+ clock-names = "cpu";
};
[email protected] {
compatible = "arm,cortex-a9";
@@ -106,6 +108,11 @@
ti,hwmods = "counter_32k";
};

+ dpll_mpu: dpll_mpu {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+
omap4_pmx_core: [email protected] {
compatible = "ti,omap4-padconf", "pinctrl-single";
reg = <0x4a100040 0x0196>;
--
1.7.9.5

2013-04-12 22:56:39

by Nishanth Menon

[permalink] [raw]
Subject: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c.
However, this presents an obstacle for using these clock nodes in
Device Tree definitions. This is especially true for board specific
clocks initially. The fixed clocks are currently found via clock
aliases table. There are many possible approaches to this problem as
discussed in the following thread:
http://marc.info/?t=136370325600009&r=1&w=2.
Highlights of the options:
a) device specific clk_add_alias:
cons: driver handling required
b) using an generic clk node and indexing to reach the clock required.
This is similar in approach taken by tegra and few other platforms.
Example usage: clock = <&clk 5>;
cons: potential to have mismatches in indexed table and associated
dtb data. In addition, managing continued documentation in bindings
as clock indexing increases. Even though readability angle could be
improved by using preprocessing of DT using macros, indexed
approach is inherently risky from cases like the following:
clk indexes in kernel:
1 - mpu_dpll
2 - aux_clk1
3 - core_clk
DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's
say kernel updates indices to:
1 - mpu_dpll
2 - per_dpll
3 - aux_clk1
4 - core_clk
using the old dtb(or dts missing an update), on new kernel which
has updated indices will result in per_dpll now controlled for
peripheral X without warning or any potential error detection.

Even though we could claim this is user error, such errors are hard
to track down and fix.

An alternate approach introduced here is to introduce device tree
bindings corresponding to the clock nodes required in DT definition
for SoC which automatically maps back to the definitions in
cclockXYZ_data.c.

The driver introduced here to do this mapping will eventually be the
place where the clock handling will migrate to. We need to consider
this angle as well so that the solution will be an valid transition
point for moving the clock data out of kernel image (into device tree
or firmware load etc..).

Overall strategy introduced here is simple: a clock node described in
device tree blob is used to identify the exact clock provided in the
SoC specific data. This is then linked back using of_clk_add_provider
to the device node to be accessible by of_clk_get.

Based on discussion contributions from Roger Quadros, Grygorii Strashko
and others.

Cc: Kevin Hilman <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Paul Walmsley <[email protected]>
[[email protected]: co-developed]
Signed-off-by: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---

RFC of this patch was discussed in http://marc.info/?t=136370325600009&r=1&w=2
along with the detailed context as to why this is an intermediate step.

Previous revisions of cpufreq-cpu0 support do not use this approach.
[Might eventually belong to Mike's tree.]

.../devicetree/bindings/clock/omap-clock.txt | 40 ++++++++
drivers/clk/Makefile | 1 +
drivers/clk/omap/Makefile | 1 +
drivers/clk/omap/clk.c | 96 ++++++++++++++++++++
4 files changed, 138 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/omap-clock.txt
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c

diff --git a/Documentation/devicetree/bindings/clock/omap-clock.txt b/Documentation/devicetree/bindings/clock/omap-clock.txt
new file mode 100644
index 0000000..047c1e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/omap-clock.txt
@@ -0,0 +1,40 @@
+Device Tree Clock bindings for Texas Instrument's OMAP compatible platforms
+
+This binding is an initial minimal binding that may be enhanced as part of
+transitioning OMAP clock data out of kernel image.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,omap-clock"
+- #clock-cells : from common clock binding; shall be set to 0.
+NOTE:
+node name should map to clock database in arch/arm/mach-omap2/cclock<SoC>_data.c
+Since all clocks are described with _ck, the node name is optimized to drop the
+usage of _ck. For example, a clock called dpll1_ck will be defined as dpll1.
+
+Example #1: describing clock node for CPU on OMAP34xx platform:
+Ref: arch/arm/mach-omap2/cclock3xxx_data.c
+describes the CPU clock to be as follows
+ CLK(NULL, "dpll1_ck", &dpll1_ck, CK_3XXX),
+Corresponding binding will be:
+ dpll1: dpll1 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&dpll1>;
+
+Example #2: describing clock node for auxilary clock #3 on OMAP443x platform:
+Ref: arch/arm/mach-omap2/cclock44xx_data.c
+describes the auxclk3 clock to be as follows:
+ CLK(NULL, "auxclk3_ck", &auxclk3_ck, CK_443X),
+Corresponding binding will be:
+ auxclk3: auxclk3 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&auxclk3>;
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e7f7fe9..dbf9b13 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_U8500) += ux500/
obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/
+obj-$(CONFIG_ARCH_OMAP) += omap/

obj-$(CONFIG_X86) += x86/

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
new file mode 100644
index 0000000..8195931
--- /dev/null
+++ b/drivers/clk/omap/Makefile
@@ -0,0 +1 @@
+obj-y += clk.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
new file mode 100644
index 0000000..6f12023
--- /dev/null
+++ b/drivers/clk/omap/clk.c
@@ -0,0 +1,96 @@
+/*
+ * Texas Instruments OMAP Clock driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Nishanth Menon <[email protected]>
+ * Tony Lindgren <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+
+static const struct of_device_id omap_clk_of_match[] = {
+ {.compatible = "ti,omap-clock",},
+ {},
+};
+
+/**
+ * omap_clk_src_get() - Get OMAP clock from node name when needed
+ * @clkspec: clkspec argument
+ * @data: unused
+ *
+ * REVISIT: We assume the following:
+ * 1. omap clock names end with _ck
+ * 2. omap clock names are under 32 characters in length
+ */
+static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct clk *clk;
+ char clk_name[32];
+ struct device_node *np = clkspec->np;
+
+ /* Set up things so consumer can call clk_get() with name */
+ snprintf(clk_name, 32, "%s_ck", np->name);
+ clk = clk_get(NULL, clk_name);
+ if (IS_ERR(clk)) {
+ pr_err("%s: could not get clock %s(%ld)\n", __func__,
+ clk_name, PTR_ERR(clk));
+ goto out;
+ }
+ clk_put(clk);
+
+out:
+ return clk;
+}
+
+/**
+ * omap_clk_probe() - create link from DT definition to clock data
+ * @pdev: device node
+ *
+ * NOTE: We assume that omap clocks are not removed.
+ */
+static int omap_clk_probe(struct platform_device *pdev)
+{
+ int res;
+ struct device_node *np = pdev->dev.of_node;
+
+ /* This allows the driver to of_clk_get() */
+ res = of_clk_add_provider(np, omap_clk_src_get, NULL);
+ if (res)
+ dev_err(&pdev->dev, "could not add provider(%d)\n", res);
+
+ return res;
+}
+
+/* We assume here that OMAP clocks will not be removed */
+static struct platform_driver omap_clk_driver = {
+ .probe = omap_clk_probe,
+ .driver = {
+ .name = "omap_clk",
+ .of_match_table = of_match_ptr(omap_clk_of_match),
+ },
+};
+
+static int __init omap_clk_init(void)
+{
+ return platform_driver_register(&omap_clk_driver);
+}
+arch_initcall(omap_clk_init);
+
+MODULE_DESCRIPTION("OMAP Clock driver");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5

2013-04-12 23:31:49

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

* Nishanth Menon <[email protected]> [130412 15:59]:
> --- /dev/null
> +++ b/drivers/clk/omap/clk.c
> +/**
> + * omap_clk_src_get() - Get OMAP clock from node name when needed
> + * @clkspec: clkspec argument
> + * @data: unused
> + *
> + * REVISIT: We assume the following:
> + * 1. omap clock names end with _ck
> + * 2. omap clock names are under 32 characters in length
> + */
> +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
> +{
> + struct clk *clk;
> + char clk_name[32];
> + struct device_node *np = clkspec->np;
> +
> + /* Set up things so consumer can call clk_get() with name */

I would leave out the comment above, it's a leftover from
the clk_add_alias() version that we don't need because of
of_clk_get().

> + snprintf(clk_name, 32, "%s_ck", np->name);
> + clk = clk_get(NULL, clk_name);
> + if (IS_ERR(clk)) {
> + pr_err("%s: could not get clock %s(%ld)\n", __func__,
> + clk_name, PTR_ERR(clk));
> + goto out;
> + }
> + clk_put(clk);
> +
> +out:
> + return clk;
> +}
> +
> +/**
> + * omap_clk_probe() - create link from DT definition to clock data
> + * @pdev: device node
> + *
> + * NOTE: We assume that omap clocks are not removed.
> + */

How about drop the comment on clocks being removed above.
It no longer an issue, so maybe something like this instead:

* Note that we look up the clock lazily when the consumer
* driver does of_clk_get() and initialize a NULL clock here.

> +static int omap_clk_probe(struct platform_device *pdev)
> +{
> + int res;
> + struct device_node *np = pdev->dev.of_node;
> +
> + /* This allows the driver to of_clk_get() */
> + res = of_clk_add_provider(np, omap_clk_src_get, NULL);
> + if (res)
> + dev_err(&pdev->dev, "could not add provider(%d)\n", res);
> +
> + return res;
> +}
> +
> +/* We assume here that OMAP clocks will not be removed */

Then the above comment can be removed too.

Regards,

Tony

2013-04-12 23:39:14

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

On 16:31-20130412, Tony Lindgren wrote:
> * Nishanth Menon <[email protected]> [130412 15:59]:
> > --- /dev/null
> > +++ b/drivers/clk/omap/clk.c
> > +/**
> > + * omap_clk_src_get() - Get OMAP clock from node name when needed
> > + * @clkspec: clkspec argument
> > + * @data: unused
> > + *
> > + * REVISIT: We assume the following:
> > + * 1. omap clock names end with _ck
> > + * 2. omap clock names are under 32 characters in length
> > + */
> > +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
> > +{
> > + struct clk *clk;
> > + char clk_name[32];
> > + struct device_node *np = clkspec->np;
> > +
> > + /* Set up things so consumer can call clk_get() with name */
>
> I would leave out the comment above, it's a leftover from
> the clk_add_alias() version that we don't need because of
> of_clk_get().
>
> > + snprintf(clk_name, 32, "%s_ck", np->name);
> > + clk = clk_get(NULL, clk_name);
> > + if (IS_ERR(clk)) {
> > + pr_err("%s: could not get clock %s(%ld)\n", __func__,
> > + clk_name, PTR_ERR(clk));
> > + goto out;
> > + }
> > + clk_put(clk);
> > +
> > +out:
> > + return clk;
> > +}
> > +
> > +/**
> > + * omap_clk_probe() - create link from DT definition to clock data
> > + * @pdev: device node
> > + *
> > + * NOTE: We assume that omap clocks are not removed.
> > + */
>
> How about drop the comment on clocks being removed above.
> It no longer an issue, so maybe something like this instead:
>
> * Note that we look up the clock lazily when the consumer
> * driver does of_clk_get() and initialize a NULL clock here.
>
> > +static int omap_clk_probe(struct platform_device *pdev)
> > +{
> > + int res;
> > + struct device_node *np = pdev->dev.of_node;
> > +
> > + /* This allows the driver to of_clk_get() */
> > + res = of_clk_add_provider(np, omap_clk_src_get, NULL);
> > + if (res)
> > + dev_err(&pdev->dev, "could not add provider(%d)\n", res);
> > +
> > + return res;
> > +}
> > +
> > +/* We assume here that OMAP clocks will not be removed */
>
> Then the above comment can be removed too.
Thanks for checking up. Fixed all of them below, will post part of
series again, only if I need to address further comments in other
patches..

>From f96c04860794f9bbfe240a8661641a7c90dd1640 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <[email protected]>
Date: Tue, 9 Apr 2013 19:26:40 -0500
Subject: [PATCH V5 1/6] clk: OMAP: introduce device tree binding to kernel clock
data

OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c.
However, this presents an obstacle for using these clock nodes in
Device Tree definitions. This is especially true for board specific
clocks initially. The fixed clocks are currently found via clock
aliases table. There are many possible approaches to this problem as
discussed in the following thread:
http://marc.info/?t=136370325600009&r=1&w=2.
Highlights of the options:
a) device specific clk_add_alias:
cons: driver handling required
b) using an generic clk node and indexing to reach the clock required.
This is similar in approach taken by tegra and few other platforms.
Example usage: clock = <&clk 5>;
cons: potential to have mismatches in indexed table and associated
dtb data. In addition, managing continued documentation in bindings
as clock indexing increases. Even though readability angle could be
improved by using preprocessing of DT using macros, indexed
approach is inherently risky from cases like the following:
clk indexes in kernel:
1 - mpu_dpll
2 - aux_clk1
3 - core_clk
DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's
say kernel updates indices to:
1 - mpu_dpll
2 - per_dpll
3 - aux_clk1
4 - core_clk
using the old dtb(or dts missing an update), on new kernel which
has updated indices will result in per_dpll now controlled for
peripheral X without warning or any potential error detection.

Even though we could claim this is user error, such errors are hard
to track down and fix.

An alternate approach introduced here is to introduce device tree
bindings corresponding to the clock nodes required in DT definition
for SoC which automatically maps back to the definitions in
cclockXYZ_data.c.

The driver introduced here to do this mapping will eventually be the
place where the clock handling will migrate to. We need to consider
this angle as well so that the solution will be an valid transition
point for moving the clock data out of kernel image (into device tree
or firmware load etc..).

Overall strategy introduced here is simple: a clock node described in
device tree blob is used to identify the exact clock provided in the
SoC specific data. This is then linked back using of_clk_add_provider
to the device node to be accessible by of_clk_get.

Based on discussion contributions from Roger Quadros, Grygorii Strashko
and others.

Cc: Kevin Hilman <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Paul Walmsley <[email protected]>
[[email protected]: co-developed]
Signed-off-by: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
.../devicetree/bindings/clock/omap-clock.txt | 40 +++++++++
drivers/clk/Makefile | 1 +
drivers/clk/omap/Makefile | 1 +
drivers/clk/omap/clk.c | 95 ++++++++++++++++++++
4 files changed, 137 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/omap-clock.txt
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c

diff --git a/Documentation/devicetree/bindings/clock/omap-clock.txt b/Documentation/devicetree/bindings/clock/omap-clock.txt
new file mode 100644
index 0000000..047c1e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/omap-clock.txt
@@ -0,0 +1,40 @@
+Device Tree Clock bindings for Texas Instrument's OMAP compatible platforms
+
+This binding is an initial minimal binding that may be enhanced as part of
+transitioning OMAP clock data out of kernel image.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,omap-clock"
+- #clock-cells : from common clock binding; shall be set to 0.
+NOTE:
+node name should map to clock database in arch/arm/mach-omap2/cclock<SoC>_data.c
+Since all clocks are described with _ck, the node name is optimized to drop the
+usage of _ck. For example, a clock called dpll1_ck will be defined as dpll1.
+
+Example #1: describing clock node for CPU on OMAP34xx platform:
+Ref: arch/arm/mach-omap2/cclock3xxx_data.c
+describes the CPU clock to be as follows
+ CLK(NULL, "dpll1_ck", &dpll1_ck, CK_3XXX),
+Corresponding binding will be:
+ dpll1: dpll1 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&dpll1>;
+
+Example #2: describing clock node for auxilary clock #3 on OMAP443x platform:
+Ref: arch/arm/mach-omap2/cclock44xx_data.c
+describes the auxclk3 clock to be as follows:
+ CLK(NULL, "auxclk3_ck", &auxclk3_ck, CK_443X),
+Corresponding binding will be:
+ auxclk3: auxclk3 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&auxclk3>;
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e7f7fe9..dbf9b13 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_U8500) += ux500/
obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/
+obj-$(CONFIG_ARCH_OMAP) += omap/

obj-$(CONFIG_X86) += x86/

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
new file mode 100644
index 0000000..8195931
--- /dev/null
+++ b/drivers/clk/omap/Makefile
@@ -0,0 +1 @@
+obj-y += clk.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
new file mode 100644
index 0000000..e73496b
--- /dev/null
+++ b/drivers/clk/omap/clk.c
@@ -0,0 +1,95 @@
+/*
+ * Texas Instruments OMAP Clock driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Nishanth Menon <[email protected]>
+ * Tony Lindgren <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+
+static const struct of_device_id omap_clk_of_match[] = {
+ {.compatible = "ti,omap-clock",},
+ {},
+};
+
+/**
+ * omap_clk_src_get() - Get OMAP clock from node name when needed
+ * @clkspec: clkspec argument
+ * @data: unused
+ *
+ * REVISIT: We assume the following:
+ * 1. omap clock names end with _ck
+ * 2. omap clock names are under 32 characters in length
+ */
+static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct clk *clk;
+ char clk_name[32];
+ struct device_node *np = clkspec->np;
+
+ snprintf(clk_name, 32, "%s_ck", np->name);
+ clk = clk_get(NULL, clk_name);
+ if (IS_ERR(clk)) {
+ pr_err("%s: could not get clock %s(%ld)\n", __func__,
+ clk_name, PTR_ERR(clk));
+ goto out;
+ }
+ clk_put(clk);
+
+out:
+ return clk;
+}
+
+/**
+ * omap_clk_probe() - create link from DT definition to clock data
+ * @pdev: device node
+ *
+ * NOTE: we look up the clock lazily when the consumer driver does
+ * of_clk_get() and initialize a NULL clock here.
+ */
+static int omap_clk_probe(struct platform_device *pdev)
+{
+ int res;
+ struct device_node *np = pdev->dev.of_node;
+
+ /* This allows the driver to of_clk_get() */
+ res = of_clk_add_provider(np, omap_clk_src_get, NULL);
+ if (res)
+ dev_err(&pdev->dev, "could not add provider(%d)\n", res);
+
+ return res;
+}
+
+static struct platform_driver omap_clk_driver = {
+ .probe = omap_clk_probe,
+ .driver = {
+ .name = "omap_clk",
+ .of_match_table = of_match_ptr(omap_clk_of_match),
+ },
+};
+
+static int __init omap_clk_init(void)
+{
+ return platform_driver_register(&omap_clk_driver);
+}
+arch_initcall(omap_clk_init);
+
+MODULE_DESCRIPTION("OMAP Clock driver");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5


--
Regards,
Nishanth Menon

2013-04-13 17:22:16

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

* Nishanth Menon <[email protected]> [130412 16:43]:
> Thanks for checking up. Fixed all of them below, will post part of
> series again, only if I need to address further comments in other
> patches..

Thanks it seems that the other ones are ready to go, just one
more comment below.

> --- /dev/null
> +++ b/drivers/clk/omap/clk.c
> @@ -0,0 +1,95 @@
> +/*
> + * Texas Instruments OMAP Clock driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
> + * Nishanth Menon <[email protected]>
> + * Tony Lindgren <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/string.h>
> +
> +static const struct of_device_id omap_clk_of_match[] = {
> + {.compatible = "ti,omap-clock",},
> + {},
> +};
> +
> +/**
> + * omap_clk_src_get() - Get OMAP clock from node name when needed
> + * @clkspec: clkspec argument
> + * @data: unused
> + *
> + * REVISIT: We assume the following:
> + * 1. omap clock names end with _ck
> + * 2. omap clock names are under 32 characters in length
> + */
> +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
> +{
> + struct clk *clk;
> + char clk_name[32];
> + struct device_node *np = clkspec->np;
> +
> + snprintf(clk_name, 32, "%s_ck", np->name);
> + clk = clk_get(NULL, clk_name);
> + if (IS_ERR(clk)) {
> + pr_err("%s: could not get clock %s(%ld)\n", __func__,
> + clk_name, PTR_ERR(clk));
> + goto out;
> + }
> + clk_put(clk);

It seems that clk_put() is actually wrong here. That's because
of_clk_get() should boild down to just the look up of the clock
and then clk_get() on it, so no double clk_get() is done in this
case. Once the consumer driver is done, it will just call clk_put()
on it.

> +out:
> + return clk;
> +}
> +
> +/**
> + * omap_clk_probe() - create link from DT definition to clock data
> + * @pdev: device node
> + *
> + * NOTE: we look up the clock lazily when the consumer driver does
> + * of_clk_get() and initialize a NULL clock here.
> + */
> +static int omap_clk_probe(struct platform_device *pdev)
> +{
> + int res;
> + struct device_node *np = pdev->dev.of_node;
> +
> + /* This allows the driver to of_clk_get() */
> + res = of_clk_add_provider(np, omap_clk_src_get, NULL);
> + if (res)
> + dev_err(&pdev->dev, "could not add provider(%d)\n", res);
> +
> + return res;
> +}
> +
> +static struct platform_driver omap_clk_driver = {
> + .probe = omap_clk_probe,
> + .driver = {
> + .name = "omap_clk",
> + .of_match_table = of_match_ptr(omap_clk_of_match),
> + },
> +};
> +
> +static int __init omap_clk_init(void)
> +{
> + return platform_driver_register(&omap_clk_driver);
> +}
> +arch_initcall(omap_clk_init);
> +
> +MODULE_DESCRIPTION("OMAP Clock driver");
> +MODULE_AUTHOR("Texas Instruments Inc.");
> +MODULE_LICENSE("GPL v2");

Other than that looks OK to me.

Tony

2013-04-14 21:19:33

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

On 10:22-20130413, Tony Lindgren wrote:
> * Nishanth Menon <[email protected]> [130412 16:43]:
> > Thanks for checking up. Fixed all of them below, will post part of
> > series again, only if I need to address further comments in other
> > patches..
>
> Thanks it seems that the other ones are ready to go, just one
> more comment below.
>
[...]
> > +static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
> > +{
> > + struct clk *clk;
> > + char clk_name[32];
> > + struct device_node *np = clkspec->np;
> > +
> > + snprintf(clk_name, 32, "%s_ck", np->name);
> > + clk = clk_get(NULL, clk_name);
> > + if (IS_ERR(clk)) {
> > + pr_err("%s: could not get clock %s(%ld)\n", __func__,
> > + clk_name, PTR_ERR(clk));
> > + goto out;
> > + }
> > + clk_put(clk);
>
> It seems that clk_put() is actually wrong here. That's because
> of_clk_get() should boild down to just the look up of the clock
> and then clk_get() on it, so no double clk_get() is done in this
> case. Once the consumer driver is done, it will just call clk_put()
> on it.
Yep - updated version below.
>From d0bf3fce235cff46feac7f5ef1a40e2fa0f2aa12 Mon Sep 17 00:00:00 2001
From: Nishanth Menon <[email protected]>
Date: Tue, 9 Apr 2013 19:26:40 -0500
Subject: [PATCH V5.1 1/6] clk: OMAP: introduce device tree binding to kernel clock
data

OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c.
However, this presents an obstacle for using these clock nodes in
Device Tree definitions. This is especially true for board specific
clocks initially. The fixed clocks are currently found via clock
aliases table. There are many possible approaches to this problem as
discussed in the following thread:
http://marc.info/?t=136370325600009&r=1&w=2.
Highlights of the options:
a) device specific clk_add_alias:
cons: driver handling required
b) using an generic clk node and indexing to reach the clock required.
This is similar in approach taken by tegra and few other platforms.
Example usage: clock = <&clk 5>;
cons: potential to have mismatches in indexed table and associated
dtb data. In addition, managing continued documentation in bindings
as clock indexing increases. Even though readability angle could be
improved by using preprocessing of DT using macros, indexed
approach is inherently risky from cases like the following:
clk indexes in kernel:
1 - mpu_dpll
2 - aux_clk1
3 - core_clk
DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's
say kernel updates indices to:
1 - mpu_dpll
2 - per_dpll
3 - aux_clk1
4 - core_clk
using the old dtb(or dts missing an update), on new kernel which
has updated indices will result in per_dpll now controlled for
peripheral X without warning or any potential error detection.

Even though we could claim this is user error, such errors are hard
to track down and fix.

An alternate approach introduced here is to introduce device tree
bindings corresponding to the clock nodes required in DT definition
for SoC which automatically maps back to the definitions in
cclockXYZ_data.c.

The driver introduced here to do this mapping will eventually be the
place where the clock handling will migrate to. We need to consider
this angle as well so that the solution will be an valid transition
point for moving the clock data out of kernel image (into device tree
or firmware load etc..).

Overall strategy introduced here is simple: a clock node described in
device tree blob is used to identify the exact clock provided in the
SoC specific data. This is then linked back using of_clk_add_provider
to the device node to be accessible by of_clk_get.

Based on discussion contributions from Roger Quadros, Grygorii Strashko
and others.

Cc: Kevin Hilman <[email protected]>
Cc: Mike Turquette <[email protected]>
Cc: Paul Walmsley <[email protected]>
[[email protected]: co-developed]
Signed-off-by: Tony Lindgren <[email protected]>
Signed-off-by: Nishanth Menon <[email protected]>
---
.../devicetree/bindings/clock/omap-clock.txt | 40 +++++++++
drivers/clk/Makefile | 1 +
drivers/clk/omap/Makefile | 1 +
drivers/clk/omap/clk.c | 91 ++++++++++++++++++++
4 files changed, 133 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/omap-clock.txt
create mode 100644 drivers/clk/omap/Makefile
create mode 100644 drivers/clk/omap/clk.c

diff --git a/Documentation/devicetree/bindings/clock/omap-clock.txt b/Documentation/devicetree/bindings/clock/omap-clock.txt
new file mode 100644
index 0000000..047c1e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/omap-clock.txt
@@ -0,0 +1,40 @@
+Device Tree Clock bindings for Texas Instrument's OMAP compatible platforms
+
+This binding is an initial minimal binding that may be enhanced as part of
+transitioning OMAP clock data out of kernel image.
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,omap-clock"
+- #clock-cells : from common clock binding; shall be set to 0.
+NOTE:
+node name should map to clock database in arch/arm/mach-omap2/cclock<SoC>_data.c
+Since all clocks are described with _ck, the node name is optimized to drop the
+usage of _ck. For example, a clock called dpll1_ck will be defined as dpll1.
+
+Example #1: describing clock node for CPU on OMAP34xx platform:
+Ref: arch/arm/mach-omap2/cclock3xxx_data.c
+describes the CPU clock to be as follows
+ CLK(NULL, "dpll1_ck", &dpll1_ck, CK_3XXX),
+Corresponding binding will be:
+ dpll1: dpll1 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&dpll1>;
+
+Example #2: describing clock node for auxilary clock #3 on OMAP443x platform:
+Ref: arch/arm/mach-omap2/cclock44xx_data.c
+describes the auxclk3 clock to be as follows:
+ CLK(NULL, "auxclk3_ck", &auxclk3_ck, CK_443X),
+Corresponding binding will be:
+ auxclk3: auxclk3 {
+ #clock-cells = <0>;
+ compatible = "ti,omap-clock";
+ };
+And it's usage will be:
+ clocks = <&auxclk3>;
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e7f7fe9..dbf9b13 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_ARCH_U8500) += ux500/
obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o
obj-$(CONFIG_ARCH_ZYNQ) += clk-zynq.o
obj-$(CONFIG_ARCH_TEGRA) += tegra/
+obj-$(CONFIG_ARCH_OMAP) += omap/

obj-$(CONFIG_X86) += x86/

diff --git a/drivers/clk/omap/Makefile b/drivers/clk/omap/Makefile
new file mode 100644
index 0000000..8195931
--- /dev/null
+++ b/drivers/clk/omap/Makefile
@@ -0,0 +1 @@
+obj-y += clk.o
diff --git a/drivers/clk/omap/clk.c b/drivers/clk/omap/clk.c
new file mode 100644
index 0000000..5a3c6d9
--- /dev/null
+++ b/drivers/clk/omap/clk.c
@@ -0,0 +1,91 @@
+/*
+ * Texas Instruments OMAP Clock driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
+ * Nishanth Menon <[email protected]>
+ * Tony Lindgren <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+
+static const struct of_device_id omap_clk_of_match[] = {
+ {.compatible = "ti,omap-clock",},
+ {},
+};
+
+/**
+ * omap_clk_src_get() - Get OMAP clock from node name when needed
+ * @clkspec: clkspec argument
+ * @data: unused
+ *
+ * REVISIT: We assume the following:
+ * 1. omap clock names end with _ck
+ * 2. omap clock names are under 32 characters in length
+ */
+static struct clk *omap_clk_src_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct clk *clk;
+ char clk_name[32];
+ struct device_node *np = clkspec->np;
+
+ snprintf(clk_name, 32, "%s_ck", np->name);
+ clk = clk_get(NULL, clk_name);
+ if (IS_ERR(clk))
+ pr_err("%s: could not get clock %s(%ld)\n", __func__,
+ clk_name, PTR_ERR(clk));
+
+ return clk;
+}
+
+/**
+ * omap_clk_probe() - create link from DT definition to clock data
+ * @pdev: device node
+ *
+ * NOTE: we look up the clock lazily when the consumer driver does
+ * of_clk_get() and initialize a NULL clock here.
+ */
+static int omap_clk_probe(struct platform_device *pdev)
+{
+ int res;
+ struct device_node *np = pdev->dev.of_node;
+
+ /* This allows the driver to of_clk_get() */
+ res = of_clk_add_provider(np, omap_clk_src_get, NULL);
+ if (res)
+ dev_err(&pdev->dev, "could not add provider(%d)\n", res);
+
+ return res;
+}
+
+static struct platform_driver omap_clk_driver = {
+ .probe = omap_clk_probe,
+ .driver = {
+ .name = "omap_clk",
+ .of_match_table = of_match_ptr(omap_clk_of_match),
+ },
+};
+
+static int __init omap_clk_init(void)
+{
+ return platform_driver_register(&omap_clk_driver);
+}
+arch_initcall(omap_clk_init);
+
+MODULE_DESCRIPTION("OMAP Clock driver");
+MODULE_AUTHOR("Texas Instruments Inc.");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5


--
Regards,
Nishanth Menon

2013-04-15 16:22:44

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

On 04/12/2013 04:54 PM, Nishanth Menon wrote:
> OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c.
> However, this presents an obstacle for using these clock nodes in
> Device Tree definitions. This is especially true for board specific
> clocks initially. The fixed clocks are currently found via clock
> aliases table. There are many possible approaches to this problem as
> discussed in the following thread:
> http://marc.info/?t=136370325600009&r=1&w=2.
> Highlights of the options:
> a) device specific clk_add_alias:
> cons: driver handling required
> b) using an generic clk node and indexing to reach the clock required.
> This is similar in approach taken by tegra and few other platforms.
> Example usage: clock = <&clk 5>;
> cons: potential to have mismatches in indexed table and associated
> dtb data. In addition, managing continued documentation in bindings
> as clock indexing increases. Even though readability angle could be
> improved by using preprocessing of DT using macros, indexed
> approach is inherently risky from cases like the following:
> clk indexes in kernel:
> 1 - mpu_dpll
> 2 - aux_clk1
> 3 - core_clk
> DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's
> say kernel updates indices to:
> 1 - mpu_dpll
> 2 - per_dpll
> 3 - aux_clk1
> 4 - core_clk
> using the old dtb(or dts missing an update), on new kernel which
> has updated indices will result in per_dpll now controlled for
> peripheral X without warning or any potential error detection.
>
> Even though we could claim this is user error, such errors are hard
> to track down and fix.

The error in case (b) is that you shouldn't be changing the DT bindings
after they've first been created. That would avoid the problem
situation. The person using the old DTB with the new kernel didn't
commit user error.

> An alternate approach introduced here is to introduce device tree
> bindings corresponding to the clock nodes required in DT definition
> for SoC which automatically maps back to the definitions in
> cclockXYZ_data.c.

Well, I haven't read the patches, but isn't that exactly what the "2" is
in <&clk 2>?

2013-04-15 18:43:57

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

On Mon, Apr 15, 2013 at 11:22 AM, Stephen Warren <[email protected]> wrote:
> On 04/12/2013 04:54 PM, Nishanth Menon wrote:
>> OMAP clock data is located in arch/arm/mach-omap2/cclockXYZ_data.c.
>> However, this presents an obstacle for using these clock nodes in
>> Device Tree definitions. This is especially true for board specific
>> clocks initially. The fixed clocks are currently found via clock
>> aliases table. There are many possible approaches to this problem as
>> discussed in the following thread:
>> http://marc.info/?t=136370325600009&r=1&w=2.
>> Highlights of the options:
>> a) device specific clk_add_alias:
>> cons: driver handling required
>> b) using an generic clk node and indexing to reach the clock required.
>> This is similar in approach taken by tegra and few other platforms.
>> Example usage: clock = <&clk 5>;
>> cons: potential to have mismatches in indexed table and associated
>> dtb data. In addition, managing continued documentation in bindings
>> as clock indexing increases. Even though readability angle could be
>> improved by using preprocessing of DT using macros, indexed
>> approach is inherently risky from cases like the following:
>> clk indexes in kernel:
>> 1 - mpu_dpll
>> 2 - aux_clk1
>> 3 - core_clk
>> DT entry for peripheral X uses <&clk 2> to reach aux_clk1. Now, let's
>> say kernel updates indices to:
>> 1 - mpu_dpll
>> 2 - per_dpll
>> 3 - aux_clk1
>> 4 - core_clk
>> using the old dtb(or dts missing an update), on new kernel which
>> has updated indices will result in per_dpll now controlled for
>> peripheral X without warning or any potential error detection.
>>
>> Even though we could claim this is user error, such errors are hard
>> to track down and fix.
>
> The error in case (b) is that you shouldn't be changing the DT bindings
> after they've first been created. That would avoid the problem
> situation. The person using the old DTB with the new kernel didn't
> commit user error.
That is what we'd like folks to follow, but when code is in
development, or productization, there is no saying the type of
mistakes people end up doing.
>
>> An alternate approach introduced here is to introduce device tree
>> bindings corresponding to the clock nodes required in DT definition
>> for SoC which automatically maps back to the definitions in
>> cclockXYZ_data.c.
>
> Well, I haven't read the patches, but isn't that exactly what the "2" is
> in <&clk 2>?
partly so. yes: it indexes back to the right clock node - at that
point the comparison stops. we just point back on naming as needed and
introduce nodes purely for the ones that we need.

Regards,
Nishanth Menon

2013-04-24 16:28:55

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

Quoting Nishanth Menon (2013-04-14 14:19:17)
> Overall strategy introduced here is simple: a clock node described in
> device tree blob is used to identify the exact clock provided in the
> SoC specific data. This is then linked back using of_clk_add_provider
> to the device node to be accessible by of_clk_get.
>
> Based on discussion contributions from Roger Quadros, Grygorii Strashko
> and others.
>
> Cc: Kevin Hilman <[email protected]>
> Cc: Mike Turquette <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> [[email protected]: co-developed]
> Signed-off-by: Tony Lindgren <[email protected]de.com>
> Signed-off-by: Nishanth Menon <[email protected]>

I can take this into clk-next after the merge window. Please refresh it
for -rc1 as we discussed on irc.

As an aside, will Tero's series for migrating to drivers/clk/omap[1] and
Rajendra's patch for registering clocks late[2] be refreshed after the
merge window? It would be nice to combine these efforts.

Thanks,
Mike

[1] http://article.gmane.org/gmane.linux.ports.arm.omap/95948
[2] http://www.spinics.net/lists/arm-kernel/msg231288.html

2013-04-25 07:01:45

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH V4 1/6] clk: OMAP: introduce device tree binding to kernel clock data

On Wednesday 24 April 2013 09:58 PM, Mike Turquette wrote:
> Quoting Nishanth Menon (2013-04-14 14:19:17)
>> Overall strategy introduced here is simple: a clock node described in
>> device tree blob is used to identify the exact clock provided in the
>> SoC specific data. This is then linked back using of_clk_add_provider
>> to the device node to be accessible by of_clk_get.
>>
>> Based on discussion contributions from Roger Quadros, Grygorii Strashko
>> and others.
>>
>> Cc: Kevin Hilman <[email protected]>
>> Cc: Mike Turquette <[email protected]>
>> Cc: Paul Walmsley <[email protected]>
>> [[email protected]: co-developed]
>> Signed-off-by: Tony Lindgren <[email protected]>
>> Signed-off-by: Nishanth Menon <[email protected]>
>
> I can take this into clk-next after the merge window. Please refresh it
> for -rc1 as we discussed on irc.
>
> As an aside, will Tero's series for migrating to drivers/clk/omap[1] and
> Rajendra's patch for registering clocks late[2] be refreshed after the

[2] is already in mainline.

> merge window? It would be nice to combine these efforts.
>
> Thanks,
> Mike
>
> [1] http://article.gmane.org/gmane.linux.ports.arm.omap/95948
> [2] http://www.spinics.net/lists/arm-kernel/msg231288.html
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>