[PATCH 0/3] OMAP2+: hwmod: Add support to parse clock info from DT

[PATCH 0/3] OMAP2+: hwmod: Add support to parse clock info from DT

[Rajendra Nayak]

With all of OMAP clock data now moving to DT, its possible to pass the
main or functional clock and all optional clocks information for a
device from DT instead of having these as part of hwmod static data
in the kernel.

This patch series is based on 'v3' of omap4 clock movement to DT
patches [1] and is boot tested on a omap4 panda es.

The series does leave all the interface clocks as part of hwmod (and
main and optional clocks for devices which are missing DT drivers).

[1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100117

Rajendra Nayak (3):
  ARM: OMAP2+: Add support to parse 'main_clk' info from DT
  ARM: OMAP2+: Add support to parse optional clk info from DT
  ARM: OMAP4: dts: Add main and optional clock data into DT

 arch/arm/boot/dts/omap4.dtsi               |  100 +++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod.c           |  100 +++++++++++++++++++++----
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  112 ----------------------------
 3 files changed, 185 insertions(+), 127 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] ARM: OMAP2+: Add support to parse 'main_clk' info from DT

[Rajendra Nayak]

With clocks for OMAP moving to DT, its now possible to pass the 'main_clk'
data for each device from DT instead of having it in hwmod.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5cc5123..12fa589 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -728,14 +728,18 @@ static int _del_initiator_dep(struct omap_hwmod *oh, struct omap_hwmod *init_oh)
  * functional clock pointer) if a main_clk is present.  Returns 0 on
  * success or -EINVAL on error.
  */
-static int _init_main_clk(struct omap_hwmod *oh)
+static int _init_main_clk(struct omap_hwmod *oh, struct device_node *np)
 {
 	int ret = 0;
 
-	if (!oh->main_clk)
+	if (!oh->main_clk && !of_get_property(np, "clocks", NULL))
 		return 0;
 
-	oh->_clk = clk_get(NULL, oh->main_clk);
+	if (oh->main_clk)
+		oh->_clk = clk_get(NULL, oh->main_clk);
+	else
+		oh->_clk = of_clk_get_by_name(np, "fck");
+
 	if (IS_ERR(oh->_clk)) {
 		pr_warning("omap_hwmod: %s: cannot clk_get main_clk %s\n",
 			   oh->name, oh->main_clk);
@@ -1561,7 +1565,8 @@ static int _init_clkdm(struct omap_hwmod *oh)
  * Resolves all clock names embedded in the hwmod.  Returns 0 on
  * success, or a negative error code on failure.
  */
-static int _init_clocks(struct omap_hwmod *oh, void *data)
+static int _init_clocks(struct omap_hwmod *oh, void *data,
+			struct device_node *np)
 {
 	int ret = 0;
 
@@ -1573,7 +1578,7 @@ static int _init_clocks(struct omap_hwmod *oh, void *data)
 	if (soc_ops.init_clkdm)
 		ret |= soc_ops.init_clkdm(oh);
 
-	ret |= _init_main_clk(oh);
+	ret |= _init_main_clk(oh, np);
 	ret |= _init_interface_clks(oh);
 	ret |= _init_opt_clks(oh);
 
@@ -2361,11 +2366,11 @@ static struct device_node *of_dev_hwmod_lookup(struct device_node *np,
  * are part of the device's address space can be ioremapped properly.
  * No return value.
  */
-static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
+static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data,
+				     struct device_node *np)
 {
 	struct omap_hwmod_addr_space *mem;
 	void __iomem *va_start = NULL;
-	struct device_node *np;
 
 	if (!oh)
 		return;
@@ -2381,12 +2386,10 @@ static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 			 oh->name);
 
 		/* Extract the IO space from device tree blob */
-		if (!of_have_populated_dt())
+		if (!np)
 			return;
 
-		np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
-		if (np)
-			va_start = of_iomap(np, 0);
+		va_start = of_iomap(np, 0);
 	} else {
 		va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
 	}
@@ -2418,14 +2421,19 @@ static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
 static int __init _init(struct omap_hwmod *oh, void *data)
 {
 	int r;
+	struct device_node *np = NULL;
 
 	if (oh->_state != _HWMOD_STATE_REGISTERED)
 		return 0;
 
+	/* If booting with DT, parse the DT node for IO space/clocks etc */
+	if (of_have_populated_dt())
+		np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
+
 	if (oh->class->sysc)
-		_init_mpu_rt_base(oh, NULL);
+		_init_mpu_rt_base(oh, NULL, np);
 
-	r = _init_clocks(oh, NULL);
+	r = _init_clocks(oh, NULL, np);
 	if (r < 0) {
 		WARN(1, "omap_hwmod: %s: couldn't init clocks\n", oh->name);
 		return -EINVAL;
-- 
1.7.9.5

[PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

With clocks for OMAP moving to DT, its now possible to pass all optional clock
data for each device from DT instead of having it in hwmod.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   66 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 12fa589..e5c804b 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
 	return ret;
 }
 
+static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
+				       struct device_node *np,
+				       int *opt_clks_cnt)
+{
+	int i, clks_cnt;
+	const char *clk_name;
+	const char **opt_clk_names;
+
+	clks_cnt = of_property_count_strings(np, "clock-names");
+	if (!clks_cnt)
+		return NULL;
+
+	opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
+	if (!opt_clk_names)
+		return NULL;
+
+	for (i = 0; i < clks_cnt; i++) {
+		of_property_read_string_index(np, "clock-names", i, &clk_name);
+		if (!strcmp(clk_name, "fck"))
+			continue;
+		opt_clks_cnt++;
+		opt_clk_names[i] = clk_name;
+	}
+	return opt_clk_names;
+}
+
+static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np)
+{
+	struct clk *c;
+	int i, opt_clks_cnt = 0;
+	int ret = 0;
+	const char **opt_clk_names;
+
+	opt_clk_names = _parse_opt_clks_dt(oh, np, &opt_clks_cnt);
+	if (!opt_clk_names)
+		return -EINVAL;
+
+	oh->opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *)
+			       * opt_clks_cnt, GFP_KERNEL);
+	if (!oh->opt_clks)
+		return -ENOMEM;
+
+	oh->opt_clks_cnt = opt_clks_cnt;
+
+	for (i = 0; i < oh->opt_clks_cnt; i++) {
+		c = of_clk_get_by_name(np, opt_clk_names[i]);
+		if (IS_ERR(c)) {
+			pr_warn("omap_hwmod: %s: cannot clk_get opt_clk %s\n",
+				oh->name, opt_clk_names[i]);
+			ret = -EINVAL;
+		}
+		oh->opt_clks[i]._clk = c;
+		oh->opt_clks[i].role = opt_clk_names[i];
+		oh->opt_clks_cnt++;
+		clk_prepare(oh->opt_clks[i]._clk);
+	}
+	return ret;
+}
+
 /**
  * _init_opt_clk - get a struct clk * for the the hwmod's optional clocks
  * @oh: struct omap_hwmod *
@@ -812,13 +871,16 @@ static int _init_interface_clks(struct omap_hwmod *oh)
  * Called from _init_clocks().  Populates the @oh omap_hwmod_opt_clk
  * clock pointers.  Returns 0 on success or -EINVAL on error.
  */
