[PATCH 0/3] omap_clk_associate

[PATCH 0/3] omap_clk_associate

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

In the following patches we introduce omap_clk_associate
hook to simplify clk handling for drivers. The main idea
is that we let struct clk know which device is the user
for that clk source and create a function name (or alias)
for the clk so drivers doesn't have to care about different
clk names between omap versions.

Felipe Balbi (3):
  clk: introduce omap_clk_associate
  clk: use clk_associate for musb driver
  clk: use clk_associate on watchdog driver

 Documentation/arm/OMAP/clk_function_name.txt |   13 ++++
 arch/arm/mach-omap2/board-n800-usb.c         |   26 --------
 arch/arm/mach-omap2/usb-musb.c               |   34 +++-------
 arch/arm/mach-omap2/usb-tusb6010.c           |    2 +
 arch/arm/plat-omap/clock.c                   |   38 +++++++++++-
 arch/arm/plat-omap/devices.c                 |   18 ++++--
 arch/arm/plat-omap/include/mach/clock.h      |    6 ++
 drivers/usb/musb/musb_core.c                 |   23 ++-----
 drivers/watchdog/omap_wdt.c                  |   87 ++++++++------------------
 9 files changed, 112 insertions(+), 135 deletions(-)
 create mode 100644 Documentation/arm/OMAP/clk_function_name.txt

[PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Introduce a new mechanism to omap's clk implementation to
associate the device with its clock during platform_device
registration. Also gives the clock a function name (like
mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
care about clk names any longer.

With this mechanism we can also be sure drivers won't be able
to clk_get the wrong clk (only true when we move all drivers
to this new mechanism and drop the old ones).

Clk's function names will be defined as they come necessary
but let's try to keep it as simple as possible.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/clock.c              |   38 +++++++++++++++++++++++++++++-
 arch/arm/plat-omap/include/mach/clock.h |    6 +++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 7bbfba2..c090f23 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -43,7 +43,7 @@ static struct clk_functions *arch_clock;
  */
 struct clk * clk_get(struct device *dev, const char *id)
 {
-	struct clk *p, *clk = ERR_PTR(-ENOENT);
+	struct clk *p, *clk;
 	int idno;
 
 	if (dev == NULL || dev->bus != &platform_bus_type)
@@ -53,6 +53,15 @@ struct clk * clk_get(struct device *dev, const char *id)
 
 	mutex_lock(&clocks_mutex);
 
+	list_for_each_entry(clk, &clocks, node) {
+		if (clk->function && (dev == clk->dev) &&
+				strcmp(id, clk->function) == 0)
+			goto found;
+
+		if (strcmp(id, clk->name) == 0)
+			goto found;
+	}
+
 	list_for_each_entry(p, &clocks, node) {
 		if (p->id == idno &&
 		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
@@ -64,10 +73,14 @@ struct clk * clk_get(struct device *dev, const char *id)
 	list_for_each_entry(p, &clocks, node) {
 		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
 			clk = p;
-			break;
+			goto found;
 		}
 	}
 
+	mutex_unlock(&clocks_mutex);
+
+	return ERR_PTR(-ENOENT);
+
 found:
 	mutex_unlock(&clocks_mutex);
 
@@ -75,6 +88,27 @@ found:
 }
 EXPORT_SYMBOL(clk_get);
 
+/**
+ * omap_clk_associate - associates a user to a clock so device drivers don't
+ * have to care about clock names
+ *
+ * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
+ * @dev: device pointer for the clock user
+ * @f: a function for the clock (uart_[if]ck, musb_ick, ehci_[if]ck, etc)
+ */
+void __init omap_clk_associate(const char *id, struct device *dev, const char *f)
+{
+	struct clk *clk = clk_get(NULL, id);
+
+	if (!dev || !clk || !IS_ERR(clk_get(dev, f)))
+		return;
+
+	clk->function = f;
+	clk->dev = dev;
+
+	clk_put(clk);
+};
+
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 9088925..21f18ca 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -13,6 +13,8 @@
 #ifndef __ARCH_ARM_OMAP_CLOCK_H
 #define __ARCH_ARM_OMAP_CLOCK_H
 
+#include <linux/device.h>
+
 struct module;
 struct clk;
 struct clockdomain;
@@ -61,8 +63,10 @@ struct dpll_data {
 
 struct clk {
 	struct list_head	node;
+	struct device		*dev;
 	struct module		*owner;
 	const char		*name;
+	const char		*function;
 	int			id;
 	struct clk		*parent;
 	unsigned long		rate;
@@ -116,6 +120,8 @@ struct clk_functions {
 
 extern unsigned int mpurate;
 
+extern void omap_clk_associate(const char *id,
+		struct device *dev, const char *func);
 extern int clk_init(struct clk_functions *custom_clocks);
 extern int clk_register(struct clk *clk);
 extern void clk_unregister(struct clk *clk);
-- 
1.6.0.1.196.g01914

[PATCH 2/3] clk: use clk_associate for musb driver

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Let musb use clk_associate to avoid introducing non-standard
clk functions and passing a clk name via pdata.

Also introduce txt file to track clk function names.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 Documentation/arm/OMAP/clk_function_name.txt |    9 +++++++
 arch/arm/mach-omap2/board-n800-usb.c         |   26 -------------------
 arch/arm/mach-omap2/usb-musb.c               |   34 +++++++-------------------
 arch/arm/mach-omap2/usb-tusb6010.c           |    2 +
 drivers/usb/musb/musb_core.c                 |   23 +++++------------
 5 files changed, 27 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/arm/OMAP/clk_function_name.txt

diff --git a/Documentation/arm/OMAP/clk_function_name.txt b/Documentation/arm/OMAP/clk_function_name.txt
new file mode 100644
index 0000000..0828812
--- /dev/null
+++ b/Documentation/arm/OMAP/clk_function_name.txt
@@ -0,0 +1,9 @@
+			OMAP Clock Function Names
+
+When moving a new driver to use clk_associate, please add here the clk
+function name given to the clks in omap1, omap2 and omap3.
+
+
+Function name		|     OMAP1	|         OMAP2		|      OMAP3
+-------------------------------------------------------------------------------
+musb_ick		|	-	|   osc_ck/usbhs_ick	|   hsotgusb_ick
diff --git a/arch/arm/mach-omap2/board-n800-usb.c b/arch/arm/mach-omap2/board-n800-usb.c
index f8df19e..820f285 100644
--- a/arch/arm/mach-omap2/board-n800-usb.c
+++ b/arch/arm/mach-omap2/board-n800-usb.c
@@ -25,7 +25,6 @@
 #define GPIO_TUSB_ENABLE	0
 
 static int tusb_set_power(int state);
-static int tusb_set_clock(struct clk *osc_ck, int state);
 
 #if	defined(CONFIG_USB_MUSB_OTG)
 #	define BOARD_MODE	MUSB_OTG
@@ -82,10 +81,8 @@ static struct musb_hdrc_config musb_config = {
 static struct musb_hdrc_platform_data tusb_data = {
 	.mode		= BOARD_MODE,
 	.set_power	= tusb_set_power,
-	.set_clock	= tusb_set_clock,
 	.min_power	= 25,	/* x2 = 50 mA drawn from VBUS as peripheral */
 	.power		= 100,	/* Max 100 mA VBUS for host mode */
-	.clock		= "osc_ck",
 	.config		= &musb_config,
 };
 
@@ -121,29 +118,6 @@ static int tusb_set_power(int state)
 	return retval;
 }
 
-static int		osc_ck_on;
-
-static int tusb_set_clock(struct clk *osc_ck, int state)
-{
-	if (state) {
-		if (osc_ck_on > 0)
-			return -ENODEV;
-
-		omap2_block_sleep();
-		clk_enable(osc_ck);
-		osc_ck_on = 1;
-	} else {
-		if (osc_ck_on == 0)
-			return -ENODEV;
-
-		clk_disable(osc_ck);
-		osc_ck_on = 0;
-		omap2_allow_sleep();
-	}
-
-	return 0;
-}
-
 void __init n800_usb_init(void)
 {
 	int ret = 0;
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 61c5709..44f2ab0 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -31,6 +31,7 @@
 #include <mach/pm.h>
 #include <mach/mux.h>
 #include <mach/usb.h>
+#include <mach/clock.h>
 
 #ifdef CONFIG_USB_MUSB_SOC
 static struct resource musb_resources[] = {
@@ -53,27 +54,6 @@ static struct resource musb_resources[] = {
 	},
 };
 
-static int clk_on;
-
-static int musb_set_clock(struct clk *clk, int state)
-{
-	if (state) {
-		if (clk_on > 0)
-			return -ENODEV;
-
-		clk_enable(clk);
-		clk_on = 1;
-	} else {
-		if (clk_on == 0)
-			return -ENODEV;
-
-		clk_disable(clk);
-		clk_on = 0;
-	}
-
-	return 0;
-}
-
 static struct musb_hdrc_eps_bits musb_eps[] = {
 	{	"ep1_tx", 10,	},
 	{	"ep1_rx", 10,	},
@@ -127,10 +107,6 @@ static struct musb_hdrc_platform_data musb_plat = {
 #elif defined(CONFIG_USB_GADGET_MUSB_HDRC)
 	.mode		= MUSB_PERIPHERAL,
 #endif
-	.clock		= cpu_is_omap34xx()
-			? "hsotgusb_ick"
-			: "usbhs_ick",
-	.set_clock	= musb_set_clock,
 	.config		= &musb_config,
 
 	/* REVISIT charge pump on TWL4030 can supply up to
@@ -159,6 +135,14 @@ static struct platform_device musb_device = {
 void __init usb_musb_init(void)
 {
 #ifdef CONFIG_USB_MUSB_SOC
+	/* associate musb clocks with musb device */
+
+	if (cpu_is_omap34xx())
+		omap_clk_associate("hsotgusb_ick", &musb_device.dev, "musb_ick");
+
+	if (cpu_is_omap24xx())
+		omap_clk_associate("usbhs_ick", &musb_device.dev, "musb_ick");
+
 	if (platform_device_register(&musb_device) < 0) {
 		printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
 		return;
diff --git a/arch/arm/mach-omap2/usb-tusb6010.c b/arch/arm/mach-omap2/usb-tusb6010.c
index 2c88207..d69d26b 100644
--- a/arch/arm/mach-omap2/usb-tusb6010.c
+++ b/arch/arm/mach-omap2/usb-tusb6010.c
@@ -18,6 +18,7 @@
 #include <mach/gpmc.h>
 #include <mach/gpio.h>
 #include <mach/mux.h>
+#include <mach/clock.h>
 
 
 static u8		async_cs, sync_cs;
@@ -250,6 +251,7 @@ tusb6010_setup_interface(struct musb_hdrc_platform_data *data,
 		printk(error, 1, status);
 		return status;
 	}
+	omap_clk_associate("osc_ck", &tusb_device.dev, "musb_ick");
 	tusb_resources[0].end = tusb_resources[0].start + 0x9ff;
 	async_cs = async;
 	gpmc_cs_write_reg(async, GPMC_CS_CONFIG1,
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 128e949..4a9070e 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1922,7 +1922,6 @@ bad_config:
 	spin_lock_init(&musb->lock);
 	musb->board_mode = plat->mode;
 	musb->board_set_power = plat->set_power;
-	musb->set_clock = plat->set_clock;
 	musb->min_power = plat->min_power;
 
 	/* Clock usage is chip-specific ... functional clock (DaVinci,
@@ -1930,13 +1929,11 @@ bad_config:
 	 * code does is make sure a clock handle is available; platform
 	 * code manages it during start/stop and suspend/resume.
 	 */
-	if (plat->clock) {
-		musb->clock = clk_get(dev, plat->clock);
-		if (IS_ERR(musb->clock)) {
-			status = PTR_ERR(musb->clock);
-			musb->clock = NULL;
-			goto fail;
-		}
+	musb->clock = clk_get(dev, "musb_ick");
+	if (IS_ERR(musb->clock)) {
+		status = PTR_ERR(musb->clock);
+		musb->clock = NULL;
+		goto fail;
 	}
 
 	/* assume vbus is off */
@@ -2159,10 +2156,7 @@ static int musb_suspend(struct platform_device *pdev, pm_message_t message)
 		 */
 	}
 
-	if (musb->set_clock)
-		musb->set_clock(musb->clock, 0);
-	else
-		clk_disable(musb->clock);
+	clk_disable(musb->clock);
 	spin_unlock_irqrestore(&musb->lock, flags);
 	return 0;
 }
@@ -2177,10 +2171,7 @@ static int musb_resume(struct platform_device *pdev)
 
 	spin_lock_irqsave(&musb->lock, flags);
 
-	if (musb->set_clock)
-		musb->set_clock(musb->clock, 1);
-	else
-		clk_enable(musb->clock);
+	clk_enable(musb->clock);
 
 	/* for static cmos like DaVinci, register values were preserved
 	 * unless for some reason the whole soc powered down and we're
-- 
1.6.0.1.196.g01914

[PATCH 3/3] clk: use clk_associate on watchdog driver

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Get rid of clk mess on omap_watchdog driver by
using clk_associate.

Cc: Wim Van Sebroeck <wim@iguana.be>
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 Documentation/arm/OMAP/clk_function_name.txt |    4 +
 arch/arm/plat-omap/devices.c                 |   18 ++++--
 drivers/watchdog/omap_wdt.c                  |   87 ++++++++------------------
 3 files changed, 43 insertions(+), 66 deletions(-)

diff --git a/Documentation/arm/OMAP/clk_function_name.txt b/Documentation/arm/OMAP/clk_function_name.txt
index 0828812..f5ddcc6 100644
--- a/Documentation/arm/OMAP/clk_function_name.txt
+++ b/Documentation/arm/OMAP/clk_function_name.txt
@@ -7,3 +7,7 @@ function name given to the clks in omap1, omap2 and omap3.
 Function name		|     OMAP1	|         OMAP2		|      OMAP3
 -------------------------------------------------------------------------------
 musb_ick		|	-	|   osc_ck/usbhs_ick	|   hsotgusb_ick
+-------------------------------------------------------------------------------
+wdt_fck			| armwdt_ck	|   mpu_wdt_fck		|   wdt2_fck
+-------------------------------------------------------------------------------
+wdt_ick			|	-	|   mpu_wdt_fck		|   wdt2_ick
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index c22bd5f..c1e253c 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -298,16 +298,24 @@ static struct platform_device omap_wdt_device = {
 
 static void omap_init_wdt(void)
 {
-	if (cpu_is_omap16xx())
+	if (cpu_is_omap16xx()) {
+		omap_clk_associate("armwdt_ck", &omap_wdt_device.dev, "wdt_fck");
 		wdt_resources[0].start = 0xfffeb000;
-	else if (cpu_is_omap2420())
+	} else if (cpu_is_omap2420()) {
+		omap_clk_associate("mpu_wdt_fck", &omap_wdt_device.dev, "wdt_fck");
+		omap_clk_associate("mpu_wdt_ick", &omap_wdt_device.dev, "wdt_ick");
 		wdt_resources[0].start = 0x48022000; /* WDT2 */
-	else if (cpu_is_omap2430())
+	} else if (cpu_is_omap2430()) {
+		omap_clk_associate("mpu_wdt_fck", &omap_wdt_device.dev, "wdt_fck");
+		omap_clk_associate("mpu_wdt_ick", &omap_wdt_device.dev, "wdt_ick");
 		wdt_resources[0].start = 0x49016000; /* WDT2 */
-	else if (cpu_is_omap343x())
+	} else if (cpu_is_omap343x()) {
+		omap_clk_associate("wdt2_fck", &omap_wdt_device.dev, "wdt_fck");
+		omap_clk_associate("wdt2_ick", &omap_wdt_device.dev, "wdt_ick");
 		wdt_resources[0].start = 0x48314000; /* WDT2 */
-	else
+	} else {
 		return;
+	}
 
 	wdt_resources[0].end = wdt_resources[0].start + 0x4f;
 
diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 7bcbb7f..e31d7f5 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -60,9 +60,8 @@ struct omap_wdt_dev {
 	void __iomem    *base;          /* physical */
 	struct device   *dev;
 	int             omap_wdt_users;
-	struct clk      *armwdt_ck;
-	struct clk      *mpu_wdt_ick;
-	struct clk      *mpu_wdt_fck;
+	struct clk      *ick;
+	struct clk      *fck;
 	struct resource *mem;
 	struct miscdevice omap_wdt_miscdev;
 };
@@ -146,13 +145,10 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(1, (unsigned long *)&(wdev->omap_wdt_users)))
 		return -EBUSY;
 
-	if (cpu_is_omap16xx())
-		clk_enable(wdev->armwdt_ck);	/* Enable the clock */
-
-	if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-		clk_enable(wdev->mpu_wdt_ick);    /* Enable the interface clock */
-		clk_enable(wdev->mpu_wdt_fck);    /* Enable the functional clock */
-	}
+	if (wdev->fck)
+		clk_enable(wdev->fck);    /* Enable the functional clock */
+	if (wdev->ick)
+		clk_enable(wdev->ick);    /* Enable the interface clock */
 
 	/* initialize prescaler */
 	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
@@ -303,44 +299,20 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 	wdev->omap_wdt_users = 0;
 	wdev->mem = mem;
 
-	if (cpu_is_omap16xx()) {
-		wdev->armwdt_ck = clk_get(&pdev->dev, "armwdt_ck");
-		if (IS_ERR(wdev->armwdt_ck)) {
-			ret = PTR_ERR(wdev->armwdt_ck);
-			wdev->armwdt_ck = NULL;
-			goto err_clk;
-		}
+	wdev->fck = clk_get(&pdev->dev, "wdt_fck");
+	if (IS_ERR(wdev->fck)) {
+		ret = PTR_ERR(wdev->fck);
+		wdev->fck = NULL;
+		goto err_clk;
 	}
 
-	if (cpu_is_omap24xx()) {
-		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "mpu_wdt_ick");
-		if (IS_ERR(wdev->mpu_wdt_ick)) {
-			ret = PTR_ERR(wdev->mpu_wdt_ick);
-			wdev->mpu_wdt_ick = NULL;
-			goto err_clk;
-		}
-		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "mpu_wdt_fck");
-		if (IS_ERR(wdev->mpu_wdt_fck)) {
-			ret = PTR_ERR(wdev->mpu_wdt_fck);
-			wdev->mpu_wdt_fck = NULL;
-			goto err_clk;
-		}
+	wdev->ick = clk_get(&pdev->dev, "wdt_ick");
+	if (IS_ERR(wdev->ick)) {
+		ret = PTR_ERR(wdev->ick);
+		wdev->ick = NULL;
+		goto err_clk;
 	}
 
-	if (cpu_is_omap34xx()) {
-		wdev->mpu_wdt_ick = clk_get(&pdev->dev, "wdt2_ick");
-		if (IS_ERR(wdev->mpu_wdt_ick)) {
-			ret = PTR_ERR(wdev->mpu_wdt_ick);
-			wdev->mpu_wdt_ick = NULL;
-			goto err_clk;
-		}
-		wdev->mpu_wdt_fck = clk_get(&pdev->dev, "wdt2_fck");
-		if (IS_ERR(wdev->mpu_wdt_fck)) {
-			ret = PTR_ERR(wdev->mpu_wdt_fck);
-			wdev->mpu_wdt_fck = NULL;
-			goto err_clk;
-		}
-	}
 	wdev->base = ioremap(res->start, res->end - res->start + 1);
 	if (!wdev->base) {
 		ret = -ENOMEM;
@@ -380,12 +352,10 @@ err_ioremap:
 	wdev->base = NULL;
 
 err_clk:
-	if (wdev->armwdt_ck)
-		clk_put(wdev->armwdt_ck);
-	if (wdev->mpu_wdt_ick)
-		clk_put(wdev->mpu_wdt_ick);
-	if (wdev->mpu_wdt_fck)
-		clk_put(wdev->mpu_wdt_fck);
+	if (wdev->ick)
+		clk_put(wdev->ick);
+	if (wdev->fck)
+		clk_put(wdev->fck);
 	kfree(wdev);
 
 err_kzalloc:
@@ -417,19 +387,14 @@ static int omap_wdt_remove(struct platform_device *pdev)
 	release_mem_region(res->start, res->end - res->start + 1);
 	platform_set_drvdata(pdev, NULL);
 
-	if (wdev->armwdt_ck) {
-		clk_put(wdev->armwdt_ck);
-		wdev->armwdt_ck = NULL;
-	}
-
-	if (wdev->mpu_wdt_ick) {
-		clk_put(wdev->mpu_wdt_ick);
-		wdev->mpu_wdt_ick = NULL;
+	if (wdev->ick) {
+		clk_put(wdev->ick);
+		wdev->ick = NULL;
 	}
 
-	if (wdev->mpu_wdt_fck) {
-		clk_put(wdev->mpu_wdt_fck);
-		wdev->mpu_wdt_fck = NULL;
+	if (wdev->fck) {
+		clk_put(wdev->fck);
+		wdev->fck = NULL;
 	}
 	iounmap(wdev->base);
 
-- 
1.6.0.1.196.g01914

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Paul Walmsley]

Hi Felipe,

so I'll put most of my comments here.

On Tue, 7 Oct 2008, Felipe Balbi wrote:

> Introduce a new mechanism to omap's clk implementation to
> associate the device with its clock during platform_device
> registration. Also gives the clock a function name (like
> mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
> care about clk names any longer.

Let's make the function names shorter, like "fclk" and "iclk".  That 
should make them even easier to use in situations where the device name 
itself changes, e.g., "mmc"/"hsmmc" etc.  Plus the linker will be able to 
merge many them together into single constant strings.  For a device with 
multiple fclks like DSS, we can use "tv_fclk" also, etc.

> With this mechanism we can also be sure drivers won't be able
> to clk_get the wrong clk (only true when we move all drivers
> to this new mechanism and drop the old ones).
> 
> Clk's function names will be defined as they come necessary
> but let's try to keep it as simple as possible.
> 
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  arch/arm/plat-omap/clock.c              |   38 +++++++++++++++++++++++++++++-
>  arch/arm/plat-omap/include/mach/clock.h |    6 +++++
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index 7bbfba2..c090f23 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -43,7 +43,7 @@ static struct clk_functions *arch_clock;
>   */
>  struct clk * clk_get(struct device *dev, const char *id)
>  {
> -	struct clk *p, *clk = ERR_PTR(-ENOENT);
> +	struct clk *p, *clk;
>  	int idno;
>  
>  	if (dev == NULL || dev->bus != &platform_bus_type)
> @@ -53,6 +53,15 @@ struct clk * clk_get(struct device *dev, const char *id)
>  
>  	mutex_lock(&clocks_mutex);
>  
> +	list_for_each_entry(clk, &clocks, node) {
> +		if (clk->function && (dev == clk->dev) &&
> +				strcmp(id, clk->function) == 0)
> +			goto found;

Please avoid the gotos and use the previously-used form for returning 
success, e.g., iterate on p; if found, then assign to clk, and break.

Also, is there some reason why there are two list_for_each_entry() blocks?  
Those should be merged into one.

> +
> +		if (strcmp(id, clk->name) == 0)
> +			goto found;

Doesn't the following code already handle the above case?

> +	}
> +
>  	list_for_each_entry(p, &clocks, node) {
>  		if (p->id == idno &&
>  		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
> @@ -64,10 +73,14 @@ struct clk * clk_get(struct device *dev, const char *id)
>  	list_for_each_entry(p, &clocks, node) {
>  		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
>  			clk = p;
> -			break;
> +			goto found;
>  		}
>  	}
>  
> +	mutex_unlock(&clocks_mutex);
> +
> +	return ERR_PTR(-ENOENT);

the above two lines will then become unnecessary.

> +
>  found:
>  	mutex_unlock(&clocks_mutex);
>  
> @@ -75,6 +88,27 @@ found:
>  }
>  EXPORT_SYMBOL(clk_get);
>  
> +/**

First, thank you for using kerneldoc.  But ...

> + * omap_clk_associate - associates a user to a clock so device drivers don't
> + * have to care about clock names

... Documentation/kernel-doc-nano-HOWTO.txt requires the short function 
description to fit on one line.  If you need more room, please put the 
larger description after a blank line after the last argument 
documentation line (e.g., line beginning with '@').

> + *

This blank line needs to be removed per kerneldoc - "The @argument 
descriptions must begin on the very next line ..."

> + * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
> + * @dev: device pointer for the clock user
> + * @f: a function for the clock (uart_[if]ck, musb_ick, ehci_[if]ck, etc)
> + */
> +void __init omap_clk_associate(const char *id, struct device *dev, const char *f)
> +{
> +	struct clk *clk = clk_get(NULL, id);
> +
> +	if (!dev || !clk || !IS_ERR(clk_get(dev, f)))
> +		return;

Please break the clk_get() test above out into its own statement, and 
clk_put() it before returning.

> +
> +	clk->function = f;
> +	clk->dev = dev;

There needs to be a test before these lines to ensure that some driver has 
not already associated a function with this clock, or a device with this 
clock, and to WARN_ON(1) if it has.

But there seems to be a deeper problem.  What happens when multiple device 
drivers want to associate to the same clock?  osc_ck is the pathological 
case.  Seems like you'll need a different data structure, like a list, to 
store in the struct clk.

> +
> +	clk_put(clk);
> +};
> +
>  int clk_enable(struct clk *clk)
>  {
>  	unsigned long flags;
> diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
> index 9088925..21f18ca 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -13,6 +13,8 @@
>  #ifndef __ARCH_ARM_OMAP_CLOCK_H
>  #define __ARCH_ARM_OMAP_CLOCK_H
>  
> +#include <linux/device.h>
> +
>  struct module;
>  struct clk;
>  struct clockdomain;
> @@ -61,8 +63,10 @@ struct dpll_data {
>  
>  struct clk {
>  	struct list_head	node;
> +	struct device		*dev;
>  	struct module		*owner;
>  	const char		*name;
> +	const char		*function;
>  	int			id;
>  	struct clk		*parent;
>  	unsigned long		rate;
> @@ -116,6 +120,8 @@ struct clk_functions {
>  
>  extern unsigned int mpurate;
>  
> +extern void omap_clk_associate(const char *id,
> +		struct device *dev, const char *func);
>  extern int clk_init(struct clk_functions *custom_clocks);
>  extern int clk_register(struct clk *clk);
>  extern void clk_unregister(struct clk *clk);
> -- 
> 1.6.0.1.196.g01914
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


- Paul

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

On Tue, Oct 14, 2008 at 11:08:48AM -0600, Paul Walmsley wrote:
> Hi Felipe,
> 
> so I'll put most of my comments here.
> 
> On Tue, 7 Oct 2008, Felipe Balbi wrote:
> 
> > Introduce a new mechanism to omap's clk implementation to
> > associate the device with its clock during platform_device
> > registration. Also gives the clock a function name (like
> > mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
> > care about clk names any longer.
> 
> Let's make the function names shorter, like "fclk" and "iclk".  That 
> should make them even easier to use in situations where the device name 
> itself changes, e.g., "mmc"/"hsmmc" etc.  Plus the linker will be able to 
> merge many them together into single constant strings.  For a device with 
> multiple fclks like DSS, we can use "tv_fclk" also, etc.

I didn't quite get you here. The idea of mmc_fck is so that

clk_get(dev, "mmc_fck");

works fine and returns the correct clock. If we have several fck and ick
function names, how will we clk_get() the right one ??

> > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> > index 7bbfba2..c090f23 100644
> > --- a/arch/arm/plat-omap/clock.c
> > +++ b/arch/arm/plat-omap/clock.c
> > @@ -43,7 +43,7 @@ static struct clk_functions *arch_clock;
> >   */
> >  struct clk * clk_get(struct device *dev, const char *id)
> >  {
> > -	struct clk *p, *clk = ERR_PTR(-ENOENT);
> > +	struct clk *p, *clk;
> >  	int idno;
> >  
> >  	if (dev == NULL || dev->bus != &platform_bus_type)
> > @@ -53,6 +53,15 @@ struct clk * clk_get(struct device *dev, const char *id)
> >  
> >  	mutex_lock(&clocks_mutex);
> >  
> > +	list_for_each_entry(clk, &clocks, node) {
> > +		if (clk->function && (dev == clk->dev) &&
> > +				strcmp(id, clk->function) == 0)
> > +			goto found;
> 
> Please avoid the gotos and use the previously-used form for returning 
> success, e.g., iterate on p; if found, then assign to clk, and break.
> 
> Also, is there some reason why there are two list_for_each_entry() blocks?  
> Those should be merged into one.

Will do.

> 
> > +
> > +		if (strcmp(id, clk->name) == 0)
> > +			goto found;
> 
> Doesn't the following code already handle the above case?

my bad, when merging both list_for_each_entry() this will disappear.

> > +/**
> 
> First, thank you for using kerneldoc.  But ...
> 
> > + * omap_clk_associate - associates a user to a clock so device drivers don't
> > + * have to care about clock names
> 
> ... Documentation/kernel-doc-nano-HOWTO.txt requires the short function 
> description to fit on one line.  If you need more room, please put the 
> larger description after a blank line after the last argument 
> documentation line (e.g., line beginning with '@').

Will fix.

> 
> > + *
> 
> This blank line needs to be removed per kerneldoc - "The @argument 
> descriptions must begin on the very next line ..."
> 
> > + * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
> > + * @dev: device pointer for the clock user
> > + * @f: a function for the clock (uart_[if]ck, musb_ick, ehci_[if]ck, etc)
> > + */
> > +void __init omap_clk_associate(const char *id, struct device *dev, const char *f)
> > +{
> > +	struct clk *clk = clk_get(NULL, id);
> > +
> > +	if (!dev || !clk || !IS_ERR(clk_get(dev, f)))
> > +		return;
> 
> Please break the clk_get() test above out into its own statement, and 
> clk_put() it before returning.

ok.

> There needs to be a test before these lines to ensure that some driver has 
> not already associated a function with this clock, or a device with this 
> clock, and to WARN_ON(1) if it has.

sounds good.

> But there seems to be a deeper problem.  What happens when multiple device 
> drivers want to associate to the same clock?  osc_ck is the pathological 
> case.  Seems like you'll need a different data structure, like a list, to 
> store in the struct clk.

Yeah, have to think about that, but then again, how can several users
concurrently enable and disable the same clock ?

I mean, imagine driver A clk_enable(osc_ck) and while needing that
clock, driver B clk_disable(osc_ck). That would break driver A, right ?

How is omap clk fw handling that case right now ? I'd say we should have
one user per clk and in the case of osc_ck, that would be a clk input
for generating another clk, or something like that, so driver A can
clk_enable(osc_ck_A) and yet driver B could clk_disable(osc_ck_B) and
everything still works.

-- 
balbi

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[David Brownell]

On Tuesday 14 October 2008, Felipe Balbi wrote:
> I didn't quite get you here. The idea of mmc_fck is so that
> 
> clk_get(dev, "mmc_fck");
> 
> works fine and returns the correct clock. If we have several fck and ick
> function names, how will we clk_get() the right one ??

If "dev" is an MMC device, there's no way to confuse
its "fck" and "ick" with those for, say, I2C.   Right?
That's the whole point of associating logical clock
names with the device.

And as Paul noted, if a device has several such clocks,
then it needs several such names.

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

On Tue, Oct 14, 2008 at 10:47:44AM -0700, David Brownell wrote:
> On Tuesday 14 October 2008, Felipe Balbi wrote:
> > I didn't quite get you here. The idea of mmc_fck is so that
> > 
> > clk_get(dev, "mmc_fck");
> > 
> > works fine and returns the correct clock. If we have several fck and ick
> > function names, how will we clk_get() the right one ??
> 
> If "dev" is an MMC device, there's no way to confuse
> its "fck" and "ick" with those for, say, I2C.   Right?
> That's the whole point of associating logical clock
> names with the device.
> 
> And as Paul noted, if a device has several such clocks,
> then it needs several such names.

hmm... that's true. Forgot about matching dev as well :-p
hehehe. Makes sense to me, let's use fclk and iclk then :-)

The main idea then would be that clk(dev, "iclk") translates to english
into "get me the interface clock of mmc device" (when dev is an mmc
device, of course).

-- 
balbi

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [081014 12:31]:
> On Tue, Oct 14, 2008 at 10:47:44AM -0700, David Brownell wrote:
> > On Tuesday 14 October 2008, Felipe Balbi wrote:
> > > I didn't quite get you here. The idea of mmc_fck is so that
> > > 
> > > clk_get(dev, "mmc_fck");
> > > 
> > > works fine and returns the correct clock. If we have several fck and ick
> > > function names, how will we clk_get() the right one ??
> > 
> > If "dev" is an MMC device, there's no way to confuse
> > its "fck" and "ick" with those for, say, I2C.   Right?
> > That's the whole point of associating logical clock
> > names with the device.
> > 
> > And as Paul noted, if a device has several such clocks,
> > then it needs several such names.
> 
> hmm... that's true. Forgot about matching dev as well :-p
> hehehe. Makes sense to me, let's use fclk and iclk then :-)
> 
> The main idea then would be that clk(dev, "iclk") translates to english
> into "get me the interface clock of mmc device" (when dev is an mmc
> device, of course).

And that we can use same naming in the driver no matter which omap :)

Tony

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Igor Stoppa]

On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:

> And that we can use same naming in the driver no matter which omap :)

Wasn't that one of the main features of the clock FW, when it was
designed?

-- 

Cheers, Igor

---

Igor Stoppa
Maemo Software - Nokia Devices R&D - Helsinki

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Tony Lindgren]

* Igor Stoppa <igor.stoppa@nokia.com> [081014 14:12]:
> On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:
> 
> > And that we can use same naming in the driver no matter which omap :)
> 
> Wasn't that one of the main features of the clock FW, when it was
> designed?

Yes. But we adding separate ick and fck during omap2420 confused that.

Tony

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

On Tue, Oct 14, 2008 at 02:24:10PM -0700, Tony Lindgren wrote:
> * Igor Stoppa <igor.stoppa@nokia.com> [081014 14:12]:
> > On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:
> > 
> > > And that we can use same naming in the driver no matter which omap :)
> > 
> > Wasn't that one of the main features of the clock FW, when it was
> > designed?
> 
> Yes. But we adding separate ick and fck during omap2420 confused that.

Also, it's useful to keep the name shown in TRM to ease the task of
searching TRM for information about that particular clock.

-- 
balbi

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Högander Jouni]

"ext Tony Lindgren" <tony@atomide.com> writes:

> * Igor Stoppa <igor.stoppa@nokia.com> [081014 14:12]:
>> On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:
>> 
>> > And that we can use same naming in the driver no matter which omap :)
>> 
>> Wasn't that one of the main features of the clock FW, when it was
>> designed?
>
> Yes. But we adding separate ick and fck during omap2420 confused
> that.

Why this was originally done? I mean we have possibility to autogate
interface clocks. AFAIK those could be just fine be enabled on boot and let
the hw to take care of them.

-- 
Jouni Högander

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1886 bytes --]

Hello Jouni,

On Wed, 15 Oct 2008, Högander Jouni wrote:

> "ext Tony Lindgren" <tony@atomide.com> writes:
> 
> > * Igor Stoppa <igor.stoppa@nokia.com> [081014 14:12]:
> >> On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:
> >> 
> >> > And that we can use same naming in the driver no matter which omap :)
> >> 
> >> Wasn't that one of the main features of the clock FW, when it was
> >> designed?
> >
> > Yes. But we adding separate ick and fck during omap2420 confused
> > that.
> 
> Why this was originally done? I mean we have possibility to autogate
> interface clocks. AFAIK those could be just fine be enabled on boot and let
> the hw to take care of them.

Richard is really the best person to ask about this, so he's been added to 
the cc.  My current understanding is below.  Perhaps he can respond with 
more detail and correct any errors.

In terms of original motivation, you might want to look at 34xx TRM 
4.12.2.4.3.  I don't think this actually works in practice on OMAP3.

Current practical use is different.  Autogating only takes effect during 
sleep & wakeup transitions for the entire CORE_Lx clockdomain (34xx TRM 
4.12.2.4.4).  So even if a module's functional clocks are disabled, the 
PRCM won't autogate the module's interface clock until the entire CORE_Lx 
clockdomain transitions to inactive.  The theory here is that this wastes 
power and that manually disabling the interface clock when the device is 
known to be inactive should save some power.

There also was some discussion last year that modules like GPTIMER can 
have their functional clocks enabled, but their interface clocks disabled 
when register access is not needed.  Wakeups from the module are still 
sent asynchronously to the PRCM.  Again the motivation is to save power.  
I don't think the current code tries to do this.


- Paul

RE: [PATCH 1/3] clk: introduce omap_clk_associate

[Woodruff, Richard]

> >> Wasn't that one of the main features of the clock FW, when it was
> >> designed?
> >
> > Yes. But we adding separate ick and fck during omap2420 confused
> > that.
>
> Why this was originally done? I mean we have possibility to autogate
> interface clocks. AFAIK those could be just fine be enabled on boot and
> let
> the hw to take care of them.

Autogating only happens when you get all preconditions met.  For much of OMAP2 time this was not the case.  For l-o I’m not sure today if it is yet the case.  For OMAP3 this has been a target.

You do need selective control for some modules.

The ideal should be to just control F and let I be auto handled.  In retrospect something like v-clock's which handle the set for a module, but also have explicit access when its needed to individual ones.  Perhaps a v clock with separate id's for individual clocks...  Some of this may be good for next gen omaps.

Regards,
Richard W.

RE: [PATCH 1/3] clk: introduce omap_clk_associate

[Woodruff, Richard]

> Richard is really the best person to ask about this, so he's been added
> to
> the cc.  My current understanding is below.  Perhaps he can respond with
> more detail and correct any errors.
>
> In terms of original motivation, you might want to look at 34xx TRM
> 4.12.2.4.3.  I don't think this actually works in practice on OMAP3.

See other mail response.

Paul hit some major points.

We do target auto handling on I-Clocks for active use cases.  We depend on it to hit our given power targets for things like MP3.  You can't handle in software all I clocks and even get close.  All drivers have to be just so to make it work.  Having individual control makes it possible especially for ones which are bugged in some fashion (hw or software).

Moving to some combined clock for the general case and having specific control of others seems like a good future goal.

Back in omap2 time the hardware designers did indicate they added all this flexibility as a way to compensate for possible design errors.  This was a correct and needed a lot in omap2 and somewhat in omap3 to meet aggressive power targets.  Our feed back at the time was that’s all fine, but it would have better to have a standard 'simple' interface which could be bypassed for direct usage if bugs/flexibility were needed.  All the variables which exist can be confusing with out a lot of explaining.

Regards,
Richard W.

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Högander Jouni]

"ext Paul Walmsley" <paul@pwsan.com> writes:

> Hello Jouni,
>
> On Wed, 15 Oct 2008, Högander Jouni wrote:
>
>> "ext Tony Lindgren" <tony@atomide.com> writes:
>> 
>> > * Igor Stoppa <igor.stoppa@nokia.com> [081014 14:12]:
>> >> On Tue, 2008-10-14 at 13:59 -0700, ext Tony Lindgren wrote:
>> >> 
>> >> > And that we can use same naming in the driver no matter which omap :)
>> >> 
>> >> Wasn't that one of the main features of the clock FW, when it was
>> >> designed?
>> >
>> > Yes. But we adding separate ick and fck during omap2420 confused
>> > that.
>> 
>> Why this was originally done? I mean we have possibility to autogate
>> interface clocks. AFAIK those could be just fine be enabled on boot and let
>> the hw to take care of them.
>
> Richard is really the best person to ask about this, so he's been added to 
> the cc.  My current understanding is below.  Perhaps he can respond with 
> more detail and correct any errors.
>
> In terms of original motivation, you might want to look at 34xx TRM 
> 4.12.2.4.3.  I don't think this actually works in practice on OMAP3.
>
> Current practical use is different.  Autogating only takes effect during 
> sleep & wakeup transitions for the entire CORE_Lx clockdomain (34xx TRM 
> 4.12.2.4.4).  So even if a module's functional clocks are disabled, the 
> PRCM won't autogate the module's interface clock until the entire CORE_Lx 
> clockdomain transitions to inactive.  The theory here is that this wastes 
> power and that manually disabling the interface clock when the device is 
> known to be inactive should save some power.

L3 and L4 are anyway on when not in wfi, because of sleep/wkdep, so no
power save. This is the case at least with OMAP3 not sure about OMAP2.

>
> There also was some discussion last year that modules like GPTIMER can 
> have their functional clocks enabled, but their interface clocks disabled 
> when register access is not needed.  Wakeups from the module are still 
> sent asynchronously to the PRCM.  Again the motivation is to save power.  
> I don't think the current code tries to do this.
>

In many modules there is also possibility to gate ick by HW when not
in wfi (see AUTOIDLE in SYCONFIG). I'm not sure about this but
extremely also fcks could be taken care by HW (see CLOCKACTIVITY in
SYSCONFIG).

>
> - Paul

-- 
Jouni Högander

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1030 bytes --]

On Wed, 15 Oct 2008, Högander Jouni wrote:

> L3 and L4 are anyway on when not in wfi, because of sleep/wkdep, so no
> power save. This is the case at least with OMAP3 not sure about OMAP2.

I thought that disabling the iclken bit still disabled some gates 
in the OCP interface.  Richard, do you know if that is the case?

> > There also was some discussion last year that modules like GPTIMER can 
> > have their functional clocks enabled, but their interface clocks disabled 
> > when register access is not needed.  Wakeups from the module are still 
> > sent asynchronously to the PRCM.  Again the motivation is to save power.  
> > I don't think the current code tries to do this.
> >
> 
> In many modules there is also possibility to gate ick by HW when not
> in wfi (see AUTOIDLE in SYCONFIG). I'm not sure about this but
> extremely also fcks could be taken care by HW (see CLOCKACTIVITY in
> SYSCONFIG).

How will the PRCM know when the device driver is accessing the module 
registers?


- Paul

RE: [PATCH 1/3] clk: introduce omap_clk_associate

[Woodruff, Richard]

> From: Högander Jouni [mailto:jouni.hogander@nokia.com]
> Sent: Wednesday, October 15, 2008 8:04 AM

> L3 and L4 are anyway on when not in wfi, because of sleep/wkdep, so no
> power save. This is the case at least with OMAP3 not sure about OMAP2.

Probably some savings inside of individual domains when they are in idle like camera.  Defiantly internal autogating in CortexA8 saves power if MPU domain is in idle.  ... Recall if DSP is up and running L3/L4 might be up, but MPU could be in WFI with its domain gated off (also in INACTIVE/RET/OFF).

A 1st order power waster is fan out from L3/L4 as you say.

Future OMAPs add more domains to reduce fan out effect even if L3/L4 is on.

> In many modules there is also possibility to gate ick by HW when not
> in wfi (see AUTOIDLE in SYCONFIG). I'm not sure about this but
> extremely also fcks could be taken care by HW (see CLOCKACTIVITY in
> SYSCONFIG).

Module AUTOIDLE can save power by selective gating inside the module.  An easy experiment is to measure power while stopped in the debugger and enable/disable individual auto idles.  You'll be surprised how much you get back on something like security accelerators (all I-clock, so gating inside of modules helps, even if l3/l4 is up).

Clock activity in some modules does do extra gating at segment idle time.

Regards,
Richard W.

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Paul Walmsley]

On Tue, 14 Oct 2008, Felipe Balbi wrote:

> On Tue, Oct 14, 2008 at 11:08:48AM -0600, Paul Walmsley wrote:
> > On Tue, 7 Oct 2008, Felipe Balbi wrote:
> > 
> > > Introduce a new mechanism to omap's clk implementation to
> > > associate the device with its clock during platform_device
> > > registration. Also gives the clock a function name (like
> > > mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
> > > care about clk names any longer.
> > 
> > Let's make the function names shorter, like "fclk" and "iclk".  That 
> > should make them even easier to use in situations where the device name 
> > itself changes, e.g., "mmc"/"hsmmc" etc.  Plus the linker will be able to 
> > merge many them together into single constant strings.  For a device with 
> > multiple fclks like DSS, we can use "tv_fclk" also, etc.
> 
> I didn't quite get you here. The idea of mmc_fck is so that
> 
> clk_get(dev, "mmc_fck");
> 
> works fine and returns the correct clock. If we have several fck and ick
> function names, how will we clk_get() the right one ??

sounds like David has already clarified this; we can discuss further if 
not.

> > > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> > > index 7bbfba2..c090f23 100644
> > > --- a/arch/arm/plat-omap/clock.c
> > > +++ b/arch/arm/plat-omap/clock.c
> > > + * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
> > > + * @dev: device pointer for the clock user
> > > + * @f: a function for the clock (uart_[if]ck, musb_ick, ehci_[if]ck, etc)
> > > + */
> > > +void __init omap_clk_associate(const char *id, struct device *dev, const char *f)
> > > +{
> > > +	struct clk *clk = clk_get(NULL, id);
> > > +
> > > +	if (!dev || !clk || !IS_ERR(clk_get(dev, f)))
> > > +		return;
> 
> > But there seems to be a deeper problem.  What happens when multiple device 
> > drivers want to associate to the same clock?  osc_ck is the pathological 
> > case.  Seems like you'll need a different data structure, like a list, to 
> > store in the struct clk.
> 
> Yeah, have to think about that, but then again, how can several users
> concurrently enable and disable the same clock ?
> 
> I mean, imagine driver A clk_enable(osc_ck) and while needing that
> clock, driver B clk_disable(osc_ck). That would break driver A, right ?

no - see below

> How is omap clk fw handling that case right now ? I'd say we should have
> one user per clk and in the case of osc_ck, that would be a clk input
> for generating another clk, or something like that, so driver A can
> clk_enable(osc_ck_A) and yet driver B could clk_disable(osc_ck_B) and
> everything still works.

clk_enable() increments the struct clk.usecount field each time it is 
called.  clk_disable() decrements it, and only disables the clock when it 
reaches 0.  The code that does this is in the first few lines of 
omap2_clk_enable() and omap2_clk_disable() in arch/arm/mach-omap2/clock.c.

[ In the medium term we will probably switch away from using clk_enable() 
and clk_disable() on osc_ck to keep the external oscillator powered on. 
But this could still apply to other clocks, e.g., boards could have 
multiple devices hanging off of the sys_clkoutX lines, etc. ]


- Paul

[PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Introduce a new mechanism to omap's clk implementation to
associate the device with its clock during platform_device
registration. Also gives the clock a function name (like
mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
care about clk names any longer.

With this mechanism we can also be sure drivers won't be able
to clk_get the wrong clk (only true when we move all drivers
to this new mechanism and drop the old ones).

Clk's function names will be defined as they come necessary
but let's try to keep it as simple as possible.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/clock.c              |   47 ++++++++++++++++++++++++++++--
 arch/arm/plat-omap/include/mach/clock.h |    4 ++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 5178701..083a3ff 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -44,6 +44,7 @@ static struct clk_functions *arch_clock;
 struct clk * clk_get(struct device *dev, const char *id)
 {
 	struct clk *p, *clk = ERR_PTR(-ENOENT);
+	struct device *d;
 	int idno;
 
 	if (dev == NULL || dev->bus != &platform_bus_type)
@@ -54,27 +55,65 @@ struct clk * clk_get(struct device *dev, const char *id)
 	mutex_lock(&clocks_mutex);
 
 	list_for_each_entry(p, &clocks, node) {
+		list_for_each_entry(d, &p->consumers, node) {
+			if (dev == d) {
+				clk = p;
+				break;
+			}
+		}
+
 		if (p->id == idno &&
 		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
 			clk = p;
-			goto found;
+			break;
 		}
-	}
 
-	list_for_each_entry(p, &clocks, node) {
 		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
 			clk = p;
 			break;
 		}
 	}
 
-found:
 	mutex_unlock(&clocks_mutex);
 
 	return clk;
 }
 EXPORT_SYMBOL(clk_get);
 
+/**
+ * omap_clk_associate - associates a clock to its consumer device(s)
+ * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
+ * @dev: device pointer for the clock consumer
+ *
+ * This function attempts to simplify clock handling for device drivers
+ * by letting struct clk know which device is the correct consumer for
+ * that clock. By making this trick, we let drivers get the correct
+ * clock without knowing about different clock names between omap
+ * versions.
+ */
+void __init omap_clk_associate(const char *id, struct device *dev)
+{
+	struct clk *clk = clk_get(NULL, id);
+
+	if (!clk || IS_ERR(clk)) {
+		pr_debug("%s: unable to get clock\n", __func__);
+		return;
+	}
+
+	if (!dev) {
+		pr_debug("%s: missing device to associate with\n", __func__);
+		clk_put(clk);
+		return;
+	}
+
+	if (list_empty(&clk->consumers))
+		INIT_LIST_HEAD(&clk->consumers);
+
+	/* add device to consumer list */
+	list_add(&dev->node, &clk->consumers);
+	clk_put(clk);
+};
+
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 9088925..1600a14 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -13,6 +13,8 @@
 #ifndef __ARCH_ARM_OMAP_CLOCK_H
 #define __ARCH_ARM_OMAP_CLOCK_H
 
+#include <linux/device.h>
+
 struct module;
 struct clk;
 struct clockdomain;
@@ -61,6 +63,7 @@ struct dpll_data {
 
 struct clk {
 	struct list_head	node;
+	struct list_head	consumers;
 	struct module		*owner;
 	const char		*name;
 	int			id;
@@ -116,6 +119,7 @@ struct clk_functions {
 
 extern unsigned int mpurate;
 
+extern void omap_clk_associate(const char *id, struct device *dev);
 extern int clk_init(struct clk_functions *custom_clocks);
 extern int clk_register(struct clk *clk);
 extern void clk_unregister(struct clk *clk);
-- 
1.6.0.2.307.gc427

[PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Introduce a new mechanism to omap's clk implementation to
associate consumer device(s) with its clock during
platform_device registration.

With this mechanism we can also be sure drivers won't be able
to clk_get the wrong clk (only true when we move all drivers
to this new mechanism and drop the old ones).

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/clock.c              |   47 ++++++++++++++++++++++++++++--
 arch/arm/plat-omap/include/mach/clock.h |    4 ++
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 5178701..083a3ff 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -44,6 +44,7 @@ static struct clk_functions *arch_clock;
 struct clk * clk_get(struct device *dev, const char *id)
 {
 	struct clk *p, *clk = ERR_PTR(-ENOENT);
+	struct device *d;
 	int idno;
 
 	if (dev == NULL || dev->bus != &platform_bus_type)
@@ -54,27 +55,65 @@ struct clk * clk_get(struct device *dev, const char *id)
 	mutex_lock(&clocks_mutex);
 
 	list_for_each_entry(p, &clocks, node) {
+		list_for_each_entry(d, &p->consumers, node) {
+			if (dev == d) {
+				clk = p;
+				break;
+			}
+		}
+
 		if (p->id == idno &&
 		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
 			clk = p;
-			goto found;
+			break;
 		}
-	}
 
-	list_for_each_entry(p, &clocks, node) {
 		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
 			clk = p;
 			break;
 		}
 	}
 
-found:
 	mutex_unlock(&clocks_mutex);
 
 	return clk;
 }
 EXPORT_SYMBOL(clk_get);
 
+/**
+ * omap_clk_associate - associates a clock to its consumer device(s)
+ * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
+ * @dev: device pointer for the clock consumer
+ *
+ * This function attempts to simplify clock handling for device drivers
+ * by letting struct clk know which device is the correct consumer for
+ * that clock. By making this trick, we let drivers get the correct
+ * clock without knowing about different clock names between omap
+ * versions.
+ */
+void __init omap_clk_associate(const char *id, struct device *dev)
+{
+	struct clk *clk = clk_get(NULL, id);
+
+	if (!clk || IS_ERR(clk)) {
+		pr_debug("%s: unable to get clock\n", __func__);
+		return;
+	}
+
+	if (!dev) {
+		pr_debug("%s: missing device to associate with\n", __func__);
+		clk_put(clk);
+		return;
+	}
+
+	if (list_empty(&clk->consumers))
+		INIT_LIST_HEAD(&clk->consumers);
+
+	/* add device to consumer list */
+	list_add(&dev->node, &clk->consumers);
+	clk_put(clk);
+};
+
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 9088925..1600a14 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -13,6 +13,8 @@
 #ifndef __ARCH_ARM_OMAP_CLOCK_H
 #define __ARCH_ARM_OMAP_CLOCK_H
 
+#include <linux/device.h>
+
 struct module;
 struct clk;
 struct clockdomain;
@@ -61,6 +63,7 @@ struct dpll_data {
 
 struct clk {
 	struct list_head	node;
+	struct list_head	consumers;
 	struct module		*owner;
 	const char		*name;
 	int			id;
@@ -116,6 +119,7 @@ struct clk_functions {
 
 extern unsigned int mpurate;
 
+extern void omap_clk_associate(const char *id, struct device *dev);
 extern int clk_init(struct clk_functions *custom_clocks);
 extern int clk_register(struct clk *clk);
 extern void clk_unregister(struct clk *clk);
-- 
1.6.0.2.307.gc427

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

Sorry,

forgot to add RFC since I didn't test these patches yet.

anyways, this is the new version of the omap_clk_associate()
mechanism. The patch header will have to change, just saw it.

Paul, could you take a look at it, does it look better now ?

I dropped the third argument (function name) since we should
be matching only against the consumer device, if the consumer
device is correct, it doesn't matter which name we give to
clk_get().

On Sat, Oct 25, 2008 at 10:55:46PM +0300, Felipe Balbi wrote:
> From: Felipe Balbi <felipe.balbi@nokia.com>
> 
> Introduce a new mechanism to omap's clk implementation to
> associate the device with its clock during platform_device
> registration. Also gives the clock a function name (like
> mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
> care about clk names any longer.
> 
> With this mechanism we can also be sure drivers won't be able
> to clk_get the wrong clk (only true when we move all drivers
> to this new mechanism and drop the old ones).
> 
> Clk's function names will be defined as they come necessary
> but let's try to keep it as simple as possible.
> 
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  arch/arm/plat-omap/clock.c              |   47 ++++++++++++++++++++++++++++--
>  arch/arm/plat-omap/include/mach/clock.h |    4 ++
>  2 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index 5178701..083a3ff 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -44,6 +44,7 @@ static struct clk_functions *arch_clock;
>  struct clk * clk_get(struct device *dev, const char *id)
>  {
>  	struct clk *p, *clk = ERR_PTR(-ENOENT);
> +	struct device *d;
>  	int idno;
>  
>  	if (dev == NULL || dev->bus != &platform_bus_type)
> @@ -54,27 +55,65 @@ struct clk * clk_get(struct device *dev, const char *id)
>  	mutex_lock(&clocks_mutex);
>  
>  	list_for_each_entry(p, &clocks, node) {
> +		list_for_each_entry(d, &p->consumers, node) {
> +			if (dev == d) {
> +				clk = p;
> +				break;
> +			}
> +		}
> +
>  		if (p->id == idno &&
>  		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
>  			clk = p;
> -			goto found;
> +			break;
>  		}
> -	}
>  
> -	list_for_each_entry(p, &clocks, node) {
>  		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
>  			clk = p;
>  			break;
>  		}
>  	}

here I merged both list_for_each_entry() in only one to make it a bit
cheaper. Hope there's no problem with it.

>  
> -found:
>  	mutex_unlock(&clocks_mutex);
>  
>  	return clk;
>  }
>  EXPORT_SYMBOL(clk_get);
>  
> +/**
> + * omap_clk_associate - associates a clock to its consumer device(s)
> + * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
> + * @dev: device pointer for the clock consumer
> + *
> + * This function attempts to simplify clock handling for device drivers
> + * by letting struct clk know which device is the correct consumer for
> + * that clock. By making this trick, we let drivers get the correct
> + * clock without knowing about different clock names between omap
> + * versions.
> + */
> +void __init omap_clk_associate(const char *id, struct device *dev)
> +{
> +	struct clk *clk = clk_get(NULL, id);
> +
> +	if (!clk || IS_ERR(clk)) {
> +		pr_debug("%s: unable to get clock\n", __func__);
> +		return;
> +	}
> +
> +	if (!dev) {
> +		pr_debug("%s: missing device to associate with\n", __func__);
> +		clk_put(clk);
> +		return;
> +	}
> +
> +	if (list_empty(&clk->consumers))
> +		INIT_LIST_HEAD(&clk->consumers);

this I need to test. I didn't know about other mechanism to see if
clk->consumers wasn't initialized so I used list_empty(). I need to see
if it won't break in runtime.

> +
> +	/* add device to consumer list */
> +	list_add(&dev->node, &clk->consumers);
> +	clk_put(clk);
> +};
> +
>  int clk_enable(struct clk *clk)
>  {
>  	unsigned long flags;
> diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
> index 9088925..1600a14 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -13,6 +13,8 @@
>  #ifndef __ARCH_ARM_OMAP_CLOCK_H
>  #define __ARCH_ARM_OMAP_CLOCK_H
>  
> +#include <linux/device.h>
> +
>  struct module;
>  struct clk;
>  struct clockdomain;
> @@ -61,6 +63,7 @@ struct dpll_data {
>  
>  struct clk {
>  	struct list_head	node;
> +	struct list_head	consumers;
>  	struct module		*owner;
>  	const char		*name;
>  	int			id;
> @@ -116,6 +119,7 @@ struct clk_functions {
>  
>  extern unsigned int mpurate;
>  
> +extern void omap_clk_associate(const char *id, struct device *dev);
>  extern int clk_init(struct clk_functions *custom_clocks);
>  extern int clk_register(struct clk *clk);
>  extern void clk_unregister(struct clk *clk);
> -- 
> 1.6.0.2.307.gc427

-- 
balbi

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

On Sat, Oct 25, 2008 at 11:03:50PM +0300, Felipe Balbi wrote:
> Sorry,
> 
> forgot to add RFC since I didn't test these patches yet.
> 
> anyways, this is the new version of the omap_clk_associate()
> mechanism. The patch header will have to change, just saw it.
> 
> Paul, could you take a look at it, does it look better now ?
> 
> I dropped the third argument (function name) since we should
> be matching only against the consumer device, if the consumer
> device is correct, it doesn't matter which name we give to
> clk_get().

I tested this today and got some weird behavior on hsmmc.c. If I just
apply the patch, clk_enable() doesn't seem to work and we get a
"non-linefecth" exception.

when I put some printks on hsmmc.c, it seemed to help as there was no
BUG anymore, but instead clk_disable() was getting stuck.

Paul, do you have a clue about what could it be ?? I was wondering if
clk_get() was kinda depending on the delay created by running
list_for_each_entry() twice ?? I merged all of them into one only loop.

BTW, I changed one little thing in the patch:
INIT_LIST_HEAD() is done in clk_register().

> On Sat, Oct 25, 2008 at 10:55:46PM +0300, Felipe Balbi wrote:
> > From: Felipe Balbi <felipe.balbi@nokia.com>
> > 
> > Introduce a new mechanism to omap's clk implementation to
> > associate the device with its clock during platform_device
> > registration. Also gives the clock a function name (like
> > mmc_fck, uart_fck, ehci_fck, etc) so drivers won't have to
> > care about clk names any longer.
> > 
> > With this mechanism we can also be sure drivers won't be able
> > to clk_get the wrong clk (only true when we move all drivers
> > to this new mechanism and drop the old ones).
> > 
> > Clk's function names will be defined as they come necessary
> > but let's try to keep it as simple as possible.
> > 
> > Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> > ---
> >  arch/arm/plat-omap/clock.c              |   47 ++++++++++++++++++++++++++++--
> >  arch/arm/plat-omap/include/mach/clock.h |    4 ++
> >  2 files changed, 47 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> > index 5178701..083a3ff 100644
> > --- a/arch/arm/plat-omap/clock.c
> > +++ b/arch/arm/plat-omap/clock.c
> > @@ -44,6 +44,7 @@ static struct clk_functions *arch_clock;
> >  struct clk * clk_get(struct device *dev, const char *id)
> >  {
> >  	struct clk *p, *clk = ERR_PTR(-ENOENT);
> > +	struct device *d;
> >  	int idno;
> >  
> >  	if (dev == NULL || dev->bus != &platform_bus_type)
> > @@ -54,27 +55,65 @@ struct clk * clk_get(struct device *dev, const char *id)
> >  	mutex_lock(&clocks_mutex);
> >  
> >  	list_for_each_entry(p, &clocks, node) {
> > +		list_for_each_entry(d, &p->consumers, node) {
> > +			if (dev == d) {
> > +				clk = p;
> > +				break;
> > +			}
> > +		}
> > +
> >  		if (p->id == idno &&
> >  		    strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
> >  			clk = p;
> > -			goto found;
> > +			break;
> >  		}
> > -	}
> >  
> > -	list_for_each_entry(p, &clocks, node) {
> >  		if (strcmp(id, p->name) == 0 && try_module_get(p->owner)) {
> >  			clk = p;
> >  			break;
> >  		}
> >  	}
> 
> here I merged both list_for_each_entry() in only one to make it a bit
> cheaper. Hope there's no problem with it.
> 
> >  
> > -found:
> >  	mutex_unlock(&clocks_mutex);
> >  
> >  	return clk;
> >  }
> >  EXPORT_SYMBOL(clk_get);
> >  
> > +/**
> > + * omap_clk_associate - associates a clock to its consumer device(s)
> > + * @id: clock id as defined in arch/arm/mach-omapX/clkxxxx.h
> > + * @dev: device pointer for the clock consumer
> > + *
> > + * This function attempts to simplify clock handling for device drivers
> > + * by letting struct clk know which device is the correct consumer for
> > + * that clock. By making this trick, we let drivers get the correct
> > + * clock without knowing about different clock names between omap
> > + * versions.
> > + */
> > +void __init omap_clk_associate(const char *id, struct device *dev)
> > +{
> > +	struct clk *clk = clk_get(NULL, id);
> > +
> > +	if (!clk || IS_ERR(clk)) {
> > +		pr_debug("%s: unable to get clock\n", __func__);
> > +		return;
> > +	}
> > +
> > +	if (!dev) {
> > +		pr_debug("%s: missing device to associate with\n", __func__);
> > +		clk_put(clk);
> > +		return;
> > +	}
> > +
> > +	if (list_empty(&clk->consumers))
> > +		INIT_LIST_HEAD(&clk->consumers);
> 
> this I need to test. I didn't know about other mechanism to see if
> clk->consumers wasn't initialized so I used list_empty(). I need to see
> if it won't break in runtime.
> 
> > +
> > +	/* add device to consumer list */
> > +	list_add(&dev->node, &clk->consumers);
> > +	clk_put(clk);
> > +};
> > +
> >  int clk_enable(struct clk *clk)
> >  {
> >  	unsigned long flags;
> > diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
> > index 9088925..1600a14 100644
> > --- a/arch/arm/plat-omap/include/mach/clock.h
> > +++ b/arch/arm/plat-omap/include/mach/clock.h
> > @@ -13,6 +13,8 @@
> >  #ifndef __ARCH_ARM_OMAP_CLOCK_H
> >  #define __ARCH_ARM_OMAP_CLOCK_H
> >  
> > +#include <linux/device.h>
> > +
> >  struct module;
> >  struct clk;
> >  struct clockdomain;
> > @@ -61,6 +63,7 @@ struct dpll_data {
> >  
> >  struct clk {
> >  	struct list_head	node;
> > +	struct list_head	consumers;
> >  	struct module		*owner;
> >  	const char		*name;
> >  	int			id;
> > @@ -116,6 +119,7 @@ struct clk_functions {
> >  
> >  extern unsigned int mpurate;
> >  
> > +extern void omap_clk_associate(const char *id, struct device *dev);
> >  extern int clk_init(struct clk_functions *custom_clocks);
> >  extern int clk_register(struct clk *clk);
> >  extern void clk_unregister(struct clk *clk);
> > -- 
> > 1.6.0.2.307.gc427
> 
> -- 
> balbi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi

Re: [PATCH 1/3] clk: introduce omap_clk_associate

[Felipe Balbi]

Hi all,

a couple of changes to omap_clk_associate,

On Sat, Oct 25, 2008 at 11:03:50PM +0300, Felipe Balbi wrote:
> > +void omap_clk_associate(const char *id, struct device *dev);

This i changed to omap_clk_associate(struct device *dev, const char *id)
as it looks closer to clk_get().

> > +		list_for_each_entry(d, &p->consumers, node) {

The list_head in struct device was changed to a klist. I already fixed
the patches. Will test them tomorrow afternoon and send the new version
here.

I'll also Cc Greg as there's a symbol from driver core that would be useful
to get it exported.

-- 
balbi