Re: [PATCH v5 3/4] clk: introduce the common clock framework

From: Sascha Hauer
Date: Tue Mar 13 2012 - 07:25:09 EST


On Sat, Mar 03, 2012 at 12:29:00AM -0800, Mike Turquette wrote:
> The common clock framework defines a common struct clk useful across
> most platforms as well as an implementation of the clk api that drivers
> can use safely for managing clocks.
>
> The net result is consolidation of many different struct clk definitions
> and platform-specific clock framework implementations.
>
> This patch introduces the common struct clk, struct clk_ops and an
> implementation of the well-known clock api in include/clk/clk.h.
> Platforms may define their own hardware-specific clock structure and
> their own clock operation callbacks, so long as it wraps an instance of
> struct clk_hw.
>
> See Documentation/clk.txt for more details.
>
> This patch is based on the work of Jeremy Kerr, which in turn was based
> on the work of Ben Herrenschmidt.
>
> Signed-off-by: Mike Turquette <mturquette@xxxxxxxxxx>
> Signed-off-by: Mike Turquette <mturquette@xxxxxx>
> Cc: Jeremy Kerr <jeremy.kerr@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Arnd Bergman <arnd.bergmann@xxxxxxxxxx>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Cc: Shawn Guo <shawn.guo@xxxxxxxxxxxxx>
> Cc: Richard Zhao <richard.zhao@xxxxxxxxxx>
> Cc: Saravana Kannan <skannan@xxxxxxxxxxxxxx>
> Cc: Magnus Damm <magnus.damm@xxxxxxxxx>
> Cc: Rob Herring <rob.herring@xxxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx>
> Cc: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> Cc: Amit Kucheria <amit.kucheria@xxxxxxxxxx>
> Cc: Deepak Saxena <dsaxena@xxxxxxxxxx>
> Cc: Grant Likely <grant.likely@xxxxxxxxxxxx>
> Cc: Andrew Lunn <andrew@xxxxxxx>
> ---
> drivers/clk/Kconfig | 28 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk.c | 1323 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/clk-private.h | 68 +++
> include/linux/clk-provider.h | 169 ++++++
> include/linux/clk.h | 68 ++-
> 6 files changed, 1652 insertions(+), 5 deletions(-)
> create mode 100644 drivers/clk/clk.c
> create mode 100644 include/linux/clk-private.h
> create mode 100644 include/linux/clk-provider.h
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 3912576..18eb8c2 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -11,3 +11,31 @@ config HAVE_MACH_CLKDEV
>
> config HAVE_CLK_PREPARE
> bool
> +
> +menuconfig COMMON_CLK
> + bool "Common Clock Framework"
> + select HAVE_CLK_PREPARE
> + ---help---
> + The common clock framework is a single definition of struct
> + clk, useful across many platforms, as well as an
> + implementation of the clock API in include/linux/clk.h.
> + Architectures utilizing the common struct clk should select
> + this automatically, but it may be necessary to manually select
> + this option for loadable modules requiring the common clock
> + framework.
> +
> + If in doubt, say "N".
> +
> +if COMMON_CLK
> +
> +config COMMON_CLK_DEBUG
> + bool "DebugFS representation of clock tree"
> + depends on COMMON_CLK
> + ---help---
> + Creates a directory hierchy in debugfs for visualizing the clk
> + tree structure. Each directory contains read-only members
> + that export information specific to that clk node: clk_rate,
> + clk_flags, clk_prepare_count, clk_enable_count &
> + clk_notifier_count.
> +
> +endif
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 07613fa..ff362c4 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -1,2 +1,3 @@
>
> obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
> +obj-$(CONFIG_COMMON_CLK) += clk.o
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> new file mode 100644
> index 0000000..b979d74
> --- /dev/null
> +++ b/drivers/clk/clk.c
> @@ -0,0 +1,1323 @@
> +/*
> + * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@xxxxxxxxxxxxx>
> + * Copyright (C) 2011-2012 Linaro Ltd <mturquette@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Standard functionality for the common clock API. See Documentation/clk.txt
> + */
> +
> +#include <linux/clk-private.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <linux/err.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +static DEFINE_SPINLOCK(enable_lock);
> +static DEFINE_MUTEX(prepare_lock);
> +
> +static HLIST_HEAD(clk_root_list);
> +static HLIST_HEAD(clk_orphan_list);
> +static LIST_HEAD(clk_notifier_list);
> +
> +/*** debugfs support ***/
> +
> +#ifdef CONFIG_COMMON_CLK_DEBUG
> +#include <linux/debugfs.h>
> +
> +static struct dentry *rootdir;
> +static struct dentry *orphandir;
> +static int inited = 0;
> +
> +/* caller must hold prepare_lock */
> +static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
> +{
> + struct dentry *d;
> + int ret = -ENOMEM;
> +
> + if (!clk || !pdentry) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + d = debugfs_create_dir(clk->name, pdentry);
> + if (!d)
> + goto out;
> +
> + clk->dentry = d;
> +
> + d = debugfs_create_u32("clk_rate", S_IRUGO, clk->dentry,
> + (u32 *)&clk->rate);
> + if (!d)
> + goto err_out;
> +
> + d = debugfs_create_x32("clk_flags", S_IRUGO, clk->dentry,
> + (u32 *)&clk->flags);
> + if (!d)
> + goto err_out;
> +
> + d = debugfs_create_u32("clk_prepare_count", S_IRUGO, clk->dentry,
> + (u32 *)&clk->prepare_count);
> + if (!d)
> + goto err_out;
> +
> + d = debugfs_create_u32("clk_enable_count", S_IRUGO, clk->dentry,
> + (u32 *)&clk->enable_count);
> + if (!d)
> + goto err_out;
> +
> + d = debugfs_create_u32("clk_notifier_count", S_IRUGO, clk->dentry,
> + (u32 *)&clk->notifier_count);
> + if (!d)
> + goto err_out;
> +
> + ret = 0;
> + goto out;
> +
> +err_out:
> + debugfs_remove(clk->dentry);
> +out:
> + return ret;
> +}
> +
> +/* caller must hold prepare_lock */
> +static int clk_debug_create_subtree(struct clk *clk, struct dentry *pdentry)
> +{
> + struct clk *child;
> + struct hlist_node *tmp;
> + int ret = -EINVAL;;
> +
> + if (!clk || !pdentry)
> + goto out;
> +
> + ret = clk_debug_create_one(clk, pdentry);
> +
> + if (ret)
> + goto out;
> +
> + hlist_for_each_entry(child, tmp, &clk->children, child_node)
> + clk_debug_create_subtree(child, clk->dentry);
> +
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +/**
> + * clk_debug_register - add a clk node to the debugfs clk tree
> + * @clk: the clk being added to the debugfs clk tree
> + *
> + * Dynamically adds a clk to the debugfs clk tree if debugfs has been
> + * initialized. Otherwise it bails out early since the debugfs clk tree
> + * will be created lazily by clk_debug_init as part of a late_initcall.
> + *
> + * Caller must hold prepare_lock. Only clk_init calls this function (so
> + * far) so this is taken care.
> + */
> +static int clk_debug_register(struct clk *clk)
> +{
> + struct clk *parent;
> + struct dentry *pdentry;
> + int ret = 0;
> +
> + if (!inited)
> + goto out;
> +
> + parent = clk->parent;
> +
> + /*
> + * Check to see if a clk is a root clk. Also check that it is
> + * safe to add this clk to debugfs
> + */
> + if (!parent)
> + if (clk->flags & CLK_IS_ROOT)
> + pdentry = rootdir;
> + else
> + pdentry = orphandir;
> + else
> + if (parent->dentry)
> + pdentry = parent->dentry;
> + else
> + goto out;
> +
> + ret = clk_debug_create_subtree(clk, pdentry);
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * clk_debug_init - lazily create the debugfs clk tree visualization
> + *
> + * clks are often initialized very early during boot before memory can
> + * be dynamically allocated and well before debugfs is setup.
> + * clk_debug_init walks the clk tree hierarchy while holding
> + * prepare_lock and creates the topology as part of a late_initcall,
> + * thus insuring that clks initialized very early will still be
> + * represented in the debugfs clk tree. This function should only be
> + * called once at boot-time, and all other clks added dynamically will
> + * be done so with clk_debug_register.
> + */
> +static int __init clk_debug_init(void)
> +{
> + struct clk *clk;
> + struct hlist_node *tmp;
> +
> + rootdir = debugfs_create_dir("clk", NULL);
> +
> + if (!rootdir)
> + return -ENOMEM;
> +
> + orphandir = debugfs_create_dir("orphans", rootdir);
> +
> + if (!orphandir)
> + return -ENOMEM;
> +
> + mutex_lock(&prepare_lock);
> +
> + hlist_for_each_entry(clk, tmp, &clk_root_list, child_node)
> + clk_debug_create_subtree(clk, rootdir);
> +
> + hlist_for_each_entry(clk, tmp, &clk_orphan_list, child_node)
> + clk_debug_create_subtree(clk, orphandir);
> +
> + inited = 1;
> +
> + mutex_unlock(&prepare_lock);
> +
> + return 0;
> +}
> +late_initcall(clk_debug_init);
> +#else
> +static inline int clk_debug_register(struct clk *clk) { return 0; }
> +#endif /* CONFIG_COMMON_CLK_DEBUG */
> +
> +/*** helper functions ***/
> +
> +inline const char *__clk_get_name(struct clk *clk)
> +{
> + return !clk ? NULL : clk->name;
> +}
> +
> +inline struct clk_hw *__clk_get_hw(struct clk *clk)
> +{
> + return !clk ? NULL : clk->hw;
> +}
> +
> +inline u8 __clk_get_num_parents(struct clk *clk)
> +{
> + return !clk ? -EINVAL : clk->num_parents;
> +}
> +
> +inline struct clk *__clk_get_parent(struct clk *clk)
> +{
> + return !clk ? NULL : clk->parent;
> +}
> +
> +inline unsigned long __clk_get_rate(struct clk *clk)
> +{
> + return !clk ? -EINVAL : clk->rate;
> +}
> +
> +inline unsigned long __clk_get_flags(struct clk *clk)
> +{
> + return !clk ? -EINVAL : clk->flags;
> +}
> +
> +static struct clk *__clk_lookup_subtree(const char *name, struct clk *clk)
> +{
> + struct clk *child;
> + struct clk *ret;
> + struct hlist_node *tmp;
> +
> + if (!strcmp(clk->name, name))
> + return clk;
> +
> + hlist_for_each_entry(child, tmp, &clk->children, child_node) {
> + ret = __clk_lookup_subtree(name, child);
> + if (ret)
> + return ret;
> + }
> +
> + return NULL;
> +}
> +
> +struct clk *__clk_lookup(const char *name)
> +{
> + struct clk *root_clk;
> + struct clk *ret;
> + struct hlist_node *tmp;
> +

This should have a check for NULL pointers. NULL pointers can happen
here if a mux has holes in it. In this case the parent name array would
look like this:

{ "parentclk1", "parentclk2", NULL /* reserved */, "parentclk3" }

Without checking for NULL pointers we dereference the NULL pointer here.

> +
> +/**
> + * __clk_round_rate - round the given rate for a clk
> + * @clk: round the rate of this clock
> + *
> + * Caller must hold prepare_lock. Useful for clk_ops such as .set_rate
> + */
> +unsigned long __clk_round_rate(struct clk *clk, unsigned long rate)
> +{
> + if (!clk && !clk->ops->round_rate)
> + return -EINVAL;

This should return the parents rate if the clock itself does not
have a round_rate function. Also, must be || instead of &&

> +
> + return clk->ops->round_rate(clk->hw, rate, NULL);
> +}
> +
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/