-static int _init_opt_clks(struct omap_hwmod *oh)
+static int _init_opt_clks(struct omap_hwmod *oh, struct device_node *np)
 {
 	struct omap_hwmod_opt_clk *oc;
 	struct clk *c;
 	int i;
 	int ret = 0;
 
+	if (of_get_property(np, "clocks", NULL))
+		return _init_opt_clks_dt(oh, np);
+
 	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++) {
 		c = clk_get(NULL, oc->clk);
 		if (IS_ERR(c)) {
@@ -1580,7 +1642,7 @@ static int _init_clocks(struct omap_hwmod *oh, void *data,
 
 	ret |= _init_main_clk(oh, np);
 	ret |= _init_interface_clks(oh);
-	ret |= _init_opt_clks(oh);
+	ret |= _init_opt_clks(oh, np);
 
 	if (!ret)
 		oh->_state = _HWMOD_STATE_CLKS_INITED;
-- 
1.7.9.5

[PATCH 3/3] ARM: OMAP4: dts: Add main and optional clock data into DT

[Rajendra Nayak]

With support to parse clock data from DT, move all main and optional
clock information from hwmod to DT.

We still retain clocks in hwmod for devices which do not have a DT node.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/boot/dts/omap4.dtsi               |  100 +++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  112 ----------------------------
 2 files changed, 100 insertions(+), 112 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 8e142f9..4dddf64 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -106,6 +106,8 @@
 			compatible = "ti,omap-counter32k";
 			reg = <0x4a304000 0x20>;
 			ti,hwmods = "counter_32k";
+			clocks = <&sys_32k_ck>;
+			clock-names = "fck";
 		};
 
 		omap4_pmx_core: pinmux@4a100040 {
@@ -135,6 +137,8 @@
 			#dma-cells = <1>;
 			#dma-channels = <32>;
 			#dma-requests = <127>;
+			clocks = <&l3_div_ck>;
+			clock-names = "fck";
 		};
 
 		gpio1: gpio@4a310000 {
@@ -147,6 +151,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_wkup_clk_mux_ck>, <&gpio1_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpio2: gpio@48055000 {
@@ -158,6 +164,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_div_ck>, <&gpio2_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpio3: gpio@48057000 {
@@ -169,6 +177,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_div_ck>, <&gpio3_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpio4: gpio@48059000 {
@@ -180,6 +190,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_div_ck>, <&gpio4_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpio5: gpio@4805b000 {
@@ -191,6 +203,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_div_ck>, <&gpio5_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpio6: gpio@4805d000 {
@@ -202,6 +216,8 @@
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			clocks = <&l4_div_ck>, <&gpio6_dbclk>;
+			clock-names = "fck", "dbclk";
 		};
 
 		gpmc: gpmc@50000000 {
@@ -221,6 +237,8 @@
 			interrupts = <0 72 0x4>;
 			ti,hwmods = "uart1";
 			clock-frequency = <48000000>;
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		uart2: serial@4806c000 {
@@ -229,6 +247,8 @@
 			interrupts = <0 73 0x4>;
 			ti,hwmods = "uart2";
 			clock-frequency = <48000000>;
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		uart3: serial@48020000 {
@@ -237,6 +257,8 @@
 			interrupts = <0 74 0x4>;
 			ti,hwmods = "uart3";
 			clock-frequency = <48000000>;
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		uart4: serial@4806e000 {
@@ -245,6 +267,8 @@
 			interrupts = <0 70 0x4>;
 			ti,hwmods = "uart4";
 			clock-frequency = <48000000>;
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		i2c1: i2c@48070000 {
@@ -254,6 +278,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "i2c1";
+			clocks = <&func_96m_fclk>;
+			clock-names = "fck";
 		};
 
 		i2c2: i2c@48072000 {
@@ -263,6 +289,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "i2c2";
+			clocks = <&func_96m_fclk>;
+			clock-names = "fck";
 		};
 
 		i2c3: i2c@48060000 {
@@ -272,6 +300,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "i2c3";
+			clocks = <&func_96m_fclk>;
+			clock-names = "fck";
 		};
 
 		i2c4: i2c@48350000 {
@@ -281,6 +311,8 @@
 			#address-cells = <1>;
 			#size-cells = <0>;
 			ti,hwmods = "i2c4";
+			clocks = <&func_96m_fclk>;
+			clock-names = "fck";
 		};
 
 		mcspi1: spi@48098000 {
@@ -301,6 +333,8 @@
 			       <&sdma 42>;
 			dma-names = "tx0", "rx0", "tx1", "rx1",
 				    "tx2", "rx2", "tx3", "rx3";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mcspi2: spi@4809a000 {
@@ -316,6 +350,8 @@
 			       <&sdma 45>,
 			       <&sdma 46>;
 			dma-names = "tx0", "rx0", "tx1", "rx1";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mcspi3: spi@480b8000 {
@@ -328,6 +364,8 @@
 			ti,spi-num-cs = <2>;
 			dmas = <&sdma 15>, <&sdma 16>;
 			dma-names = "tx0", "rx0";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mcspi4: spi@480ba000 {
@@ -340,6 +378,8 @@
 			ti,spi-num-cs = <1>;
 			dmas = <&sdma 70>, <&sdma 71>;
 			dma-names = "tx0", "rx0";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mmc1: mmc@4809c000 {
@@ -351,6 +391,8 @@
 			ti,needs-special-reset;
 			dmas = <&sdma 61>, <&sdma 62>;
 			dma-names = "tx", "rx";
+			clocks = <&hsmmc1_fclk>;
+			clock-names = "fck";
 		};
 
 		mmc2: mmc@480b4000 {
@@ -361,6 +403,8 @@
 			ti,needs-special-reset;
 			dmas = <&sdma 47>, <&sdma 48>;
 			dma-names = "tx", "rx";
+			clocks = <&hsmmc2_fclk>;
+			clock-names = "fck";
 		};
 
 		mmc3: mmc@480ad000 {
@@ -371,6 +415,8 @@
 			ti,needs-special-reset;
 			dmas = <&sdma 77>, <&sdma 78>;
 			dma-names = "tx", "rx";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mmc4: mmc@480d1000 {
@@ -381,6 +427,8 @@
 			ti,needs-special-reset;
 			dmas = <&sdma 57>, <&sdma 58>;
 			dma-names = "tx", "rx";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		mmc5: mmc@480d5000 {
@@ -391,6 +439,8 @@
 			ti,needs-special-reset;
 			dmas = <&sdma 59>, <&sdma 60>;
 			dma-names = "tx", "rx";
+			clocks = <&func_48m_fclk>;
+			clock-names = "fck";
 		};
 
 		wdt2: wdt@4a314000 {
@@ -398,6 +448,8 @@
 			reg = <0x4a314000 0x80>;
 			interrupts = <0 80 0x4>;
 			ti,hwmods = "wd_timer2";
+			clocks = <&sys_32k_ck>;
+			clock-names = "fck";
 		};
 
 		mcpdm: mcpdm@40132000 {
@@ -410,6 +462,8 @@
 			dmas = <&sdma 65>,
 			       <&sdma 66>;
 			dma-names = "up_link", "dn_link";
+			clocks = <&pad_clks_ck>;
+			clock-names = "fck";
 		};
 
 		dmic: dmic@4012e000 {
@@ -421,6 +475,8 @@
 			ti,hwmods = "dmic";
 			dmas = <&sdma 67>;
 			dma-names = "up_link";
+			clocks = <&func_dmic_abe_gfclk>;
+			clock-names = "fck";
 		};
 
 		mcbsp1: mcbsp@40122000 {
@@ -435,6 +491,8 @@
 			dmas = <&sdma 33>,
 			       <&sdma 34>;
 			dma-names = "tx", "rx";
+			clocks = <&func_mcbsp1_gfclk>, <&pad_clks_ck>, <&mcbsp1_sync_mux_ck>;
+			clock-names = "fck", "pad_fck", "prcm_fck";
 		};
 
 		mcbsp2: mcbsp@40124000 {
@@ -449,6 +507,8 @@
 			dmas = <&sdma 17>,
 			       <&sdma 18>;
 			dma-names = "tx", "rx";
+			clocks = <&func_mcbsp2_gfclk>, <&pad_clks_ck>, <&mcbsp2_sync_mux_ck>;
+			clock-names = "fck", "pad_fck", "prcm_fck";
 		};
 
 		mcbsp3: mcbsp@40126000 {
@@ -463,6 +523,8 @@
 			dmas = <&sdma 19>,
 			       <&sdma 20>;
 			dma-names = "tx", "rx";
+			clocks = <&func_mcbsp3_gfclk>, <&pad_clks_ck>, <&mcbsp3_sync_mux_ck>;
+			clock-names = "fck", "pad_fck", "prcm_fck";
 		};
 
 		mcbsp4: mcbsp@48096000 {
@@ -476,6 +538,8 @@
 			dmas = <&sdma 31>,
 			       <&sdma 32>;
 			dma-names = "tx", "rx";
+			clocks = <&per_mcbsp4_gfclk>, <&pad_clks_ck>, <&mcbsp4_sync_mux_ck>;
+			clock-names = "fck", "pad_fck", "prcm_fck";
 		};
 
 		keypad: keypad@4a31c000 {
@@ -484,6 +548,8 @@
 			interrupts = <0 120 0x4>;
 			reg-names = "mpu";
 			ti,hwmods = "kbd";
+			clocks = <&sys_32k_ck>;
+			clock-names = "fck";
 		};
 
 		emif1: emif@4c000000 {
@@ -495,6 +561,8 @@
 			hw-caps-read-idle-ctrl;
 			hw-caps-ll-interface;
 			hw-caps-temp-alert;
+			clocks = <&ddrphy_ck>;
+			clock-names = "fck";
 		};
 
 		emif2: emif@4d000000 {
@@ -506,6 +574,8 @@
 			hw-caps-read-idle-ctrl;
 			hw-caps-ll-interface;
 			hw-caps-temp-alert;
+			clocks = <&ddrphy_ck>;
+			clock-names = "fck";
 		};
 
 		ocp2scp@4a0ad000 {
@@ -515,6 +585,8 @@
 			#size-cells = <1>;
 			ranges;
 			ti,hwmods = "ocp2scp_usb_phy";
+			clocks = <&ocp2scp_usb_phy_phy_48m>;
+			clock-names = "fck";
 			usb2_phy: usb2phy@4a0ad080 {
 				compatible = "ti,omap-usb2";
 				reg = <0x4a0ad080 0x58>;
@@ -528,6 +600,8 @@
 			interrupts = <0 37 0x4>;
 			ti,hwmods = "timer1";
 			ti,timer-alwon;
+			clocks = <&dmt1_clk_mux>;
+			clock-names = "fck";
 		};
 
 		timer2: timer@48032000 {
@@ -535,6 +609,8 @@
 			reg = <0x48032000 0x80>;
 			interrupts = <0 38 0x4>;
 			ti,hwmods = "timer2";
+			clocks = <&cm2_dm2_mux>;
+			clock-names = "fck";
 		};
 
 		timer3: timer@48034000 {
@@ -542,6 +618,8 @@
 			reg = <0x48034000 0x80>;
 			interrupts = <0 39 0x4>;
 			ti,hwmods = "timer3";
+			clocks = <&cm2_dm3_mux>;
+			clock-names = "fck";
 		};
 
 		timer4: timer@48036000 {
@@ -549,6 +627,8 @@
 			reg = <0x48036000 0x80>;
 			interrupts = <0 40 0x4>;
 			ti,hwmods = "timer4";
+			clocks = <&cm2_dm4_mux>;
+			clock-names = "fck";
 		};
 
 		timer5: timer@40138000 {
@@ -558,6 +638,8 @@
 			interrupts = <0 41 0x4>;
 			ti,hwmods = "timer5";
 			ti,timer-dsp;
+			clocks = <&timer5_sync_mux>;
+			clock-names = "fck";
 		};
 
 		timer6: timer@4013a000 {
@@ -567,6 +649,8 @@
 			interrupts = <0 42 0x4>;
 			ti,hwmods = "timer6";
 			ti,timer-dsp;
+			clocks = <&timer6_sync_mux>;
+			clock-names = "fck";
 		};
 
 		timer7: timer@4013c000 {
@@ -576,6 +660,8 @@
 			interrupts = <0 43 0x4>;
 			ti,hwmods = "timer7";
 			ti,timer-dsp;
+			clocks = <&timer7_sync_mux>;
+			clock-names = "fck";
 		};
 
 		timer8: timer@4013e000 {
@@ -586,6 +672,8 @@
 			ti,hwmods = "timer8";
 			ti,timer-pwm;
 			ti,timer-dsp;
+			clocks = <&timer8_sync_mux>;
+			clock-names = "fck";
 		};
 
 		timer9: timer@4803e000 {
@@ -594,6 +682,8 @@
 			interrupts = <0 45 0x4>;
 			ti,hwmods = "timer9";
 			ti,timer-pwm;
+			clocks = <&cm2_dm9_mux>;
+			clock-names = "fck";
 		};
 
 		timer10: timer@48086000 {
@@ -602,6 +692,8 @@
 			interrupts = <0 46 0x4>;
 			ti,hwmods = "timer10";
 			ti,timer-pwm;
+			clocks = <&cm2_dm10_mux>;
+			clock-names = "fck";
 		};
 
 		timer11: timer@48088000 {
@@ -610,6 +702,8 @@
 			interrupts = <0 47 0x4>;
 			ti,hwmods = "timer11";
 			ti,timer-pwm;
+			clocks = <&cm2_dm11_mux>;
+			clock-names = "fck";
 		};
 
 		usbhstll: usbhstll@4a062000 {
@@ -617,6 +711,8 @@
 			reg = <0x4a062000 0x1000>;
 			interrupts = <0 78 0x4>;
 			ti,hwmods = "usb_tll_hs";
+			clocks = <&usb_tll_hs_ick>;
+			clock-names = "fck";
 		};
 
 		usbhshost: usbhshost@4a064000 {
@@ -626,6 +722,8 @@
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
+			clocks = <&usb_host_hs_fck>;
+			clock-names = "fck";
 
 			usbhsohci: ohci@4a064800 {
 				compatible = "ti,ohci-omap3", "usb-ohci";
@@ -661,6 +759,8 @@
 			num-eps = <16>;
 			ram-bits = <12>;
 			ti,has-mailbox;
+			clocks = <&usb_otg_hs_ick>;
+			clock-names = "fck";
 		};
 	};
 };
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 848b6dc..5ea01ea 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -419,7 +419,6 @@ static struct omap_hwmod omap44xx_counter_32k_hwmod = {
 	.class		= &omap44xx_counter_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
 	.flags		= HWMOD_SWSUP_SIDLE,
-	.main_clk	= "sys_32k_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_WKUP_SYNCTIMER_CLKCTRL_OFFSET,
@@ -570,7 +569,6 @@ static struct omap_hwmod omap44xx_dma_system_hwmod = {
 	.class		= &omap44xx_dma_hwmod_class,
 	.clkdm_name	= "l3_dma_clkdm",
 	.mpu_irqs	= omap44xx_dma_system_irqs,
-	.main_clk	= "l3_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_SDMA_SDMA_CLKCTRL_OFFSET,
@@ -617,7 +615,6 @@ static struct omap_hwmod omap44xx_dmic_hwmod = {
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_dmic_irqs,
 	.sdma_reqs	= omap44xx_dmic_sdma_reqs,
-	.main_clk	= "func_dmic_abe_gfclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_DMIC_CLKCTRL_OFFSET,
@@ -1036,7 +1033,6 @@ static struct omap_hwmod omap44xx_emif1_hwmod = {
 	.clkdm_name	= "l3_emif_clkdm",
 	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
 	.mpu_irqs	= omap44xx_emif1_irqs,
-	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_MEMIF_EMIF_1_CLKCTRL_OFFSET,
@@ -1058,7 +1054,6 @@ static struct omap_hwmod omap44xx_emif2_hwmod = {
 	.clkdm_name	= "l3_emif_clkdm",
 	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
 	.mpu_irqs	= omap44xx_emif2_irqs,
-	.main_clk	= "ddrphy_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_MEMIF_EMIF_2_CLKCTRL_OFFSET,
@@ -1153,16 +1148,11 @@ static struct omap_hwmod_irq_info omap44xx_gpio1_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio1_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio1_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio1_hwmod = {
 	.name		= "gpio1",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
 	.mpu_irqs	= omap44xx_gpio1_irqs,
-	.main_clk	= "l4_wkup_clk_mux_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_WKUP_GPIO1_CLKCTRL_OFFSET,
@@ -1170,8 +1160,6 @@ static struct omap_hwmod omap44xx_gpio1_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio1_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio1_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1181,17 +1169,12 @@ static struct omap_hwmod_irq_info omap44xx_gpio2_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio2_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio2_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio2_hwmod = {
 	.name		= "gpio2",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.mpu_irqs	= omap44xx_gpio2_irqs,
-	.main_clk	= "l4_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_GPIO2_CLKCTRL_OFFSET,
@@ -1199,8 +1182,6 @@ static struct omap_hwmod omap44xx_gpio2_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio2_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio2_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1210,17 +1191,12 @@ static struct omap_hwmod_irq_info omap44xx_gpio3_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio3_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio3_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio3_hwmod = {
 	.name		= "gpio3",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.mpu_irqs	= omap44xx_gpio3_irqs,
-	.main_clk	= "l4_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_GPIO3_CLKCTRL_OFFSET,
@@ -1228,8 +1204,6 @@ static struct omap_hwmod omap44xx_gpio3_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio3_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio3_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1239,17 +1213,12 @@ static struct omap_hwmod_irq_info omap44xx_gpio4_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio4_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio4_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio4_hwmod = {
 	.name		= "gpio4",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.mpu_irqs	= omap44xx_gpio4_irqs,
-	.main_clk	= "l4_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_GPIO4_CLKCTRL_OFFSET,
@@ -1257,8 +1226,6 @@ static struct omap_hwmod omap44xx_gpio4_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio4_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio4_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1268,17 +1235,12 @@ static struct omap_hwmod_irq_info omap44xx_gpio5_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio5_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio5_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio5_hwmod = {
 	.name		= "gpio5",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.mpu_irqs	= omap44xx_gpio5_irqs,
-	.main_clk	= "l4_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_GPIO5_CLKCTRL_OFFSET,
@@ -1286,8 +1248,6 @@ static struct omap_hwmod omap44xx_gpio5_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio5_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio5_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1297,17 +1257,12 @@ static struct omap_hwmod_irq_info omap44xx_gpio6_irqs[] = {
 	{ .irq = -1 }
 };
 
-static struct omap_hwmod_opt_clk gpio6_opt_clks[] = {
-	{ .role = "dbclk", .clk = "gpio6_dbclk" },
-};
-
 static struct omap_hwmod omap44xx_gpio6_hwmod = {
 	.name		= "gpio6",
 	.class		= &omap44xx_gpio_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.mpu_irqs	= omap44xx_gpio6_irqs,
-	.main_clk	= "l4_div_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_GPIO6_CLKCTRL_OFFSET,
@@ -1315,8 +1270,6 @@ static struct omap_hwmod omap44xx_gpio6_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.opt_clks	= gpio6_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(gpio6_opt_clks),
 	.dev_attr	= &gpio_dev_attr,
 };
 
@@ -1551,7 +1504,6 @@ static struct omap_hwmod omap44xx_i2c1_hwmod = {
 	.flags		= HWMOD_16BIT_REG | HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_i2c1_irqs,
 	.sdma_reqs	= omap44xx_i2c1_sdma_reqs,
-	.main_clk	= "func_96m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_I2C1_CLKCTRL_OFFSET,
@@ -1581,7 +1533,6 @@ static struct omap_hwmod omap44xx_i2c2_hwmod = {
 	.flags		= HWMOD_16BIT_REG | HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_i2c2_irqs,
 	.sdma_reqs	= omap44xx_i2c2_sdma_reqs,
-	.main_clk	= "func_96m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_I2C2_CLKCTRL_OFFSET,
@@ -1611,7 +1562,6 @@ static struct omap_hwmod omap44xx_i2c3_hwmod = {
 	.flags		= HWMOD_16BIT_REG | HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_i2c3_irqs,
 	.sdma_reqs	= omap44xx_i2c3_sdma_reqs,
-	.main_clk	= "func_96m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_I2C3_CLKCTRL_OFFSET,
@@ -1641,7 +1591,6 @@ static struct omap_hwmod omap44xx_i2c4_hwmod = {
 	.flags		= HWMOD_16BIT_REG | HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_i2c4_irqs,
 	.sdma_reqs	= omap44xx_i2c4_sdma_reqs,
-	.main_clk	= "func_96m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_I2C4_CLKCTRL_OFFSET,
@@ -1830,7 +1779,6 @@ static struct omap_hwmod omap44xx_kbd_hwmod = {
 	.class		= &omap44xx_kbd_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
 	.mpu_irqs	= omap44xx_kbd_irqs,
-	.main_clk	= "sys_32k_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_WKUP_KEYBOARD_CLKCTRL_OFFSET,
@@ -1962,18 +1910,12 @@ static struct omap_hwmod_dma_info omap44xx_mcbsp1_sdma_reqs[] = {
 	{ .dma_req = -1 }
 };
 
-static struct omap_hwmod_opt_clk mcbsp1_opt_clks[] = {
-	{ .role = "pad_fck", .clk = "pad_clks_ck" },
-	{ .role = "prcm_fck", .clk = "mcbsp1_sync_mux_ck" },
-};
-
 static struct omap_hwmod omap44xx_mcbsp1_hwmod = {
 	.name		= "mcbsp1",
 	.class		= &omap44xx_mcbsp_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_mcbsp1_irqs,
 	.sdma_reqs	= omap44xx_mcbsp1_sdma_reqs,
-	.main_clk	= "func_mcbsp1_gfclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_MCBSP1_CLKCTRL_OFFSET,
@@ -1981,8 +1923,6 @@ static struct omap_hwmod omap44xx_mcbsp1_hwmod = {
 			.modulemode   = MODULEMODE_SWCTRL,
 		},
 	},
-	.opt_clks	= mcbsp1_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(mcbsp1_opt_clks),
 };
 
 /* mcbsp2 */
@@ -1997,18 +1937,12 @@ static struct omap_hwmod_dma_info omap44xx_mcbsp2_sdma_reqs[] = {
 	{ .dma_req = -1 }
 };
 
-static struct omap_hwmod_opt_clk mcbsp2_opt_clks[] = {
-	{ .role = "pad_fck", .clk = "pad_clks_ck" },
-	{ .role = "prcm_fck", .clk = "mcbsp2_sync_mux_ck" },
-};
-
 static struct omap_hwmod omap44xx_mcbsp2_hwmod = {
 	.name		= "mcbsp2",
 	.class		= &omap44xx_mcbsp_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_mcbsp2_irqs,
 	.sdma_reqs	= omap44xx_mcbsp2_sdma_reqs,
-	.main_clk	= "func_mcbsp2_gfclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_MCBSP2_CLKCTRL_OFFSET,
@@ -2016,8 +1950,6 @@ static struct omap_hwmod omap44xx_mcbsp2_hwmod = {
 			.modulemode   = MODULEMODE_SWCTRL,
 		},
 	},
-	.opt_clks	= mcbsp2_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(mcbsp2_opt_clks),
 };
 
 /* mcbsp3 */
@@ -2032,18 +1964,12 @@ static struct omap_hwmod_dma_info omap44xx_mcbsp3_sdma_reqs[] = {
 	{ .dma_req = -1 }
 };
 
-static struct omap_hwmod_opt_clk mcbsp3_opt_clks[] = {
-	{ .role = "pad_fck", .clk = "pad_clks_ck" },
-	{ .role = "prcm_fck", .clk = "mcbsp3_sync_mux_ck" },
-};
-
 static struct omap_hwmod omap44xx_mcbsp3_hwmod = {
 	.name		= "mcbsp3",
 	.class		= &omap44xx_mcbsp_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_mcbsp3_irqs,
 	.sdma_reqs	= omap44xx_mcbsp3_sdma_reqs,
-	.main_clk	= "func_mcbsp3_gfclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_MCBSP3_CLKCTRL_OFFSET,
@@ -2051,8 +1977,6 @@ static struct omap_hwmod omap44xx_mcbsp3_hwmod = {
 			.modulemode   = MODULEMODE_SWCTRL,
 		},
 	},
-	.opt_clks	= mcbsp3_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(mcbsp3_opt_clks),
 };
 
 /* mcbsp4 */
@@ -2067,11 +1991,6 @@ static struct omap_hwmod_dma_info omap44xx_mcbsp4_sdma_reqs[] = {
 	{ .dma_req = -1 }
 };
 
-static struct omap_hwmod_opt_clk mcbsp4_opt_clks[] = {
-	{ .role = "pad_fck", .clk = "pad_clks_ck" },
-	{ .role = "prcm_fck", .clk = "mcbsp4_sync_mux_ck" },
-};
-
 static struct omap_hwmod omap44xx_mcbsp4_hwmod = {
 	.name		= "mcbsp4",
 	.class		= &omap44xx_mcbsp_hwmod_class,
@@ -2086,8 +2005,6 @@ static struct omap_hwmod omap44xx_mcbsp4_hwmod = {
 			.modulemode   = MODULEMODE_SWCTRL,
 		},
 	},
-	.opt_clks	= mcbsp4_opt_clks,
-	.opt_clks_cnt	= ARRAY_SIZE(mcbsp4_opt_clks),
 };
 
 /*
@@ -2141,7 +2058,6 @@ static struct omap_hwmod omap44xx_mcpdm_hwmod = {
 	.flags		= HWMOD_EXT_OPT_MAIN_CLK | HWMOD_SWSUP_SIDLE,
 	.mpu_irqs	= omap44xx_mcpdm_irqs,
 	.sdma_reqs	= omap44xx_mcpdm_sdma_reqs,
-	.main_clk	= "pad_clks_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_PDM_CLKCTRL_OFFSET,
@@ -2202,7 +2118,6 @@ static struct omap_hwmod omap44xx_mcspi1_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mcspi1_irqs,
 	.sdma_reqs	= omap44xx_mcspi1_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MCSPI1_CLKCTRL_OFFSET,
@@ -2238,7 +2153,6 @@ static struct omap_hwmod omap44xx_mcspi2_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mcspi2_irqs,
 	.sdma_reqs	= omap44xx_mcspi2_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MCSPI2_CLKCTRL_OFFSET,
@@ -2274,7 +2188,6 @@ static struct omap_hwmod omap44xx_mcspi3_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mcspi3_irqs,
 	.sdma_reqs	= omap44xx_mcspi3_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MCSPI3_CLKCTRL_OFFSET,
@@ -2308,7 +2221,6 @@ static struct omap_hwmod omap44xx_mcspi4_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mcspi4_irqs,
 	.sdma_reqs	= omap44xx_mcspi4_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MCSPI4_CLKCTRL_OFFSET,
@@ -2364,7 +2276,6 @@ static struct omap_hwmod omap44xx_mmc1_hwmod = {
 	.clkdm_name	= "l3_init_clkdm",
 	.mpu_irqs	= omap44xx_mmc1_irqs,
 	.sdma_reqs	= omap44xx_mmc1_sdma_reqs,
-	.main_clk	= "hsmmc1_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_MMC1_CLKCTRL_OFFSET,
@@ -2393,7 +2304,6 @@ static struct omap_hwmod omap44xx_mmc2_hwmod = {
 	.clkdm_name	= "l3_init_clkdm",
 	.mpu_irqs	= omap44xx_mmc2_irqs,
 	.sdma_reqs	= omap44xx_mmc2_sdma_reqs,
-	.main_clk	= "hsmmc2_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_MMC2_CLKCTRL_OFFSET,
@@ -2421,7 +2331,6 @@ static struct omap_hwmod omap44xx_mmc3_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mmc3_irqs,
 	.sdma_reqs	= omap44xx_mmc3_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MMCSD3_CLKCTRL_OFFSET,
@@ -2449,7 +2358,6 @@ static struct omap_hwmod omap44xx_mmc4_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mmc4_irqs,
 	.sdma_reqs	= omap44xx_mmc4_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MMCSD4_CLKCTRL_OFFSET,
@@ -2477,7 +2385,6 @@ static struct omap_hwmod omap44xx_mmc5_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_mmc5_irqs,
 	.sdma_reqs	= omap44xx_mmc5_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_MMCSD5_CLKCTRL_OFFSET,
@@ -2729,7 +2636,6 @@ static struct omap_hwmod omap44xx_ocp2scp_usb_phy_hwmod = {
 	 * So listing ocp2scp_usb_phy_phy_48m as a main_clk here seems
 	 * to be the best workaround.
 	 */
-	.main_clk	= "ocp2scp_usb_phy_phy_48m",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_USBPHYOCP2SCP_CLKCTRL_OFFSET,
@@ -3189,7 +3095,6 @@ static struct omap_hwmod omap44xx_timer2_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_timer2_irqs,
-	.main_clk	= "cm2_dm2_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER2_CLKCTRL_OFFSET,
@@ -3210,7 +3115,6 @@ static struct omap_hwmod omap44xx_timer3_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_timer3_irqs,
-	.main_clk	= "cm2_dm3_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER3_CLKCTRL_OFFSET,
@@ -3231,7 +3135,6 @@ static struct omap_hwmod omap44xx_timer4_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_timer4_irqs,
-	.main_clk	= "cm2_dm4_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER4_CLKCTRL_OFFSET,
@@ -3252,7 +3155,6 @@ static struct omap_hwmod omap44xx_timer5_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_timer5_irqs,
-	.main_clk	= "timer5_sync_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_TIMER5_CLKCTRL_OFFSET,
@@ -3274,7 +3176,6 @@ static struct omap_hwmod omap44xx_timer6_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_timer6_irqs,
-	.main_clk	= "timer6_sync_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_TIMER6_CLKCTRL_OFFSET,
@@ -3296,7 +3197,6 @@ static struct omap_hwmod omap44xx_timer7_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_timer7_irqs,
-	.main_clk	= "timer7_sync_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_TIMER7_CLKCTRL_OFFSET,
@@ -3318,7 +3218,6 @@ static struct omap_hwmod omap44xx_timer8_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
 	.mpu_irqs	= omap44xx_timer8_irqs,
-	.main_clk	= "timer8_sync_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_TIMER8_CLKCTRL_OFFSET,
@@ -3340,7 +3239,6 @@ static struct omap_hwmod omap44xx_timer9_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_timer9_irqs,
-	.main_clk	= "cm2_dm9_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER9_CLKCTRL_OFFSET,
@@ -3363,7 +3261,6 @@ static struct omap_hwmod omap44xx_timer10_hwmod = {
 	.clkdm_name	= "l4_per_clkdm",
 	.flags		= HWMOD_SET_DEFAULT_CLOCKACT,
 	.mpu_irqs	= omap44xx_timer10_irqs,
-	.main_clk	= "cm2_dm10_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER10_CLKCTRL_OFFSET,
@@ -3385,7 +3282,6 @@ static struct omap_hwmod omap44xx_timer11_hwmod = {
 	.class		= &omap44xx_timer_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
 	.mpu_irqs	= omap44xx_timer11_irqs,
-	.main_clk	= "cm2_dm11_mux",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_DMTIMER11_CLKCTRL_OFFSET,
@@ -3437,7 +3333,6 @@ static struct omap_hwmod omap44xx_uart1_hwmod = {
 	.flags		= HWMOD_SWSUP_SIDLE_ACT,
 	.mpu_irqs	= omap44xx_uart1_irqs,
 	.sdma_reqs	= omap44xx_uart1_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_UART1_CLKCTRL_OFFSET,
@@ -3466,7 +3361,6 @@ static struct omap_hwmod omap44xx_uart2_hwmod = {
 	.flags		= HWMOD_SWSUP_SIDLE_ACT,
 	.mpu_irqs	= omap44xx_uart2_irqs,
 	.sdma_reqs	= omap44xx_uart2_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_UART2_CLKCTRL_OFFSET,
@@ -3496,7 +3390,6 @@ static struct omap_hwmod omap44xx_uart3_hwmod = {
 				HWMOD_SWSUP_SIDLE_ACT,
 	.mpu_irqs	= omap44xx_uart3_irqs,
 	.sdma_reqs	= omap44xx_uart3_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
@@ -3525,7 +3418,6 @@ static struct omap_hwmod omap44xx_uart4_hwmod = {
 	.flags		= HWMOD_SWSUP_SIDLE_ACT,
 	.mpu_irqs	= omap44xx_uart4_irqs,
 	.sdma_reqs	= omap44xx_uart4_sdma_reqs,
-	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L4PER_UART4_CLKCTRL_OFFSET,
@@ -3617,7 +3509,6 @@ static struct omap_hwmod omap44xx_usb_host_hs_hwmod = {
 	.name		= "usb_host_hs",
 	.class		= &omap44xx_usb_host_hs_hwmod_class,
 	.clkdm_name	= "l3_init_clkdm",
-	.main_clk	= "usb_host_hs_fck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_USB_HOST_CLKCTRL_OFFSET,
@@ -3716,7 +3607,6 @@ static struct omap_hwmod omap44xx_usb_otg_hs_hwmod = {
 	.clkdm_name	= "l3_init_clkdm",
 	.flags		= HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
 	.mpu_irqs	= omap44xx_usb_otg_hs_irqs,
-	.main_clk	= "usb_otg_hs_ick",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_USB_OTG_CLKCTRL_OFFSET,
@@ -3759,7 +3649,6 @@ static struct omap_hwmod omap44xx_usb_tll_hs_hwmod = {
 	.class		= &omap44xx_usb_tll_hs_hwmod_class,
 	.clkdm_name	= "l3_init_clkdm",
 	.mpu_irqs	= omap44xx_usb_tll_hs_irqs,
-	.main_clk	= "usb_tll_hs_ick",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_L3INIT_USB_TLL_CLKCTRL_OFFSET,
@@ -3804,7 +3693,6 @@ static struct omap_hwmod omap44xx_wd_timer2_hwmod = {
 	.class		= &omap44xx_wd_timer_hwmod_class,
 	.clkdm_name	= "l4_wkup_clkdm",
 	.mpu_irqs	= omap44xx_wd_timer2_irqs,
-	.main_clk	= "sys_32k_ck",
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_WKUP_WDT2_CLKCTRL_OFFSET,
-- 
1.7.9.5

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Nishanth Menon]

Hi Rajendra,

On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
[..]
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 12fa589..e5c804b 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>         return ret;
>  }
>
> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> +                                      struct device_node *np,
> +                                      int *opt_clks_cnt)
> +{
> +       int i, clks_cnt;
> +       const char *clk_name;
> +       const char **opt_clk_names;
> +
> +       clks_cnt = of_property_count_strings(np, "clock-names");
> +       if (!clks_cnt)
> +               return NULL;
> +
> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> +       if (!opt_clk_names)
> +               return NULL;
> +
> +       for (i = 0; i < clks_cnt; i++) {
> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
> +               if (!strcmp(clk_name, "fck"))

Could we instead parse for names that are "optional,role_name" instead
of assuming anything other than fck is optional clocks?

> +                       continue;
> +               opt_clks_cnt++;
> +               opt_clk_names[i] = clk_name;
> +       }
> +       return opt_clk_names;
> +}
> +
[...]
--
Regards,
Nishanth Menon

Re: [PATCH 1/3] ARM: OMAP2+: Add support to parse 'main_clk' info from DT

[menon.nishanth]

Hi Rajendra,
On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:

[...]
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5cc5123..12fa589 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -2381,12 +2386,10 @@ static void __init _init_mpu_rt_base(struct omap_hwmod *oh, void *data)
>                          oh->name);
>
>                 /* Extract the IO space from device tree blob */
> -               if (!of_have_populated_dt())
> +               if (!np)
>                         return;
>
> -               np = of_dev_hwmod_lookup(of_find_node_by_name(NULL, "ocp"), oh);
> -               if (np)
> -                       va_start = of_iomap(np, 0);
> +               va_start = of_iomap(np, 0);

This hunk needs a rebase with 3.11-rc5
due to commit 130142d91467e8a07f3a863db369225a89e84d75
(ARM: OMAP2+: hwmod: rt address space index for DT)

>         } else {
>                 va_start = ioremap(mem->pa_start, mem->pa_end - mem->pa_start);
>         }

[...]

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
> Hi Rajendra,
> 
> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
> [..]
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 12fa589..e5c804b 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>         return ret;
>>  }
>>
>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>> +                                      struct device_node *np,
>> +                                      int *opt_clks_cnt)
>> +{
>> +       int i, clks_cnt;
>> +       const char *clk_name;
>> +       const char **opt_clk_names;
>> +
>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>> +       if (!clks_cnt)
>> +               return NULL;
>> +
>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>> +       if (!opt_clk_names)
>> +               return NULL;
>> +
>> +       for (i = 0; i < clks_cnt; i++) {
>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>> +               if (!strcmp(clk_name, "fck"))
> 
> Could we instead parse for names that are "optional,role_name" instead
> of assuming anything other than fck is optional clocks?

you mean look for anything with optional,*? because the role names would change.

> 
>> +                       continue;
>> +               opt_clks_cnt++;
>> +               opt_clk_names[i] = clk_name;
>> +       }
>> +       return opt_clk_names;
>> +}
>> +
> [...]
> --
> Regards,
> Nishanth Menon
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Nishanth Menon]

On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>> Hi Rajendra,
>>
>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>> [..]
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index 12fa589..e5c804b 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>          return ret;
>>>   }
>>>
>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>> +                                      struct device_node *np,
>>> +                                      int *opt_clks_cnt)
>>> +{
>>> +       int i, clks_cnt;
>>> +       const char *clk_name;
>>> +       const char **opt_clk_names;
>>> +
>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>> +       if (!clks_cnt)
>>> +               return NULL;
>>> +
>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>> +       if (!opt_clk_names)
>>> +               return NULL;
>>> +
>>> +       for (i = 0; i < clks_cnt; i++) {
>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>> +               if (!strcmp(clk_name, "fck"))
>>
>> Could we instead parse for names that are "optional,role_name" instead
>> of assuming anything other than fck is optional clocks?
>
> you mean look for anything with optional,*? because the role names would change.
>

yes. the idea being, we now have a meaning to the clock name - there are 
two types of clocks here.. functional and optional, we *might* have 
facility to add interface clock(we dont know interface clock handling 
yet, but something in the future).. we might increase the support for 
number of functional clocks.. it might help to keep the format such that 
it is a "bit extendable".

>>
>>> +                       continue;
>>> +               opt_clks_cnt++;
>>> +               opt_clk_names[i] = clk_name;
>>> +       }
>>> +       return opt_clk_names;
>>> +}
>>> +
>> [...]
>> --
>> Regards,
>> Nishanth Menon
>>
>


-- 
Regards,
Nishanth Menon

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 07:09 PM, Nishanth Menon wrote:
> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>> Hi Rajendra,
>>>
>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>> [..]
>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>> index 12fa589..e5c804b 100644
>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>          return ret;
>>>>   }
>>>>
>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>> +                                      struct device_node *np,
>>>> +                                      int *opt_clks_cnt)
>>>> +{
>>>> +       int i, clks_cnt;
>>>> +       const char *clk_name;
>>>> +       const char **opt_clk_names;
>>>> +
>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>> +       if (!clks_cnt)
>>>> +               return NULL;
>>>> +
>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>> +       if (!opt_clk_names)
>>>> +               return NULL;
>>>> +
>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>> +               if (!strcmp(clk_name, "fck"))
>>>
>>> Could we instead parse for names that are "optional,role_name" instead
>>> of assuming anything other than fck is optional clocks?
>>
>> you mean look for anything with optional,*? because the role names would change.
>>
> 
> yes. the idea being, we now have a meaning to the clock name - there are two types of clocks here.. functional and optional, we *might* have facility to add interface clock(we dont know interface clock handling yet, but something in the future).. we might increase the support for number of functional clocks.. it might help to keep the format such that it is a "bit extendable".

right, makes sense, will update.

> 
>>>
>>>> +                       continue;
>>>> +               opt_clks_cnt++;
>>>> +               opt_clk_names[i] = clk_name;
>>>> +       }
>>>> +       return opt_clk_names;
>>>> +}
>>>> +
>>> [...]
>>> -- 
>>> Regards,
>>> Nishanth Menon
>>>
>>
> 
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Mark Rutland]

[Adding Mike Turquette and dt maintainers to Cc]

On Tue, Jul 23, 2013 at 07:24:38AM +0100, Rajendra Nayak wrote:
> With clocks for OMAP moving to DT, its now possible to pass all optional clock
> data for each device from DT instead of having it in hwmod.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |   66 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 12fa589..e5c804b 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>  	return ret;
>  }
>  
> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> +				       struct device_node *np,
> +				       int *opt_clks_cnt)
> +{
> +	int i, clks_cnt;
> +	const char *clk_name;
> +	const char **opt_clk_names;
> +
> +	clks_cnt = of_property_count_strings(np, "clock-names");
> +	if (!clks_cnt)
> +		return NULL;
> +
> +	opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> +	if (!opt_clk_names)
> +		return NULL;
> +
> +	for (i = 0; i < clks_cnt; i++) {
> +		of_property_read_string_index(np, "clock-names", i, &clk_name);
> +		if (!strcmp(clk_name, "fck"))
> +			continue;
> +		opt_clks_cnt++;
> +		opt_clk_names[i] = clk_name;
> +	}
> +	return opt_clk_names;
> +}
> +
> +static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np)
> +{
> +	struct clk *c;
> +	int i, opt_clks_cnt = 0;
> +	int ret = 0;
> +	const char **opt_clk_names;
> +
> +	opt_clk_names = _parse_opt_clks_dt(oh, np, &opt_clks_cnt);
> +	if (!opt_clk_names)
> +		return -EINVAL;
> +
> +	oh->opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *)
> +			       * opt_clks_cnt, GFP_KERNEL);
> +	if (!oh->opt_clks)
> +		return -ENOMEM;
> +
> +	oh->opt_clks_cnt = opt_clks_cnt;
> +
> +	for (i = 0; i < oh->opt_clks_cnt; i++) {
> +		c = of_clk_get_by_name(np, opt_clk_names[i]);
> +		if (IS_ERR(c)) {
> +			pr_warn("omap_hwmod: %s: cannot clk_get opt_clk %s\n",
> +				oh->name, opt_clk_names[i]);
> +			ret = -EINVAL;
> +		}
> +		oh->opt_clks[i]._clk = c;
> +		oh->opt_clks[i].role = opt_clk_names[i];
> +		oh->opt_clks_cnt++;
> +		clk_prepare(oh->opt_clks[i]._clk);
> +	}
> +	return ret;
> +}

I don't like this. 

clock-names is used to represent the names of clocks as inputs to the
device. The driver must know the names of each and every one of the
clock inputs it intends to use -- there's a finite number, and if it
doesn't know about it it clearly has no idea how that clock's meant to
be used.

Consider a future revision of the hardware that has an additional clock
input. Some new feature may require that clock, but your driver won't
support that new feature, so you don't need it. Preparing that clock is
a waste of power, and could cause issues if for some reason the clock
was mutually exlcusive with another clock (so preparing it would make
the hardware unusable). If the new revision *requires* that clock to
provide the same interface otherwise, it's not backwards compatible and
needs a new binding, and the driver needs to be extended to support it.

Given that, preparing all the clocks you've been handed is a hack.
Simply request by name the ones you know you need, and attempt to
request the optional ones as necessary. Don't blindly go and prepare
everything.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Mark Rutland]

[Adding Mike Turquette and dt maintainers]

On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
> > On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
> >> Hi Rajendra,
> >>
> >> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
> >> [..]
> >>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >>> index 12fa589..e5c804b 100644
> >>> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
> >>>          return ret;
> >>>   }
> >>>
> >>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> >>> +                                      struct device_node *np,
> >>> +                                      int *opt_clks_cnt)
> >>> +{
> >>> +       int i, clks_cnt;
> >>> +       const char *clk_name;
> >>> +       const char **opt_clk_names;
> >>> +
> >>> +       clks_cnt = of_property_count_strings(np, "clock-names");
> >>> +       if (!clks_cnt)
> >>> +               return NULL;
> >>> +
> >>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> >>> +       if (!opt_clk_names)
> >>> +               return NULL;
> >>> +
> >>> +       for (i = 0; i < clks_cnt; i++) {
> >>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
> >>> +               if (!strcmp(clk_name, "fck"))
> >>
> >> Could we instead parse for names that are "optional,role_name" instead
> >> of assuming anything other than fck is optional clocks?
> >
> > you mean look for anything with optional,*? because the role names would change.
> >
> 
> yes. the idea being, we now have a meaning to the clock name - there are 
> two types of clocks here.. functional and optional, we *might* have 
> facility to add interface clock(we dont know interface clock handling 
> yet, but something in the future).. we might increase the support for 
> number of functional clocks.. it might help to keep the format such that 
> it is a "bit extendable".

I completely disagree. The only things that should appear in clock-names
are the names of the clock inputs that appear in the manual for the
device. The driver should know which ones are optional, as that's a
fixed property of the IP and the way the driver uses it.

You should not be embedding additional semantics in name properties.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 07:15 PM, Mark Rutland wrote:
> [Adding Mike Turquette and dt maintainers to Cc]
> 
> On Tue, Jul 23, 2013 at 07:24:38AM +0100, Rajendra Nayak wrote:
>> With clocks for OMAP moving to DT, its now possible to pass all optional clock
>> data for each device from DT instead of having it in hwmod.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> ---
>>  arch/arm/mach-omap2/omap_hwmod.c |   66 ++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 64 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 12fa589..e5c804b 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>  	return ret;
>>  }
>>  
>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>> +				       struct device_node *np,
>> +				       int *opt_clks_cnt)
>> +{
>> +	int i, clks_cnt;
>> +	const char *clk_name;
>> +	const char **opt_clk_names;
>> +
>> +	clks_cnt = of_property_count_strings(np, "clock-names");
>> +	if (!clks_cnt)
>> +		return NULL;
>> +
>> +	opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>> +	if (!opt_clk_names)
>> +		return NULL;
>> +
>> +	for (i = 0; i < clks_cnt; i++) {
>> +		of_property_read_string_index(np, "clock-names", i, &clk_name);
>> +		if (!strcmp(clk_name, "fck"))
>> +			continue;
>> +		opt_clks_cnt++;
>> +		opt_clk_names[i] = clk_name;
>> +	}
>> +	return opt_clk_names;
>> +}
>> +
>> +static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np)
>> +{
>> +	struct clk *c;
>> +	int i, opt_clks_cnt = 0;
>> +	int ret = 0;
>> +	const char **opt_clk_names;
>> +
>> +	opt_clk_names = _parse_opt_clks_dt(oh, np, &opt_clks_cnt);
>> +	if (!opt_clk_names)
>> +		return -EINVAL;
>> +
>> +	oh->opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *)
>> +			       * opt_clks_cnt, GFP_KERNEL);
>> +	if (!oh->opt_clks)
>> +		return -ENOMEM;
>> +
>> +	oh->opt_clks_cnt = opt_clks_cnt;
>> +
>> +	for (i = 0; i < oh->opt_clks_cnt; i++) {
>> +		c = of_clk_get_by_name(np, opt_clk_names[i]);
>> +		if (IS_ERR(c)) {
>> +			pr_warn("omap_hwmod: %s: cannot clk_get opt_clk %s\n",
>> +				oh->name, opt_clk_names[i]);
>> +			ret = -EINVAL;
>> +		}
>> +		oh->opt_clks[i]._clk = c;
>> +		oh->opt_clks[i].role = opt_clk_names[i];
>> +		oh->opt_clks_cnt++;
>> +		clk_prepare(oh->opt_clks[i]._clk);
>> +	}
>> +	return ret;
>> +}
> 
> I don't like this. 
> 
> clock-names is used to represent the names of clocks as inputs to the
> device. The driver must know the names of each and every one of the
> clock inputs it intends to use -- there's a finite number, and if it
> doesn't know about it it clearly has no idea how that clock's meant to
> be used.
> 
> Consider a future revision of the hardware that has an additional clock
> input. Some new feature may require that clock, but your driver won't
> support that new feature, so you don't need it. Preparing that clock is
> a waste of power, and could cause issues if for some reason the clock
> was mutually exlcusive with another clock (so preparing it would make
> the hardware unusable). If the new revision *requires* that clock to
> provide the same interface otherwise, it's not backwards compatible and
> needs a new binding, and the driver needs to be extended to support it.
> 
> Given that, preparing all the clocks you've been handed is a hack.

Mark, this is a piece of platform code (hwmod framework for omap) which
does a enable/reset/idle of all devices on the SoC early at boot to get
rid of bootloader dependencies. This isn;t something used by the drivers
when they enable the devices. I don't see any issue with 'waste of power'.
The framework (unlike the driver) has no knowledge of what clocks are
needed and hence does a enable all momentarily to reset and put the device in a
known state.

> Simply request by name the ones you know you need, and attempt to
> request the optional ones as necessary. Don't blindly go and prepare
> everything.
> 
> Thanks,
> Mark.
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Russell King - ARM Linux]

On Wed, Aug 14, 2013 at 02:49:04PM +0100, Mark Rutland wrote:
> [Adding Mike Turquette and dt maintainers]
> 
> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
> > yes. the idea being, we now have a meaning to the clock name - there are 
> > two types of clocks here.. functional and optional, we *might* have 
> > facility to add interface clock(we dont know interface clock handling 
> > yet, but something in the future).. we might increase the support for 
> > number of functional clocks.. it might help to keep the format such that 
> > it is a "bit extendable".
> 
> I completely disagree. The only things that should appear in clock-names
> are the names of the clock inputs that appear in the manual for the
> device. The driver should know which ones are optional, as that's a
> fixed property of the IP and the way the driver uses it.
> 
> You should not be embedding additional semantics in name properties.

Agreed.  I've been on at people about this for years, and every time they
go off and do something else, it ultimately ends up coming back to bite
them.  Clock names _are_ the clock input names as far as device drivers
are concerned, and nothing else.

If there are optional clocks, then the driver should be doing as Mark
says - the driver should try to get them, and if it fails to get them
then they're quite simply not provided by the platform.  If there are
optional clocks which the device driver does not know about (or even
need to know about) then that too should not be a problem - the driver
just doesn't have to touch them.

If they're optional, but required for the device to function, then the
driver should get them and control them as necessary.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Nishanth Menon]

On 08/14/2013 08:49 AM, Mark Rutland wrote:
> [Adding Mike Turquette and dt maintainers]
>
> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>>> Hi Rajendra,
>>>>
>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>>> [..]
>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>>> index 12fa589..e5c804b 100644
>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>>           return ret;
>>>>>    }
>>>>>
>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>>> +                                      struct device_node *np,
>>>>> +                                      int *opt_clks_cnt)
>>>>> +{
>>>>> +       int i, clks_cnt;
>>>>> +       const char *clk_name;
>>>>> +       const char **opt_clk_names;
>>>>> +
>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>>> +       if (!clks_cnt)
>>>>> +               return NULL;
>>>>> +
>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>>> +       if (!opt_clk_names)
>>>>> +               return NULL;
>>>>> +
>>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>>> +               if (!strcmp(clk_name, "fck"))
>>>>
>>>> Could we instead parse for names that are "optional,role_name" instead
>>>> of assuming anything other than fck is optional clocks?
>>>
>>> you mean look for anything with optional,*? because the role names would change.
>>>
>>
>> yes. the idea being, we now have a meaning to the clock name - there are
>> two types of clocks here.. functional and optional, we *might* have
>> facility to add interface clock(we dont know interface clock handling
>> yet, but something in the future).. we might increase the support for
>> number of functional clocks.. it might help to keep the format such that
>> it is a "bit extendable".
>
> I completely disagree. The only things that should appear in clock-names
> are the names of the clock inputs that appear in the manual for the
> device. The driver should know which ones are optional, as that's a
> fixed property of the IP and the way the driver uses it.
>
> You should not be embedding additional semantics in name properties.

we use an level of abstraction called omap_device and hwmod to allow 
devices to use a generic pm_runtime. drivers for specific blocks dont 
normally need to know about the clocks to deal with. This allows maximum 
reuse consider concept is generic enough.

The traditional way of dealing with this has been to encode the *names* 
*inside* hwmod!!! which is obviously the wrong way to deal with with 
clock nodes in dts.

This series moves away from that approach - which is good. Now, the 
question is:

clocks-names = "....."  is not sufficient to indicate a list of clocks 
of different functions

3 types of clocks may exist for OMAP h/w blocks:
a) functional clock (1 usually, but we are starting to see multiple 
fclks). this drives the actual functionality
b) optional clocks - additional clocks such as debounce clock etc. - 
there can be n such clocks
c) interface clocks - this drives the register interface on the bus.  - 
there can be n such clocks and additional headaches for interface - 
currently embedded inside hwmod.

you could do:
A) embedd in the sematics of the name
clock-names="fck", "optional,role1",  "optional,role2";
clocks= <&clk_a>, <&clk_x>, <&clk_y>;
OR:
B) embedd in the property name
optional-clock-names="optional,role1",  "optional,role2";
optional-clocks=<&clk_x>, <&clk_y>;
functional-clock-names="fck";
functional-clocks=<&clk_a>;

are you suggesting (b) here?

-- 
Regards,
Nishanth Menon

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Mark Rutland]

On Wed, Aug 14, 2013 at 02:54:57PM +0100, Rajendra Nayak wrote:
> On Wednesday 14 August 2013 07:15 PM, Mark Rutland wrote:
> > [Adding Mike Turquette and dt maintainers to Cc]
> > 
> > On Tue, Jul 23, 2013 at 07:24:38AM +0100, Rajendra Nayak wrote:
> >> With clocks for OMAP moving to DT, its now possible to pass all optional clock
> >> data for each device from DT instead of having it in hwmod.
> >>
> >> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> >> ---
> >>  arch/arm/mach-omap2/omap_hwmod.c |   66 ++++++++++++++++++++++++++++++++++++--
> >>  1 file changed, 64 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >> index 12fa589..e5c804b 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
> >>  	return ret;
> >>  }
> >>  
> >> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> >> +				       struct device_node *np,
> >> +				       int *opt_clks_cnt)
> >> +{
> >> +	int i, clks_cnt;
> >> +	const char *clk_name;
> >> +	const char **opt_clk_names;
> >> +
> >> +	clks_cnt = of_property_count_strings(np, "clock-names");
> >> +	if (!clks_cnt)
> >> +		return NULL;
> >> +
> >> +	opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> >> +	if (!opt_clk_names)
> >> +		return NULL;
> >> +
> >> +	for (i = 0; i < clks_cnt; i++) {
> >> +		of_property_read_string_index(np, "clock-names", i, &clk_name);
> >> +		if (!strcmp(clk_name, "fck"))
> >> +			continue;
> >> +		opt_clks_cnt++;
> >> +		opt_clk_names[i] = clk_name;
> >> +	}
> >> +	return opt_clk_names;
> >> +}
> >> +
> >> +static int _init_opt_clks_dt(struct omap_hwmod *oh, struct device_node *np)
> >> +{
> >> +	struct clk *c;
> >> +	int i, opt_clks_cnt = 0;
> >> +	int ret = 0;
> >> +	const char **opt_clk_names;
> >> +
> >> +	opt_clk_names = _parse_opt_clks_dt(oh, np, &opt_clks_cnt);
> >> +	if (!opt_clk_names)
> >> +		return -EINVAL;
> >> +
> >> +	oh->opt_clks = kzalloc(sizeof(struct omap_hwmod_opt_clk *)
> >> +			       * opt_clks_cnt, GFP_KERNEL);
> >> +	if (!oh->opt_clks)
> >> +		return -ENOMEM;
> >> +
> >> +	oh->opt_clks_cnt = opt_clks_cnt;
> >> +
> >> +	for (i = 0; i < oh->opt_clks_cnt; i++) {
> >> +		c = of_clk_get_by_name(np, opt_clk_names[i]);
> >> +		if (IS_ERR(c)) {
> >> +			pr_warn("omap_hwmod: %s: cannot clk_get opt_clk %s\n",
> >> +				oh->name, opt_clk_names[i]);
> >> +			ret = -EINVAL;
> >> +		}
> >> +		oh->opt_clks[i]._clk = c;
> >> +		oh->opt_clks[i].role = opt_clk_names[i];
> >> +		oh->opt_clks_cnt++;
> >> +		clk_prepare(oh->opt_clks[i]._clk);
> >> +	}
> >> +	return ret;
> >> +}
> > 
> > I don't like this. 
> > 
> > clock-names is used to represent the names of clocks as inputs to the
> > device. The driver must know the names of each and every one of the
> > clock inputs it intends to use -- there's a finite number, and if it
> > doesn't know about it it clearly has no idea how that clock's meant to
> > be used.
> > 
> > Consider a future revision of the hardware that has an additional clock
> > input. Some new feature may require that clock, but your driver won't
> > support that new feature, so you don't need it. Preparing that clock is
> > a waste of power, and could cause issues if for some reason the clock
> > was mutually exlcusive with another clock (so preparing it would make
> > the hardware unusable). If the new revision *requires* that clock to
> > provide the same interface otherwise, it's not backwards compatible and
> > needs a new binding, and the driver needs to be extended to support it.
> > 
> > Given that, preparing all the clocks you've been handed is a hack.
> 
> Mark, this is a piece of platform code (hwmod framework for omap) which
> does a enable/reset/idle of all devices on the SoC early at boot to get
> rid of bootloader dependencies. This isn;t something used by the drivers
> when they enable the devices. I don't see any issue with 'waste of power'.
> The framework (unlike the driver) has no knowledge of what clocks are
> needed and hence does a enable all momentarily to reset and put the device in a
> known state.

Ok, I'd misunderstood there. Apologies for the noise.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 07:28 PM, Nishanth Menon wrote:
> On 08/14/2013 08:49 AM, Mark Rutland wrote:
>> [Adding Mike Turquette and dt maintainers]
>>
>> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
>>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>>>> Hi Rajendra,
>>>>>
>>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>>>> [..]
>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>> index 12fa589..e5c804b 100644
>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>>>           return ret;
>>>>>>    }
>>>>>>
>>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>>>> +                                      struct device_node *np,
>>>>>> +                                      int *opt_clks_cnt)
>>>>>> +{
>>>>>> +       int i, clks_cnt;
>>>>>> +       const char *clk_name;
>>>>>> +       const char **opt_clk_names;
>>>>>> +
>>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>>>> +       if (!clks_cnt)
>>>>>> +               return NULL;
>>>>>> +
>>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>>>> +       if (!opt_clk_names)
>>>>>> +               return NULL;
>>>>>> +
>>>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>>>> +               if (!strcmp(clk_name, "fck"))
>>>>>
>>>>> Could we instead parse for names that are "optional,role_name" instead
>>>>> of assuming anything other than fck is optional clocks?
>>>>
>>>> you mean look for anything with optional,*? because the role names would change.
>>>>
>>>
>>> yes. the idea being, we now have a meaning to the clock name - there are
>>> two types of clocks here.. functional and optional, we *might* have
>>> facility to add interface clock(we dont know interface clock handling
>>> yet, but something in the future).. we might increase the support for
>>> number of functional clocks.. it might help to keep the format such that
>>> it is a "bit extendable".
>>
>> I completely disagree. The only things that should appear in clock-names
>> are the names of the clock inputs that appear in the manual for the
>> device. The driver should know which ones are optional, as that's a
>> fixed property of the IP and the way the driver uses it.
>>
>> You should not be embedding additional semantics in name properties.
> 
> we use an level of abstraction called omap_device and hwmod to allow devices to use a generic pm_runtime. drivers for specific blocks dont normally need to know about the clocks to deal with. This allows maximum reuse consider concept is generic enough.

They do know about the optional clocks though and request and release them when needed. The need for hwmod to know about optional clocks
(and enable all) arises from the fact that some of these devices need *some* optional clocks for a successful reset.
And given hmwod has no knowledge about which optional ones (if at all) will be needed, it goes ahead and enables all before doing a reset.
This is something done only at init time and *not* something thats done every time the device is enabled by the driver using pm_runtime.

> 
> The traditional way of dealing with this has been to encode the *names* *inside* hwmod!!! which is obviously the wrong way to deal with with clock nodes in dts.
> 
> This series moves away from that approach - which is good. Now, the question is:
> 
> clocks-names = "....."  is not sufficient to indicate a list of clocks of different functions
> 
> 3 types of clocks may exist for OMAP h/w blocks:
> a) functional clock (1 usually, but we are starting to see multiple fclks). this drives the actual functionality
> b) optional clocks - additional clocks such as debounce clock etc. - there can be n such clocks
> c) interface clocks - this drives the register interface on the bus.  - there can be n such clocks and additional headaches for interface - currently embedded inside hwmod.
> 
> you could do:
> A) embedd in the sematics of the name
> clock-names="fck", "optional,role1",  "optional,role2";
> clocks= <&clk_a>, <&clk_x>, <&clk_y>;
> OR:
> B) embedd in the property name
> optional-clock-names="optional,role1",  "optional,role2";
> optional-clocks=<&clk_x>, <&clk_y>;
> functional-clock-names="fck";
> functional-clocks=<&clk_a>;
> 
> are you suggesting (b) here?
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Mark Rutland]

On Wed, Aug 14, 2013 at 02:58:25PM +0100, Nishanth Menon wrote:
> On 08/14/2013 08:49 AM, Mark Rutland wrote:
> > [Adding Mike Turquette and dt maintainers]
> >
> > On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
> >> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
> >>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
> >>>> Hi Rajendra,
> >>>>
> >>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
> >>>> [..]
> >>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >>>>> index 12fa589..e5c804b 100644
> >>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
> >>>>>           return ret;
> >>>>>    }
> >>>>>
> >>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> >>>>> +                                      struct device_node *np,
> >>>>> +                                      int *opt_clks_cnt)
> >>>>> +{
> >>>>> +       int i, clks_cnt;
> >>>>> +       const char *clk_name;
> >>>>> +       const char **opt_clk_names;
> >>>>> +
> >>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
> >>>>> +       if (!clks_cnt)
> >>>>> +               return NULL;
> >>>>> +
> >>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> >>>>> +       if (!opt_clk_names)
> >>>>> +               return NULL;
> >>>>> +
> >>>>> +       for (i = 0; i < clks_cnt; i++) {
> >>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
> >>>>> +               if (!strcmp(clk_name, "fck"))
> >>>>
> >>>> Could we instead parse for names that are "optional,role_name" instead
> >>>> of assuming anything other than fck is optional clocks?
> >>>
> >>> you mean look for anything with optional,*? because the role names would change.
> >>>
> >>
> >> yes. the idea being, we now have a meaning to the clock name - there are
> >> two types of clocks here.. functional and optional, we *might* have
> >> facility to add interface clock(we dont know interface clock handling
> >> yet, but something in the future).. we might increase the support for
> >> number of functional clocks.. it might help to keep the format such that
> >> it is a "bit extendable".
> >
> > I completely disagree. The only things that should appear in clock-names
> > are the names of the clock inputs that appear in the manual for the
> > device. The driver should know which ones are optional, as that's a
> > fixed property of the IP and the way the driver uses it.
> >
> > You should not be embedding additional semantics in name properties.
> 
> we use an level of abstraction called omap_device and hwmod to allow 
> devices to use a generic pm_runtime. drivers for specific blocks dont 
> normally need to know about the clocks to deal with. This allows maximum 
> reuse consider concept is generic enough.
> 
> The traditional way of dealing with this has been to encode the *names* 
> *inside* hwmod!!! which is obviously the wrong way to deal with with 
> clock nodes in dts.
> 
> This series moves away from that approach - which is good. Now, the 
> question is:
> 
> clocks-names = "....."  is not sufficient to indicate a list of clocks 
> of different functions
> 
> 3 types of clocks may exist for OMAP h/w blocks:
> a) functional clock (1 usually, but we are starting to see multiple 
> fclks). this drives the actual functionality
> b) optional clocks - additional clocks such as debounce clock etc. - 
> there can be n such clocks
> c) interface clocks - this drives the register interface on the bus.  - 
> there can be n such clocks and additional headaches for interface - 
> currently embedded inside hwmod.

I don't see why these can't all be supported without embedding
additional details in the clock-names property. When you say "there can
be n such clocks", do you mean that different devices may have different
numbers of clocks, or that a given device could have an arbitrary set?
Surely the design of the IP imposes a maximum limit on the number of
clocks a device may have?

> 
> you could do:
> A) embedd in the sematics of the name
> clock-names="fck", "optional,role1",  "optional,role2";
> clocks= <&clk_a>, <&clk_x>, <&clk_y>;
> OR:

Does the name of the clock itself not encode this?

Surely "fclk" would be your functional clock, "dbncclk" or similar would
be an (optional) debounce clock, "apbclk" would be an interface clock?
Surely you must know their type by which input they are and how they are
to be used?

>From the sounds of your other reply, the issue is that you're poking
clocks for arbitrary devices, without knowing the semantics of those
clocks, because you're doing it form outside the driver. I don't have a
good solution to this, but your proposed solution is not one I'm happy
with.

> B) embedd in the property name
> optional-clock-names="optional,role1",  "optional,role2";
> optional-clocks=<&clk_x>, <&clk_y>;
> functional-clock-names="fck";
> functional-clocks=<&clk_a>;
> 
> are you suggesting (b) here?

I'm suggesting neither.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 07:35 PM, Rajendra Nayak wrote:
> On Wednesday 14 August 2013 07:28 PM, Nishanth Menon wrote:
>> On 08/14/2013 08:49 AM, Mark Rutland wrote:
>>> [Adding Mike Turquette and dt maintainers]
>>>
>>> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
>>>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>>>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>>>>> Hi Rajendra,
>>>>>>
>>>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>>>>> [..]
>>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>> index 12fa589..e5c804b 100644
>>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>>>>           return ret;
>>>>>>>    }
>>>>>>>
>>>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>>>>> +                                      struct device_node *np,
>>>>>>> +                                      int *opt_clks_cnt)
>>>>>>> +{
>>>>>>> +       int i, clks_cnt;
>>>>>>> +       const char *clk_name;
>>>>>>> +       const char **opt_clk_names;
>>>>>>> +
>>>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>>>>> +       if (!clks_cnt)
>>>>>>> +               return NULL;
>>>>>>> +
>>>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>>>>> +       if (!opt_clk_names)
>>>>>>> +               return NULL;
>>>>>>> +
>>>>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>>>>> +               if (!strcmp(clk_name, "fck"))
>>>>>>
>>>>>> Could we instead parse for names that are "optional,role_name" instead
>>>>>> of assuming anything other than fck is optional clocks?
>>>>>
>>>>> you mean look for anything with optional,*? because the role names would change.
>>>>>
>>>>
>>>> yes. the idea being, we now have a meaning to the clock name - there are
>>>> two types of clocks here.. functional and optional, we *might* have
>>>> facility to add interface clock(we dont know interface clock handling
>>>> yet, but something in the future).. we might increase the support for
>>>> number of functional clocks.. it might help to keep the format such that
>>>> it is a "bit extendable".
>>>
>>> I completely disagree. The only things that should appear in clock-names
>>> are the names of the clock inputs that appear in the manual for the
>>> device. The driver should know which ones are optional, as that's a
>>> fixed property of the IP and the way the driver uses it.
>>>
>>> You should not be embedding additional semantics in name properties.
>>
>> we use an level of abstraction called omap_device and hwmod to allow devices to use a generic pm_runtime. drivers for specific blocks dont normally need to know about the clocks to deal with. This allows maximum reuse consider concept is generic enough.
> 
> They do know about the optional clocks though and request and release them when needed. The need for hwmod to know about optional clocks
> (and enable all) arises from the fact that some of these devices need *some* optional clocks for a successful reset.
> And given hmwod has no knowledge about which optional ones (if at all) will be needed, it goes ahead and enables all before doing a reset.
> This is something done only at init time and *not* something thats done every time the device is enabled by the driver using pm_runtime.

We do seem to have a flag 'HWMOD_CONTROL_OPT_CLKS_IN_RESET' which says which ones really need optional clocks to be controlled.
I guess for the rest hwmod can even ignore optional clocks completely.

> 
>>
>> The traditional way of dealing with this has been to encode the *names* *inside* hwmod!!! which is obviously the wrong way to deal with with clock nodes in dts.
>>
>> This series moves away from that approach - which is good. Now, the question is:
>>
>> clocks-names = "....."  is not sufficient to indicate a list of clocks of different functions
>>
>> 3 types of clocks may exist for OMAP h/w blocks:
>> a) functional clock (1 usually, but we are starting to see multiple fclks). this drives the actual functionality
>> b) optional clocks - additional clocks such as debounce clock etc. - there can be n such clocks
>> c) interface clocks - this drives the register interface on the bus.  - there can be n such clocks and additional headaches for interface - currently embedded inside hwmod.
>>
>> you could do:
>> A) embedd in the sematics of the name
>> clock-names="fck", "optional,role1",  "optional,role2";
>> clocks= <&clk_a>, <&clk_x>, <&clk_y>;
>> OR:
>> B) embedd in the property name
>> optional-clock-names="optional,role1",  "optional,role2";
>> optional-clocks=<&clk_x>, <&clk_y>;
>> functional-clock-names="fck";
>> functional-clocks=<&clk_a>;
>>
>> are you suggesting (b) here?
>>
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

[]..
> 
> From the sounds of your other reply, the issue is that you're poking
> clocks for arbitrary devices, without knowing the semantics of those
> clocks, because you're doing it form outside the driver.

Thats right.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Mark Rutland]

On Wed, Aug 14, 2013 at 03:05:25PM +0100, Rajendra Nayak wrote:
> On Wednesday 14 August 2013 07:28 PM, Nishanth Menon wrote:
> > On 08/14/2013 08:49 AM, Mark Rutland wrote:
> >> [Adding Mike Turquette and dt maintainers]
> >>
> >> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
> >>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
> >>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
> >>>>> Hi Rajendra,
> >>>>>
> >>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
> >>>>> [..]
> >>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >>>>>> index 12fa589..e5c804b 100644
> >>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
> >>>>>>           return ret;
> >>>>>>    }
> >>>>>>
> >>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
> >>>>>> +                                      struct device_node *np,
> >>>>>> +                                      int *opt_clks_cnt)
> >>>>>> +{
> >>>>>> +       int i, clks_cnt;
> >>>>>> +       const char *clk_name;
> >>>>>> +       const char **opt_clk_names;
> >>>>>> +
> >>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
> >>>>>> +       if (!clks_cnt)
> >>>>>> +               return NULL;
> >>>>>> +
> >>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
> >>>>>> +       if (!opt_clk_names)
> >>>>>> +               return NULL;
> >>>>>> +
> >>>>>> +       for (i = 0; i < clks_cnt; i++) {
> >>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
> >>>>>> +               if (!strcmp(clk_name, "fck"))
> >>>>>
> >>>>> Could we instead parse for names that are "optional,role_name" instead
> >>>>> of assuming anything other than fck is optional clocks?
> >>>>
> >>>> you mean look for anything with optional,*? because the role names would change.
> >>>>
> >>>
> >>> yes. the idea being, we now have a meaning to the clock name - there are
> >>> two types of clocks here.. functional and optional, we *might* have
> >>> facility to add interface clock(we dont know interface clock handling
> >>> yet, but something in the future).. we might increase the support for
> >>> number of functional clocks.. it might help to keep the format such that
> >>> it is a "bit extendable".
> >>
> >> I completely disagree. The only things that should appear in clock-names
> >> are the names of the clock inputs that appear in the manual for the
> >> device. The driver should know which ones are optional, as that's a
> >> fixed property of the IP and the way the driver uses it.
> >>
> >> You should not be embedding additional semantics in name properties.
> > 
> > we use an level of abstraction called omap_device and hwmod to allow devices to use a generic pm_runtime. drivers for specific blocks dont normally need to know about the clocks to deal with. This allows maximum reuse consider concept is generic enough.
> 
> They do know about the optional clocks though and request and release them when needed. The need for hwmod to know about optional clocks
> (and enable all) arises from the fact that some of these devices need *some* optional clocks for a successful reset.
> And given hmwod has no knowledge about which optional ones (if at all) will be needed, it goes ahead and enables all before doing a reset.
> This is something done only at init time and *not* something thats done every time the device is enabled by the driver using pm_runtime.

To clarify:

I was initially confused as to the purpose of the code. I'm not against
a one-off clock initialisation to put everything into a sane state. If
we can't trust the bootloaders, that seems like a necessary evil. I'll
leave Mike to comment on whether and how that should be done.

I do not think we should be embedding clock semantics in clock-names.
That's not the way the property is intended to be used, it breaks
uniformity, and it's an abuse of the system that may come back to bite
us later.

Thanks,
Mark.

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Rajendra Nayak]

On Wednesday 14 August 2013 07:43 PM, Mark Rutland wrote:
> On Wed, Aug 14, 2013 at 03:05:25PM +0100, Rajendra Nayak wrote:
>> On Wednesday 14 August 2013 07:28 PM, Nishanth Menon wrote:
>>> On 08/14/2013 08:49 AM, Mark Rutland wrote:
>>>> [Adding Mike Turquette and dt maintainers]
>>>>
>>>> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
>>>>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>>>>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>>>>>> Hi Rajendra,
>>>>>>>
>>>>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>>>>>> [..]
>>>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>> index 12fa589..e5c804b 100644
>>>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>>>>>           return ret;
>>>>>>>>    }
>>>>>>>>
>>>>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>>>>>> +                                      struct device_node *np,
>>>>>>>> +                                      int *opt_clks_cnt)
>>>>>>>> +{
>>>>>>>> +       int i, clks_cnt;
>>>>>>>> +       const char *clk_name;
>>>>>>>> +       const char **opt_clk_names;
>>>>>>>> +
>>>>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>>>>>> +       if (!clks_cnt)
>>>>>>>> +               return NULL;
>>>>>>>> +
>>>>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>>>>>> +       if (!opt_clk_names)
>>>>>>>> +               return NULL;
>>>>>>>> +
>>>>>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>>>>>> +               if (!strcmp(clk_name, "fck"))
>>>>>>>
>>>>>>> Could we instead parse for names that are "optional,role_name" instead
>>>>>>> of assuming anything other than fck is optional clocks?
>>>>>>
>>>>>> you mean look for anything with optional,*? because the role names would change.
>>>>>>
>>>>>
>>>>> yes. the idea being, we now have a meaning to the clock name - there are
>>>>> two types of clocks here.. functional and optional, we *might* have
>>>>> facility to add interface clock(we dont know interface clock handling
>>>>> yet, but something in the future).. we might increase the support for
>>>>> number of functional clocks.. it might help to keep the format such that
>>>>> it is a "bit extendable".
>>>>
>>>> I completely disagree. The only things that should appear in clock-names
>>>> are the names of the clock inputs that appear in the manual for the
>>>> device. The driver should know which ones are optional, as that's a
>>>> fixed property of the IP and the way the driver uses it.
>>>>
>>>> You should not be embedding additional semantics in name properties.
>>>
>>> we use an level of abstraction called omap_device and hwmod to allow devices to use a generic pm_runtime. drivers for specific blocks dont normally need to know about the clocks to deal with. This allows maximum reuse consider concept is generic enough.
>>
>> They do know about the optional clocks though and request and release them when needed. The need for hwmod to know about optional clocks
>> (and enable all) arises from the fact that some of these devices need *some* optional clocks for a successful reset.
>> And given hmwod has no knowledge about which optional ones (if at all) will be needed, it goes ahead and enables all before doing a reset.
>> This is something done only at init time and *not* something thats done every time the device is enabled by the driver using pm_runtime.
> 
> To clarify:
> 
> I was initially confused as to the purpose of the code. I'm not against
> a one-off clock initialisation to put everything into a sane state. If
> we can't trust the bootloaders, that seems like a necessary evil. I'll
> leave Mike to comment on whether and how that should be done.
> 
> I do not think we should be embedding clock semantics in clock-names.
> That's not the way the property is intended to be used, it breaks
> uniformity, and it's an abuse of the system that may come back to bite
> us later.

Mark, that makes sense.

Nishanth, thinking some more of this, the 'optional,role-name' also won't work
for the simple reason that drivers who do clk_get(node, 'role-name') would
then simply fail.

So I guess we need to figure out a better way to handle this.

> 
> Thanks,
> Mark.
> 

Re: [PATCH 2/3] ARM: OMAP2+: Add support to parse optional clk info from DT

[Nishanth Menon]

On 08/14/2013 09:20 AM, Rajendra Nayak wrote:
> On Wednesday 14 August 2013 07:43 PM, Mark Rutland wrote:
>> On Wed, Aug 14, 2013 at 03:05:25PM +0100, Rajendra Nayak wrote:
>>> On Wednesday 14 August 2013 07:28 PM, Nishanth Menon wrote:
>>>> On 08/14/2013 08:49 AM, Mark Rutland wrote:
>>>>> [Adding Mike Turquette and dt maintainers]
>>>>>
>>>>> On Wed, Aug 14, 2013 at 02:39:44PM +0100, Nishanth Menon wrote:
>>>>>> On 08/14/2013 08:20 AM, Rajendra Nayak wrote:
>>>>>>> On Wednesday 14 August 2013 06:18 PM, Nishanth Menon wrote:
>>>>>>>> Hi Rajendra,
>>>>>>>>
>>>>>>>> On Tue, Jul 23, 2013 at 1:24 AM, Rajendra Nayak <rnayak@ti.com> wrote:
>>>>>>>> [..]
>>>>>>>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>>> index 12fa589..e5c804b 100644
>>>>>>>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>>>>>>>> @@ -805,6 +805,65 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>>>>>>>>>           return ret;
>>>>>>>>>    }
>>>>>>>>>
>>>>>>>>> +static const char **_parse_opt_clks_dt(struct omap_hwmod *oh,
>>>>>>>>> +                                      struct device_node *np,
>>>>>>>>> +                                      int *opt_clks_cnt)
>>>>>>>>> +{
>>>>>>>>> +       int i, clks_cnt;
>>>>>>>>> +       const char *clk_name;
>>>>>>>>> +       const char **opt_clk_names;
>>>>>>>>> +
>>>>>>>>> +       clks_cnt = of_property_count_strings(np, "clock-names");
>>>>>>>>> +       if (!clks_cnt)
>>>>>>>>> +               return NULL;
>>>>>>>>> +
>>>>>>>>> +       opt_clk_names = kzalloc(sizeof(char *)*clks_cnt, GFP_KERNEL);
>>>>>>>>> +       if (!opt_clk_names)
>>>>>>>>> +               return NULL;
>>>>>>>>> +
>>>>>>>>> +       for (i = 0; i < clks_cnt; i++) {
>>>>>>>>> +               of_property_read_string_index(np, "clock-names", i, &clk_name);
>>>>>>>>> +               if (!strcmp(clk_name, "fck"))
>>>>>>>>
>>>>>>>> Could we instead parse for names that are "optional,role_name" instead
>>>>>>>> of assuming anything other than fck is optional clocks?
>>>>>>>
>>>>>>> you mean look for anything with optional,*? because the role names would change.
>>>>>>>
>>>>>>
>>>>>> yes. the idea being, we now have a meaning to the clock name - there are
>>>>>> two types of clocks here.. functional and optional, we *might* have
>>>>>> facility to add interface clock(we dont know interface clock handling
>>>>>> yet, but something in the future).. we might increase the support for
>>>>>> number of functional clocks.. it might help to keep the format such that
>>>>>> it is a "bit extendable".
>>>>>
>>>>> I completely disagree. The only things that should appear in clock-names
>>>>> are the names of the clock inputs that appear in the manual for the
>>>>> device. The driver should know which ones are optional, as that's a
>>>>> fixed property of the IP and the way the driver uses it.
>>>>>
>>>>> You should not be embedding additional semantics in name properties.
>>>>
>>>> we use an level of abstraction called omap_device and hwmod to allow devices to use a generic pm_runtime. drivers for specific blocks dont normally need to know about the clocks to deal with. This allows maximum reuse consider concept is generic enough.
>>>
>>> They do know about the optional clocks though and request and release them when needed. The need for hwmod to know about optional clocks
>>> (and enable all) arises from the fact that some of these devices need *some* optional clocks for a successful reset.
>>> And given hmwod has no knowledge about which optional ones (if at all) will be needed, it goes ahead and enables all before doing a reset.
>>> This is something done only at init time and *not* something thats done every time the device is enabled by the driver using pm_runtime.
>>
>> To clarify:
>>
>> I was initially confused as to the purpose of the code. I'm not against
>> a one-off clock initialisation to put everything into a sane state. If
>> we can't trust the bootloaders, that seems like a necessary evil. I'll
>> leave Mike to comment on whether and how that should be done.
>>
>> I do not think we should be embedding clock semantics in clock-names.
>> That's not the way the property is intended to be used, it breaks
>> uniformity, and it's an abuse of the system that may come back to bite
>> us later.
> 
> Mark, that makes sense.
> 
> Nishanth, thinking some more of this, the 'optional,role-name' also won't work
> for the simple reason that drivers who do clk_get(node, 'role-name') would
> then simply fail.
> 
> So I guess we need to figure out a better way to handle this.

you are right :( we do have types of clock inputs to a device and
therein lies our problem :(


-- 
Regards,
Nishanth Menon

Re: [PATCH 3/3] ARM: OMAP4: dts: Add main and optional clock data into DT

[Paul Walmsley]

Hi

On Tue, 23 Jul 2013, Rajendra Nayak wrote:

> With support to parse clock data from DT, move all main and optional
> clock information from hwmod to DT.
> 
> We still retain clocks in hwmod for devices which do not have a DT node.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi               |  100 +++++++++++++++++++++++++
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  112 ----------------------------
>  2 files changed, 100 insertions(+), 112 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 8e142f9..4dddf64 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -106,6 +106,8 @@
>  			compatible = "ti,omap-counter32k";
>  			reg = <0x4a304000 0x20>;
>  			ti,hwmods = "counter_32k";
> +			clocks = <&sys_32k_ck>;
> +			clock-names = "fck";

Shouldn't these patches be using the official DT clock binding format?  
Either the 'clocks' property should be something like:

    clocks = <&prm 10>;

or a change to the bindings needs to be discussed and implemented.


- Paul

Re: [PATCH 3/3] ARM: OMAP4: dts: Add main and optional clock data into DT

[Rajendra Nayak]

On Wednesday 21 August 2013 05:27 AM, Paul Walmsley wrote:
> Hi
> 
> On Tue, 23 Jul 2013, Rajendra Nayak wrote:
> 
>> With support to parse clock data from DT, move all main and optional
>> clock information from hwmod to DT.
>>
>> We still retain clocks in hwmod for devices which do not have a DT node.
>>
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> ---
>>  arch/arm/boot/dts/omap4.dtsi               |  100 +++++++++++++++++++++++++
>>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |  112 ----------------------------
>>  2 files changed, 100 insertions(+), 112 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 8e142f9..4dddf64 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -106,6 +106,8 @@
>>  			compatible = "ti,omap-counter32k";
>>  			reg = <0x4a304000 0x20>;
>>  			ti,hwmods = "counter_32k";
>> +			clocks = <&sys_32k_ck>;
>> +			clock-names = "fck";
> 
> Shouldn't these patches be using the official DT clock binding format?

They are, already.
 
> Either the 'clocks' property should be something like:
> 
>     clocks = <&prm 10>;

That depends on the #clock-cells for the clock provider node. These patches
are based on Teros series to move all OMAP clocks to DT [1], and the clock providers
in his series have the #clock-cells set to 0.

So the clock bindings do not mandate
	clocks = <&clk-provider index>
or
	clocks = <&clk-provider>

It just depends on how the clk-provider node is defined.

[1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100117

> 
> or a change to the bindings needs to be discussed and implemented.
> 
> 
> - Paul
>